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
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".
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.
A refresh request that carried a scope was silently ignored: the response omitted the scope field while the rotated token kept the grant's original permission, so a client narrowing a write token to read believed it held a read-only token but received a write-capable one, and a broadening request was accepted rather than rejected. Validate the requested scope against the grant (RFC 6749 §6): reject anything beyond it with invalid_scope, narrow the rotated token to a requested subset, and always echo the effective scope.
598de77 to
0cd462c
Compare
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cd462c536
ℹ️ 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".
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.