From 122a41b58d734d9eab2915f449c49dc24431afd0 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 17 Oct 2026 02:22:29 -0700 Subject: [PATCH 1/3] Record each redeemed authorization code, so one can't mint again once its grant is gone The only record of a redemption was the grant it minted, which a replay, revocation or disconnecting the app destroys. Within the code's 60 seconds, a third exchange then minted a fresh grant. Redemption now records the code's jti in its own table, in the grant's transaction, and keeps it 25 hours (rubric GP-4, GP-5). A daily job prunes older rows. --- app/models/identity/access_token.rb | 13 ++++--- .../oauth/redeemed_authorization_code.rb | 17 +++++++++ config/recurring.yml | 3 ++ ...eate_oauth_redeemed_authorization_codes.rb | 14 ++++++++ db/schema.rb | 9 ++++- db/schema_sqlite.rb | 9 ++++- docs/oauth/spec_conformance.md | 2 +- test/integration/oauth_flow_test.rb | 36 +++++++++++++++++++ .../oauth/redeemed_authorization_code_test.rb | 21 +++++++++++ 9 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 app/models/oauth/redeemed_authorization_code.rb create mode 100644 db/migrate/20261017140000_create_oauth_redeemed_authorization_codes.rb create mode 100644 test/models/oauth/redeemed_authorization_code_test.rb diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 8d4199f213..38dde5c28d 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -126,11 +126,16 @@ def cleanup end end - # An authorization code redeems at most once (RFC 6749 §4.1.2): the grant it - # mints is stamped with the code's jti, and the unique index refuses a second - # stamp. A replay also revokes the grant the first redemption issued. + # An authorization code redeems at most once (RFC 6749 §4.1.2). Redeeming + # records its jti (Oauth::RedeemedAuthorizationCode) in the same transaction + # as the grant it mints, and the unique index refuses a second record. A + # replay also revokes the grant the first redemption issued, and the record + # outlives that grant, so a third attempt mints nothing either. def redeem(authorization_code, **attributes) - create! authorization_code_jti: authorization_code.jti, **attributes + transaction do + Oauth::RedeemedAuthorizationCode.create! jti: authorization_code.jti + create! authorization_code_jti: authorization_code.jti, **attributes + end rescue ActiveRecord::RecordNotUnique where(authorization_code_jti: authorization_code.jti).destroy_all nil diff --git a/app/models/oauth/redeemed_authorization_code.rb b/app/models/oauth/redeemed_authorization_code.rb new file mode 100644 index 0000000000..85e0d2afed --- /dev/null +++ b/app/models/oauth/redeemed_authorization_code.rb @@ -0,0 +1,17 @@ +# An authorization code that has been redeemed, by its jti. Codes are +# stateless, and the grant a code minted carries its jti too, but that grant +# can be destroyed: by a replay of the code, by revocation, by disconnecting +# the app. This record outlives it, so the code still can't be redeemed again +# for as long as it could otherwise be presented (RFC 6749 §4.1.2; rubric +# GP-4, GP-5). +class Oauth::RedeemedAuthorizationCode < ApplicationRecord + # Well past the code's 60-second lifetime, as the rubric asks (24 hours past + # expiry), so a skewed clock or a slow replay is still recognized. + RETENTION = 25.hours + + scope :stale, -> { where(created_at: ...RETENTION.ago) } + + def self.cleanup + stale.in_batches.delete_all + end +end diff --git a/config/recurring.yml b/config/recurring.yml index 3929a4d39b..c84189393a 100644 --- a/config/recurring.yml +++ b/config/recurring.yml @@ -36,6 +36,9 @@ production: &production cleanup_lapsed_oauth_grants: command: "Identity::AccessToken.cleanup" schedule: every day at 04:32 + cleanup_oauth_redeemed_authorization_codes: + command: "Oauth::RedeemedAuthorizationCode.cleanup" + schedule: every day at 04:42 cleanup_exports: command: "Export.cleanup" schedule: every hour at minute 20 diff --git a/db/migrate/20261017140000_create_oauth_redeemed_authorization_codes.rb b/db/migrate/20261017140000_create_oauth_redeemed_authorization_codes.rb new file mode 100644 index 0000000000..7dafaf29e4 --- /dev/null +++ b/db/migrate/20261017140000_create_oauth_redeemed_authorization_codes.rb @@ -0,0 +1,14 @@ +# A record of every authorization code redeemed, kept past the code's own +# lifetime, so a code can't be redeemed again once the grant it minted is gone +# (revoked by a replay, by the person, or by its app). +class CreateOauthRedeemedAuthorizationCodes < ActiveRecord::Migration[8.2] + def change + create_table :oauth_redeemed_authorization_codes, id: :uuid do |t| + t.string :jti, null: false, limit: 36 + t.datetime :created_at, null: false + + t.index :jti, unique: true + t.index :created_at + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 84cc4c91b7..3ac374fabc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2026_10_15_120200) do +ActiveRecord::Schema[8.2].define(version: 2026_10_17_140000) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -454,6 +454,13 @@ t.index ["client_id"], name: "index_oauth_clients_on_client_id", unique: true end + create_table "oauth_redeemed_authorization_codes", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.datetime "created_at", null: false + t.string "jti", limit: 36, null: false + t.index ["created_at"], name: "index_oauth_redeemed_authorization_codes_on_created_at" + t.index ["jti"], name: "index_oauth_redeemed_authorization_codes_on_jti", unique: true + end + create_table "oauth_retired_refresh_tokens", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.uuid "access_token_id", null: false t.datetime "created_at", null: false diff --git a/db/schema_sqlite.rb b/db/schema_sqlite.rb index 1033aa3dc9..16399330f9 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2026_10_15_120200) do +ActiveRecord::Schema[8.2].define(version: 2026_10_17_140000) do create_table "accesses", id: :uuid, force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -454,6 +454,13 @@ t.index ["client_id"], name: "index_oauth_clients_on_client_id", unique: true end + create_table "oauth_redeemed_authorization_codes", id: :uuid, force: :cascade do |t| + t.string "jti", limit: 36, null: false + t.datetime "created_at", null: false + t.index ["created_at"], name: "index_oauth_redeemed_authorization_codes_on_created_at" + t.index ["jti"], name: "index_oauth_redeemed_authorization_codes_on_jti", unique: true + end + create_table "oauth_retired_refresh_tokens", id: :uuid, force: :cascade do |t| t.uuid "access_token_id", null: false t.string "refresh_token_digest", limit: 80, null: false diff --git a/docs/oauth/spec_conformance.md b/docs/oauth/spec_conformance.md index 856dcffc5d..c7cc64ba22 100644 --- a/docs/oauth/spec_conformance.md +++ b/docs/oauth/spec_conformance.md @@ -65,7 +65,7 @@ Status key: | Requirement | Status | |---|---| -| Authorization codes are single-use; reuse is refused and the grant minted from that code is revoked (§4.1.2, §10.5; OAuth 2.1 §4.1.3) | **Fixed** #2296 `bd447dd7a` "Redeem each authorization code once…". A `jti` in the code plus a unique `identity_access_tokens.authorization_code_jti`. Reuse is checked only after PKCE, `redirect_uri` and `client_id` pass, so a leaked code without its verifier can't revoke the grant. Limit: if the grant is revoked within the code's 60s life, the row is gone and one more redemption is possible. No tombstones were added. | +| Authorization codes are single-use; reuse is refused and the grant minted from that code is revoked (§4.1.2, §10.5; OAuth 2.1 §4.1.3) | **Fixed** #2296 `bd447dd7a` "Redeem each authorization code once…". A `jti` in the code plus a unique `identity_access_tokens.authorization_code_jti`. Reuse is checked only after PKCE, `redirect_uri` and `client_id` pass, so a leaked code without its verifier can't revoke the grant. Since #PRNUM, each redemption also records the code's `jti` in `oauth_redeemed_authorization_codes`, in the same transaction as the grant, kept 25 hours (rubric GP-4, GP-5: at least 24 hours past expiry). So once a replay, the person or the app has revoked the grant, the code still mints nothing. | | `client_id` required and matched to the code for public clients (§4.1.3) | **Fixed** #2296 `bd447dd7a`; one check shared by both grants in #3081 `f9a1885ff` | | Missing or malformed parameters answer `invalid_request`, not `invalid_grant` or `unsupported_grant_type` (§5.2) | **Fixed** #2296 `bd447dd7a`, `ac884df5f` "Reject non-string token request parameters as malformed" (an array `refresh_token` had been accepted as an `IN` list; a hash had raised a 500); #3081 `f9a1885ff`, `2b2b7f2d6` | | Exact `redirect_uri` equality with the code (§4.1.3) | **Conformant** | diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index bcf3b6851f..144874c32f 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -778,6 +778,42 @@ class OauthFlowTest < ActionDispatch::IntegrationTest assert_response :success end + test "a code replayed again after its grant was revoked mints nothing" do + code_verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + client = oauth_clients(:mcp_client) + exchange = { grant_type: "authorization_code", client_id: client.client_id, code: authorization_code_for(client, code_verifier: code_verifier), + redirect_uri: "http://127.0.0.1:8888/callback", code_verifier: code_verifier } + + untenanted { post oauth_token_path, params: exchange, as: :json } + assert_response :success + + untenanted { post oauth_token_path, params: exchange, as: :json } + assert_equal "invalid_grant", response.parsed_body["error"] + + assert_no_difference "Identity::AccessToken.count" do + untenanted { post oauth_token_path, params: exchange, as: :json } + end + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + + test "a code can't be redeemed again once the person disconnects the app" do + code_verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + client = oauth_clients(:mcp_client) + exchange = { grant_type: "authorization_code", client_id: client.client_id, code: authorization_code_for(client, code_verifier: code_verifier), + redirect_uri: "http://127.0.0.1:8888/callback", code_verifier: code_verifier } + + untenanted { post oauth_token_path, params: exchange, as: :json } + assert_response :success + identities(:david).access_tokens.oauth.where(oauth_client: client).destroy_all + + assert_no_difference "Identity::AccessToken.count" do + untenanted { post oauth_token_path, params: exchange, as: :json } + end + assert_response :bad_request + assert_equal "invalid_grant", response.parsed_body["error"] + end + test "a replayed code that fails PKCE does not revoke the grant" do code_verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" client = oauth_clients(:mcp_client) diff --git a/test/models/oauth/redeemed_authorization_code_test.rb b/test/models/oauth/redeemed_authorization_code_test.rb new file mode 100644 index 0000000000..793243bece --- /dev/null +++ b/test/models/oauth/redeemed_authorization_code_test.rb @@ -0,0 +1,21 @@ +require "test_helper" + +class Oauth::RedeemedAuthorizationCodeTest < ActiveSupport::TestCase + test "a jti is recorded once" do + Oauth::RedeemedAuthorizationCode.create!(jti: "a5f1c0de-0000-4000-8000-000000000001") + + assert_raises(ActiveRecord::RecordNotUnique) do + Oauth::RedeemedAuthorizationCode.create!(jti: "a5f1c0de-0000-4000-8000-000000000001") + end + end + + test "cleanup keeps records well past the code's lifetime, then removes them" do + kept = travel_to(24.hours.ago) { Oauth::RedeemedAuthorizationCode.create!(jti: "a5f1c0de-0000-4000-8000-000000000002") } + stale = travel_to(26.hours.ago) { Oauth::RedeemedAuthorizationCode.create!(jti: "a5f1c0de-0000-4000-8000-000000000003") } + + Oauth::RedeemedAuthorizationCode.cleanup + + assert Oauth::RedeemedAuthorizationCode.exists?(kept.id) + assert_not Oauth::RedeemedAuthorizationCode.exists?(stale.id) + end +end From b45c7dfb443c014463a999a995ca877bced2c343 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 17 Oct 2026 02:23:47 -0700 Subject: [PATCH 2/3] Point the ledger at #3206 --- docs/oauth/spec_conformance.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/oauth/spec_conformance.md b/docs/oauth/spec_conformance.md index c7cc64ba22..0297d92f4b 100644 --- a/docs/oauth/spec_conformance.md +++ b/docs/oauth/spec_conformance.md @@ -65,7 +65,7 @@ Status key: | Requirement | Status | |---|---| -| Authorization codes are single-use; reuse is refused and the grant minted from that code is revoked (§4.1.2, §10.5; OAuth 2.1 §4.1.3) | **Fixed** #2296 `bd447dd7a` "Redeem each authorization code once…". A `jti` in the code plus a unique `identity_access_tokens.authorization_code_jti`. Reuse is checked only after PKCE, `redirect_uri` and `client_id` pass, so a leaked code without its verifier can't revoke the grant. Since #PRNUM, each redemption also records the code's `jti` in `oauth_redeemed_authorization_codes`, in the same transaction as the grant, kept 25 hours (rubric GP-4, GP-5: at least 24 hours past expiry). So once a replay, the person or the app has revoked the grant, the code still mints nothing. | +| Authorization codes are single-use; reuse is refused and the grant minted from that code is revoked (§4.1.2, §10.5; OAuth 2.1 §4.1.3) | **Fixed** #2296 `bd447dd7a` "Redeem each authorization code once…". A `jti` in the code plus a unique `identity_access_tokens.authorization_code_jti`. Reuse is checked only after PKCE, `redirect_uri` and `client_id` pass, so a leaked code without its verifier can't revoke the grant. Since #3206, each redemption also records the code's `jti` in `oauth_redeemed_authorization_codes`, in the same transaction as the grant, kept 25 hours (rubric GP-4, GP-5: at least 24 hours past expiry). So once a replay, the person or the app has revoked the grant, the code still mints nothing. | | `client_id` required and matched to the code for public clients (§4.1.3) | **Fixed** #2296 `bd447dd7a`; one check shared by both grants in #3081 `f9a1885ff` | | Missing or malformed parameters answer `invalid_request`, not `invalid_grant` or `unsupported_grant_type` (§5.2) | **Fixed** #2296 `bd447dd7a`, `ac884df5f` "Reject non-string token request parameters as malformed" (an array `refresh_token` had been accepted as an `IN` list; a hash had raised a 500); #3081 `f9a1885ff`, `2b2b7f2d6` | | Exact `redirect_uri` equality with the code (§4.1.3) | **Conformant** | From eff0d09a2c271ec030fcd92282d8a9d5908b5de1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 17 Oct 2026 02:57:49 -0700 Subject: [PATCH 3/3] Remember a code redeemed before redemptions were recorded, once it's replayed A grant redeemed by earlier code, or by an old process mid-deploy, has no record. Its first replay's record rolled back with the grant insert that the unique jti refused, so a later replay could mint again. Redemption is now its own savepoint, and a refused one records the code outside it. --- app/models/identity/access_token.rb | 10 +++++++++- test/integration/oauth_flow_test.rb | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/models/identity/access_token.rb b/app/models/identity/access_token.rb index 38dde5c28d..4a016ca8f6 100644 --- a/app/models/identity/access_token.rb +++ b/app/models/identity/access_token.rb @@ -132,16 +132,24 @@ def cleanup # replay also revokes the grant the first redemption issued, and the record # outlives that grant, so a third attempt mints nothing either. def redeem(authorization_code, **attributes) - transaction do + transaction requires_new: true do Oauth::RedeemedAuthorizationCode.create! jti: authorization_code.jti create! authorization_code_jti: authorization_code.jti, **attributes end rescue ActiveRecord::RecordNotUnique where(authorization_code_jti: authorization_code.jti).destroy_all + remember_redeemed authorization_code nil end private + # A code redeemed before redemptions were recorded has a grant but no + # record, and the failed transaction above rolled back the one it tried + # to write. Write it now, so the code mints nothing once that grant is gone. + def remember_redeemed(authorization_code) + Oauth::RedeemedAuthorizationCode.create_or_find_by!(jti: authorization_code.jti) + end + # MySQL compares `token` case-insensitively, so a match is confirmed # byte for byte: a differently cased token neither authenticates nor # installs a digest that the real token would then never match. diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index 144874c32f..a56a461431 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -797,6 +797,23 @@ class OauthFlowTest < ActionDispatch::IntegrationTest assert_equal "invalid_grant", response.parsed_body["error"] end + test "a code redeemed before redemptions were recorded is remembered when it's replayed" do + code_verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + client = oauth_clients(:mcp_client) + code = authorization_code_for(client, code_verifier: code_verifier) + exchange = { grant_type: "authorization_code", client_id: client.client_id, code: code, + redirect_uri: "http://127.0.0.1:8888/callback", code_verifier: code_verifier } + identities(:david).access_tokens.create!(oauth_client: client, authorization_code_jti: Oauth::AuthorizationCode.parse(code).jti) + + untenanted { post oauth_token_path, params: exchange, as: :json } + assert_equal "invalid_grant", response.parsed_body["error"] + + assert_no_difference "Identity::AccessToken.count" do + untenanted { post oauth_token_path, params: exchange, as: :json } + end + assert_equal "invalid_grant", response.parsed_body["error"] + end + test "a code can't be redeemed again once the person disconnects the app" do code_verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" client = oauth_clients(:mcp_client)