Adjust the specs based on feedback from Claire to ensure we're still testing the invalid scope for authorization code grant flow case

spec/requests/oauth/token_spec.rb

@@ -5,20 +5,17 @@ require 'rails_helper'
 RSpec.describe 'Managing OAuth Tokens' do
   describe 'POST /oauth/token' do
     subject do
-      post '/oauth/token', params: params, headers: headers
+      post '/oauth/token', params: params.merge(additional_params), headers: headers
     end
 
+    let(:headers) { nil }
+    let(:additional_params) { {} }
+    let(:params) { {} }
+
     let(:application) do
       Fabricate(:application, scopes: 'read write follow', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob')
     end
 
-    # This is using the OAuth client_secret_basic client authentication method
-    let(:headers) do
-      {
-        Authorization: ActionController::HttpAuthentication::Basic.encode_credentials(application.uid, application.secret),
-      }
-    end
-
     context "with grant_type 'authorization_code'" do
       let(:params) do
         {
@@ -33,13 +30,31 @@ RSpec.describe 'Managing OAuth Tokens' do
         access_grant.plaintext_token
       end
 
-      shared_examples 'original scope request preservation' do
-        it 'returns all scopes requested by the authorization code' do
+      shared_examples 'returns a correctly scoped access token' do
+        it 'returns the scopes requested by the authorization code' do
           subject
 
           expect(response).to have_http_status(200)
           expect(response.parsed_body[:scope]).to eq 'read write'
         end
+
+        context 'with additional parameters not used by the grant type' do
+          # When performing an authorization code grant flow, the `/oauth/token`
+          # endpoint does not accept a `scope` parameter, and should not
+          # override the scopes from the authorization grant.
+          let(:additional_params) do
+            {
+              scope: 'write',
+            }
+          end
+
+          it 'returns the scopes requested by the authorization code' do
+            subject
+
+            expect(response).to have_http_status(200)
+            expect(response.parsed_body[:scope]).to eq 'read write'
+          end
+        end
       end
 
       context 'with client authentication via params' do
@@ -54,19 +69,73 @@ RSpec.describe 'Managing OAuth Tokens' do
           }
         end
 
-        it_behaves_like 'original scope request preservation'
+        it_behaves_like 'returns a correctly scoped access token'
       end
 
-      it_behaves_like 'original scope request preservation'
+      context 'with client authentication via basic auth' do
+        let(:headers) do
+          {
+            Authorization: ActionController::HttpAuthentication::Basic.encode_credentials(application.uid, application.secret),
+          }
+        end
+
+        it_behaves_like 'returns a correctly scoped access token'
+      end
     end
 
     context "with grant_type 'client_credentials'" do
-      let(:scope) { nil }
-      let(:params) do
-        {
-          grant_type: 'client_credentials',
-          scope: scope,
-        }
+      shared_examples 'returns the correct scopes' do
+        context 'with no scopes specified' do
+          let(:scope) { nil }
+
+          it 'returns only the default scope' do
+            subject
+
+            expect(response).to have_http_status(200)
+            expect(response.parsed_body[:scope]).to eq('read')
+          end
+        end
+
+        context 'with scopes specified' do
+          context 'when the scopes belong to the application' do
+            let(:scope) { 'read write' }
+
+            it 'returns all the requested scopes' do
+              subject
+
+              expect(response).to have_http_status(200)
+              expect(response.parsed_body[:scope]).to eq 'read write'
+            end
+          end
+
+          context 'when some scopes do not belong to the application' do
+            let(:scope) { 'read write push' }
+
+            it 'returns an error' do
+              subject
+
+              expect(response).to have_http_status(400)
+              expect(response.parsed_body[:error]).to eq 'invalid_scope'
+            end
+          end
+        end
+      end
+
+      context 'with client authentication via basic auth' do
+        let(:headers) do
+          {
+            Authorization: ActionController::HttpAuthentication::Basic.encode_credentials(application.uid, application.secret),
+          }
+        end
+
+        let(:params) do
+          {
+            grant_type: 'client_credentials',
+            scope: scope,
+          }
+        end
+
+        it_behaves_like 'returns the correct scopes'
       end
 
       context 'with client authentication via params' do
@@ -80,47 +149,7 @@ RSpec.describe 'Managing OAuth Tokens' do
           }
         end
 
-        it 'returns only the default scope' do
-          subject
-
-          expect(response).to have_http_status(200)
-          expect(response.parsed_body[:scope]).to eq('read')
-        end
-      end
-
-      context 'with no scopes specified' do
-        let(:scope) { nil }
-
-        it 'returns only the default scope' do
-          subject
-
-          expect(response).to have_http_status(200)
-          expect(response.parsed_body[:scope]).to eq('read')
-        end
-      end
-
-      context 'with scopes specified' do
-        context 'when the scopes belong to the application' do
-          let(:scope) { 'read write' }
-
-          it 'returns all the requested scopes' do
-            subject
-
-            expect(response).to have_http_status(200)
-            expect(response.parsed_body[:scope]).to eq 'read write'
-          end
-        end
-
-        context 'when some scopes do not belong to the application' do
-          let(:scope) { 'read write push' }
-
-          it 'returns an error' do
-            subject
-
-            expect(response).to have_http_status(400)
-            expect(response.parsed_body[:error]).to eq 'invalid_scope'
-          end
-        end
+        it_behaves_like 'returns the correct scopes'
       end
     end
   end