Repository navigation
Record each redeemed authorization code, so one can't mint again once its grant is gone - #3206
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🔵 Needs a closer look
A maintainer must confirm integration-branch sequencing and the manual migration against the shared production database before deployment.
No open issues 🎉
What changed in this PR
Prevents redeemed OAuth authorization codes from minting another grant after the original grant is revoked or deleted.
Changes:
- Records redeemed code identifiers atomically with grant creation.
- Retains records for 25 hours, with daily cleanup.
- Adds replay and retention tests and updates conformance documentation.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
| test/models/oauth/redeemed_authorization_code_test.rb | Tests uniqueness and retention. |
| test/integration/oauth_flow_test.rb | Tests replay after revocation and disconnection. |
| docs/oauth/spec_conformance.md | Documents persistent replay protection. |
| db/schema.rb | Adds the MySQL table and indexes. |
| db/schema_sqlite.rb | Adds the SQLite table and indexes. |
| db/migrate/20261017140000_create_oauth_redeemed_authorization_codes.rb | Creates the redemption-record table. |
| config/recurring.yml | Schedules daily cleanup. |
| app/models/oauth/redeemed_authorization_code.rb | Defines retention and batched cleanup. |
| app/models/identity/access_token.rb | Records redemption within the grant transaction. |
Review effort: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6e95b2ecd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dcb914c to
3a2bb79
Compare
d626be9 to
d8ae638
Compare
d8ae638 to
fc836c9
Compare
82cdd19 to
73f186d
Compare
fc836c9 to
8823486
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88234866c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
73f186d to
1b3ab1f
Compare
8823486 to
f000e51
Compare
… 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.
…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.
0af0b6d to
eff0d09
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Restacked onto #3192 |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ping partway * origin/oauth-token-digests: (54 commits) Make this branch's migrations safe to run again after stopping partway Make this branch's migrations safe to run again after stopping partway Make the OAuth migrations safe to run again after stopping partway Redact retired refresh tokens from logged SQL Filter the code parameter by its whole name, so code_challenge stays readable Bring main's tests up to this branch: Ruby 4, webhook attribution, token digests Pin the fizzy_mcp accessory to fizzy-mcp-server v0.1.0 by digest Scope webhook dispatch by account as well as board Test the reviewer audit against the structured log line production writes Refuse an OAuth parameter that arrives as a list where one value is meant Revoke on the email alone, and record each use in the request's structured log Remove the reviewer's access tokens and passkeys on sign-out, so a code holder can't keep a way back in Wire the reviewer sign-in into production from Deploy/Fizzy Close the reviewer's live sockets on sign-out, and keep the identity id out of Sentry End the reviewer's sessions on rotation, so the old code's sessions don't outlive it Refuse a code that normalizes to nothing, not just a blank one Let the directory reviewer sign in with a fixed code, SaaS only and off unless configured Refuse docker-dev's shared default port in a git worktree Ruby 4.0.9 (#3234) Retry the offline clear only in an active worker ...
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ece7739273
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
* origin/oauth-token-digests: Answer 404 from a dark endpoint however many times it's asked
* tp/oauth-token-digests: Name v0.2.0 in the pin's comment Pin fizzy-mcp-server v0.2.0 Refuse a join code that was reset while it was being redeemed Filter presigned storage signatures from logs Keep the reviewer's memberships in reach, and print the member audit escaped Import an export that lacks a column this database has only just gained List the reviewer accounts' members when signing the reviewer out Bump thruster from 0.1.26 to 0.1.27 (#3199) Give the asset build a placeholder direct-upload credential Anchor the stubbed storage host pattern Sign direct uploads with a write-only storage credential Lock the reviewer's email address, so a code holder can't move its membership out of revocation's reach Use Ruby's GC defaults and log job memory (#3238)
* tp/oauth-token-digests: Bring main's tests up to this branch: Ruby 4 and the access token finder
* tp/oauth-token-digests: Pin the load balancers to kamal-proxy v0.10.2, and say why configure-lb-mcp.sh still checks Give the load balancer the real client IP
…in order * tp/oauth-pats-as-grants: Hand the issuer's tokens back before removing it, and run down only once no code knows it Remove the personal access issuer before handing its tokens back Pin both personal token forms to the train's dark delivery and replica heal Refuse to destroy the personal access issuer by its stored client_id Keep a read-only personal access token's write a 401, whichever form it takes Find the personal access issuer a concurrent first use provisioned Point the conformance record at #3205 Record personal access tokens as tokens of a first-party client Provision the personal access client and backfill existing tokens onto it Issue personal access tokens from a first-party client, and read both forms as personal Remember a code redeemed before redemptions were recorded, once it's replayed Report a grace-window retry whose scope is refused as a replay Point the ledger at #3206 Count no family revocation for a personal access token deleted with its identity Record each redeemed authorization code, so one can't mint again once its grant is gone Name an event's identity only from its grant or explicitly, and label errors only by the client that asked Label an OAuth error with the client that asked Record AU-1 and OB-1 in the conformance record Log OAuth events to the structured log and count them in Yabeda Report OAuth lifecycle events, and announce errors, throttling and timing # Conflicts: # app/controllers/oauth/authorizations_controller.rb # db/schema.rb # db/schema_sqlite.rb
* tp/oauth-token-digests: Bring main's SQL redaction tests up to bearer lookup by find_honored Redact OAuth client statements from logged SQL, now that a client has a secret
* tp/oauth-token-digests: Redact retired refresh tokens from logged SQL
* tp/oauth-token-digests: Filter the code parameter by its whole name, so code_challenge stays readable
* tp/oauth-token-digests: Read a replayed refresh token's grant past the query cache
… on main in order * tp/oauth-code-replay-record: Remember a code redeemed before redemptions were recorded, once it's replayed Point the ledger at basecamp#3206 Record each redeemed authorization code, so one can't mint again once its grant is gone # Conflicts: # app/models/identity/access_token.rb
…main in order * tp/oauth-audit-metrics: Remember a code redeemed before redemptions were recorded, once it's replayed Report a grace-window retry whose scope is refused as a replay Point the ledger at basecamp#3206 Count no family revocation for a personal access token deleted with its identity Record each redeemed authorization code, so one can't mint again once its grant is gone Name an event's identity only from its grant or explicitly, and label errors only by the client that asked Label an OAuth error with the client that asked Record AU-1 and OB-1 in the conformance record Log OAuth events to the structured log and count them in Yabeda Report OAuth lifecycle events, and announce errors, throttling and timing # Conflicts: # app/models/oauth/client.rb # db/schema.rb # db/schema_sqlite.rb
…in order * tp/oauth-audience: Hand the issuer's tokens back before removing it, and run down only once no code knows it Remove the personal access issuer before handing its tokens back Pin both personal token forms to the train's dark delivery and replica heal Refuse to destroy the personal access issuer by its stored client_id Keep a read-only personal access token's write a 401, whichever form it takes Find the personal access issuer a concurrent first use provisioned Point the conformance record at basecamp#3205 Record personal access tokens as tokens of a first-party client Provision the personal access client and backfill existing tokens onto it Issue personal access tokens from a first-party client, and read both forms as personal Remember a code redeemed before redemptions were recorded, once it's replayed Report a grace-window retry whose scope is refused as a replay Point the ledger at basecamp#3206 Count no family revocation for a personal access token deleted with its identity Record each redeemed authorization code, so one can't mint again once its grant is gone Name an event's identity only from its grant or explicitly, and label errors only by the client that asked Label an OAuth error with the client that asked Record AU-1 and OB-1 in the conformance record Log OAuth events to the structured log and count them in Yabeda Report OAuth lifecycle events, and announce errors, throttling and timing # Conflicts: # app/controllers/users/push_subscriptions_controller.rb # app/models/identity/access_token.rb # app/models/oauth/client.rb # app/models/webhook.rb # config/recurring.yml
Warning
This PR contains a migration. Don't merge it directly. It has to go through the beta integration branch (
oauth-integration-2026-10-15) and be migrated by hand on beta before it lands. beta1 runs againstfizzy_production(saas/config/database.ymlendsbeta: *production), so migrating on beta1 is migrating production. The row to add to that branch's inventory is below. This PR doesn't push to the integration branch.Stacked on #3192 (train: #2296 → #3080 → #3081 → #3082 → #3158 → #3160 → #3161 → #3159 → #3163 → #3169 → #3192 → this).
Rubric checks GP-4 and GP-5 (bc3
doc/oauth/rubric.md, basecamp/bc3#13882): an authorization code is redeemed at most once, and a replay revokes what the first redemption minted. A replay without the verifier revokes nothing, and the code stays on record at least 24 hours past expiry.The gap
Codes are stateless (
Oauth::AuthorizationCode, an encrypted payload with ajti). The only record that a code had been redeemed was the grant it minted,identity_access_tokens.authorization_code_jti. A replay destroys that grant, and so do revocation and disconnecting the app. After that, nothing remembered the code, so within its 60-second life a third exchange with the same code and verifier minted a fresh grant. Found in review on #2296 (thread).What changes
oauth_redeemed_authorization_codes(jtiunique,created_atindexed).Identity::AccessToken.redeemwrites the row in the same transaction as the grant. A second redemption hits the unique index (RecordNotUnique), the existing path revokes any grant still carrying thatjti, and nothing new is minted. That holds whether the grant still exists or was revoked meanwhile.Oauth::RedeemedAuthorizationCode.cleanupruns daily at 04:42 (config/recurring.yml) and deletes older rows in batches.redeem, so a replay without the verifier still revokes nothing.jtiis the nonce inside the encrypted code, not the code, and it can't be turned back into one. It isn't a bearer artifact, so it isn't digested (TK-2).docs/oauth/spec_conformance.md: the single-use row no longer lists the "no tombstones" limit.Migration inventory row for the integration branch
Continues #3192's rows 8–10 and #3205's rows 11–12. The D2 and D3 drafts (#3202, #3203) are labelled D2-1 and D3-1.
20261017140000_create_oauth_redeemed_authorization_codes.rbCREATE TABLE oauth_redeemed_authorization_codes(uuid id,jtistring(36) NOT NULL unique,created_atNOT NULL indexed)bin/kamal app exec 'bin/rails db:migrate:down:primary VERSION=20261017140000' -d beta1 -p -r web -e MIGRATE:1. That drops the table. Code from before this PR never reads it.Tests
jti; cleanup keeps 24-hour-old rows and removes 26-hour-old ones. Both integration tests fail without the change (checked).db/schema.rbin column order only (physical vs sorted), as for the train's other migrations. On SQLite the dump matchesdb/schema_sqlite.rbexactly.