Skip to content

Fix token reuse ignoring custom attributes when the request has none - #1853

Merged
nbulaj merged 1 commit into
doorkeeper-gem:mainfrom
55728:fix/custom-attributes-token-reuse
Jul 15, 2026
Merged

Fix token reuse ignoring custom attributes when the request has none#1853
nbulaj merged 1 commit into
doorkeeper-gem:mainfrom
55728:fix/custom-attributes-token-reuse

Conversation

@55728

@55728 55728 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

With reuse_access_token enabled and custom_access_token_attributes configured, a token request that doesn't specify any custom attributes could be handed back an existing token that was created with custom attribute values — a partition mix-up within the same application and resource owner.

Doorkeeper.configure do
  reuse_access_token
  custom_access_token_attributes [:tenant_name]
end

AccessToken.find_or_create_for(application: app, resource_owner: owner, scopes: scopes, tenant_name: "acme")
# => token A (tenant_name: "acme")

AccessToken.find_or_create_for(application: app, resource_owner: owner, scopes: scopes)
# before: token A is reused, tenant_name "acme" leaks into an attribute-less request
# after:  a new token without custom attributes is created

Cause

AccessTokenMixin.find_or_create_for and the client credentials Creator both collapse the extracted custom attributes with .presence, so {} (attributes configured, but not supplied in this request) becomes nil. custom_attributes_match? treats nil as "ignore custom attributes entirely" (return true if custom_attributes.nil?) and matches any token regardless of its stored values.

The bug is one-directional: requests that do supply custom attributes were already compared correctly and are unaffected.

Fix

Keep the empty hash intact instead of collapsing it to nil. custom_attributes_match? already handles {} correctly — each configured attribute is compared, so a token holding values no longer matches a request that supplies none, while two attribute-less tokens still match each other. The nil = "ignore custom attributes" contract is preserved for callers that rely on it (e.g. AuthorizationsController#matching_token? during pre-auth).

The change is deliberately confined to the two lookup call sites (plus docs/comments) so it can be backported to 5.x cleanly.

Note this is distinct from #1693, which is about reuse_access_token returning the same token for different access grants — grants aren't part of the matching there, whereas this PR is about configured custom attributes being ignored during matching.

With reuse_access_token enabled and custom_access_token_attributes
configured, a token request that doesn't specify any custom attributes
could be handed an existing token that was created with custom
attribute values (same application and resource owner).

AccessTokenMixin.find_or_create_for and the client credentials Creator
collapsed the extracted custom attributes with `.presence`, turning {}
(attributes configured but not supplied in this request) into nil
(ignore custom attributes entirely), so custom_attributes_match?
matched any token regardless of its stored custom attributes.

Keep the empty hash intact so such lookups only match tokens without
custom attribute values. Requests that do supply custom attributes were
already matched correctly and keep working as before.
@55728
55728 force-pushed the fix/custom-attributes-token-reuse branch from c05d60f to 44b4173 Compare July 14, 2026 12:56
@nbulaj

nbulaj commented Jul 14, 2026

Copy link
Copy Markdown
Member

Note: backport into https://github.com/doorkeeper-gem/doorkeeper/tree/v5.9-stable when merged

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.

Pull request overview

Fixes a token-reuse partitioning bug in Doorkeeper’s access token lookup when reuse_access_token is enabled and custom_access_token_attributes is configured: requests that do not supply custom attributes should no longer reuse tokens that do have custom attribute values set.

Changes:

  • Preserve an empty custom-attributes hash ({}) at token lookup call sites instead of collapsing it to nil, so custom attribute matching is not silently skipped.
  • Add regression specs covering both AccessToken.find_or_create_for and client-credentials token creation behavior with/without custom attribute inputs.
  • Document the nil vs {} contract for custom_attributes: matching, and add a changelog entry.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
spec/models/doorkeeper/access_token_spec.rb Adds regression coverage for matching_token_for and find_or_create_for to ensure empty custom-attribute requests don’t reuse custom-attributed tokens.
spec/lib/oauth/client_credentials/creator_spec.rb Adds regression coverage for client-credentials token reuse behavior when custom attributes are configured and omitted/present.
lib/doorkeeper/oauth/client_credentials/creator.rb Stops collapsing extracted custom attributes via .presence, keeping {} distinct from nil for matching semantics.
lib/doorkeeper/models/access_token_mixin.rb Stops collapsing extracted custom attributes via .presence for find_or_create_for, and clarifies nil vs {} matching behavior in docs/comments.
CHANGELOG.md Records the user-visible bugfix under ## main.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@55728
55728 force-pushed the fix/custom-attributes-token-reuse branch from 44b4173 to b8d9a43 Compare July 14, 2026 15:20

@nbulaj nbulaj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM ✅

@nbulaj
nbulaj merged commit 1fe5dd3 into doorkeeper-gem:main Jul 15, 2026
22 checks passed
@55728
55728 deleted the fix/custom-attributes-token-reuse branch July 15, 2026 10:42
nbulaj added a commit that referenced this pull request Jul 15, 2026
Fix token reuse ignoring custom attributes when the request has none
nbulaj added a commit that referenced this pull request Jul 15, 2026
…butes-token-reuse

Backport #1853: Fix token reuse ignoring custom attributes when the request has none
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.

3 participants