fix: include isTokenBinding in CCA cache key to prevent bearer/PoP collision - #3867
Merged
Gladwin Johnson VR (gladjohn) merged 4 commits intoJun 17, 2026
Merged
Gladwin Johnson VR (gladjohn) merged 4 commits into
Gladwin Johnson VR (gladjohn) merged 4 commits into
Conversation
Add GetOrBuildCca_BearerThenTokenBinding_ShouldReturnDifferentInstances to demonstrate that GetApplicationKey does not include the isTokenBinding flag. A bearer call (isTokenBinding=false) caches a CCA with a string-assertion credential, and a subsequent PoP call (isTokenBinding=true) incorrectly reuses the same instance. MSAL then throws 'A string-returning client assertion callback cannot be used over mTLS' because the cached CCA was never wired with the ClientSignedAssertion bundle overload. This test is intentionally failing (Assert.NotSame on the same instance). Once the cache key is fixed to include isTokenBinding, it will pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…llision GetApplicationKey did not include the isTokenBinding flag, so a CCA built for bearer (WithClientAssertion string delegate) was cached and reused for mTLS PoP requests. MSAL then threw 'A string-returning client assertion callback cannot be used over mTLS' because the cached CCA was never wired with the ClientSignedAssertion bundle overload. Fix: append '-tokenBinding' to the cache key when isTokenBinding=true so bearer and PoP builds get separate cache entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ievgen Polyvanyi (cpp11nullptr)
approved these changes
Jun 16, 2026
Addresses review feedback from cpp11nullptr: add GetOrBuildCca_TokenBindingThenBearer_DoesNotReturnCachedPopApp to cover the reverse cache-key collision direction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bogdan Gavril (bgavrilMS)
approved these changes
Jun 16, 2026
Address bgavrilMS review: remove default value from isTokenBinding parameter to force callers to explicitly specify the flag. Add XML doc explaining why isTokenBinding is part of the cache key (bearer and mTLS PoP use incompatible MSAL credential wirings). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bogdan Gavril (bgavrilMS)
approved these changes
Jun 17, 2026
Gladwin Johnson VR (gladjohn)
deleted the
gladjohn/fix-cca-cache-key-tokenbinding
branch
June 17, 2026 13:01
This was referenced Jun 24, 2026
Merged
Closed
Closed
Closed
This was referenced Aug 3, 2026
Closed
This was referenced Aug 10, 2026
Merged
This was referenced Aug 19, 2026
This was referenced Sep 7, 2026
This was referenced Sep 18, 2026
This was referenced Sep 28, 2026
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.
Problem
GetApplicationKeydid not include theisTokenBindingflag in the CCA cache key. When the same app made a bearer call followed by an mTLS PoP call (or vice versa), both resolved to the same cache key — so the second call reused the CCA built for the first.This caused MSAL to throw at request time:
The root cause is that bearer builds the CCA with
WithClientAssertion(Func<string>)while PoP builds it withWithClientAssertion(Func<ClientSignedAssertion>)orWithCertificate()— these are fundamentally incompatible credential wirings that must not share a cache entry.Fix
Append
-tokenBindingto the cache key whenisTokenBinding = true. This is a minimal, backwards-compatible change:false(default) → their keys are unchanged.Test
Added
GetOrBuildCca_BearerThenTokenBinding_DoesNotReturnCachedBearerAppwhich:isTokenBinding: false) — succeeds and caches.isTokenBinding: true) — with the fix, this is a cache miss and attempts a fresh build. Since the test uses a secret credential (which can't do mTLS binding), it correctly throws IDW10115.Results
Microsoft.Identity.Web.Testpass (net9.0, 4 pre-existing skips)GetApplicationKeyis private, parameter has a default value