Add forwarded_client_claims support to confidential client flows#937
Open
Robbie-Microsoft wants to merge 11 commits into
Open
Add forwarded_client_claims support to confidential client flows#937Robbie-Microsoft wants to merge 11 commits into
Robbie-Microsoft wants to merge 11 commits into
Conversation
… flows Port of msal-dotnet PR 5999 (WithClaimsFromClient). Adds a `client_claims` keyword argument to `ConfidentialClientApplication.acquire_token_for_client` and `ManagedIdentityClient.acquire_token_for_client` for forwarding client-originated claims (e.g. the network security perimeter `xms_az_nwperimid` claim) to ESTS/IMDS. Unlike `claims_challenge` (server-issued, bypasses the cache), `client_claims` tokens are cached and the cache entry is keyed on the claims value, reusing the existing `_compute_ext_cache_key` mechanism (the `fmi_path` precedent). - token_cache: add `_parse_claims_or_raise`, `_deep_merge_dict`, `_merge_claims` helpers; `claims` stays excluded from the ext cache key while `client_claims` participates in it. - oauth2: strip the cache-key-only `client_claims` pseudo-parameter from the wire body while preserving it for the cache-add event. - application: validate `client_claims`, merge it into the OAuth `claims` body parameter, and isolate the cache by claims value. - managed_identity: support `client_claims` on the IMDS (Azure VM) source only (sent as the `claims` query parameter); other sources raise; MSIv1 restricts the claims JSON to only the `xms_az_nwperimid` key. Adds unit tests covering cache isolation, wire shape, claim merging, source restrictions, and MSIv1 validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for forwarding client-originated claims via a new client_claims argument in confidential-client and managed-identity client-credentials flows, while ensuring tokens remain cacheable and isolated by the claims value (distinct from server-issued claims_challenge behavior).
Changes:
- Introduces shared claims parsing/merging helpers and uses
client_claimsto participate in the extended cache key (while keepingclaimsexcluded). - Ensures
client_claimsis merged into the OAuthclaimsparameter but stripped from the actual HTTP request body (cache-key-only pseudo-parameter). - Adds managed identity support for
client_claimson IMDS/Azure VM only (including MSIv1 validation), with unit tests covering caching and wire-shape behaviors.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
msal/token_cache.py |
Adds claims helpers and leverages existing ext-cache-key mechanism to isolate cached tokens by client_claims. |
msal/oauth2cli/oauth2.py |
Prevents client_claims from being sent on the wire while preserving it for cache add events. |
msal/application.py |
Adds client_claims to CCA acquire-token-for-client flow, validates/merges claims, and isolates cache entries by claims value. |
msal/managed_identity.py |
Adds client_claims support for IMDS/Azure VM only, isolates cache by claims, and validates MSIv1 claim constraints. |
tests/test_token_cache.py |
Adds unit tests for cache-key isolation and claims helper behavior. |
tests/test_mi.py |
Adds managed-identity unit tests for IMDS forwarding, cache isolation, and unsupported source errors. |
tests/test_application.py |
Adds CCA unit tests for wire shape, merging behavior, and cache isolation with client_claims. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… code, silent) Phase 1 added client_claims to acquire_token_for_client. This extends it to the remaining confidential client flows so client-originated claims are forwarded and cache-isolated consistently, mirroring msal-dotnet PR 5999's WithClaimsFromClient (which applies to all confidential client builders): - acquire_token_on_behalf_of (OBO) - acquire_token_by_user_federated_identity_credential (FIC) - acquire_token_by_authorization_code - acquire_token_silent / acquire_token_silent_with_error (cache-read isolation plus refresh-token request merge) A shared _stash_client_claims() helper validates the value and stashes it into the request data, so it (a) contributes to the extended cache key and (b) is merged into the OAuth "claims" parameter while being stripped from the wire body. Adds unit tests for each flow (wire shape, validation, cache isolation). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- token_cache._parse_claims_or_raise now also catches TypeError (raised when the input is not a str/bytes) and surfaces the same friendly ValueError, so every caller behaves consistently regardless of the bad input's type. - ManagedIdentityClient.acquire_token_for_client now rejects non-string client_claims with a ValueError (mirroring the confidential-client flows), preventing a raw TypeError leak and inconsistent extended-cache-key hashing. - ConfidentialClientApplication.acquire_token_for_client now reuses the shared _stash_client_claims() helper instead of duplicating the validate-and-stash logic, removing the risk of the two paths diverging. - Add cross-referencing docstring notes disambiguating the per-request client_claims (a JSON string forwarded in the request) from the pre-existing constructor client_claims (a dict of claims signed into the client-assertion JWT). - Add unit tests for non-string client_claims on managed identity and for non-string inputs to _parse_claims_or_raise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4gust
approved these changes
Jun 29, 2026
Avery-Dunn
reviewed
Jun 29, 2026
Avery-Dunn
reviewed
Jun 29, 2026
Address reviewer feedback on PR #937: - Rename the new per-request parameter `client_claims` -> `forwarded_client_claims` across all confidential-client flows (acquire_token_for_client, on_behalf_of, user FIC, auth code, silent) and the Managed Identity acquire_token_for_client. This removes the naming collision with the pre-existing `client_claims` *constructor* parameter (a dict signed into the client-assertion JWT), which a second reviewer also flagged as confusing. The public keyword is the only thing renamed. The internal request-data key "client_claims" (used by the oauth2 wire-strip, _compute_ext_cache_key cache isolation, and _merge_claims) and the private Managed Identity plumbing keep their existing names, so cache keying and wire behavior are unchanged. - Add test_forwarded_client_claims_merged_with_claims_challenge, covering the previously untested three-way merge of server-issued claims_challenge + client capabilities + forwarded_client_claims into the single OAuth "claims" wire parameter. 200 passed across test_token_cache.py, test_mi.py, test_application.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply six fixes derived from the sibling MSAL PRs (go #629, java #1039, js #8686) to the forwarded_client_claims port: 1. Make _compute_ext_cache_key injective. Switch from separator-less key+value concatenation to length-prefixed pairs ("{len(k)}:{k}{len(v)}:{v}"), matching Go's post-collision-fix CacheExtKeyGenerator. Without this, fmi_path + client_claims (which now co-occur in acquire_token_for_client) could collide and return the wrong cached token. Adds boundary-collision regression tests. NOTE: hashes are now intentionally not byte-identical to MSAL .NET (which still uses unprefixed concat); caches are not shared across languages, so within-process injectivity is what matters. 2. Remove the MSIv1 client-side allow-list (_validate_msiv1_claims). Forward any JSON-object claims value as-is and let IMDS decide which keys it accepts, matching go/java. 3. Validate the managed-identity source before the cache read. Reject unsupported sources (Service Fabric, App Service, Machine Learning, Azure Arc) up front so an unsupported source never returns a cached client-claims token. _obtain_token keeps its per-source guards as a backstop. 4. Add merge-conflict precedence tests: on a direct leaf conflict the client-originated value wins (merged last); disjoint claims are preserved. 5. Drop the first-party xms_az_nwperimid example from public docstrings; use generic "client-originated claims" wording. 6. Document that the same forwarded_client_claims value must be sent on every request that should share the cached token (it is part of the cache key). 204 tests pass across test_token_cache.py, test_mi.py, test_application.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore _compute_ext_cache_key to MSAL .NET's ComputeAccessTokenExtCacheKey encoding: sorted, separator-less key+value concatenation -> SHA-256 -> base64url. This makes the ext cache key byte-identical to MSAL .NET again. The earlier hardening commit (4fc3639) had switched to Go's post-#629 length-prefixed encoding to make the key injective. Per maintainer decision, msal-python should match MSAL .NET, not current Go, so that change is reverted: - token_cache.py: restore plain key+value concatenation; docstring now notes the .NET match and the deliberate divergence from Go's #629 length-prefixed form. - test_token_cache.py: restore the .NET parity hashes (bns2ytmx..., 3-rg6_wy..., rn_gkpxx...) and rename the parity tests to *_matches_dotnet; remove the two length-prefix boundary-collision regression tests (they asserted the injective property that .NET's encoding does not provide). Hardening fixes #2-#6 (MI allow-list removal, MI source pre-validation, merge conflict-precedence tests, generic docs, send-on-every-request docs) are unchanged. 202 tests pass across test_token_cache.py, test_mi.py, test_application.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use kwargs.get("data", {}) instead of the truthiness form in acquire_token_silent and acquire_token_silent_with_error so a caller-provided empty mapping is not replaced with a fresh dict. Matches the fmi_path / for_client sibling sites. Per Copilot review.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Robbie-Microsoft
changed the title
The forwarded client_claims value (and the merged "claims") are kept in the cache-add event data only for ext_cache_key computation, but TokenCache.add() debug-logs event["data"] and previously masked only password/client_secret/refresh_token/assertion/user_fic. Add "client_claims" and "claims" to the masked fields so DEBUG logging cannot emit raw claim contents. Per Copilot review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…te_ext_cache_key Clarify that pseudo-parameters like client_claims intentionally feed the extended cache key hash while being stripped from the wire, so different client-originated claims route to separate cache entries. Doc-only. Per Copilot review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avery-Dunn
approved these changes
Jul 1, 2026
4gust
approved these changes
Jul 8, 2026
…entity Revert msal/managed_identity.py and tests/test_mi.py to their pre-PR (dev) state so forwarded_client_claims is supported only on the confidential client flows. The shared _parse_claims_or_raise / _compute_ext_cache_key helpers remain in use by the confidential client path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Robbie-Microsoft
changed the title
- Rewrap the _stash_client_claims docstring so no sentence dangles mid-line. - Split the same-value cache-hit assertions out of the leaf-conflict test into test_same_forwarded_client_claims_hits_cache, so each test covers one behavior. Co-authored-by: Copilot App <223556219+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
Ports msal-dotnet's
WithClaimsFromClientas a per-requestforwarded_client_claimskeyword — a JSON string of client-originated claims forwarded to ESTS. Scoped to confidential client, matching .NET (where it lives on the CCA builder base).Flows (
msal/application.py):acquire_token_for_client,acquire_token_on_behalf_of,acquire_token_by_user_federated_identity_credential,acquire_token_by_authorization_code,acquire_token_silent(_with_error).Behavior
claims_challenge.claims_challengeis server-issued and bypasses the cache;forwarded_client_claimsis cached, with the entry keyed on the claims value (reuses the existing_compute_ext_cache_key/fmi_pathmechanism). Send the same value on every request that should share a token; a different or absent value is a cache miss. Isolation is bidirectional.claims_challengeinto the single OAuthclaimsparameter; the client value wins a direct leaf conflict.ValueError. Raw value never appears in errors or DEBUG logs. No-op when absent.Naming
Renamed from
client_claims→forwarded_client_claimsto avoid colliding with the constructor'sclient_claims(a dict signed into the client-assertion JWT). The internal request-data key and wire behavior are unchanged.Files
msal/application.py_stash_client_claims()helper + wiring into the 5 flows.msal/token_cache.py_compute_ext_cache_keymatches MSAL .NET byte-for-byte.msal/oauth2cli/oauth2.pyclient_claimspseudo-param from the wire body.Testing