Skip to content

refactor: allow multiple UaaTokenEnhancers#3904

Open
peterhaochen47 wants to merge 1 commit intodevelopfrom
pr/develop/refactor-allow-multiple-token-enhancers-2
Open

refactor: allow multiple UaaTokenEnhancers#3904
peterhaochen47 wants to merge 1 commit intodevelopfrom
pr/develop/refactor-allow-multiple-token-enhancers-2

Conversation

@peterhaochen47
Copy link
Copy Markdown
Member

@peterhaochen47 peterhaochen47 commented May 6, 2026

  • previously, only one is allowed
  • add setUaaTokenEnhancers function to allow adding multiple UaaTokenEnhancers (this func will be implicitly called
    when UaaTokenEnhancer bean/beans are supplied)
  • refactor setUaaTokenEnhancer function (but keep same general business logic as before) for backward compatibility; mark as deprecated

incorporated feedback from earlier PR: #3899

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates UaaTokenServices to support configuring multiple UaaTokenEnhancer instances (instead of a single enhancer), while keeping the legacy setUaaTokenEnhancer API for backward compatibility (now deprecated).

Changes:

  • Add setUaaTokenEnhancers(List<UaaTokenEnhancer>) and migrate token-claim enrichment to iterate over multiple enhancers.
  • Deprecate setUaaTokenEnhancer(UaaTokenEnhancer) and adapt it to delegate to the new list-based setter.
  • Add/extend tests to verify that claims from multiple enhancers are applied to generated tokens.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java Introduces list-based enhancer injection and merges enhancer-produced claims into access tokens; deprecates the single-enhancer setter.
uaa/src/test/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServicesTests.java Adds coverage ensuring multiple enhancers’ claims appear in issued JWT access tokens.
server/src/test/java/org/cloudfoundry/identity/uaa/oauth/DeprecatedUaaTokenServicesTests.java Adds coverage ensuring the deprecated token services path also supports multiple enhancers.

Comment thread server/src/main/java/org/cloudfoundry/identity/uaa/oauth/UaaTokenServices.java Outdated
if (enhancer != null) {
Map<String, Object> claims = enhancer.enhance(emptyMap(), authentication);
if (claims != null) {
additionalRootClaims.putAll(claims);
Copy link
Copy Markdown
Member Author

@peterhaochen47 peterhaochen47 May 8, 2026

Choose a reason for hiding this comment

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

I am not sure merging the claim values from different token enhancers is a good idea; that would result in unpredictable/unintended final outcome. It seems more rigorous that an enhancer has deterministic power of the final outcome of the claim it touches, and if there is a conflict, the later enhancer wins.

If we do wanna let two enhancers touch on the same claim (not a use case now), we could pass in the previously added claims resulted from prior enhancers to the current enhancer, like an assembly line, so the current enhancer knows what has already been added to what claims (and merge if needed):

enhancer.enhance(additionalRootClaims(), authentication);

But we don't need this for now; we could always add this when we need to. But once we add this, we can't remove it without a breaking change. So I'm holding off on that for now.

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.

I like this idea, so each successive enhancer (ordered by @order) can see previous changes and update as needed.

- previously, only one is allowed
- add setUaaTokenEnhancers function to
allow adding multiple UaaTokenEnhancers
(this func will be implicitly called
when UaaTokenEnhancer bean/beans are
supplied)
- refactor setUaaTokenEnhancer function
(but keep same general business logic as before)
for backward compatibility; mark as
deprecated
@peterhaochen47 peterhaochen47 force-pushed the pr/develop/refactor-allow-multiple-token-enhancers-2 branch from c38902f to 28feee4 Compare May 8, 2026 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants