From 2068675173cf5bae44524ff06baed5ae64994ca4 Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Mon, 20 Apr 2026 18:11:54 +1000 Subject: [PATCH] Collapse auth-code replay revocation to two update_all queries The previous implementation iterated find_each(&:revoke!) on both the access-token and refresh-token associations. OidcAccessToken#revoke! also cascades to its refresh tokens, so a chain of N access tokens with their refresh tokens produced ~3N UPDATEs (outer loop + cascade + outer refresh loop double-writing) all while holding a pessimistic lock on the auth_code row. Replace with scoped update_all on each association -- 2 UPDATEs total, no behavior change. Also hoist the repeated refresh_token_record.oidc_authorization_code lookup in the rotation path to a named local and drop the duplicated inline comment. Co-Authored-By: Claude Opus 4 --- app/controllers/oidc_controller.rb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index 9d63567..412747e 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -510,15 +510,12 @@ class OidcController < ApplicationController # Check if code has already been used (CRITICAL: check AFTER locking) if auth_code.used? - # Per OAuth 2.0 spec, if an auth code is reused, revoke all tokens issued from it + # Per OAuth 2.0 spec, if an auth code is reused, revoke every token + # descended from it (both generations across any rotations). Rails.logger.warn "OAuth Security: Authorization code reuse detected for code #{auth_code.id}" - - # Revoke the entire token chain derived from this code. revoke! sets - # revoked_at and cascades from each access token to its refresh tokens. - # Iterating refresh tokens directly is defensive in case rotation ever - # leaves an access token without its linked refresh token. - auth_code.oidc_access_tokens.find_each(&:revoke!) - auth_code.oidc_refresh_tokens.find_each(&:revoke!) + now = Time.current + auth_code.oidc_access_tokens.where(revoked_at: nil).update_all(revoked_at: now) + auth_code.oidc_refresh_tokens.where(revoked_at: nil).update_all(revoked_at: now) render json: { error: "invalid_grant", @@ -693,11 +690,15 @@ class OidcController < ApplicationController refresh_token_record.revoke! # Generate new access token record (opaque token with BCrypt hashing) + # Carry the authorization-code FK forward across rotations so replay + # revocation reaches every descendant token in the chain. + issuing_auth_code = refresh_token_record.oidc_authorization_code + new_access_token = OidcAccessToken.create!( application: application, user: user, scope: refresh_token_record.scope, - oidc_authorization_code: refresh_token_record.oidc_authorization_code # Carry FK so replay revocation catches rotated tokens + oidc_authorization_code: issuing_auth_code ) # Generate new refresh token (token rotation) @@ -705,7 +706,7 @@ class OidcController < ApplicationController application: application, user: user, oidc_access_token: new_access_token, - oidc_authorization_code: refresh_token_record.oidc_authorization_code, # Carry FK so replay revocation catches rotated tokens + oidc_authorization_code: issuing_auth_code, scope: refresh_token_record.scope, token_family_id: refresh_token_record.token_family_id, # Keep same family for rotation tracking auth_time: refresh_token_record.auth_time, # Carry over original auth_time