Expire OAuth access tokens and rotate refresh tokens - #3081
Conversation
There was a problem hiding this comment.
Pull request overview
Adds OAuth 2.1 access-token expiry, refresh-token rotation, expired-token rejection, and refresh-token revocation while preserving personal-token behavior.
Changes:
- Adds expiring OAuth tokens and rotating refresh tokens.
- Supports refresh grants and revocation by either token.
- Advertises and tests the expanded OAuth lifecycle.
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.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
app/models/identity/access_token.rb |
Implements expiry and token rotation. |
app/models/identity.rb |
Rejects expired bearer tokens. |
app/controllers/oauth/tokens_controller.rb |
Handles refresh grants and token responses. |
app/controllers/oauth/revocations_controller.rb |
Supports refresh-token revocation. |
app/controllers/oauth/metadata_controller.rb |
Advertises refresh grants. |
app/controllers/oauth/clients_controller.rb |
Adds refresh grants to DCR responses. |
db/migrate/20260827100000_add_oauth_token_expiry.rb |
Adds lifecycle columns and index. |
db/schema.rb |
Updates the MySQL schema. |
db/schema_sqlite.rb |
Updates the SQLite schema. |
test/models/identity/access_token_test.rb |
Tests expiry and rotation behavior. |
test/integration/oauth_flow_test.rb |
Tests refresh and revocation flows. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
7041285 to
8af37c3
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7041285c7b
ℹ️ 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".
4d696f6 to
530a7bb
Compare
OAuth-issued access tokens now expire an hour after issuance and carry a refresh token, generated at creation. Personal access tokens are untouched: no expiry, no refresh token. The token endpoint gains the refresh_token grant with OAuth 2.1 rotation semantics: each refresh rotates both the access and refresh token on the grant record, so the previous refresh token dies with the rotation and the Connected Apps list still shows one grant per client. Refresh requests must present the client_id the grant was issued to. Bearer authentication ignores expired tokens, and revocation accepts either token, killing the whole grant. Discovery and DCR responses advertise the refresh_token grant.
Refresh-token consumption raced: two concurrent refreshes could both load the grant and both answer 200, one with already-dead credentials. Rotation now updates guarded on the presented refresh token, so the loser matches no row and gets invalid_grant. The rename to refresh follows STYLE.md's bang rule — no non-bang counterpart, no bang. Also assert grant_types in the discovery and DCR responses so the advertised refresh grant can't silently regress.
530a7bb to
598de77
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 598de77115
ℹ️ 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".
| if @access_token.refresh | ||
| render json: token_response(@access_token) |
There was a problem hiding this comment.
When a refresh request includes scope, this branch ignores it and returns no scope field while retaining the grant's original permission. For example, refreshing a write token with scope=read still produces a write-capable token, but the omitted response field tells an OAuth client that its requested read-only scope was granted; requesting an expanded scope is likewise accepted instead of returning invalid_scope. Validate the requested scope against the original grant and either narrow the token or reject unsupported changes.
Useful? React with 👍 / 👎.
Stacked on #3080. Second of three gap-closers for #2296.
The minimal stack issued non-expiring access tokens with no refresh path. 7041285 adds OAuth 2.1 token lifecycle:
refresh_tokengrant with rotation semantics: each refresh rotates both the access token and the refresh token on the grant record, so the previous refresh token dies with the rotation and Connected Apps still shows one grant per client. Refresh requests must present theclient_idthe grant was issued to.Identity.find_by_permissable_access_tokengoes through anactivescope).refresh_tokengrant.Token responses now include
expires_inandrefresh_token; the refresh response omitsscope(unchanged from the original grant, per RFC 6749 §6).Deliberately not done: refresh-token reuse detection beyond rotation (a reused rotated token simply fails as unknown), and refresh-token expiry — grants live until revoked from Connected Apps, matching the existing revocation model.