Merge branch 'fix-oauth-resource-owner-docs' into 'master'

Fix OAuth documentation and tests for Resource Owner Password Credentials Grant

See merge request gitlab-org/gitlab!32878

doc/api/oauth2.md

@@ -173,11 +173,14 @@ the following parameters:
 }
 ```
 
+Also you must use HTTP Basic authentication using the `client_id` and`client_secret`
+values to authenticate the client that performs a request.
+
 Example cURL request:
 
 ```shell
 echo 'grant_type=password&username=<your_username>&password=<your_password>' > auth.txt
-curl --data "@auth.txt" --request POST "https://gitlab.example.com/oauth/token"
+curl --data "@auth.txt" --user client_id:client_secret --request POST "https://gitlab.example.com/oauth/token"
 ```
 
 Then, you'll receive the access token back in the response:
@@ -190,6 +193,8 @@ Then, you'll receive the access token back in the response:
 }
 ```
 
+By default, the scope of the access token is `api`, which provides complete read/write access.
+
 For testing, you can use the `oauth2` Ruby gem:
 
 ```ruby

spec/requests/api/oauth_tokens_spec.rb

@@ -4,15 +4,32 @@ require 'spec_helper'
 
 describe 'OAuth tokens' do
   context 'Resource Owner Password Credentials' do
-    def request_oauth_token(user)
-      post '/oauth/token', params: { username: user.username, password: user.password, grant_type: 'password' }
+    def basic_auth_header(username, password)
+      {
+        'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(
+          username,
+          password
+        )
+      }
     end
 
+    def client_basic_auth_header(client)
+      basic_auth_header(client.uid, client.secret)
+    end
+
+    def request_oauth_token(user, headers = {})
+      post '/oauth/token',
+         params: { username: user.username, password: user.password, grant_type: 'password' },
+         headers: headers
+    end
+
+    let_it_be(:client) { create(:oauth_application) }
+
     context 'when user has 2FA enabled' do
       it 'does not create an access token' do
         user = create(:user, :two_factor)
 
-        request_oauth_token(user)
+        request_oauth_token(user, client_basic_auth_header(client))
 
         expect(response).to have_gitlab_http_status(:unauthorized)
         expect(json_response['error']).to eq('invalid_grant')
@@ -20,13 +37,46 @@ describe 'OAuth tokens' do
     end
 
     context 'when user does not have 2FA enabled' do
-      it 'creates an access token' do
-        user = create(:user)
+      # NOTE: using ROPS grant flow without client credentials will be deprecated
+      # and removed in the next version of Doorkeeper.
+      # See https://gitlab.com/gitlab-org/gitlab/-/issues/219137
+      context 'when no client credentials provided' do
+        it 'creates an access token' do
+          user = create(:user)
+
+          request_oauth_token(user)
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['access_token']).not_to be_nil
+        end
+      end
+
+      context 'when client credentials provided' do
+        context 'with valid credentials' do
+          it 'creates an access token' do
+            user = create(:user)
+
+            request_oauth_token(user, client_basic_auth_header(client))
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['access_token']).not_to be_nil
+          end
+        end
+
+        context 'with invalid credentials' do
+          it 'does not create an access token' do
+            # NOTE: remove this after update to Doorkeeper 5.5 or newer, see
+            # https://gitlab.com/gitlab-org/gitlab/-/issues/219137
+            pending 'Enable this example after upgrading Doorkeeper to 5.5 or newer'
+
+            user = create(:user)
 
-        request_oauth_token(user)
+            request_oauth_token(user, basic_auth_header(client.uid, 'invalid secret'))
 
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(json_response['access_token']).not_to be_nil
+            expect(response).to have_gitlab_http_status(:unauthorized)
+            expect(json_response['error']).to eq('invalid_client')
+          end
+        end
       end
     end
 
@@ -40,7 +90,7 @@ describe 'OAuth tokens' do
       before do
         user.block
 
-        request_oauth_token(user)
+        request_oauth_token(user, client_basic_auth_header(client))
       end
 
       include_examples 'does not create an access token'
@@ -50,7 +100,7 @@ describe 'OAuth tokens' do
       before do
         user.ldap_block
 
-        request_oauth_token(user)
+        request_oauth_token(user, client_basic_auth_header(client))
       end
 
       include_examples 'does not create an access token'
@@ -60,7 +110,7 @@ describe 'OAuth tokens' do
       before do
         user.update!(confirmed_at: nil)
 
-        request_oauth_token(user)
+        request_oauth_token(user, client_basic_auth_header(client))
       end
 
       include_examples 'does not create an access token'