Revoke full token chain on OIDC authorization-code replay
The replay handler previously used a created_at time-range filter to target access tokens and called update_all(expires_at:), which left revoked_at nil, skipped refresh tokens entirely, and could miss or falsely catch tokens from concurrent flows. Add an oidc_authorization_code FK on both token tables, carry it through refresh-token rotation, and use the association to revoke every descendant via revoke! (which sets revoked_at and cascades access -> refresh). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
This commit is contained in:
@@ -513,12 +513,12 @@ class OidcController < ApplicationController
|
|||||||
# 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 all tokens issued from it
|
||||||
Rails.logger.warn "OAuth Security: Authorization code reuse detected for code #{auth_code.id}"
|
Rails.logger.warn "OAuth Security: Authorization code reuse detected for code #{auth_code.id}"
|
||||||
|
|
||||||
# Revoke all access tokens issued from this authorization code
|
# Revoke the entire token chain derived from this code. revoke! sets
|
||||||
OidcAccessToken.where(
|
# revoked_at and cascades from each access token to its refresh tokens.
|
||||||
application: application,
|
# Iterating refresh tokens directly is defensive in case rotation ever
|
||||||
user: auth_code.user,
|
# leaves an access token without its linked refresh token.
|
||||||
created_at: auth_code.created_at..Time.current
|
auth_code.oidc_access_tokens.find_each(&:revoke!)
|
||||||
).update_all(expires_at: Time.current)
|
auth_code.oidc_refresh_tokens.find_each(&:revoke!)
|
||||||
|
|
||||||
render json: {
|
render json: {
|
||||||
error: "invalid_grant",
|
error: "invalid_grant",
|
||||||
@@ -559,7 +559,8 @@ class OidcController < ApplicationController
|
|||||||
access_token_record = OidcAccessToken.create!(
|
access_token_record = OidcAccessToken.create!(
|
||||||
application: application,
|
application: application,
|
||||||
user: user,
|
user: user,
|
||||||
scope: auth_code.scope
|
scope: auth_code.scope,
|
||||||
|
oidc_authorization_code: auth_code
|
||||||
)
|
)
|
||||||
|
|
||||||
# Generate refresh token (opaque, with hashing)
|
# Generate refresh token (opaque, with hashing)
|
||||||
@@ -567,6 +568,7 @@ class OidcController < ApplicationController
|
|||||||
application: application,
|
application: application,
|
||||||
user: user,
|
user: user,
|
||||||
oidc_access_token: access_token_record,
|
oidc_access_token: access_token_record,
|
||||||
|
oidc_authorization_code: auth_code,
|
||||||
scope: auth_code.scope,
|
scope: auth_code.scope,
|
||||||
auth_time: auth_code.auth_time,
|
auth_time: auth_code.auth_time,
|
||||||
acr: auth_code.acr
|
acr: auth_code.acr
|
||||||
@@ -694,7 +696,8 @@ class OidcController < ApplicationController
|
|||||||
new_access_token = OidcAccessToken.create!(
|
new_access_token = OidcAccessToken.create!(
|
||||||
application: application,
|
application: application,
|
||||||
user: user,
|
user: user,
|
||||||
scope: refresh_token_record.scope
|
scope: refresh_token_record.scope,
|
||||||
|
oidc_authorization_code: refresh_token_record.oidc_authorization_code # Carry FK so replay revocation catches rotated tokens
|
||||||
)
|
)
|
||||||
|
|
||||||
# Generate new refresh token (token rotation)
|
# Generate new refresh token (token rotation)
|
||||||
@@ -702,6 +705,7 @@ class OidcController < ApplicationController
|
|||||||
application: application,
|
application: application,
|
||||||
user: user,
|
user: user,
|
||||||
oidc_access_token: new_access_token,
|
oidc_access_token: new_access_token,
|
||||||
|
oidc_authorization_code: refresh_token_record.oidc_authorization_code, # Carry FK so replay revocation catches rotated tokens
|
||||||
scope: refresh_token_record.scope,
|
scope: refresh_token_record.scope,
|
||||||
token_family_id: refresh_token_record.token_family_id, # Keep same family for rotation tracking
|
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
|
auth_time: refresh_token_record.auth_time, # Carry over original auth_time
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
class OidcAccessToken < ApplicationRecord
|
class OidcAccessToken < ApplicationRecord
|
||||||
belongs_to :application
|
belongs_to :application
|
||||||
belongs_to :user
|
belongs_to :user
|
||||||
|
belongs_to :oidc_authorization_code, optional: true
|
||||||
has_many :oidc_refresh_tokens, dependent: :destroy
|
has_many :oidc_refresh_tokens, dependent: :destroy
|
||||||
|
|
||||||
before_validation :generate_token, on: :create
|
before_validation :generate_token, on: :create
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
class OidcAuthorizationCode < ApplicationRecord
|
class OidcAuthorizationCode < ApplicationRecord
|
||||||
belongs_to :application
|
belongs_to :application
|
||||||
belongs_to :user
|
belongs_to :user
|
||||||
|
has_many :oidc_access_tokens
|
||||||
|
has_many :oidc_refresh_tokens
|
||||||
|
|
||||||
attr_accessor :plaintext_code
|
attr_accessor :plaintext_code
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ class OidcRefreshToken < ApplicationRecord
|
|||||||
belongs_to :application
|
belongs_to :application
|
||||||
belongs_to :user
|
belongs_to :user
|
||||||
belongs_to :oidc_access_token
|
belongs_to :oidc_access_token
|
||||||
|
belongs_to :oidc_authorization_code, optional: true
|
||||||
|
|
||||||
before_validation :generate_token, on: :create
|
before_validation :generate_token, on: :create
|
||||||
before_validation :set_expiry, on: :create
|
before_validation :set_expiry, on: :create
|
||||||
|
|||||||
@@ -0,0 +1,8 @@
|
|||||||
|
class AddOidcAuthorizationCodeIdToTokens < ActiveRecord::Migration[8.1]
|
||||||
|
def change
|
||||||
|
add_reference :oidc_access_tokens, :oidc_authorization_code,
|
||||||
|
null: true, foreign_key: true, index: true
|
||||||
|
add_reference :oidc_refresh_tokens, :oidc_authorization_code,
|
||||||
|
null: true, foreign_key: true, index: true
|
||||||
|
end
|
||||||
|
end
|
||||||
8
db/schema.rb
generated
8
db/schema.rb
generated
@@ -10,7 +10,7 @@
|
|||||||
#
|
#
|
||||||
# It's strongly recommended that you check this file into your version control system.
|
# It's strongly recommended that you check this file into your version control system.
|
||||||
|
|
||||||
ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
|
ActiveRecord::Schema[8.1].define(version: 2026_04_20_073319) do
|
||||||
create_table "active_storage_attachments", force: :cascade do |t|
|
create_table "active_storage_attachments", force: :cascade do |t|
|
||||||
t.bigint "blob_id", null: false
|
t.bigint "blob_id", null: false
|
||||||
t.datetime "created_at", null: false
|
t.datetime "created_at", null: false
|
||||||
@@ -118,6 +118,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
|
|||||||
t.integer "application_id", null: false
|
t.integer "application_id", null: false
|
||||||
t.datetime "created_at", null: false
|
t.datetime "created_at", null: false
|
||||||
t.datetime "expires_at", null: false
|
t.datetime "expires_at", null: false
|
||||||
|
t.integer "oidc_authorization_code_id"
|
||||||
t.datetime "revoked_at"
|
t.datetime "revoked_at"
|
||||||
t.string "scope"
|
t.string "scope"
|
||||||
t.string "token_hmac"
|
t.string "token_hmac"
|
||||||
@@ -126,6 +127,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
|
|||||||
t.index ["application_id", "user_id"], name: "index_oidc_access_tokens_on_application_id_and_user_id"
|
t.index ["application_id", "user_id"], name: "index_oidc_access_tokens_on_application_id_and_user_id"
|
||||||
t.index ["application_id"], name: "index_oidc_access_tokens_on_application_id"
|
t.index ["application_id"], name: "index_oidc_access_tokens_on_application_id"
|
||||||
t.index ["expires_at"], name: "index_oidc_access_tokens_on_expires_at"
|
t.index ["expires_at"], name: "index_oidc_access_tokens_on_expires_at"
|
||||||
|
t.index ["oidc_authorization_code_id"], name: "index_oidc_access_tokens_on_oidc_authorization_code_id"
|
||||||
t.index ["revoked_at"], name: "index_oidc_access_tokens_on_revoked_at"
|
t.index ["revoked_at"], name: "index_oidc_access_tokens_on_revoked_at"
|
||||||
t.index ["token_hmac"], name: "index_oidc_access_tokens_on_token_hmac", unique: true
|
t.index ["token_hmac"], name: "index_oidc_access_tokens_on_token_hmac", unique: true
|
||||||
t.index ["user_id"], name: "index_oidc_access_tokens_on_user_id"
|
t.index ["user_id"], name: "index_oidc_access_tokens_on_user_id"
|
||||||
@@ -162,6 +164,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
|
|||||||
t.datetime "created_at", null: false
|
t.datetime "created_at", null: false
|
||||||
t.datetime "expires_at", null: false
|
t.datetime "expires_at", null: false
|
||||||
t.integer "oidc_access_token_id", null: false
|
t.integer "oidc_access_token_id", null: false
|
||||||
|
t.integer "oidc_authorization_code_id"
|
||||||
t.datetime "revoked_at"
|
t.datetime "revoked_at"
|
||||||
t.string "scope"
|
t.string "scope"
|
||||||
t.integer "token_family_id"
|
t.integer "token_family_id"
|
||||||
@@ -172,6 +175,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
|
|||||||
t.index ["application_id"], name: "index_oidc_refresh_tokens_on_application_id"
|
t.index ["application_id"], name: "index_oidc_refresh_tokens_on_application_id"
|
||||||
t.index ["expires_at"], name: "index_oidc_refresh_tokens_on_expires_at"
|
t.index ["expires_at"], name: "index_oidc_refresh_tokens_on_expires_at"
|
||||||
t.index ["oidc_access_token_id"], name: "index_oidc_refresh_tokens_on_oidc_access_token_id"
|
t.index ["oidc_access_token_id"], name: "index_oidc_refresh_tokens_on_oidc_access_token_id"
|
||||||
|
t.index ["oidc_authorization_code_id"], name: "index_oidc_refresh_tokens_on_oidc_authorization_code_id"
|
||||||
t.index ["revoked_at"], name: "index_oidc_refresh_tokens_on_revoked_at"
|
t.index ["revoked_at"], name: "index_oidc_refresh_tokens_on_revoked_at"
|
||||||
t.index ["token_family_id"], name: "index_oidc_refresh_tokens_on_token_family_id"
|
t.index ["token_family_id"], name: "index_oidc_refresh_tokens_on_token_family_id"
|
||||||
t.index ["token_hmac"], name: "index_oidc_refresh_tokens_on_token_hmac", unique: true
|
t.index ["token_hmac"], name: "index_oidc_refresh_tokens_on_token_hmac", unique: true
|
||||||
@@ -274,11 +278,13 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
|
|||||||
add_foreign_key "application_user_claims", "applications", on_delete: :cascade
|
add_foreign_key "application_user_claims", "applications", on_delete: :cascade
|
||||||
add_foreign_key "application_user_claims", "users", on_delete: :cascade
|
add_foreign_key "application_user_claims", "users", on_delete: :cascade
|
||||||
add_foreign_key "oidc_access_tokens", "applications"
|
add_foreign_key "oidc_access_tokens", "applications"
|
||||||
|
add_foreign_key "oidc_access_tokens", "oidc_authorization_codes"
|
||||||
add_foreign_key "oidc_access_tokens", "users"
|
add_foreign_key "oidc_access_tokens", "users"
|
||||||
add_foreign_key "oidc_authorization_codes", "applications"
|
add_foreign_key "oidc_authorization_codes", "applications"
|
||||||
add_foreign_key "oidc_authorization_codes", "users"
|
add_foreign_key "oidc_authorization_codes", "users"
|
||||||
add_foreign_key "oidc_refresh_tokens", "applications"
|
add_foreign_key "oidc_refresh_tokens", "applications"
|
||||||
add_foreign_key "oidc_refresh_tokens", "oidc_access_tokens"
|
add_foreign_key "oidc_refresh_tokens", "oidc_access_tokens"
|
||||||
|
add_foreign_key "oidc_refresh_tokens", "oidc_authorization_codes"
|
||||||
add_foreign_key "oidc_refresh_tokens", "users"
|
add_foreign_key "oidc_refresh_tokens", "users"
|
||||||
add_foreign_key "oidc_user_consents", "applications"
|
add_foreign_key "oidc_user_consents", "applications"
|
||||||
add_foreign_key "oidc_user_consents", "users"
|
add_foreign_key "oidc_user_consents", "users"
|
||||||
|
|||||||
@@ -846,4 +846,62 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
|
|||||||
old_token_record = OidcRefreshToken.find(refresh_token.id)
|
old_token_record = OidcRefreshToken.find(refresh_token.id)
|
||||||
assert old_token_record.revoked?
|
assert old_token_record.revoked?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "code replay revokes the full token chain including rotated descendants" do
|
||||||
|
OidcUserConsent.create!(
|
||||||
|
user: @user,
|
||||||
|
application: @application,
|
||||||
|
scopes_granted: "openid profile",
|
||||||
|
granted_at: Time.current,
|
||||||
|
sid: "test-sid-chain"
|
||||||
|
)
|
||||||
|
|
||||||
|
auth_code = OidcAuthorizationCode.create!(
|
||||||
|
application: @application,
|
||||||
|
user: @user,
|
||||||
|
redirect_uri: "http://localhost:4000/callback",
|
||||||
|
scope: "openid profile",
|
||||||
|
expires_at: 10.minutes.from_now
|
||||||
|
)
|
||||||
|
|
||||||
|
basic = "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
|
||||||
|
|
||||||
|
# Initial exchange -> A1 + R1
|
||||||
|
post "/oauth/token", params: {
|
||||||
|
grant_type: "authorization_code",
|
||||||
|
code: auth_code.plaintext_code,
|
||||||
|
redirect_uri: "http://localhost:4000/callback"
|
||||||
|
}, headers: {"Authorization" => basic}
|
||||||
|
assert_response :success
|
||||||
|
first_refresh = JSON.parse(@response.body)["refresh_token"]
|
||||||
|
|
||||||
|
# Rotate once -> A2 + R2 (same auth_code FK carried forward)
|
||||||
|
post "/oauth/token", params: {
|
||||||
|
grant_type: "refresh_token",
|
||||||
|
refresh_token: first_refresh
|
||||||
|
}, headers: {"Authorization" => basic}
|
||||||
|
assert_response :success
|
||||||
|
|
||||||
|
# Sanity: the full chain is now linked to the auth_code
|
||||||
|
assert_equal 2, auth_code.oidc_access_tokens.count
|
||||||
|
assert_equal 2, auth_code.oidc_refresh_tokens.count
|
||||||
|
|
||||||
|
# Replay the original code
|
||||||
|
post "/oauth/token", params: {
|
||||||
|
grant_type: "authorization_code",
|
||||||
|
code: auth_code.plaintext_code,
|
||||||
|
redirect_uri: "http://localhost:4000/callback"
|
||||||
|
}, headers: {"Authorization" => basic}
|
||||||
|
assert_response :bad_request
|
||||||
|
|
||||||
|
# Every descendant token must now have revoked_at set
|
||||||
|
auth_code.oidc_access_tokens.each do |token|
|
||||||
|
assert_not_nil token.reload.revoked_at,
|
||||||
|
"access token #{token.id} should have revoked_at set after replay"
|
||||||
|
end
|
||||||
|
auth_code.oidc_refresh_tokens.each do |token|
|
||||||
|
assert_not_nil token.reload.revoked_at,
|
||||||
|
"refresh token #{token.id} should have revoked_at set after replay"
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user