From 5a5f1a371819b65f66bd4d629d2780bedbf39c93 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 16 Apr 2025 22:07:30 +0200 Subject: [PATCH 1/2] Change /oauth/token request specs to use client_secret_basic authentication --- spec/requests/oauth/token_spec.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/spec/requests/oauth/token_spec.rb b/spec/requests/oauth/token_spec.rb index 7be65e7ab3..b10e912012 100644 --- a/spec/requests/oauth/token_spec.rb +++ b/spec/requests/oauth/token_spec.rb @@ -1,21 +1,28 @@ # frozen_string_literal: true require 'rails_helper' +require 'debug' RSpec.describe 'Managing OAuth Tokens' do describe 'POST /oauth/token' do subject do - post '/oauth/token', params: params + post '/oauth/token', params: params, headers: headers end 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 + let(:params) do { grant_type: grant_type, - client_id: application.uid, - client_secret: application.secret, redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', code: code, scope: scope, From 77cf2abb3a2789df35851462d09ef82a2caf22b3 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Sat, 19 Apr 2025 01:51:33 +0200 Subject: [PATCH 2/2] Improve /oauth/token request specs Previously these specs passed incorrect parameters to both the authorization_code and client_credentials grant flows. The authorization_code flow does not accept a `scope` parameter, instead the scope is set when the access grant is created, per RFC 6749 Section 4.1.2. The `code` parameter is accepted by this flow. https://www.rfc-editor.org/rfc/rfc6749#section-4.1.2 The client_credentials flow does not accept a `code` parameter, and instead accepts a `scope` parameter, per RFC 6749 Section 4.4.1 https://www.rfc-editor.org/rfc/rfc6749#section-4.4.1 This ensures we're only testing valid oauth flows, and not deviating from the specification. The OAuth flows should ignore any unknown parameters (i.e., passing `code` to client_credentials would have no impact on the functionality, and this would be asserted at the Doorkeeper level). --- spec/requests/oauth/token_spec.rb | 83 ++++++++++++++++++------------- 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/spec/requests/oauth/token_spec.rb b/spec/requests/oauth/token_spec.rb index b10e912012..d6031cbece 100644 --- a/spec/requests/oauth/token_spec.rb +++ b/spec/requests/oauth/token_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'rails_helper' -require 'debug' RSpec.describe 'Managing OAuth Tokens' do describe 'POST /oauth/token' do @@ -20,24 +19,22 @@ RSpec.describe 'Managing OAuth Tokens' do } end - let(:params) do - { - grant_type: grant_type, - redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', - code: code, - scope: scope, - } - end - context "with grant_type 'authorization_code'" do - let(:grant_type) { 'authorization_code' } + let(:params) do + { + grant_type: 'authorization_code', + redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', + code: code, + } + end + let(:code) do access_grant = Fabricate(:access_grant, application: application, redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', scopes: 'read write') access_grant.plaintext_token end shared_examples 'original scope request preservation' do - it 'returns all scopes requested for the given code' do + it 'returns all scopes requested by the authorization code' do subject expect(response).to have_http_status(200) @@ -45,36 +42,51 @@ RSpec.describe 'Managing OAuth Tokens' do end end - context 'with no scopes specified' do - let(:scope) { nil } + context 'with client authentication via params' do + let(:headers) { nil } + let(:params) do + { + grant_type: 'authorization_code', + redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', + client_id: application.uid, + client_secret: application.secret, + code: code, + } + end it_behaves_like 'original scope request preservation' end - context 'with scopes specified' do - context 'when the scopes were requested for this code' do - let(:scope) { 'write' } - - it_behaves_like 'original scope request preservation' - end - - context 'when the scope was not requested for the code' do - let(:scope) { 'follow' } - - it_behaves_like 'original scope request preservation' - end - - context 'when the scope does not belong to the application' do - let(:scope) { 'push' } - - it_behaves_like 'original scope request preservation' - end - end + it_behaves_like 'original scope request preservation' end context "with grant_type 'client_credentials'" do - let(:grant_type) { 'client_credentials' } - let(:code) { nil } + let(:scope) { nil } + let(:params) do + { + grant_type: 'client_credentials', + scope: scope, + } + end + + context 'with client authentication via params' do + let(:headers) { nil } + let(:params) do + { + grant_type: 'client_credentials', + client_id: application.uid, + client_secret: application.secret, + scope: scope, + } + 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 } @@ -106,6 +118,7 @@ RSpec.describe 'Managing OAuth Tokens' do subject expect(response).to have_http_status(400) + expect(response.parsed_body[:error]).to eq 'invalid_scope' end end end