From 5c6ad1a0e59ed10653885fce86c742666239b4fa Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 16 May 2024 17:12:47 +0200 Subject: [PATCH 01/13] Ensure only confidential clients can use Client Credentials grant flow --- config/initializers/doorkeeper.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 516db258df..8e0cd9c84d 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -167,6 +167,16 @@ Doorkeeper.configure do grant_flows %w(authorization_code client_credentials) + # If the client is not a confidential client, it should not be able to use the + # client_credentials grant flow, since it cannot keep a secret. + allow_grant_flow_for_client do |grant_flow, client| + if grant_flow == Doorkeeper::OAuth::CLIENT_CREDENTIALS + client.confidential? + else + true + end + end + # Under some circumstances you might want to have applications auto-approved, # so that the user skips the authorization step. # For example if dealing with a trusted application. From b21e7d8fdb42b65ecb94c33f3e5c32cc6f4f9ee5 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 16 May 2024 17:13:30 +0200 Subject: [PATCH 02/13] Add support for public clients to OAuth Application creation - parameter name TBD --- app/controllers/api/v1/apps_controller.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/apps_controller.rb b/app/controllers/api/v1/apps_controller.rb index 50feaf1854..068d57aae5 100644 --- a/app/controllers/api/v1/apps_controller.rb +++ b/app/controllers/api/v1/apps_controller.rb @@ -16,14 +16,19 @@ class Api::V1::AppsController < Api::BaseController redirect_uri: app_params[:redirect_uris], scopes: app_scopes_or_default, website: app_params[:website], + confidential: app_confidential?, } end + def app_confidential? + !app_params[:token_endpoint_auth_method] || app_params[:token_endpoint_auth_method] != 'none' + end + def app_scopes_or_default app_params[:scopes] || Doorkeeper.configuration.default_scopes end def app_params - params.permit(:client_name, :scopes, :website, :redirect_uris, redirect_uris: []) + params.permit(:client_name, :scopes, :website, :token_endpoint_auth_method, :redirect_uris, redirect_uris: []) end end From fad8f7b148d78f314cf9fc0689dd3d00969fb092 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 16 May 2024 17:13:58 +0200 Subject: [PATCH 03/13] Only return client_secret for confidential clients --- app/serializers/rest/credential_application_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/rest/credential_application_serializer.rb b/app/serializers/rest/credential_application_serializer.rb index 0532390c9a..d4faedb848 100644 --- a/app/serializers/rest/credential_application_serializer.rb +++ b/app/serializers/rest/credential_application_serializer.rb @@ -8,7 +8,7 @@ class REST::CredentialApplicationSerializer < REST::ApplicationSerializer end def client_secret - object.secret + object.secret if object.confidential? end # Added for future forwards compatibility when we may decide to expire OAuth From 47e4f8478fd2a461d10a64e0bb5c890eaf499b4c Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 16 May 2024 17:16:02 +0200 Subject: [PATCH 04/13] Enable expiry of OAuth Access Tokens granted to public clients --- config/initializers/doorkeeper.rb | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 8e0cd9c84d..b427a2f34a 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -31,10 +31,19 @@ Doorkeeper.configure do # If you want to disable expiration, set this to nil. access_token_expires_in nil - # Assign a custom TTL for implicit grants. - # custom_access_token_expires_in do |oauth_client| - # oauth_client.application.additional_settings.implicit_oauth_expiration - # end + # context.grant_type to compare with Doorkeeper::OAUTH grant type constants + # context.client for client (Doorkeeper::Application) + # context.scopes for scopes + custom_access_token_expires_in do |context| + # If the client is confidential (all clients pre 4.3), then we don't want to + # expire access tokens. Applications created by users are also considered + # confidential. + if context.client.confidential? + nil + else + 15.minutes.to_i + end + end # Use a custom class for generating the access token. # https://github.com/doorkeeper-gem/doorkeeper#custom-access-token-generator From 2250aead46dff5e2e9eab285a1c213d02e5c551a Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 9 Apr 2025 21:24:08 +0200 Subject: [PATCH 05/13] WIP --- app/controllers/api/v1/apps_controller.rb | 15 ++++- config/initializers/doorkeeper.rb | 4 +- spec/requests/api/v1/apps_spec.rb | 71 +++++++++++++++++++++++ 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/apps_controller.rb b/app/controllers/api/v1/apps_controller.rb index 068d57aae5..fbfae2937a 100644 --- a/app/controllers/api/v1/apps_controller.rb +++ b/app/controllers/api/v1/apps_controller.rb @@ -2,6 +2,9 @@ class Api::V1::AppsController < Api::BaseController skip_before_action :require_authenticated_user! + before_action :validate_token_endpoint_auth_method! + + TOKEN_ENDPOINT_AUTH_METHODS = %w(none client_secret_basic client_secret_post).freeze def create @app = Doorkeeper::Application.create!(application_options) @@ -16,12 +19,18 @@ class Api::V1::AppsController < Api::BaseController redirect_uri: app_params[:redirect_uris], scopes: app_scopes_or_default, website: app_params[:website], - confidential: app_confidential?, + confidential: !app_public?, } end - def app_confidential? - !app_params[:token_endpoint_auth_method] || app_params[:token_endpoint_auth_method] != 'none' + def validate_token_endpoint_auth_method! + return unless app_params.include? :token_endpoint_auth_method + + bad_request unless TOKEN_ENDPOINT_AUTH_METHODS.include? app_params[:token_endpoint_auth_method] + end + + def app_public? + app_params[:token_endpoint_auth_method] == 'none' end def app_scopes_or_default diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index b427a2f34a..d7084647ff 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -129,7 +129,9 @@ Doorkeeper.configure do # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then # falls back to the `:client_id` and `:client_secret` params from the `params` object. # Check out the wiki for more information on customization - # client_credentials :from_basic, :from_params + # + # This is the default value: + client_credentials :from_basic, :from_params # Change the way access token is authenticated from the request object. # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then diff --git a/spec/requests/api/v1/apps_spec.rb b/spec/requests/api/v1/apps_spec.rb index 3120ab9c64..c54a371060 100644 --- a/spec/requests/api/v1/apps_spec.rb +++ b/spec/requests/api/v1/apps_spec.rb @@ -36,6 +36,7 @@ RSpec.describe 'Apps' do expect(app).to be_present expect(app.scopes.to_s).to eq scopes expect(app.redirect_uris).to eq redirect_uris + expect(app.confidential).to be true expect(response.parsed_body).to match( a_hash_including( @@ -55,6 +56,76 @@ RSpec.describe 'Apps' do end end + context 'without being a confidential application' do + let(:client_name) { 'Test confidential app' } + let(:params) do + { + client_name: client_name, + redirect_uris: redirect_uris, + scopes: scopes, + website: website, + token_endpoint_auth_method: 'none', + } + end + + it 'creates an public OAuth app', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + + app = Doorkeeper::Application.find_by(name: client_name) + + expect(app).to be_present + expect(app.scopes.to_s).to eq scopes + expect(app.redirect_uris).to eq redirect_uris + expect(app.confidential).to be false + + expect(response.parsed_body).to match( + a_hash_including( + id: app.id.to_s, + client_id: app.uid, + client_secret: nil, + client_secret_expires_at: 0, + name: client_name, + website: website, + scopes: ['read', 'write'], + redirect_uris: redirect_uris, + # Deprecated properties as of 4.3: + redirect_uri: redirect_uri, + vapid_key: Rails.configuration.x.vapid_public_key + ) + ) + end + end + + context 'when token_endpoint_auth_method is unknown' do + let(:client_name) { 'Test unknown auth app' } + let(:params) do + { + client_name: client_name, + redirect_uris: redirect_uris, + scopes: scopes, + website: website, + # Not yet supported: + token_endpoint_auth_method: 'private_key_jwt', + } + end + + it 'does not create an OAuth app', :aggregate_failures do + subject + + expect(response).to have_http_status(400) + expect(response.content_type) + .to start_with('application/json') + + app = Doorkeeper::Application.find_by(name: client_name) + + expect(app).to_not be_present + end + end + context 'without scopes being supplied' do let(:scopes) { nil } From 7898619d7442c8a42d545d243bf9d2dfdb4f74c4 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 10 Apr 2025 21:56:22 +0200 Subject: [PATCH 06/13] Prevent only using offline_access scope --- app/controllers/oauth/authorizations_controller.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index deafedeaef..9677c72f0f 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -19,7 +19,13 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController end def render_success - if skip_authorization? || (matching_token? && !truthy_param?('force_login')) + # FIXME: Find a better way to apply this validation: if the scopes only + # includes offline_access, then it's not valid, since offline_access doesn't + # actually give access to resources: + if pre_auth.scopes.all?('offline_access') + error = Doorkeeper::OAuth::InvalidRequestResponse.new(reason: :offline_access_only, missing_param: nil) + render :error, locals: { error_response: error }, status: 400 + elsif skip_authorization? || (matching_token? && !truthy_param?('force_login')) redirect_or_render authorize_response elsif Doorkeeper.configuration.api_only render json: pre_auth From 1af6ae19b94e46dca52a01b3834aa646d472fa3a Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 10 Apr 2025 21:57:25 +0200 Subject: [PATCH 07/13] Add offline_access scope --- app/lib/scope_transformer.rb | 2 ++ config/initializers/doorkeeper.rb | 9 +++++---- config/locales/doorkeeper.en.yml | 3 +++ spec/lib/scope_transformer_spec.rb | 6 ++++++ 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/lib/scope_transformer.rb b/app/lib/scope_transformer.rb index 7dda709229..79498cfdef 100644 --- a/app/lib/scope_transformer.rb +++ b/app/lib/scope_transformer.rb @@ -14,6 +14,8 @@ class ScopeTransformer < Parslet::Transform # # override for profile scope which is read only @access = %w(read) if @term == 'profile' + # Override offline_access since it doesn't imply read or write access: + @access = %w(offline) if @term == 'offline_access' end def key diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index d7084647ff..25ddee4659 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -35,10 +35,10 @@ Doorkeeper.configure do # context.client for client (Doorkeeper::Application) # context.scopes for scopes custom_access_token_expires_in do |context| - # If the client is confidential (all clients pre 4.3), then we don't want to - # expire access tokens. Applications created by users are also considered - # confidential. - if context.client.confidential? + # If the client is confidential (all clients pre 4.3) and it hasn't + # requested offline_access, then we don't want to expire access tokens. + # Applications created by users are also considered confidential. + if context.client.confidential? && !context.scopes.exists?('offline_access') nil else 15.minutes.to_i @@ -80,6 +80,7 @@ Doorkeeper.configure do # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes default_scopes :read optional_scopes :profile, + :offline_access, :write, :'write:accounts', :'write:blocks', diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 3b3b141afa..da48ef97db 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -89,6 +89,7 @@ en: invalid_request: missing_param: 'Missing required parameter: %{value}.' request_not_authorized: Request need to be authorized. Required parameter for authorizing request is missing or invalid. + offline_access_only: The offline_access scope can only be used with other scopes. unknown: The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed. invalid_resource_owner: The provided resource owner credentials are not valid, or resource owner cannot be found invalid_scope: The requested scope is invalid, unknown, or malformed. @@ -118,6 +119,7 @@ en: read: Read-only access read/write: Read and write access write: Write-only access + offline: Access for an extended period of time title: accounts: Accounts admin/accounts: Administration of accounts @@ -138,6 +140,7 @@ en: notifications: Notifications profile: Your Mastodon profile push: Push notifications + offline_access: Offline access reports: Reports search: Search statuses: Posts diff --git a/spec/lib/scope_transformer_spec.rb b/spec/lib/scope_transformer_spec.rb index f4003352e4..e3e4932250 100644 --- a/spec/lib/scope_transformer_spec.rb +++ b/spec/lib/scope_transformer_spec.rb @@ -23,6 +23,12 @@ RSpec.describe ScopeTransformer do it_behaves_like 'a scope', nil, 'profile', 'read' end + context 'with scope "offline_access"' do + let(:input) { 'offline_access' } + + it_behaves_like 'a scope', nil, 'offline_access', 'offline' + end + context 'with scope "read"' do let(:input) { 'read' } From 463d5dd4d5eee9fdc62cf95a4c1f227e399aabb2 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Fri, 11 Apr 2025 22:10:03 +0200 Subject: [PATCH 08/13] Try to fix the usage of doorkeeper configuration --- app/models/session_activation.rb | 20 ++++++++++++++----- app/models/user.rb | 10 ++++++---- app/services/app_sign_up_service.rb | 12 ++++++----- config/initializers/doorkeeper.rb | 4 ++++ .../oauth/authorizations_controller_spec.rb | 17 +++++++++++----- 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/app/models/session_activation.rb b/app/models/session_activation.rb index d99ecf8adb..af686a0a74 100644 --- a/app/models/session_activation.rb +++ b/app/models/session_activation.rb @@ -65,12 +65,22 @@ class SessionActivation < ApplicationRecord end def access_token_attributes + app = Doorkeeper::Application.find_by(superapp: true) + scopes = Doorkeeper::OAuth::Scopes.from_array(DEFAULT_SCOPES) + + context = Doorkeeper::OAuth::Authorization::Token.build_context( + app, + Doorkeeper::OAuth::AUTHORIZATION_CODE, + scopes, + user_id + ) + { - application_id: Doorkeeper::Application.find_by(superapp: true)&.id, - resource_owner_id: user_id, - scopes: DEFAULT_SCOPES.join(' '), - expires_in: Doorkeeper.configuration.access_token_expires_in, - use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?, + application_id: context.client, + resource_owner_id: context.resource_owner, + scopes: context.scopes, + expires_in: Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context), + use_refresh_token: Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context), } end end diff --git a/app/models/user.rb b/app/models/user.rb index 72f7490043..3de3974bef 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -310,10 +310,12 @@ class User < ApplicationRecord def token_for_app(app) return nil if app.nil? || app.owner != self - Doorkeeper::AccessToken.find_or_create_by(application_id: app.id, resource_owner_id: id) do |t| - t.scopes = app.scopes - t.expires_in = Doorkeeper.configuration.access_token_expires_in - t.use_refresh_token = Doorkeeper.configuration.refresh_token_enabled? + context = Doorkeeper::OAuth::Authorization::Token.build_context(app, Doorkeeper::OAuth::AUTHORIZATION_CODE, app.scopes, app.owner) + + Doorkeeper::AccessToken.find_or_create_by(application_id: context.client.id, resource_owner_id: context.resource_owner.id) do |t| + t.scopes = context.scopes + t.expires_in = Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context) + t.use_refresh_token = Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context) end end diff --git a/app/services/app_sign_up_service.rb b/app/services/app_sign_up_service.rb index a4399efd65..a53c143e28 100644 --- a/app/services/app_sign_up_service.rb +++ b/app/services/app_sign_up_service.rb @@ -27,12 +27,14 @@ class AppSignUpService < BaseService end def create_access_token! + context = Doorkeeper::OAuth::Authorization::Token.build_context(@app, Doorkeeper::OAuth::AUTHORIZATION_CODE, @app.scopes, @user.id) + @access_token = Doorkeeper::AccessToken.create!( - application: @app, - resource_owner_id: @user.id, - scopes: @app.scopes, - expires_in: Doorkeeper.configuration.access_token_expires_in, - use_refresh_token: Doorkeeper.configuration.refresh_token_enabled? + application: context.client, + resource_owner_id: context.resource_owner, + scopes: context.scopes, + expires_in: Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context), + use_refresh_token: Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context) ) end diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 25ddee4659..ce4d93b837 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -45,6 +45,10 @@ Doorkeeper.configure do end end + use_refresh_token do |context| + context.scopes.exists?('offline_access') + end + # Use a custom class for generating the access token. # https://github.com/doorkeeper-gem/doorkeeper#custom-access-token-generator # access_token_generator "::Doorkeeper::JWT" diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index cfc80b8650..b9e2b64c37 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -30,12 +30,19 @@ RSpec.describe Oauth::AuthorizationsController do context 'when app is already authorized' do before do + context = Doorkeeper::OAuth::Authorization::Token.build_context( + app, + Doorkeeper::OAuth::AUTHORIZATION_CODE, + app.scopes, + user.id + ) + Doorkeeper::AccessToken.find_or_create_for( - application: app, - resource_owner: user.id, - scopes: app.scopes, - expires_in: Doorkeeper.configuration.access_token_expires_in, - use_refresh_token: Doorkeeper.configuration.refresh_token_enabled? + application: context.client, + resource_owner: context.resource_owner, + scopes: context.scopes, + expires_in: Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context), + use_refresh_token: Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context) ) end From fcd238cb4b2f173f29749e86023fd98bd80ebaaf Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 16 Apr 2025 18:08:00 +0200 Subject: [PATCH 09/13] Fix issues with null appearing for user owned access tokens --- app/models/session_activation.rb | 2 +- app/models/user.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/session_activation.rb b/app/models/session_activation.rb index af686a0a74..400ab1d146 100644 --- a/app/models/session_activation.rb +++ b/app/models/session_activation.rb @@ -76,7 +76,7 @@ class SessionActivation < ApplicationRecord ) { - application_id: context.client, + application_id: context.client.id, resource_owner_id: context.resource_owner, scopes: context.scopes, expires_in: Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context), diff --git a/app/models/user.rb b/app/models/user.rb index 3de3974bef..bff65d1749 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -310,9 +310,9 @@ class User < ApplicationRecord def token_for_app(app) return nil if app.nil? || app.owner != self - context = Doorkeeper::OAuth::Authorization::Token.build_context(app, Doorkeeper::OAuth::AUTHORIZATION_CODE, app.scopes, app.owner) + context = Doorkeeper::OAuth::Authorization::Token.build_context(app, Doorkeeper::OAuth::AUTHORIZATION_CODE, app.scopes, app.owner.id) - Doorkeeper::AccessToken.find_or_create_by(application_id: context.client.id, resource_owner_id: context.resource_owner.id) do |t| + Doorkeeper::AccessToken.find_or_create_by(application_id: context.client.id, resource_owner_id: context.resource_owner) do |t| t.scopes = context.scopes t.expires_in = Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context) t.use_refresh_token = Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context) From 9e0eb997474724491329bebf812d3a35ee5c5841 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 16 Apr 2025 22:07:30 +0200 Subject: [PATCH 10/13] Change /oauth/token request specs to use client_secret_basic authentication --- spec/requests/oauth/token_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/requests/oauth/token_spec.rb b/spec/requests/oauth/token_spec.rb index 74f301c577..04e3d3fc88 100644 --- a/spec/requests/oauth/token_spec.rb +++ b/spec/requests/oauth/token_spec.rb @@ -5,17 +5,19 @@ require 'rails_helper' RSpec.describe 'Managing OAuth Tokens' do describe 'POST /oauth/token' do subject do - post '/oauth/token', params: params + post '/oauth/token', params: params, headers: { + # This is using the OAuth client_secret_basic client authentication method + Authorization: ActionController::HttpAuthentication::Basic.encode_credentials(application.uid, application.secret), + } end let(:application) do Fabricate(:application, scopes: 'read write follow', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob') 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 81b79fefb7f9d00380728ebced82ee601a636c88 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 16 Apr 2025 22:08:04 +0200 Subject: [PATCH 11/13] WIP: refresh tokens --- config/initializers/doorkeeper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index ce4d93b837..c3e6503a4b 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -181,7 +181,7 @@ Doorkeeper.configure do # http://tools.ietf.org/html/rfc6819#section-4.4.3 # - grant_flows %w(authorization_code client_credentials) + grant_flows %w(authorization_code client_credentials refresh_token) # If the client is not a confidential client, it should not be able to use the # client_credentials grant flow, since it cannot keep a secret. From 677e0b8a370d1f4ecbacd7deab0f74736afe6367 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Wed, 16 Apr 2025 23:37:48 +0200 Subject: [PATCH 12/13] Add 'none' to token_endpoint_auth_methods_supported to indicate public client support --- app/presenters/oauth_metadata_presenter.rb | 2 +- spec/requests/well_known/oauth_metadata_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/presenters/oauth_metadata_presenter.rb b/app/presenters/oauth_metadata_presenter.rb index f488a62925..3925cfb951 100644 --- a/app/presenters/oauth_metadata_presenter.rb +++ b/app/presenters/oauth_metadata_presenter.rb @@ -61,7 +61,7 @@ class OauthMetadataPresenter < ActiveModelSerializers::Model end def token_endpoint_auth_methods_supported - %w(client_secret_basic client_secret_post) + %w(none client_secret_basic client_secret_post) end def code_challenge_methods_supported diff --git a/spec/requests/well_known/oauth_metadata_spec.rb b/spec/requests/well_known/oauth_metadata_spec.rb index 42a6c1b328..30c8a0b4bd 100644 --- a/spec/requests/well_known/oauth_metadata_spec.rb +++ b/spec/requests/well_known/oauth_metadata_spec.rb @@ -25,7 +25,7 @@ RSpec.describe 'The /.well-known/oauth-authorization-server request' do scopes_supported: Doorkeeper.configuration.scopes.map(&:to_s), response_types_supported: Doorkeeper.configuration.authorization_response_types, response_modes_supported: Doorkeeper.configuration.authorization_response_flows.flat_map(&:response_mode_matches).uniq, - token_endpoint_auth_methods_supported: %w(client_secret_basic client_secret_post), + token_endpoint_auth_methods_supported: %w(none client_secret_basic client_secret_post), grant_types_supported: grant_types_supported, code_challenge_methods_supported: Doorkeeper.configuration.pkce_code_challenge_methods_supported, # non-standard extension: From a2a34fbadda92e01349d574738ea8e0bac0bfdd0 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 17 Apr 2025 20:17:38 +0200 Subject: [PATCH 13/13] Migrate Web::PushSubscription and SessionActivation records to the new access token after refresh --- config/initializers/doorkeeper.rb | 7 ++++ spec/fabricators/application_fabricator.rb | 2 +- spec/requests/oauth/token_spec.rb | 47 ++++++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index c3e6503a4b..bf40143de9 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -49,6 +49,13 @@ Doorkeeper.configure do context.scopes.exists?('offline_access') end + after_successful_strategy_response do |request, _response| + if request.is_a? Doorkeeper::OAuth::RefreshTokenRequest + Web::PushSubscription.where(access_token_id: request.refresh_token.id).update!(access_token_id: request.access_token.id) + SessionActivation.where(access_token_id: request.refresh_token.id).update!(access_token_id: request.access_token.id) + end + end + # Use a custom class for generating the access token. # https://github.com/doorkeeper-gem/doorkeeper#custom-access-token-generator # access_token_generator "::Doorkeeper::JWT" diff --git a/spec/fabricators/application_fabricator.rb b/spec/fabricators/application_fabricator.rb index 272821304c..072f355bfa 100644 --- a/spec/fabricators/application_fabricator.rb +++ b/spec/fabricators/application_fabricator.rb @@ -3,5 +3,5 @@ Fabricator(:application, from: Doorkeeper::Application) do name 'Example' website 'http://example.com' - redirect_uri 'http://example.com/callback' + redirect_uri 'urn:ietf:wg:oauth:2.0:oob' end diff --git a/spec/requests/oauth/token_spec.rb b/spec/requests/oauth/token_spec.rb index 04e3d3fc88..27bbbe7ac5 100644 --- a/spec/requests/oauth/token_spec.rb +++ b/spec/requests/oauth/token_spec.rb @@ -105,6 +105,53 @@ RSpec.describe 'Managing OAuth Tokens' do end end end + + context "with grant_type 'refresh_token'" do + let(:grant_type) { 'refresh_token' } + + let!(:user) { Fabricate(:user) } + let!(:application) { Fabricate(:application, scopes: 'read offline_access') } + let!(:access_token) do + Fabricate( + :accessible_access_token, + resource_owner_id: user.id, + application: application, + # Even though the `application` uses the `offline_access` scope, the + # generation of a refresh token only happens when the model is created + # with `use_refresh_token: true`. + # + # This is normally determined from the request to create the access + # token, but here we are just creating the access token model, so we + # need to force the `access_token` to have `use_refresh_token: true` + use_refresh_token: true + ) + end + + let!(:web_push_subscription) { Fabricate(:web_push_subscription, user: user, access_token: access_token) } + + let(:params) do + { + grant_type: grant_type, + refresh_token: access_token.refresh_token, + } + end + + it 'updates the Web::PushSubscription when refreshed' do + expect { subject } + .to change { access_token.reload.revoked_at }.from(nil).to(be_present) + + expect(response).to have_http_status(200) + + new_token = Doorkeeper::AccessToken.by_token(response.parsed_body[:access_token]) + + expect(web_push_subscription.reload.access_token_id).to eq(new_token.id) + + # Assert that there are definitely no subscriptions left for the + # previous access token: + expect(Web::PushSubscription.where(access_token: access_token.id).count) + .to eq(0) + end + end end describe 'POST /oauth/revoke' do