Conversation
The migration and initializer templates are ERB that renders Ruby source, not HTML, so GuardRails reads every `<%= %>` in them as an unescaped output and reports each new migration template as "Potential XSS (unquoted template variable)" on the line every one of them shares, `ActiveRecord::Migration<%= migration_version %>`. It did so on #1865, #1932, #1942 and #1944. `migration_version` is built from ActiveRecord::VERSION and nothing in these files ever reaches a browser. The ignore file takes the templates directory out of the scan, the same way .codeclimate.yml already scopes that scanner. The gemspec builds the gem from app/, config/, lib/ and vendor/, so the file ships nowhere.
Every refresh creates a new oauth_access_tokens record, and nothing linked the records of one refresh chain together. Revoking a refresh token at the revocation endpoint therefore revoked the record of the presented token only, while RFC 7009 §2.1 says the authorization server SHOULD also invalidate the access tokens based on the same authorization grant. Earlier records of the chain can still be usable at that point: with the previous_refresh_token column the previous record lives until the new access token is first used. The records of one chain now share an identifier in a new nullable refresh_token_family_id column on oauth_access_tokens (part of the install migration, added to existing installs by the new doorkeeper:refresh_token_family_id generator, with an index). - AccessToken#generate_refresh_token starts a family when a refresh token is issued outside the refresh grant (authorization code, password, or a host app creating the record), and keeps it across validations of the same new record. - RefreshTokenRequest carries the family of the presented refresh token onto the record it creates. A refresh token that predates the column is given a family at that point (#ensure_refresh_token_family_id!, written only while the column is still empty so that concurrent refreshes continue the same family), so no backfill is needed. - RevocableRefreshToken#revoke goes through the new AccessToken#revoke_refresh_token_family, which revokes every unrevoked record of the family in one write, whichever refresh token of the chain is presented. A presented refresh token whose own record is already revoked stays revocable, since the records issued after it along the chain can still be live. The application is part of the condition, so a family identifier can never reach the tokens of another client. Revoking an access token, #revoke and .revoke_all_for are unchanged. Without the column, for records without a family, and for access token models that do not implement the new methods (the Sequel and MongoDB extensions ship their own mixins), only the presented record is revoked, as before. The reader and writer of the attribute ignore a missing column. Closes #1945
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Family revocation can be skipped for previously revoked members and can race with concurrent refresh issuance.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (3)
What changed in this PR
Adds refresh-token family tracking so revoking a refresh token can revoke its entire authorization chain.
Changes:
- Adds the indexed
refresh_token_family_idcolumn and generator. - Propagates family IDs across refreshes and performs family-wide revocation.
- Adds documentation and comprehensive model, request, controller, and generator specs.
| File | Description |
|---|---|
.guardrails/ignore |
Excludes Ruby generator templates from XSS checks. |
CHANGELOG.md |
Documents refresh-family revocation. |
README.md |
Explains setup and behavior. |
lib/doorkeeper/models/access_token_mixin.rb |
Implements family assignment and revocation. |
lib/doorkeeper/oauth/refresh_token_request.rb |
Carries family IDs across refreshes. |
lib/doorkeeper/revocable_tokens/revocable_refresh_token.rb |
Delegates refresh-token revocation to the family API. |
lib/generators/doorkeeper/refresh_token_family_id_generator.rb |
Adds the migration generator. |
lib/generators/doorkeeper/templates/add_refresh_token_family_id_to_access_tokens.rb.erb |
Defines the upgrade migration. |
lib/generators/doorkeeper/templates/migration.rb.erb |
Adds the column for new installations. |
spec/controllers/tokens_controller_spec.rb |
Tests endpoint family revocation and fallback. |
spec/dummy/db/migrate/20260920000000_add_refresh_token_family_id_to_access_tokens.rb |
Updates the dummy database. |
spec/dummy/db/schema.rb |
Records the new column and index. |
spec/generators/refresh_token_family_id_generator_spec.rb |
Tests migration generation. |
spec/lib/oauth/refresh_token_request_spec.rb |
Tests family propagation and compatibility. |
spec/models/doorkeeper/access_token_spec.rb |
Tests assignment, migration, and revocation behavior. |
spec/requests/flows/refresh_token_spec.rb |
Tests family creation through authorization flows. |
spec/requests/flows/revoke_token_spec.rb |
Tests end-to-end chain revocation. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
55728
force-pushed
the
feat/refresh-token-family
branch
from
September 20, 2026 09:27
2f037e0 to
1c6a490
Compare
Member
Author
|
Hey @nbulaj 👋 Converting this to draft to lighten your review queue — I know there are a few PRs from me stacked up right now and I don't want to overwhelm you. The code is ready whenever you have bandwidth; I'll mark it ready for review again when the timing feels right. No rush at all! 😊 |
55728
marked this pull request as draft
September 21, 2026 10:00
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1945.
Every refresh creates a new
oauth_access_tokensrecord, and nothing links the records of one refresh chain together. Revoking a refresh token at the revocation endpoint therefore revokes the record of the presented token only, while RFC 7009 §2.1 says:During the #1787 grace period (
previous_refresh_tokencolumn), the previous record lives until the new access token is first used. With #1944 (revoke_previous_access_token_on_refresh false), every previous access token that has not expired yet remains usable, which is the limitation #1944 documents.What changes
A nullable
refresh_token_family_idcolumn onoauth_access_tokens, following the additive-column pattern ofprevious_refresh_token,resource,refresh_token_scopes(#1932) andrefresh_token_revoked_at(#1944): part of the install migration, added to existing installs byrails generate doorkeeper:refresh_token_family_id, detected withcolumn_names.include?. The generated migration also adds a (non-unique) index, since revocation looks records up by this column.AccessToken#generate_refresh_tokenstarts a family (SecureRandom.uuid) when a refresh token is issued outside the refresh grant. Doing it there, likerefresh_token_scopes, covers the authorization code and password grants as well as host apps that create the record themselves. It is kept across validations of the same new record, and an explicitly assigned value wins.RefreshTokenRequestcopies the family of the presented refresh token onto the record it creates.AccessToken#ensure_refresh_token_family_id!). The identifier is written with a conditionalUPDATE ... WHERE refresh_token_family_id IS NULLand read back, so concurrent refreshes of the same token during the grace period all continue one family.RevocableRefreshToken#revokegoes through the newAccessToken#revoke_refresh_token_family, which revokes the record and every record of its family that#revoked?does not already report as revoked (so a revocation time in the future is brought forward, as#revokedoes for a single record), whichever refresh token of the chain is presented.application_idis part of the condition, so a family identifier can never reach another client's tokens, whatever a host app stores in the column.Important
Compatibility
AccessToken#revokeandAccessToken.revoke_all_forare unchanged (RFC 7009 §2.1 makes the access token case a MAY).refresh_token_scopes=).#revokefor models that do not implement the new methods.RefreshTokenRequest#create_access_tokenwas at theMetrics/AbcSizelimit, so the two attributes a refresh carries along the chain (refresh_token_scopesfrom Keep the granted scope on refresh tokens across narrowed refreshes #1932 and the family) moved into one privaterefresh_chain_attributes. No behavior change..guardrails/ignorefor the generator templates (same commit as on Add opt-in to keep the previous access token usable after a refresh #1944; whichever lands first makes the other a no-op).Relation to other work
main. Once Add opt-in to keep the previous access token usable after a refresh #1944 is merged this branch needs a rebase (conflicts are all "both sides appended at the same anchor"); I checked the combination locally: withrevoke_previous_access_token_on_refresh false, revoking the current refresh token reaches the previous access tokens that were kept alive. The "Things to keep in mind" bullet in Add opt-in to keep the previous access token usable after a refresh #1944's README section should then point to this feature.