Skip to content

Record each redeemed authorization code, so one can't mint again once its grant is gone - #3206

Merged
jeremy merged 12 commits into
mainfrom
oauth-code-replay-record
Oct 27, 2026
Merged

jeremy merged 12 commits into
mainfrom
oauth-code-replay-record

Conversation

@jeremy

@jeremy jeremy commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

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 against fizzy_production (saas/config/database.yml ends beta: *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 a jti). 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 (jti unique, created_at indexed). Identity::AccessToken.redeem writes 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 that jti, and nothing new is minted. That holds whether the grant still exists or was revoked meanwhile.
  • Retention: 25 hours, which is the rubric's "at least 24 hours past expiry" with a margin. Oauth::RedeemedAuthorizationCode.cleanup runs daily at 04:42 (config/recurring.yml) and deletes older rows in batches.
  • Order of checks unchanged: client, redirect URI and PKCE are checked before redeem, so a replay without the verifier still revokes nothing.
  • What's stored: a jti is 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.

# Version / file PR What it does Production table MySQL 8.4 DDL
13 20261017140000_create_oauth_redeemed_authorization_codes.rb this CREATE TABLE oauth_redeemed_authorization_codes (uuid id, jti string(36) NOT NULL unique, created_at NOT NULL indexed) new New table; no lock on anything existing
  • Undo by version: 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.
  • Before running it: this PR's code writes the table on every code exchange, so migrate before beta boots this code. OAuth is dark on beta, so no exchange happens there in any case.

Tests

  • New: a code replayed again after its grant was revoked mints nothing; a code can't be redeemed again once the person disconnects the app; the unique jti; cleanup keeps 24-hour-old rows and removes 26-hour-old ones. Both integration tests fail without the change (checked).
  • Thelio, dedicated MySQL 8.4 container, at b6e95b2, exit status gated: SaaS (MySQL) 2299 tests, OSS (SQLite) 2131, OSS (MySQL) 2131, all 0 failures and 0 errors.
  • Migration on MySQL 8.4 (OSS), from Store tokens and client secrets as digests (D1 of 3: read by digest, plaintext fallback) #3192's schema: up, down, up. The dump differs from the committed db/schema.rb in column order only (physical vs sorted), as for the train's other migrations. On SQLite the dump matches db/schema_sqlite.rb exactly.

Copilot AI balanced review requested due to automatic review settings October 17, 2026 09:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T03:02:25.273355Z 007aaae New commits
🔒 Security Review ✅ Completed 2026-10-26T03:03:49.120409Z 007aaae New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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 run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to 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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

@jeremy
jeremy requested a balanced review from Copilot October 17, 2026 09:44
@jeremy
jeremy force-pushed the oauth-token-digests branch from dcb914c to 3a2bb79 Compare October 17, 2026 09:58
jeremy added a commit that referenced this pull request Oct 17, 2026
@jeremy
jeremy force-pushed the oauth-code-replay-record branch from d626be9 to d8ae638 Compare October 17, 2026 09:58
Copilot stopped reviewing on behalf of jeremy due to an error October 17, 2026 10:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human verification must confirm the integration-branch migration runs against the shared production database before this code is deployed.

No open issues 🎉

Review effort: Balanced

jeremy added a commit that referenced this pull request Oct 17, 2026
@jeremy
jeremy force-pushed the oauth-code-replay-record branch from d8ae638 to fc836c9 Compare October 17, 2026 10:31
@jeremy
jeremy force-pushed the oauth-token-digests branch from 82cdd19 to 73f186d Compare October 17, 2026 10:32
jeremy added a commit that referenced this pull request Oct 17, 2026
@jeremy
jeremy force-pushed the oauth-code-replay-record branch from fc836c9 to 8823486 Compare October 17, 2026 10:32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread app/models/identity/access_token.rb

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human verification must confirm the required integration-branch migration against beta’s shared production database before this code lands.

No open issues 🎉

Review effort: Balanced

@jeremy
jeremy force-pushed the oauth-token-digests branch from 73f186d to 1b3ab1f Compare October 17, 2026 11:10
jeremy added a commit that referenced this pull request Oct 17, 2026
@jeremy
jeremy force-pushed the oauth-code-replay-record branch from 8823486 to f000e51 Compare October 17, 2026 11:11
@jeremy
jeremy requested a balanced review from Copilot October 17, 2026 11:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human verification is required for the integration-branch migration against beta’s shared production database before deployment.

No open issues 🎉

Review effort: Balanced

… 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.
@jeremy
jeremy force-pushed the oauth-code-replay-record branch from 0af0b6d to eff0d09 Compare October 18, 2026 05:56
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The required integration-branch rollout and manual beta migration against the shared production database need human confirmation.

No open issues 🎉

Review effort: Balanced

@jeremy

jeremy commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@jeremy

jeremy commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

Restacked onto #3192 617edb87b ("Keep identity_access_tokens.refresh_token until it can go with token"), now at eff0d09a2. On thelio: rubocop clean; SaaS (MySQL), OSS (SQLite) and OSS (MySQL) suites pass, 0 failures, 0 errors. CI is green and Copilot reviewed this head with no open issues. Codex didn't review it: two requests returned "You have reached your Codex usage limits for code reviews". Re-request it once the limit resets.

@jeremy

jeremy commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: eff0d09a2c

ℹ️ 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".

…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
  ...

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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
jeremy added a commit that referenced this pull request Oct 26, 2026
…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
Base automatically changed from oauth-token-digests to main October 27, 2026 06:28
@jeremy
jeremy merged commit 826f257 into main Oct 27, 2026
11 of 13 checks passed
@jeremy
jeremy deleted the oauth-code-replay-record branch October 27, 2026 06:29
kheubaum pushed a commit to kheubaum/fizzy that referenced this pull request Oct 27, 2026
… 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
kheubaum pushed a commit to kheubaum/fizzy that referenced this pull request Oct 27, 2026
…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
kheubaum pushed a commit to kheubaum/fizzy that referenced this pull request Oct 27, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants