Add CacheKey to AppTokenProviderResult for cache partitioning#6019
Draft
trwalke wants to merge 1 commit into
Draft
Add CacheKey to AppTokenProviderResult for cache partitioning#6019trwalke wants to merge 1 commit into
trwalke wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for access token cache partitioning for AppTokenProvider by introducing a provider-supplied CacheKey that becomes part of MSAL’s internal access token cache key components.
Changes:
- Added
AppTokenProviderResult.CacheKeyto allow partitioning of app token provider access token cache entries. - Updated client credential flow to include the provider cache key in
AuthenticationRequestParameters.CacheKeyComponents. - Added unit tests validating partitioning behavior and ensuring no cache key component is added when
CacheKeyis absent.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Identity.Test.Unit/AppTokenProviderTests.cs | Adds tests verifying cache partitioning via provider CacheKey and default behavior when unset. |
| src/client/Microsoft.Identity.Client/PublicApi/*/PublicAPI.Unshipped.txt | Updates public API baselines to include AppTokenProviderResult.CacheKey. |
| src/client/Microsoft.Identity.Client/Internal/Requests/ClientCredentialRequest.cs | Injects provider CacheKey into request cache key components. |
| src/client/Microsoft.Identity.Client/Internal/Requests/AuthenticationRequestParameters.cs | Allows internal setting of CacheKeyComponents so internal flows can update it. |
| src/client/Microsoft.Identity.Client/Internal/Constants.cs | Introduces a constant for the reserved cache key component name. |
| src/client/Microsoft.Identity.Client/Extensibility/AppTokenProviderResult.cs | Adds the new public CacheKey property with XML docs. |
Comment on lines
+216
to
+217
| t.AdditionalCacheKeyComponents.TryGetValue("appTokenProviderKey", out string _), | ||
| "Cached AT does not contain the 'appTokenProviderKey' cache component."); |
| } | ||
|
|
||
| var keysSeen = allTokens | ||
| .Select(t => t.AdditionalCacheKeyComponents["appTokenProviderKey"]) |
Comment on lines
+256
to
+258
| token.AdditionalCacheKeyComponents.ContainsKey("appTokenProviderKey"); | ||
| Assert.IsFalse(hasProviderKey, | ||
| "When AppTokenProviderResult.CacheKey is not set, the 'appTokenProviderKey' component must not be added."); |
Comment on lines
+34
to
+36
| /// to partition the cache. When provided, this value is stored in the request's | ||
| /// <c>CacheKeyComponents</c> under the key <c>"appTokenProviderKey"</c> and contributes to the | ||
| /// computed access token cache key. |
| { | ||
| if (AuthenticationRequestParameters.CacheKeyComponents == null) | ||
| { | ||
| AuthenticationRequestParameters.CacheKeyComponents = new SortedList<string, string>(); |
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.
This pull request introduces support for partitioning the access token cache in MSAL by allowing an
AppTokenProviderto specify a custom cache key via the newCacheKeyproperty inAppTokenProviderResult. This enables scenarios where tokens should be cached separately based on provider-specific criteria. The changes also include updates to the public API, internal logic for cache key handling, and comprehensive unit tests to verify the new behavior.App Token Provider Cache Partitioning:
CacheKeyproperty toAppTokenProviderResult, allowing the app token provider to partition the access token cache. This value is included in the cache key components under"appTokenProviderKey", ensuring tokens with different cache keys are stored separately.CacheKeyproperty in all relevantPublicAPI.Unshipped.txtfiles.AppTokenProviderCacheKeyinConstants.csto standardize the cache key component name.ClientCredentialRequestto add theCacheKeytoAuthenticationRequestParameters.CacheKeyComponentsif provided by the app token provider.CacheKeyComponentsinAuthenticationRequestParameterstointernalto allow internal updates.Testing:
CacheKeyis not set.Fixes #Changes proposed in this request
Testing
Performance impact
Documentation