WithClientClaims() - forward client-originated claims across MSI and confidential client flows#5999
Conversation
…lows Introduces WithClientClaims(string claimsJson) as a client-originated claims API that, unlike WithClaims(), does NOT bypass the token cache. Tokens are cached and keyed on the normalized claims value. Key behaviors: - JSON is normalized (keys sorted, whitespace stripped) before use as a cache key to prevent cache fragmentation from cosmetic differences. - MSIv1 (IMDS GET): claims forwarded as 'claims' query parameter. - MSIv2 (ESTS POST): claims forwarded as 'claims' body parameter. - Cert/FIC: merged into ClaimsAndClientCapabilities sent to ESTS. - Cache key includes 'client_claims' component (via CacheKeyComponents). Files changed: - ClaimsHelper.cs: NormalizeClaimsJson, MergeClaimsObjects, SortJsonObjectKeys - AcquireTokenCommonParameters.cs: ClientClaims property - AcquireTokenForManagedIdentityParameters.cs: ClientClaims property - AcquireTokenForManagedIdentityParameterBuilder.cs: WithClientClaims() - AbstractConfidentialClientAcquireTokenParameterBuilderExtension.cs: WithClientClaims<T>() - AuthenticationRequestParameters.cs: merge ClientClaims into ClaimsAndClientCapabilities - ManagedIdentityAuthRequest.cs: propagate ClientClaims to MI parameters - AbstractManagedIdentity.cs: transport client claims (query/body) per request method - PublicAPI.Unshipped.txt (6 TFMs): new method signatures Open questions (see PR #5982): - MSIv2: IMDS team confirmation needed that ESTS endpoint accepts 'claims' body param - MSIv1 param name: confirm 'claims' vs IMDS-specific param name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- URL-encode ClientClaims with Uri.EscapeDataString before adding to QueryParameters in AbstractManagedIdentity.AuthenticateAsync. UriBuilder encodes braces/quotes but not colons when building the URI, while MockHttpMessageHandler parses the query without URL-decoding. Pre-encoding with EscapeDataString ensures the stored value (%7B%22...%7D) matches what ParseKeyValueList extracts. - Update WithClientClaimsTests to use Uri.EscapeDataString(normalizedClaims) as the expected claims value in extraQueryParameters mock setup. - Add ClaimsHelperTests (12 passing) and WithClientClaimsTests (33 passing) covering all 3 flows: IMDS GET, ESTS POST, and confidential client. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a proof-of-concept WithClientClaims() API to forward client-originated claims JSON through MSAL’s Managed Identity and confidential client token acquisition flows, while keeping tokens cacheable by keying cache entries on a normalized claims representation.
Changes:
- Added
WithClientClaims(string claimsJson)for Managed Identity and aWithClientClaims<T>(string claimsJson)extension for confidential client builders. - Implemented canonicalization helpers in
ClaimsHelper(normalize + merge) and propagated client claims through request parameters to MI transport (GET query param vs POST body). - Added unit tests for JSON normalization/merging and for expected caching / wire behavior in MI and confidential client scenarios.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/WithClientClaimsTests.cs | New tests covering MI + confidential client behavior, caching expectations, and invalid JSON handling. |
| tests/Microsoft.Identity.Test.Unit/CoreTests/OAuth2Tests/ClaimsHelperTests.cs | New unit tests for claims JSON normalization and merge behavior. |
| src/client/Microsoft.Identity.Client/PublicApi/netstandard2.0/PublicAPI.Unshipped.txt | Declares new public APIs for netstandard2.0. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0/PublicAPI.Unshipped.txt | Declares new public APIs for net8.0. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-ios/PublicAPI.Unshipped.txt | Declares new public APIs for net8.0-ios. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-android/PublicAPI.Unshipped.txt | Declares new public APIs for net8.0-android. |
| src/client/Microsoft.Identity.Client/PublicApi/net472/PublicAPI.Unshipped.txt | Declares new public APIs for net472. |
| src/client/Microsoft.Identity.Client/PublicApi/net462/PublicAPI.Unshipped.txt | Declares new public APIs for net462. |
| src/client/Microsoft.Identity.Client/ManagedIdentity/AbstractManagedIdentity.cs | Adds forwarding of client claims to MI requests (query param for GET; body param for POST). |
| src/client/Microsoft.Identity.Client/Internal/Requests/ManagedIdentityAuthRequest.cs | Propagates ClientClaims from AuthenticationRequestParameters into MI request parameters. |
| src/client/Microsoft.Identity.Client/Internal/Requests/AuthenticationRequestParameters.cs | Merges server-issued claims + client-originated claims into the claims parameter used in OAuth2 requests. |
| src/client/Microsoft.Identity.Client/Internal/ClaimsHelper.cs | Adds JSON normalization + merge helpers to canonicalize and combine claims objects. |
| src/client/Microsoft.Identity.Client/Extensibility/AbstractConfidentialClientAcquireTokenParameterBuilderExtension.cs | Adds confidential-client builder extension method WithClientClaims<T>. |
| src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenForManagedIdentityParameters.cs | Adds ClientClaims to MI parameter object and logs presence (without logging content). |
| src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenCommonParameters.cs | Adds ClientClaims to common parameters. |
| src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenForManagedIdentityParameterBuilder.cs | Adds MI builder method WithClientClaims(string) and includes normalized claims in cache key components. |
- Add ValidateUseOfExperimentalFeature() gate to WithClientClaims on both MSI builder and confidential client extension (null/whitespace still a no-op without the gate so existing no-claims callers are unaffected) - Fix double-call behavior: use SortedList indexer (last-write-wins) instead of .Add() which threw InvalidOperationException on second call - Fix security issue: remove raw claimsJson from MsalClientException message in NormalizeClaimsJson (sensitive data must not appear in logs/telemetry) - Wrap MergeClaimsObjects JSON parsing in MsalClientException to match the existing pattern from MergeClaimsIntoCapabilityJson - Fix misleading 'cached normally' comment in ManagedIdentityAuthRequest.cs to accurately describe the cache contract (keyed via CacheKeyComponents) - Remove unused System.Net.Http and System.Text.Json usings from test file - Add OIDC section 5.5 test coverage to ClaimsHelperTests: array element order preservation, null claim values, idempotency, URI-named claims, combined userinfo+id_token shape - Update all WithClientClaims tests to enable experimental features Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/client/Microsoft.Identity.Client/Internal/ClaimsHelper.cs:66
- This has the same exception-shaping gap as normalization:
ParseIntoJsonObjectcan throwInvalidOperationExceptionfor valid JSON that is not an object, but this block only translatesJsonException. A caller combiningWithClaims("[]")withWithClientClaims(...)would now get a raw exception instead of the MSAL invalid-claims error.
JObject obj1 = JsonHelper.ParseIntoJsonObject(claims1);
JObject obj2 = JsonHelper.ParseIntoJsonObject(claims2);
JObject merged = JsonHelper.Merge(obj1, obj2);
return JsonHelper.JsonObjectToString(merged);
}
catch (JsonException ex)
ParseIntoJsonObject calls JsonNode.AsObject() which throws InvalidOperationException (not JsonException) when the input is valid JSON but not an object (e.g. arrays, scalars). Broaden the catch clauses in NormalizeClaimsJson and MergeClaimsObjects to translate this into the expected MsalClientException(invalid_json_claims_format). Also adds a TODO comment in SortJsonObjectKeys noting that objects nested inside JSON arrays are not recursively key-sorted — a theoretical gap for current OIDC §5.5 callers who use string arrays, deferred. Adds 6 new tests covering the InvalidOperationException path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
src/client/Microsoft.Identity.Client/Internal/ClaimsHelper.cs:68
MergeClaimsObjectshas the same JSON-null gap: if either input is the literalnull,ParseIntoJsonObject(...).AsObject()can throwNullReferenceException, which bypasses this filter. That can happen when server claims are combined with client claims, so this should be translated to the documented MSAL invalid-claims exception just like other non-object JSON.
catch (Exception ex) when (ex is JsonException || ex is InvalidOperationException)
tests/Microsoft.Identity.Test.Unit/CoreTests/OAuth2Tests/ClaimsHelperTests.cs:309
- This assertion is reversed. The comment and expected sorted output say
id_tokenshould appear beforeuserinfo, but the assertion checks foruserinfobeforeid_token, causing the test to fail for the correct ordering.
// id_token sorts before userinfo
Assert.IsLessThan(result.IndexOf("userinfo", StringComparison.Ordinal), result.IndexOf("id_token", StringComparison.Ordinal),
"id_token must appear before userinfo after ordinal key sort.");
Bug fixes: - Fix NullReferenceException when WithClientClaims is called with the JSON literal 'null': JsonHelper.ParseIntoJsonObject now null-checks the parsed JsonNode and throws InvalidOperationException before calling .AsObject(), so callers get MsalClientException(invalid_json_claims_format) as expected - Remove raw claims payload from MsalClientException messages in MergeClaimsObjects and MergeClaimsIntoCapabilityJson — claims can contain sensitive data that must not appear in exception logs or telemetry - Fix reversed test assertions in ClaimsHelperTests: IndexOf(bronze) must be less than IndexOf(silver), and IndexOf(id_token) must be less than IndexOf(userinfo) after ordinal key sort New test coverage: - WithClientClaims_JsonNullLiteral_ThrowsMsalClientException: validates the NRE fix for the JSON 'null' literal - WithClientClaims_NonImdsSource_SetsBuilderParameter: documents that the builder parameter is set regardless of the underlying MI source (AppService) Docs: - Add XML doc <remarks> to the confidential client WithClientClaims extension noting that B2C, ADFS, and dSTS are unsupported/undefined for this API Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
InvalidOperationException is in the System namespace. The file had no using System directive, causing CS0246 on CI (Linux/net8.0 strict build). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Identity - ClaimsHelper.cs: fix 'using System' ordering (move to top per convention) - ClaimsHelper.cs: MergeClaimsIntoCapabilityJson now catches InvalidOperationException in addition to JsonException — consistent with NormalizeClaimsJson/MergeClaimsObjects - ClaimsHelper.cs: neutral error message in MergeClaimsIntoCapabilityJson; the method also handles server-issued claims from .WithClaims() so the message no longer names 'client_claims' specifically - ClaimsHelper.cs: SortJsonObjectKeys — add comment explaining why array elements are cloned as-is (OIDC array element order is semantically significant) - AbstractManagedIdentity.cs: gate ClientClaims forwarding to Imds/ImdsV2 sources only; other sources (App Service, Azure Arc, Service Fabric, etc.) have no confirmed contract for the 'claims' parameter and receiving it could cause failures - AbstractConfidentialClientAcquireTokenParameterBuilderExtension.cs: throw MsalClientException when WithClientClaims is called on GetAuthorizationRequestUrlParameterBuilder — client claims must not appear in front-channel auth URLs (security + cache key mismatch) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Directory.Packages.props was accidentally committed with MSTest bumped from 4.0.2 to 4.2.2. MSTest 4.2.2 introduces MSTEST0060 (TreatWarningsAsErrors) which flags E2E tests that use both [RunOn] and [TestMethod] — a valid pattern in this repo where [RunOn] inherits from TestMethodAttribute. Revert to 4.0.2. Also replace Assert.IsLessThan(a, b) with Assert.IsTrue(a < b) in ClaimsHelperTests to avoid argument-order ambiguity between MSTest versions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Centralize claims JSON parsing in ClaimsHelper.ParseClaimsOrThrow so all three call sites (MergeClaimsObjects, MergeClaimsIntoCapabilityJson, ValidateMsiv1Claims) produce the same friendly MsalClientException with error code InvalidJsonClaimsFormat. Previously ValidateMsiv1Claims leaked a raw JsonException on malformed input. (Avery-Dunn) - Add test: combined .WithClaims + .WithClaimsFromClient on IMDS — verifies only client claims go on the wire and .WithClaims still bypasses the cache. (neha-bhargava) - Add [DataRow]-parameterized test enumerating App Service, Azure Arc, Cloud Shell, Service Fabric, and Machine Learning sources; each must throw MsalClientException at ExecuteAsync time. (neha-bhargava) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Proof-of-concept implementation of
WithClaimsFromClient(string claimsJson)— a new per-request API that lets clients forward a JSON claims payload (e.g., NSPxms_az_nwperimid) to ESTS / IMDS. Companion to the design doc in #5982 (now merged).What this PR does
Adds the API on:
AcquireTokenForManagedIdentityParameterBuilder(MSI flows — currently MSIv1 only; MSIv2 design pending IMDS team)AcquireTokenForClientParameterBuilderand the FIC builder, viaAbstractConfidentialClientAcquireTokenParameterBuilderExtensionWires the claims through to the existing claims/capabilities merge pipeline so they participate correctly in cache keying and are sent on the wire as a standard OAuth
claimsparameter (body for ESTS, query string for IMDS v1).Out of scope (tracked separately)
/issuecredentialboolean parameter (blocked on IMDS team design)