Add an extra subject_token verification to protect token extensions #3954
Open
fhanik wants to merge 3 commits into
Open
Add an extra subject_token verification to protect token extensions #3954fhanik wants to merge 3 commits into
fhanik wants to merge 3 commits into
Conversation
…ng subject_token verification TokenExchangeGranter.getTokenActor() previously parsed the subject_token with JwtHelper.decode(), a parse-only operation that never verified the JWT signature. Because the default tokenExchangeAuthenticationManager is @ConditionalOnMissingBean, a custom replacement that weakens or omits signature verification left getTokenActor() as an unguarded path: claims from an unverified — or tampered — subject_token would be trusted and embedded verbatim in the act claim of the issued token. A new public method, ExternalOAuthAuthenticationManager.verifySubjectToken(), encapsulates the existing issuer-based IdP resolution (resolveOriginProvider, driven by the token's iss claim) and signature/expiry validation (validateToken) into a single callable that getTokenActor() now invokes. Any failure — unknownissuer, signature mismatch, or expired token — is converted to InvalidGrantException and surfaced as HTTP 400. The granter's verification isindependent of the filter layer, so it holds regardless of which tokenExchangeAuthenticationManager bean is active.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens OAuth2 Token Exchange by ensuring TokenExchangeGranter independently verifies the subject_token JWT signature (and expiry) based on the issuer-mapped registered IdP, preventing signature-check bypasses when the filter-layer authentication manager is overridden.
Changes:
- Added
ExternalOAuthAuthenticationManager.verifySubjectToken()to resolve IdP byiss, verify signature, and check expiry. - Updated
TokenExchangeGranterto callverifySubjectToken()(instead of parse-only decoding) and wired the needed bean viaOAuth2FilterConfig. - Added unit and MockMvc integration tests to prove tampered
subject_tokens are rejected both in the default setup and when the filter layer skips signature validation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenExchangeSubjectTokenSignatureBypassMockMvcTests.java |
New integration test proving granter-level verification rejects tampered tokens even if filter-layer verification is bypassed via bean override. |
uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenExchangeDefaultConfigMockMvcTests.java |
Adds default-configuration integration coverage showing tampered subject_token is blocked at the filter layer (401). |
server/src/test/java/org/cloudfoundry/identity/uaa/oauth/token/TokenExchangeGranterTests.java |
Updates unit tests for new granter dependency and adds signature-verification failure cases. |
server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java |
Introduces verifySubjectToken() API to reuse existing issuer resolution + signature/expiry validation logic. |
server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenExchangeGranter.java |
Replaces parse-only JWT decoding with signature+expiry verification for subject_token. |
server/src/main/java/org/cloudfoundry/identity/uaa/oauth/provider/config/xml/OAuth2FilterConfig.java |
Wires externalOAuthAuthenticationManager into TokenExchangeGranter via constructor injection. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
TokenExchangeGranter.getTokenActor()now callsExternalOAuthAuthenticationManager.verifySubjectToken()instead of theparse-only
JwtHelper.decode(). The new method resolves the registeredidentity provider by the token's
issclaim, verifies the JWT signatureagainst that provider's published keys, and checks expiry — reusing logic
already present in
ExternalOAuthAuthenticationManager.OAuth2FilterConfigwires the existingexternalOAuthAuthenticationManagerbean into
TokenExchangeGranteras a new constructor parameter.TokenExchangeDefaultConfigMockMvcTestsconfirms the default configurationrejects a tampered token at the filter layer (HTTP 401); the new
TokenExchangeSubjectTokenSignatureBypassMockMvcTestsconfirms the granterindependently rejects it (HTTP 400
invalid_grant) even when the filterlayer is replaced by a custom bean that skips signature checking.