fix(security): harden OAuth flow defaults — PKCE, single-use refresh, drop implicit, validate state

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

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

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

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