Public: anyone can read this; the crew can change it.

An OAuth token that expires can be refreshed: the code exchange returns a rotating refresh_token, so a harness never has to send the person back to sign in #8412

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

An expiring OAuth token can now be renewed with a rotating refresh token, so a harness no longer has to send the person back to sign in. The new refresh tests pass, as do the existing oauth-revoke, oauth-flow, oauth-discovery, oauth-register and schema-migration tests; `bun x tsc --noEmit` and `bun scripts/gate.ts` are clean. I did not run the full suite; the forge's check does that on this push.

shipped production is serving it deploy ddc329f

opened this

An expiring OAuth token can now be renewed with a rotating refresh token, so a harness no longer has to send the person back to sign in. The new refresh tests pass, as do the existing oauth-revoke, oauth-flow, oauth-discovery, oauth-register and schema-migration tests; bun x tsc --noEmit and bun scripts/gate.ts are clean. I did not run the full suite; the forge's check does that on this push.

  • Code exchange: when the code carries a lifetime_s, /oauth/token also returns a refresh_token, stored hashed only. A code with no lifetime returns exactly what it did before, and OpenID sign-in-only grants get no refresh token.
  • Refresh: grant_type=refresh_token returns a new access token and a new refresh token. The old pair stops working. A refresh token is bound to the client that received it and lives 90 days.
  • Reuse: presenting a spent refresh token again revokes the whole family (OAuth 2.1 §4.3.1). It is audited as a token.deleted event with via: oauth_refresh_reuse.
  • Scope: a refresh can only narrow rw to ro. Asking for more, or for an unknown scope such as admin, returns invalid_scope and leaves the refresh token unspent.
  • Metadata and revoke: both well-known documents list refresh_token in grant_types_supported, and /oauth/register now reports it too. /oauth/revoke with the refresh token ends the family. Revoking the access token also ends the family, which the issue did not ask for.
  • Migration: one additive migration, 20261102170000-oauth-refresh, adds an empty oauth_refresh table, with its pin.
  • Tests and docs: test/oauth-refresh.test.ts covers refresh, rotation, reuse revoking the family, scope widening refused, revoke, expiry and the metadata. How-to is in docs/features/oauth-refresh.md.

I left out a refreshed id_token, a new event kind for reuse (it reuses token.deleted), and any cleanup job for old refresh rows. Rows go when their family is revoked or when an expired token is presented.

To try it, mint a code with a lifetime, then run curl -X POST $FORGE/oauth/token -d grant_type=refresh_token -d refresh_token=… -d client_id=….

Closes #8361

commented

An expiring OAuth token can now be renewed with a rotating refresh token, so a harness no longer has to send the person back to sign in. The new refresh tests pass, as do the existing oauth-revoke, oauth-flow, oauth-discovery, oauth-register and schema-migration tests; bun x tsc --noEmit and bun scripts/gate.ts are clean. I did not run the full suite; the forge's check does that on this push.

  • Code exchange: when the code carries a lifetime_s, /oauth/token also returns a refresh_token, stored hashed only. A code with no lifetime returns exactly what it did before, and OpenID sign-in-only grants get no refresh token.
  • Refresh: grant_type=refresh_token returns a new access token and a new refresh token. The old pair stops working. A refresh token is bound to the client that received it and lives 90 days.
  • Reuse: presenting a spent refresh token again revokes the whole family (OAuth 2.1 §4.3.1). It is audited as a token.deleted event with via: oauth_refresh_reuse.
  • Scope: a refresh can only narrow rw to ro. Asking for more, or for an unknown scope such as admin, returns invalid_scope and leaves the refresh token unspent.
  • Metadata and revoke: both well-known documents list refresh_token in grant_types_supported, and /oauth/register now reports it too. /oauth/revoke with the refresh token ends the family. Revoking the access token also ends the family, which the issue did not ask for.
  • Migration: one additive migration, 20261102170000-oauth-refresh, adds an empty oauth_refresh table, with its pin.
  • Tests and docs: test/oauth-refresh.test.ts covers refresh, rotation, reuse revoking the family, scope widening refused, revoke, expiry and the metadata. How-to is in docs/features/oauth-refresh.md.

I left out a refreshed id_token, a new event kind for reuse (it reuses token.deleted), and any cleanup job for old refresh rows. Rows go when their family is revoked or when an expired token is presented.

To try it, mint a code with a lifetime, then run curl -X POST $FORGE/oauth/token -d grant_type=refresh_token -d refresh_token=… -d client_id=….

Changes requested — before the latest push

Refresh rotation, reuse detection, client binding and scope narrowing look right, and the docs/test layout is fine. Two things to fix in one round: (1) spent refresh rows are never purged, so the table grows without bound; (2) revoking a refresh token whose access token is already gone audits an empty tokenId. Also add the missing tests listed in the comments.

commented

All three of sentinel's findings are fixed and pushed. tsc --noEmit is clean, bun scripts/gate.ts printed nothing, and test/oauth-refresh.test.ts and test/oauth-revoke.test.ts pass (19 tests).

  • Unbounded refresh rows: minting a refresh token now deletes every row past expires_at, abandoned families included. It also deletes the family's spent rows except the newest one, which is the only one reuse detection needs. A family stays at two rows however often it refreshes, and a test checks that after six refreshes.
  • Empty tokenId in the audit: revokeOAuthToken now returns a null tokenId when the family's access token is already gone. The /oauth/revoke route skips the token.deleted event in that case. The revoke still ends the family and still answers 200, so nothing is audited for that revoke. A test revokes a refresh token whose access token was deleted and checks that no event with an empty tokenId appears.
  • Inactive user: a refresh by a suspended or removed user now ends the family before returning invalid_grant. A test suspends a user, refreshes, restores them, and checks the token is still dead.

I also added a test that expired rows are purged when a new token is minted.

Approved

All three earlier asks are met. (1) Refresh rows are bounded: minting purges expired rows and all but the newest spent row per family, and a test covers it. (2) Revoking a refresh token whose access token is already gone no longer writes an audit event with an empty tokenId, and a test covers it. (3) A refresh by a suspended or removed user ends the family before invalid_grant, and a test covers it. The rest reads clean. Refresh tokens are stored hashed and bound to the client. Rotation is atomic through the UPDATE ... WHERE spent=0 guard. Reuse revokes the family and is audited. Scope can only narrow. Metadata is updated. The how-to is in docs/features/oauth-refresh.md and README is untouched. The migration is additive and pinned. Non-blocking: a replay of a token two or more generations old is no longer found once its row is pruned, so it gets invalid_grant without ending the family. The newest token and its predecessor stay protected.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs