diff --git a/app/controllers/auth/authorities_controller.rb b/app/controllers/auth/authorities_controller.rb index d5dc01b..2510b89 100644 --- a/app/controllers/auth/authorities_controller.rb +++ b/app/controllers/auth/authorities_controller.rb @@ -15,7 +15,6 @@ def current begin access_token = doorkeeper_token if access_token - access_token.revoke_previous_refresh_token! auth[:token_valid] = true configure_asset_access else diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 81f79e3..bbb8f36 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -119,7 +119,23 @@ access_token_generator "::Doorkeeper::JWT" force_ssl_in_redirect_uri false - grant_flows %w[authorization_code client_credentials implicit password implicit_oidc] + + # Require non-confidential clients to use PKCE (RFC 7636) on + # authorization_code grants. Public clients without a code_verifier are + # rejected by Doorkeeper at the /oauth/token exchange. + force_pkce + + # Drop the OAuth 2.0 implicit grant (`response_type=token`). OAuth 2.1 + # §2.1.2 removes it; clients should use authorization_code with PKCE. + # + # NOTE: we must NOT use the `implicit_oidc` alias here. doorkeeper-openid_connect + # registers it as an alias for `["implicit", "id_token", "id_token token"]` + # (openid_connect.rb:83), and Doorkeeper's calculate_grant_flows expands + # aliases into the effective flow set — so listing `implicit_oidc` would + # silently re-enable the bare `implicit` flow and keep `response_type=token` + # working. Instead we list the OpenID Connect identity-token flows + # explicitly, which drops `implicit` while retaining OIDC id_token support. + grant_flows ["authorization_code", "client_credentials", "password", "id_token", "id_token token"] end Doorkeeper::JWT.configure do diff --git a/db/migrate/20260518095026_remove_previous_refresh_token_from_oauth_access_tokens.rb b/db/migrate/20260518095026_remove_previous_refresh_token_from_oauth_access_tokens.rb new file mode 100644 index 0000000..034a6f6 --- /dev/null +++ b/db/migrate/20260518095026_remove_previous_refresh_token_from_oauth_access_tokens.rb @@ -0,0 +1,17 @@ +class RemovePreviousRefreshTokenFromOauthAccessTokens < ActiveRecord::Migration[8.1] + # Dropping `previous_refresh_token` switches Doorkeeper's refresh flow + # from deferred revocation to immediate revocation: once a refresh token + # is used, it's marked revoked before the new pair is returned, so the + # same refresh token can't mint another set of tokens. + # + # Trade-off: there is no longer a grace window for the client to retry + # if it lost the response to the new pair. The accepted behaviour is to + # re-authenticate, in line with RFC 6819 §5.2.2.3. + def up + remove_column :oauth_access_tokens, :previous_refresh_token + end + + def down + add_column :oauth_access_tokens, :previous_refresh_token, :string, null: false, default: "" + end +end diff --git a/lib/omniauth/strategies/generic_oauth.rb b/lib/omniauth/strategies/generic_oauth.rb index aa63365..00150d2 100644 --- a/lib/omniauth/strategies/generic_oauth.rb +++ b/lib/omniauth/strategies/generic_oauth.rb @@ -77,8 +77,11 @@ def set_options(id) options.client_id = strat.client_id options.client_secret = strat.client_secret - # prevent csrf errors - options.provider_ignores_state = true + # Validate the OAuth2 `state` parameter on callback (OmniAuth default). + # Some legacy providers mishandle `state` during multi-factor or + # cross-domain flows; operators can opt out by setting the env var + # `OAUTH_PROVIDER_IGNORES_STATE=true`. + options.provider_ignores_state = ENV["OAUTH_PROVIDER_IGNORES_STATE"] == "true" end def access_token_options