From f8b4faa984c6ddcabb2b83e881c2e614ad13fd5c Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 26 Jun 2026 14:24:04 -0400 Subject: [PATCH 1/3] Add claimsFromClient API for client-originated claims Ports msal-dotnet PR #5999 (WithClaimsFromClient) to msal-java as a new per-request builder method claimsFromClient(String) on the Managed Identity, Client Credentials, On-Behalf-Of, and User Federated Identity Credential parameter builders. Unlike server-issued `claims` challenges (CAE), which bypass and refresh the cache, client-originated claims are forwarded on the wire as a standard OAuth `claims` parameter and are cached, keyed per distinct claims value via an extended access-token cache key (extCacheKeyHash). - New claimsFromClient(String) builders: blank is a no-op; non-object JSON throws MsalClientException(INVALID_JSON) via JsonHelper.validateJsonObjectFormat. - Wire injection in TokenRequestExecutor for confidential-client flows; IMDS transport (query string for GET, body for POST) in AbstractManagedIdentitySource. - Managed Identity restricts client claims to IMDS (MSIv1), allowing only the xms_az_nwperimid claim; other sources or keys throw INVALID_REQUEST. - Cache write/read paths keyed on extCacheKeyHash across CC/MI/OBO/FIC, including user-token isolation for the FIC flow. - Tests: new ClientClaimsTest plus FIC and Managed Identity coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../msal4j/AbstractManagedIdentitySource.java | 60 ++++ ...AcquireTokenByManagedIdentitySupplier.java | 6 + .../AcquireTokenByOnBehalfOfSupplier.java | 6 + ...erFederatedIdentityCredentialSupplier.java | 8 + .../msal4j/AcquireTokenSilentSupplier.java | 3 +- .../aad/msal4j/AuthenticationErrorCode.java | 7 + .../msal4j/ClientCredentialParameters.java | 55 +++- .../aad/msal4j/IAcquireTokenParameters.java | 26 ++ .../com/microsoft/aad/msal4j/JsonHelper.java | 22 ++ .../aad/msal4j/ManagedIdentityParameters.java | 89 +++++- .../aad/msal4j/OnBehalfOfParameters.java | 84 +++++- .../com/microsoft/aad/msal4j/TokenCache.java | 32 +- .../aad/msal4j/TokenRequestExecutor.java | 14 + ...FederatedIdentityCredentialParameters.java | 86 +++++- .../aad/msal4j/ClientClaimsTest.java | 277 ++++++++++++++++++ .../aad/msal4j/ManagedIdentityTests.java | 85 ++++++ .../UserFederatedIdentityCredentialTest.java | 80 +++++ 17 files changed, 925 insertions(+), 15 deletions(-) create mode 100644 msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java index 2593dc41..83232c57 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java @@ -9,6 +9,8 @@ import java.net.HttpURLConnection; import java.net.SocketException; import java.net.URISyntaxException; +import java.util.HashMap; +import java.util.Map; //base class for all sources that support managed identity abstract class AbstractManagedIdentitySource { @@ -16,6 +18,10 @@ abstract class AbstractManagedIdentitySource { private static final Logger LOG = LoggerFactory.getLogger(AbstractManagedIdentitySource.class); private static final String MANAGED_IDENTITY_NO_RESPONSE_RECEIVED = "[Managed Identity] Authentication unavailable. No response received from the managed identity endpoint."; + // IMDS (MSIv1) only supports this single custom claim. Any other top-level key causes IMDS to + // return HTTP 400 with no useful diagnostic, so it is rejected client-side before the network call. + private static final String XMS_AZ_NWPERIMID = "xms_az_nwperimid"; + protected final ManagedIdentityRequest managedIdentityRequest; protected final ServiceBundle serviceBundle; ManagedIdentitySourceType managedIdentitySourceType; @@ -42,6 +48,7 @@ public ManagedIdentityResponse getManagedIdentityResponse( createManagedIdentityRequest(parameters.resource); managedIdentityRequest.addTokenRevocationParametersToQuery(parameters); + addClientClaimsToRequest(parameters); IHttpResponse response; try { @@ -62,6 +69,59 @@ public ManagedIdentityResponse getManagedIdentityResponse( return handleResponse(parameters, response); } + /** + * Forwards client-originated claims (set via + * {@link ManagedIdentityParameters.ManagedIdentityParametersBuilder#claimsFromClient(String)}) to + * the managed identity endpoint. Only IMDS-based managed identity is supported; other sources fail + * fast rather than silently dropping the value (which would also pollute the cache with a key the + * endpoint never saw). For IMDS (a GET request) the claims are added as a query parameter; for any + * POST-based source they would be added to the body. + */ + private void addClientClaimsToRequest(ManagedIdentityParameters parameters) { + if (StringHelper.isNullOrBlank(parameters.clientClaims)) { + return; + } + + if (managedIdentitySourceType != ManagedIdentitySourceType.IMDS) { + throw new MsalClientException( + String.format("claimsFromClient is only supported for IMDS-based managed identity sources. " + + "The detected source is %s.", managedIdentitySourceType), + AuthenticationErrorCode.INVALID_REQUEST); + } + + // IMDS == MSIv1: validate that the only top-level claim key is xms_az_nwperimid. + validateMsiv1Claims(parameters.clientClaims); + + if (managedIdentityRequest.method == HttpMethod.GET) { + if (managedIdentityRequest.queryParameters == null) { + managedIdentityRequest.queryParameters = new HashMap<>(); + } + // The value is URL-encoded later by StringHelper.serializeQueryParameters. + managedIdentityRequest.queryParameters.put("claims", parameters.clientClaims); + LOG.info("[Managed Identity] Adding client claims to IMDS request as query parameter."); + } else { + if (managedIdentityRequest.bodyParameters == null) { + managedIdentityRequest.bodyParameters = new HashMap<>(); + } + managedIdentityRequest.bodyParameters.put("claims", parameters.clientClaims); + LOG.info("[Managed Identity] Adding client claims to request body."); + } + } + + private static void validateMsiv1Claims(String claimsJson) { + Map parsed = JsonHelper.parseJsonToMap(claimsJson); + for (String key : parsed.keySet()) { + if (!XMS_AZ_NWPERIMID.equals(key)) { + throw new MsalClientException( + String.format("MSIv1 (IMDS v1) only supports the `%s` custom claim. " + + "The claims JSON contained the unsupported key `%s`. " + + "Remove all keys other than `%s` when using claimsFromClient with MSIv1.", + XMS_AZ_NWPERIMID, key, XMS_AZ_NWPERIMID), + AuthenticationErrorCode.INVALID_REQUEST); + } + } + } + public ManagedIdentityResponse handleResponse( ManagedIdentityParameters parameters, IHttpResponse response) { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java index c6545cf7..10c0b8a8 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java @@ -67,6 +67,12 @@ AuthenticationResult execute() throws Exception { context, null); + // Propagate ext_cache_key_hash for cache isolation (e.g., client_claims) + String extCacheKeyHash = managedIdentityParameters.computeExtCacheKeyHash(); + if (!StringHelper.isBlank(extCacheKeyHash)) { + silentRequest.extCacheKeyHash(extCacheKeyHash); + } + AcquireTokenSilentSupplier supplier = new AcquireTokenSilentSupplier( this.clientApplication, silentRequest); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByOnBehalfOfSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByOnBehalfOfSupplier.java index 2f438997..3d348004 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByOnBehalfOfSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByOnBehalfOfSupplier.java @@ -40,6 +40,12 @@ AuthenticationResult execute() throws Exception { context, onBehalfOfRequest.parameters.userAssertion()); + // Propagate ext_cache_key_hash for cache isolation (e.g., client_claims) + String extCacheKeyHash = this.onBehalfOfRequest.parameters.computeExtCacheKeyHash(); + if (!StringHelper.isBlank(extCacheKeyHash)) { + silentRequest.extCacheKeyHash(extCacheKeyHash); + } + AcquireTokenSilentSupplier supplier = new AcquireTokenSilentSupplier( this.clientApplication, silentRequest); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByUserFederatedIdentityCredentialSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByUserFederatedIdentityCredentialSupplier.java index 22bb0072..e8513de1 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByUserFederatedIdentityCredentialSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByUserFederatedIdentityCredentialSupplier.java @@ -48,6 +48,14 @@ AuthenticationResult execute() throws Exception { context, null); + // Propagate ext_cache_key_hash for cache isolation (e.g., client_claims). + // User-FIC tokens are account-scoped, so the user-token read path in TokenCache + // must also filter on this hash for the isolation to take effect. + String extCacheKeyHash = this.userFicRequest.parameters.computeExtCacheKeyHash(); + if (!StringHelper.isBlank(extCacheKeyHash)) { + silentRequest.extCacheKeyHash(extCacheKeyHash); + } + AcquireTokenSilentSupplier supplier = new AcquireTokenSilentSupplier( this.clientApplication, silentRequest); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java index 84ef8071..1071d654 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java @@ -39,7 +39,8 @@ AuthenticationResult execute() throws Exception { silentRequest.parameters().account(), requestAuthority, silentRequest.parameters().scopes(), - clientApplication.clientId()); + clientApplication.clientId(), + silentRequest.extCacheKeyHash()); if (res == null) { throw new MsalClientException(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE, AuthenticationErrorCode.CACHE_MISS); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java index d5fba3df..1776f561 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java @@ -166,4 +166,11 @@ public class AuthenticationErrorCode { * This is returned by the instance discovery endpoint when the provided authority host is unknown. */ public static final String INVALID_INSTANCE = "invalid_instance"; + + /** + * Indicates that the request is malformed or uses an unsupported parameter combination, for + * example when client-originated claims are supplied to a managed identity source that does not + * support them, or when an unsupported claim key is used. + */ + public static final String INVALID_REQUEST = "invalid_request"; } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java index 3146cb26..b8c5e107 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java @@ -32,6 +32,8 @@ public class ClientCredentialParameters implements IAcquireTokenParameters { private String fmiPath; + private String clientClaims; + // Generic extended cache key components. Any optional or flow-specific parameters // that should influence token cache isolation adds an entry here. The hash of these // components is used as part of the cache key in relevant scenarios entries. @@ -40,7 +42,7 @@ public class ClientCredentialParameters implements IAcquireTokenParameters { // Memoized hash of cacheKeyComponents (computed once since parameters are immutable). private String extCacheKeyHashCache; - private ClientCredentialParameters(Set scopes, Boolean skipCache, ClaimsRequest claims, Map extraHttpHeaders, Map extraQueryParameters, String tenant, IClientCredential clientCredential, String fmiPath) { + private ClientCredentialParameters(Set scopes, Boolean skipCache, ClaimsRequest claims, Map extraHttpHeaders, Map extraQueryParameters, String tenant, IClientCredential clientCredential, String fmiPath, String clientClaims) { this.scopes = scopes; this.skipCache = skipCache; this.claims = claims; @@ -49,6 +51,7 @@ private ClientCredentialParameters(Set scopes, Boolean skipCache, Claims this.tenant = tenant; this.clientCredential = clientCredential; this.fmiPath = fmiPath; + this.clientClaims = clientClaims; // Build cache key components from any parameters that require cache isolation. this.cacheKeyComponents = buildCacheKeyComponents(); @@ -114,6 +117,16 @@ public String fmiPath() { return this.fmiPath; } + /** + * Client-originated claims set via {@link ClientCredentialParametersBuilder#claimsFromClient(String)}. + * Forwarded to the token endpoint as the OAuth {@code claims} parameter and used as part of the + * extended cache key so that distinct claim values are cached separately. + */ + @Override + public String clientClaims() { + return this.clientClaims; + } + /** * Builds the sorted map of cache key components from the parameters that require * cache isolation. Returns null if no components are present. @@ -127,6 +140,12 @@ private SortedMap buildCacheKeyComponents() { components = new TreeMap<>(); components.put("fmi_path", fmiPath); } + if (!StringHelper.isBlank(clientClaims)) { + if (components == null) { + components = new TreeMap<>(); + } + components.put("client_claims", clientClaims); + } return components; } @@ -145,7 +164,15 @@ SortedMap cacheKeyComponents() { * The result is memoized since ClientCredentialParameters is immutable after construction. * Used by both cache writes ({@link TokenCache}) and cache reads (silent lookup). */ - String computeExtCacheKeyHash() { + /** + * Computes the extended cache key hash from all cache key components. + * Returns an empty string if no components are present. + *

+ * The result is memoized since ClientCredentialParameters is immutable after construction. + * Used by both cache writes ({@link TokenCache}) and cache reads (silent lookup). + */ + @Override + public String computeExtCacheKeyHash() { if (extCacheKeyHashCache != null) { return extCacheKeyHashCache; } @@ -162,6 +189,7 @@ public static class ClientCredentialParametersBuilder { private String tenant; private IClientCredential clientCredential; private String fmiPath; + private String clientClaims; ClientCredentialParametersBuilder() { } @@ -245,8 +273,29 @@ public ClientCredentialParametersBuilder fmiPath(String fmiPath) { return this; } + /** + * Specifies client-originated claims (a raw JSON object string) to forward to the token + * endpoint as the OAuth {@code claims} request parameter. Unlike {@link #claims(ClaimsRequest)} + * (server-issued claims challenges, which bypass the cache), tokens acquired with client claims + * are cached and the cache entry is keyed on the claims value, so distinct claim values produce + * separate cache entries. Use stable, non-dynamic values to avoid cache fragmentation. + * A blank value is ignored; an invalid JSON object throws {@link MsalClientException}. + * + * @param claimsJson a valid JSON object string containing the client claims + * @return builder that can be used to construct ClientCredentialParameters + */ + public ClientCredentialParametersBuilder claimsFromClient(String claimsJson) { + if (StringHelper.isBlank(claimsJson)) { + return this; + } + + JsonHelper.validateJsonObjectFormat(claimsJson); + this.clientClaims = claimsJson; + return this; + } + public ClientCredentialParameters build() { - return new ClientCredentialParameters(this.scopes, this.skipCache, this.claims, this.extraHttpHeaders, this.extraQueryParameters, this.tenant, this.clientCredential, this.fmiPath); + return new ClientCredentialParameters(this.scopes, this.skipCache, this.claims, this.extraHttpHeaders, this.extraQueryParameters, this.tenant, this.clientCredential, this.fmiPath, this.clientClaims); } public String toString() { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IAcquireTokenParameters.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IAcquireTokenParameters.java index 5c6acb9d..fd92fcaf 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IAcquireTokenParameters.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IAcquireTokenParameters.java @@ -67,4 +67,30 @@ interface IAcquireTokenParameters { */ @Deprecated Map extraQueryParameters(); + + /** + * Gets the client-originated claims (a raw JSON string) set via the per-request + * {@code claimsFromClient(...)} builder method. + *

+ * Unlike {@link #claims()} (server-issued claims challenges, which bypass/refresh the cache), + * client claims are cached and the cache entry is keyed on the claims value, and they are sent on + * the wire as a standard OAuth {@code claims} parameter. + * + * @return the client-originated claims JSON, or null if none were provided. + */ + default String clientClaims() { + return null; + } + + /** + * Computes the extended cache-key hash contributed by this request's parameters (for example + * {@code fmi_path} or {@code client_claims}). The hash isolates cache entries so that requests + * with different component values do not collide. Returns an empty string when there are no + * extended cache-key components. + * + * @return the Base64URL-encoded SHA-256 hash of the cache-key components, or an empty string. + */ + default String computeExtCacheKeyHash() { + return ""; + } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java index d566cfb6..5c2a28b2 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java @@ -157,6 +157,28 @@ static void validateJsonFormat(String jsonString) { } } + /** + * Validates that the supplied string is a well-formed JSON object (the root token is + * {@code {}}). Throws {@link MsalClientException} with {@link AuthenticationErrorCode#INVALID_JSON} + * otherwise. The raw value is never included in the exception message, since claims payloads may + * contain sensitive data. + */ + static void validateJsonObjectFormat(String jsonString) { + try (JsonReader reader = JsonProviders.createReader(jsonString)) { + if (reader.nextToken() != JsonToken.START_OBJECT) { + throw new MsalClientException( + "The claims value is not a valid JSON object. " + + "See https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter.", + AuthenticationErrorCode.INVALID_JSON); + } + reader.skipChildren(); + } catch (IOException e) { + throw new MsalClientException( + "The claims value is not a valid JSON object: " + e.getMessage(), + AuthenticationErrorCode.INVALID_JSON); + } + } + public static String formCapabilitiesJson(Set clientCapabilities) { if (clientCapabilities == null || clientCapabilities.isEmpty()) { return null; diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityParameters.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityParameters.java index 21335802..787488b3 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityParameters.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityParameters.java @@ -5,6 +5,8 @@ import java.util.Map; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; /** * Object containing parameters for managed identity flow. Can be used as parameter to @@ -15,12 +17,24 @@ public class ManagedIdentityParameters implements IAcquireTokenParameters { String resource; boolean forceRefresh; String claims; + String clientClaims; String revokedTokenHash; - - private ManagedIdentityParameters(String resource, boolean forceRefresh, String claims) { + + // Generic extended cache key components. The hash of these components isolates cache + // entries so that requests with different client-claims values do not collide. + private SortedMap cacheKeyComponents; + + // Memoized hash of cacheKeyComponents (computed once since parameters are immutable). + private String extCacheKeyHashCache; + + private ManagedIdentityParameters(String resource, boolean forceRefresh, String claims, String clientClaims) { this.resource = resource; this.forceRefresh = forceRefresh; this.claims = claims; + this.clientClaims = clientClaims; + + // Build cache key components from any parameters that require cache isolation. + this.cacheKeyComponents = buildCacheKeyComponents(); } @Override @@ -83,10 +97,55 @@ public String revokedTokenHash() { return this.revokedTokenHash; } + /** + * Client-originated claims set via {@link ManagedIdentityParametersBuilder#claimsFromClient(String)}. + * Unlike {@link #claims()} (server-issued, cache-bypassing), these are cached and keyed on the + * raw claims string as passed by the caller. + */ + @Override + public String clientClaims() { + return this.clientClaims; + } + + /** + * Builds the sorted map of cache key components from the parameters that require cache isolation. + * Returns null if no components are present. + */ + private SortedMap buildCacheKeyComponents() { + TreeMap components = null; + if (!StringHelper.isNullOrBlank(clientClaims)) { + components = new TreeMap<>(); + components.put("client_claims", clientClaims); + } + return components; + } + + /** + * Returns the extended cache key components for this request, if any. + * Used by {@link TokenCache} for both cache writes and reads. + */ + SortedMap cacheKeyComponents() { + return this.cacheKeyComponents; + } + + /** + * Computes the extended cache key hash from all cache key components, or an empty string when + * there are none. The result is memoized since the parameters are immutable after construction. + */ + @Override + public String computeExtCacheKeyHash() { + if (extCacheKeyHashCache != null) { + return extCacheKeyHashCache; + } + extCacheKeyHashCache = StringHelper.computeExtCacheKeyHash(cacheKeyComponents); + return extCacheKeyHashCache; + } + public static class ManagedIdentityParametersBuilder { private String resource; private boolean forceRefresh; private String claims; + private String clientClaims; ManagedIdentityParametersBuilder() { } @@ -118,8 +177,32 @@ public ManagedIdentityParametersBuilder claims(String claims) { return this; } + /** + * Specifies client-originated claims (a raw JSON object string) to forward to the identity + * endpoint. Unlike {@link #claims(String)} (server-issued claims challenges, which bypass the + * cache), tokens acquired with client claims are cached and the cache entry is keyed on the + * claims value. Different claim values produce separate cache entries, so use stable, + * non-dynamic values to avoid cache fragmentation. + *

+ * Only IMDS-based managed identity is supported, and IMDS (MSIv1) only accepts the single + * custom claim {@code xms_az_nwperimid}; any other source or claim key causes the request to + * fail with an {@link MsalClientException}. A blank value is ignored. + * + * @param claimsJson a valid JSON object string containing the client claims + * @return this builder instance + */ + public ManagedIdentityParametersBuilder claimsFromClient(String claimsJson) { + if (StringHelper.isNullOrBlank(claimsJson)) { + return this; + } + + JsonHelper.validateJsonObjectFormat(claimsJson); + this.clientClaims = claimsJson; + return this; + } + public ManagedIdentityParameters build() { - return new ManagedIdentityParameters(this.resource, this.forceRefresh, this.claims); + return new ManagedIdentityParameters(this.resource, this.forceRefresh, this.claims, this.clientClaims); } public String toString() { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OnBehalfOfParameters.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OnBehalfOfParameters.java index f954c32b..07f8e345 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OnBehalfOfParameters.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OnBehalfOfParameters.java @@ -5,6 +5,8 @@ import java.util.Map; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import static com.microsoft.aad.msal4j.ParameterValidationUtils.validateNotNull; @@ -23,8 +25,16 @@ public class OnBehalfOfParameters implements IAcquireTokenParameters { private Map extraHttpHeaders; private Map extraQueryParameters; private String tenant; + private String clientClaims; - private OnBehalfOfParameters(Set scopes, Boolean skipCache, IUserAssertion userAssertion, ClaimsRequest claims, Map extraHttpHeaders, Map extraQueryParameters, String tenant) { + // Generic extended cache key components. The hash of these components isolates cache + // entries so that requests with different client-claims values do not collide. + private SortedMap cacheKeyComponents; + + // Memoized hash of cacheKeyComponents (computed once since parameters are immutable). + private String extCacheKeyHashCache; + + private OnBehalfOfParameters(Set scopes, Boolean skipCache, IUserAssertion userAssertion, ClaimsRequest claims, Map extraHttpHeaders, Map extraQueryParameters, String tenant, String clientClaims) { this.scopes = scopes; //Legacy code that made the public parameter take the Boolean class instead of the primitive, so we need a null check this.skipCache = skipCache != null && skipCache; @@ -33,6 +43,10 @@ private OnBehalfOfParameters(Set scopes, Boolean skipCache, IUserAsserti this.extraHttpHeaders = extraHttpHeaders; this.extraQueryParameters = extraQueryParameters; this.tenant = tenant; + this.clientClaims = clientClaims; + + // Build cache key components from any parameters that require cache isolation. + this.cacheKeyComponents = buildCacheKeyComponents(); } private static OnBehalfOfParametersBuilder builder() { @@ -88,6 +102,50 @@ public String tenant() { return this.tenant; } + /** + * Client-originated claims set via {@link OnBehalfOfParametersBuilder#claimsFromClient(String)}. + * Forwarded to the token endpoint as the OAuth {@code claims} parameter and used as part of the + * extended cache key so that distinct claim values are cached separately. + */ + @Override + public String clientClaims() { + return this.clientClaims; + } + + /** + * Builds the sorted map of cache key components from the parameters that require cache isolation. + * Returns null if no components are present. + */ + private SortedMap buildCacheKeyComponents() { + TreeMap components = null; + if (!StringHelper.isBlank(clientClaims)) { + components = new TreeMap<>(); + components.put("client_claims", clientClaims); + } + return components; + } + + /** + * Returns the extended cache key components for this request, if any. + * Used by {@link TokenCache} for both cache writes and reads. + */ + SortedMap cacheKeyComponents() { + return this.cacheKeyComponents; + } + + /** + * Computes the extended cache key hash from all cache key components, or an empty string when + * there are none. The result is memoized since the parameters are immutable after construction. + */ + @Override + public String computeExtCacheKeyHash() { + if (extCacheKeyHashCache != null) { + return extCacheKeyHashCache; + } + extCacheKeyHashCache = StringHelper.computeExtCacheKeyHash(cacheKeyComponents); + return extCacheKeyHashCache; + } + public static class OnBehalfOfParametersBuilder { private Set scopes; private Boolean skipCache; @@ -96,6 +154,7 @@ public static class OnBehalfOfParametersBuilder { private Map extraHttpHeaders; private Map extraQueryParameters; private String tenant; + private String clientClaims; OnBehalfOfParametersBuilder() { } @@ -156,8 +215,29 @@ public OnBehalfOfParametersBuilder tenant(String tenant) { return this; } + /** + * Specifies client-originated claims (a raw JSON object string) to forward to the token + * endpoint as the OAuth {@code claims} request parameter. Unlike {@link #claims(ClaimsRequest)} + * (server-issued claims challenges, which bypass the cache), tokens acquired with client claims + * are cached and the cache entry is keyed on the claims value, so distinct claim values produce + * separate cache entries. Use stable, non-dynamic values to avoid cache fragmentation. + * A blank value is ignored; an invalid JSON object throws {@link MsalClientException}. + * + * @param claimsJson a valid JSON object string containing the client claims + * @return this builder instance + */ + public OnBehalfOfParametersBuilder claimsFromClient(String claimsJson) { + if (StringHelper.isBlank(claimsJson)) { + return this; + } + + JsonHelper.validateJsonObjectFormat(claimsJson); + this.clientClaims = claimsJson; + return this; + } + public OnBehalfOfParameters build() { - return new OnBehalfOfParameters(this.scopes, this.skipCache, this.userAssertion, this.claims, this.extraHttpHeaders, this.extraQueryParameters, this.tenant); + return new OnBehalfOfParameters(this.scopes, this.skipCache, this.userAssertion, this.claims, this.extraHttpHeaders, this.extraQueryParameters, this.tenant, this.clientClaims); } public String toString() { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java index e1c28700..635dd9a7 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java @@ -339,12 +339,16 @@ private static AccessTokenCacheEntity createAccessTokenCacheEntity(TokenRequestE /** * Computes the extended cache key hash for a request, if applicable. - * Delegates to the generic cache key components on the parameters object. + * Delegates to the generic cache key components exposed by the request's parameters object + * ({@link IAcquireTokenParameters#computeExtCacheKeyHash()}), which covers client credentials, + * managed identity, on-behalf-of and user-FIC. Parameter types without extended cache-key + * components return an empty string via the interface default. * The algorithm uses sorted key-value concatenation → SHA-256 → Base64URL (cross-SDK compatible). */ private static String computeExtCacheKeyHashForRequest(MsalRequest msalRequest) { - if (msalRequest instanceof ClientCredentialRequest) { - return ((ClientCredentialRequest) msalRequest).parameters.computeExtCacheKeyHash(); + IAcquireTokenParameters apiParameters = msalRequest.requestContext().apiParameters(); + if (apiParameters != null) { + return apiParameters.computeExtCacheKeyHash(); } return ""; } @@ -544,11 +548,22 @@ private Optional getAccessTokenCacheEntity( Set scopes, String clientId, Set environmentAliases) { + return getAccessTokenCacheEntity(account, authority, scopes, clientId, environmentAliases, null); + } + + private Optional getAccessTokenCacheEntity( + IAccount account, + Authority authority, + Set scopes, + String clientId, + Set environmentAliases, + String extCacheKeyHash) { return accessTokens.values().stream().filter( accessToken -> accessToken.homeAccountId != null && accessToken.homeAccountId.equals(account.homeAccountId()) && + extCacheKeyHashMatches(accessToken, extCacheKeyHash) && environmentAliases.contains(accessToken.environment) && accessToken.realm.equals(authority.tenant()) && accessToken.clientId.equals(clientId) && @@ -686,6 +701,15 @@ AuthenticationResult getCachedAuthenticationResult( Authority authority, Set scopes, String clientId) { + return getCachedAuthenticationResult(account, authority, scopes, clientId, null); + } + + AuthenticationResult getCachedAuthenticationResult( + IAccount account, + Authority authority, + Set scopes, + String clientId, + String extCacheKeyHash) { AuthenticationResult.AuthenticationResultBuilder builder = AuthenticationResult.builder(); @@ -704,7 +728,7 @@ AuthenticationResult getCachedAuthenticationResult( getAccountCacheEntity(account, environmentAliases); Optional atCacheEntity = - getAccessTokenCacheEntity(account, authority, scopes, clientId, environmentAliases); + getAccessTokenCacheEntity(account, authority, scopes, clientId, environmentAliases, extCacheKeyHash); Optional idTokenCacheEntity = getIdTokenCacheEntity(account, authority, clientId, environmentAliases); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java index 3c7e3519..81995492 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java @@ -63,6 +63,20 @@ OAuthHttpRequest createOauthHttpRequest() throws MalformedURLException { params.put("claims", claimsRequest); } + // Client-originated claims (claimsFromClient) are forwarded on the wire as a standard OAuth + // "claims" parameter. They are merged here, after the capabilities/server-claims merge above, + // because that logic rebuilds the "claims" param and would otherwise clobber an earlier value. + // This single point covers the confidential-client flows (client credentials, OBO, user-FIC); + // public-client and managed-identity flows return null here and are unaffected. + String clientClaims = msalRequest.requestContext().apiParameters().clientClaims(); + if (!StringHelper.isBlank(clientClaims)) { + if (params.get("claims") != null) { + params.put("claims", JsonHelper.mergeJSONString(params.get("claims"), clientClaims)); + } else { + params.put("claims", clientClaims); + } + } + if(msalRequest.requestContext().apiParameters().extraQueryParameters() != null ){ for(String key: msalRequest.requestContext().apiParameters().extraQueryParameters().keySet()){ if(params.containsKey(key)){ diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialParameters.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialParameters.java index 8a0299dc..5cd48f09 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialParameters.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialParameters.java @@ -5,6 +5,8 @@ import java.util.Map; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.UUID; import static com.microsoft.aad.msal4j.ParameterValidationUtils.validateNotBlank; @@ -29,6 +31,14 @@ public class UserFederatedIdentityCredentialParameters implements IAcquireTokenP private Map extraHttpHeaders; private Map extraQueryParameters; private String tenant; + private String clientClaims; + + // Generic extended cache key components. The hash of these components isolates cache + // entries so that requests with different client-claims values do not collide. + private SortedMap cacheKeyComponents; + + // Memoized hash of cacheKeyComponents (computed once since parameters are immutable). + private String extCacheKeyHashCache; private UserFederatedIdentityCredentialParameters( Set scopes, @@ -39,7 +49,8 @@ private UserFederatedIdentityCredentialParameters( ClaimsRequest claims, Map extraHttpHeaders, Map extraQueryParameters, - String tenant) { + String tenant, + String clientClaims) { this.scopes = scopes; this.username = username; this.userObjectId = userObjectId; @@ -49,6 +60,10 @@ private UserFederatedIdentityCredentialParameters( this.extraHttpHeaders = extraHttpHeaders; this.extraQueryParameters = extraQueryParameters; this.tenant = tenant; + this.clientClaims = clientClaims; + + // Build cache key components from any parameters that require cache isolation. + this.cacheKeyComponents = buildCacheKeyComponents(); } /** @@ -145,6 +160,51 @@ public String tenant() { return this.tenant; } + /** + * Client-originated claims set via + * {@link UserFederatedIdentityCredentialParametersBuilder#claimsFromClient(String)}. + * Forwarded to the token endpoint as the OAuth {@code claims} parameter and used as part of the + * extended cache key so that distinct claim values are cached separately. + */ + @Override + public String clientClaims() { + return this.clientClaims; + } + + /** + * Builds the sorted map of cache key components from the parameters that require cache isolation. + * Returns null if no components are present. + */ + private SortedMap buildCacheKeyComponents() { + TreeMap components = null; + if (!StringHelper.isBlank(clientClaims)) { + components = new TreeMap<>(); + components.put("client_claims", clientClaims); + } + return components; + } + + /** + * Returns the extended cache key components for this request, if any. + * Used by {@link TokenCache} for both cache writes and reads. + */ + SortedMap cacheKeyComponents() { + return this.cacheKeyComponents; + } + + /** + * Computes the extended cache key hash from all cache key components, or an empty string when + * there are none. The result is memoized since the parameters are immutable after construction. + */ + @Override + public String computeExtCacheKeyHash() { + if (extCacheKeyHashCache != null) { + return extCacheKeyHashCache; + } + extCacheKeyHashCache = StringHelper.computeExtCacheKeyHash(cacheKeyComponents); + return extCacheKeyHashCache; + } + public static class UserFederatedIdentityCredentialParametersBuilder { private Set scopes; private String username; @@ -155,6 +215,7 @@ public static class UserFederatedIdentityCredentialParametersBuilder { private Map extraHttpHeaders; private Map extraQueryParameters; private String tenant; + private String clientClaims; UserFederatedIdentityCredentialParametersBuilder() { } @@ -225,11 +286,32 @@ public UserFederatedIdentityCredentialParametersBuilder tenant(String tenant) { return this; } + /** + * Specifies client-originated claims (a raw JSON object string) to forward to the token + * endpoint as the OAuth {@code claims} request parameter. Unlike {@link #claims(ClaimsRequest)} + * (server-issued claims challenges, which bypass the cache), tokens acquired with client claims + * are cached and the cache entry is keyed on the claims value, so distinct claim values produce + * separate cache entries. Use stable, non-dynamic values to avoid cache fragmentation. + * A blank value is ignored; an invalid JSON object throws {@link MsalClientException}. + * + * @param claimsJson a valid JSON object string containing the client claims + * @return the builder + */ + public UserFederatedIdentityCredentialParametersBuilder claimsFromClient(String claimsJson) { + if (StringHelper.isBlank(claimsJson)) { + return this; + } + + JsonHelper.validateJsonObjectFormat(claimsJson); + this.clientClaims = claimsJson; + return this; + } + public UserFederatedIdentityCredentialParameters build() { return new UserFederatedIdentityCredentialParameters( this.scopes, this.username, this.userObjectId, this.assertion, this.forceRefresh, this.claims, this.extraHttpHeaders, - this.extraQueryParameters, this.tenant); + this.extraQueryParameters, this.tenant, this.clientClaims); } } } diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java new file mode 100644 index 00000000..42a30946 --- /dev/null +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java @@ -0,0 +1,277 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +import java.util.Collections; +import java.util.HashMap; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +/** + * Tests for the per-request {@code claimsFromClient(String)} API across confidential-client flows + * (client credentials and on-behalf-of). Client-originated claims differ from server-issued + * {@code claims} challenges: they are forwarded on the wire as a standard OAuth {@code claims} + * parameter and cause cache isolation keyed on the claims value (rather than bypassing the cache). + * + * @see FmiTest for the analogous fmi_path cache-isolation tests + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class ClientClaimsTest { + + private static final String CLAIMS_A = "{\"claimA\":{\"essential\":true}}"; + private static final String CLAIMS_B = "{\"claimB\":{\"values\":[\"v1\"]}}"; + + private ConfidentialClientApplication buildCca(DefaultHttpClient httpClientMock) throws Exception { + return ConfidentialClientApplication.builder("clientId", ClientCredentialFactory.createFromSecret("secret")) + .authority("https://login.microsoftonline.com/tenant/") + .instanceDiscovery(false) + .validateAuthority(false) + .httpClient(httpClientMock) + .build(); + } + + @SafeVarargs + private final DefaultHttpClient mockHttpClient(HashMap... responses) throws Exception { + DefaultHttpClient httpClientMock = mock(DefaultHttpClient.class); + if (responses.length == 0) { + when(httpClientMock.send(any(HttpRequest.class))).thenReturn( + TestHelper.expectedResponse(HttpStatus.HTTP_OK, + TestHelper.getSuccessfulTokenResponse(new HashMap<>()))); + } else { + org.mockito.stubbing.OngoingStubbing stub = + when(httpClientMock.send(any(HttpRequest.class))); + for (HashMap response : responses) { + stub = stub.thenReturn(TestHelper.expectedResponse(HttpStatus.HTTP_OK, + TestHelper.getSuccessfulTokenResponse(response))); + } + } + return httpClientMock; + } + + private HashMap tokenResponse(String accessToken) { + HashMap values = new HashMap<>(); + values.put("access_token", accessToken); + return values; + } + + // ======================================================================== + // JSON object validation (JsonHelper.validateJsonObjectFormat) + // ======================================================================== + + @Test + void validateJsonObjectFormat_validObject_doesNotThrow() { + assertDoesNotThrow(() -> JsonHelper.validateJsonObjectFormat(CLAIMS_A)); + assertDoesNotThrow(() -> JsonHelper.validateJsonObjectFormat("{\"xms_az_nwperimid\":{\"essential\":true}}")); + } + + @Test + void validateJsonObjectFormat_malformedJson_throwsInvalidJson() { + MsalClientException ex = assertThrows(MsalClientException.class, + () -> JsonHelper.validateJsonObjectFormat("not json at all")); + assertEquals(AuthenticationErrorCode.INVALID_JSON, ex.errorCode()); + } + + @Test + void validateJsonObjectFormat_nonObjectJson_throwsInvalidJson() { + // A JSON array is valid JSON but not a JSON object. + MsalClientException arrayEx = assertThrows(MsalClientException.class, + () -> JsonHelper.validateJsonObjectFormat("[1,2,3]")); + assertEquals(AuthenticationErrorCode.INVALID_JSON, arrayEx.errorCode()); + + // A bare scalar is likewise not a JSON object. + MsalClientException scalarEx = assertThrows(MsalClientException.class, + () -> JsonHelper.validateJsonObjectFormat("\"justAString\"")); + assertEquals(AuthenticationErrorCode.INVALID_JSON, scalarEx.errorCode()); + } + + // ======================================================================== + // Builder behavior — blank is a no-op, invalid JSON throws + // ======================================================================== + + @Test + void builders_blankClaims_areNoOp() { + ClientCredentialParameters cc = ClientCredentialParameters + .builder(Collections.singleton("scope")) + .claimsFromClient(" ") + .build(); + assertNull(cc.clientClaims()); + assertEquals("", cc.computeExtCacheKeyHash()); + + OnBehalfOfParameters obo = OnBehalfOfParameters + .builder(Collections.singleton("scope"), new UserAssertion(TestHelper.signedAssertion)) + .claimsFromClient(null) + .build(); + assertNull(obo.clientClaims()); + assertEquals("", obo.computeExtCacheKeyHash()); + + ManagedIdentityParameters mi = ManagedIdentityParameters + .builder("resource") + .claimsFromClient("") + .build(); + assertNull(mi.clientClaims()); + assertEquals("", mi.computeExtCacheKeyHash()); + + UserFederatedIdentityCredentialParameters fic = UserFederatedIdentityCredentialParameters + .builder(Collections.singleton("scope"), "user@contoso.com", "assertion") + .claimsFromClient(" ") + .build(); + assertNull(fic.clientClaims()); + assertEquals("", fic.computeExtCacheKeyHash()); + } + + @Test + void builders_invalidClaims_throwInvalidJson() { + assertEquals(AuthenticationErrorCode.INVALID_JSON, assertThrows(MsalClientException.class, () -> + ClientCredentialParameters.builder(Collections.singleton("scope")).claimsFromClient("nope")).errorCode()); + assertEquals(AuthenticationErrorCode.INVALID_JSON, assertThrows(MsalClientException.class, () -> + OnBehalfOfParameters.builder(Collections.singleton("scope"), new UserAssertion(TestHelper.signedAssertion)).claimsFromClient("[1]")).errorCode()); + assertEquals(AuthenticationErrorCode.INVALID_JSON, assertThrows(MsalClientException.class, () -> + ManagedIdentityParameters.builder("resource").claimsFromClient("nope")).errorCode()); + assertEquals(AuthenticationErrorCode.INVALID_JSON, assertThrows(MsalClientException.class, () -> + UserFederatedIdentityCredentialParameters.builder(Collections.singleton("scope"), "user@contoso.com", "assertion").claimsFromClient("nope")).errorCode()); + } + + @Test + void clientClaims_distinctValues_produceDistinctCacheKeyHashes() { + ClientCredentialParameters a = ClientCredentialParameters + .builder(Collections.singleton("scope")).claimsFromClient(CLAIMS_A).build(); + ClientCredentialParameters b = ClientCredentialParameters + .builder(Collections.singleton("scope")).claimsFromClient(CLAIMS_B).build(); + + assertNotEquals("", a.computeExtCacheKeyHash()); + assertNotEquals(a.computeExtCacheKeyHash(), b.computeExtCacheKeyHash()); + } + + // ======================================================================== + // Client credentials — wire and cache isolation + // ======================================================================== + + @Test + void clientCredential_clientClaims_sentAsClaimsBodyParam() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + ClientCredentialParameters parameters = ClientCredentialParameters + .builder(Collections.singleton("https://graph.microsoft.com/.default")) + .claimsFromClient(CLAIMS_A) + .build(); + + cca.acquireToken(parameters).get(); + + verify(httpClientMock).send(argThat(request -> { + String body = request.body(); + return body.contains("claims=") && body.contains("claimA") + && body.contains("grant_type=client_credentials"); + })); + } + + @Test + void clientCredential_distinctClaims_isolateCache() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(tokenResponse("token_A"), tokenResponse("token_B")); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + IAuthenticationResult resultA = cca.acquireToken(ClientCredentialParameters + .builder(Collections.singleton("scope")).claimsFromClient(CLAIMS_A).build()).get(); + IAuthenticationResult resultB = cca.acquireToken(ClientCredentialParameters + .builder(Collections.singleton("scope")).claimsFromClient(CLAIMS_B).build()).get(); + + assertEquals("token_A", resultA.accessToken()); + assertEquals("token_B", resultB.accessToken()); + assertEquals(2, cca.tokenCache.accessTokens.size()); + verify(httpClientMock, times(2)).send(any()); + } + + @Test + void clientCredential_sameClaims_cacheHit() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + ClientCredentialParameters params = ClientCredentialParameters + .builder(Collections.singleton("scope")).claimsFromClient(CLAIMS_A).build(); + + IAuthenticationResult result1 = cca.acquireToken(params).get(); + IAuthenticationResult result2 = cca.acquireToken(params).get(); + + assertEquals(result1.accessToken(), result2.accessToken()); + assertEquals(1, cca.tokenCache.accessTokens.size()); + verify(httpClientMock, times(1)).send(any()); + } + + @Test + void clientCredential_claimsDoNotCollideWithNonClaimsTokens() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(tokenResponse("regular"), tokenResponse("with_claims")); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + cca.acquireToken(ClientCredentialParameters + .builder(Collections.singleton("scope")).build()).get(); + cca.acquireToken(ClientCredentialParameters + .builder(Collections.singleton("scope")).claimsFromClient(CLAIMS_A).build()).get(); + + assertEquals(2, cca.tokenCache.accessTokens.size()); + verify(httpClientMock, times(2)).send(any()); + } + + // ======================================================================== + // On-behalf-of — wire and cache isolation + // ======================================================================== + + @Test + void onBehalfOf_clientClaims_sentAsClaimsBodyParam() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + OnBehalfOfParameters parameters = OnBehalfOfParameters + .builder(Collections.singleton("scope"), new UserAssertion(TestHelper.signedAssertion)) + .claimsFromClient(CLAIMS_A) + .build(); + + cca.acquireToken(parameters).get(); + + verify(httpClientMock).send(argThat(request -> { + String body = request.body(); + return body.contains("claims=") && body.contains("claimA"); + })); + } + + @Test + void onBehalfOf_distinctClaims_isolateCache() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(tokenResponse("obo_A"), tokenResponse("obo_B")); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + UserAssertion assertion = new UserAssertion(TestHelper.signedAssertion); + + IAuthenticationResult resultA = cca.acquireToken(OnBehalfOfParameters + .builder(Collections.singleton("scope"), assertion).claimsFromClient(CLAIMS_A).build()).get(); + IAuthenticationResult resultB = cca.acquireToken(OnBehalfOfParameters + .builder(Collections.singleton("scope"), assertion).claimsFromClient(CLAIMS_B).build()).get(); + + assertEquals("obo_A", resultA.accessToken()); + assertEquals("obo_B", resultB.accessToken()); + assertEquals(2, cca.tokenCache.accessTokens.size()); + verify(httpClientMock, times(2)).send(any()); + } + + @Test + void onBehalfOf_sameClaims_cacheHit() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + OnBehalfOfParameters params = OnBehalfOfParameters + .builder(Collections.singleton("scope"), new UserAssertion(TestHelper.signedAssertion)) + .claimsFromClient(CLAIMS_A) + .build(); + + IAuthenticationResult result1 = cca.acquireToken(params).get(); + IAuthenticationResult result2 = cca.acquireToken(params).get(); + + assertEquals(result1.accessToken(), result2.accessToken()); + assertEquals(1, cca.tokenCache.accessTokens.size()); + verify(httpClientMock, times(1)).send(any()); + } +} diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java index f5a8ccc0..ee458057 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java @@ -930,4 +930,89 @@ private void assertMsalServiceException(String errorCode, String message) throws assertTrue(ex.getMessage().contains(message)); } } + + @Nested + class ClientClaimsTests extends BaseManagedIdentityTest { + // Client-originated claims (claimsFromClient) for managed identity. Unlike server-issued + // `claims` challenges (which bypass the cache), client claims are forwarded on the wire and + // cached, keyed on the claims value. Only IMDS (MSIv1) is supported, and MSIv1 only permits + // the `xms_az_nwperimid` custom claim. + + private final String nwperimidEssential = "{\"xms_az_nwperimid\":{\"essential\":true}}"; + private final String nwperimidValues = "{\"xms_az_nwperimid\":{\"values\":[\"perimid-1234\"]}}"; + + @Test + void imds_clientClaims_sentAsQueryParameter() throws Exception { + setUpCommonTest(IMDS, ManagedIdentityTestConstants.IMDS_ENDPOINT, ManagedIdentityId.systemAssigned()); + when(httpClientMock.send(any())).thenReturn(expectedResponse(HttpStatus.HTTP_OK, getSuccessfulResponse(ManagedIdentityTestConstants.RESOURCE))); + + miApp.acquireTokenForManagedIdentity( + ManagedIdentityParameters.builder(ManagedIdentityTestConstants.RESOURCE) + .claimsFromClient(nwperimidEssential) + .build()).get(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(HttpRequest.class); + verify(httpClientMock).send(captor.capture()); + String url = captor.getValue().url().toString(); + assertTrue(url.contains("claims="), "IMDS request URL should carry the claims query parameter"); + assertTrue(url.contains("xms_az_nwperimid"), "claims value should be present in the URL"); + } + + @Test + void imds_distinctClaims_isolateCache() throws Exception { + setUpCommonTest(IMDS, ManagedIdentityTestConstants.IMDS_ENDPOINT, ManagedIdentityId.systemAssigned()); + when(httpClientMock.send(any())).thenReturn(expectedResponse(HttpStatus.HTTP_OK, getSuccessfulResponse(ManagedIdentityTestConstants.RESOURCE))); + + miApp.acquireTokenForManagedIdentity(ManagedIdentityParameters.builder(ManagedIdentityTestConstants.RESOURCE) + .claimsFromClient(nwperimidEssential).build()).get(); + miApp.acquireTokenForManagedIdentity(ManagedIdentityParameters.builder(ManagedIdentityTestConstants.RESOURCE) + .claimsFromClient(nwperimidValues).build()).get(); + + assertEquals(2, miApp.tokenCache().accessTokens.size(), "Distinct client claims should produce distinct cache entries"); + verify(httpClientMock, times(2)).send(any()); + } + + @Test + void imds_sameClaims_cacheHit() throws Exception { + setUpCommonTest(IMDS, ManagedIdentityTestConstants.IMDS_ENDPOINT, ManagedIdentityId.systemAssigned()); + when(httpClientMock.send(any())).thenReturn(expectedResponse(HttpStatus.HTTP_OK, getSuccessfulResponse(ManagedIdentityTestConstants.RESOURCE))); + + ManagedIdentityParameters params = ManagedIdentityParameters.builder(ManagedIdentityTestConstants.RESOURCE) + .claimsFromClient(nwperimidEssential).build(); + + IAuthenticationResult first = miApp.acquireTokenForManagedIdentity(params).get(); + assertTokenFromIdentityProvider(first); + IAuthenticationResult second = miApp.acquireTokenForManagedIdentity(params).get(); + assertTokenFromCache(second); + + assertEquals(1, miApp.tokenCache().accessTokens.size()); + verify(httpClientMock, times(1)).send(any()); + } + + @Test + void unsupportedSource_withClientClaims_throwsInvalidRequest() throws Exception { + setUpCommonTest(APP_SERVICE, ManagedIdentityTestConstants.APP_SERVICE_ENDPOINT, ManagedIdentityId.systemAssigned()); + + CompletableFuture future = miApp.acquireTokenForManagedIdentity( + ManagedIdentityParameters.builder(ManagedIdentityTestConstants.RESOURCE) + .claimsFromClient(nwperimidEssential) + .build()); + + assertMsalClientException(future, AuthenticationErrorCode.INVALID_REQUEST); + verify(httpClientMock, never()).send(any()); + } + + @Test + void imds_disallowedMsiv1Key_throwsInvalidRequest() throws Exception { + setUpCommonTest(IMDS, ManagedIdentityTestConstants.IMDS_ENDPOINT, ManagedIdentityId.systemAssigned()); + + CompletableFuture future = miApp.acquireTokenForManagedIdentity( + ManagedIdentityParameters.builder(ManagedIdentityTestConstants.RESOURCE) + .claimsFromClient("{\"other_claim\":{\"essential\":true}}") + .build()); + + assertMsalClientException(future, AuthenticationErrorCode.INVALID_REQUEST); + verify(httpClientMock, never()).send(any()); + } + } } diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialTest.java index cd5b8bc7..9151d9ca 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialTest.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/UserFederatedIdentityCredentialTest.java @@ -616,4 +616,84 @@ void userFic_SingleAccountFallback_DoesNotReturnWrongUserToken() throws Exceptio // Assert: Both accounts now in cache assertEquals(2, cca.getAccounts().get().size(), "Both Alice and Bob should be in cache"); } + + // ======================================================================== + // Client claims (claimsFromClient) — wire and cache isolation + // ======================================================================== + + private static final String CLAIMS_A = "{\"claimA\":{\"essential\":true}}"; + private static final String CLAIMS_B = "{\"claimB\":{\"values\":[\"v1\"]}}"; + + @Test + void userFic_clientClaims_sentAsClaimsBodyParam() throws Exception { + // Arrange + String oid = "oid-user-claims"; + String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca"; + when(httpClientMock.send(any(HttpRequest.class))) + .thenReturn(createUserResponse(oid, TEST_UPN, "access-token", tid)); + + UserFederatedIdentityCredentialParameters parameters = UserFederatedIdentityCredentialParameters + .builder(SCOPES, TEST_UPN, FAKE_ASSERTION) + .claimsFromClient(CLAIMS_A) + .build(); + + // Act + cca.acquireToken(parameters).get(); + + // Assert — client claims are forwarded as a standard OAuth claims body parameter + verify(httpClientMock).send(argThat(request -> { + String body = request.body(); + return body.contains("claims=") && body.contains("claimA"); + })); + } + + @Test + void userFic_distinctClaims_isolateCache() throws Exception { + // Arrange — same user (same OID/UPN/tid); only the client claims differ + String oid = "oid-user-claims"; + String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca"; + AtomicInteger callCount = new AtomicInteger(0); + when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> { + int n = callCount.incrementAndGet(); + return createUserResponse(oid, TEST_UPN, "token-" + n, tid); + }); + + // Act 1 — claims A populates the cache + IAuthenticationResult resultA = cca.acquireToken(UserFederatedIdentityCredentialParameters + .builder(SCOPES, TEST_UPN, FAKE_ASSERTION).claimsFromClient(CLAIMS_A).build()).get(); + assertEquals(1, callCount.get(), "First call (claims A) should hit the IdP"); + + // Act 2 — claims B for the same user must NOT reuse the claims-A token + IAuthenticationResult resultB = cca.acquireToken(UserFederatedIdentityCredentialParameters + .builder(SCOPES, TEST_UPN, FAKE_ASSERTION).claimsFromClient(CLAIMS_B).build()).get(); + + // Assert — distinct claims produce distinct cache entries and a fresh network call + assertEquals(2, callCount.get(), "Second call (claims B) should hit the IdP, not reuse claims-A token"); + assertNotEquals(resultA.accessToken(), resultB.accessToken()); + assertEquals(2, cca.tokenCache.accessTokens.size(), "Distinct claims should yield two cached access tokens"); + } + + @Test + void userFic_sameClaims_cacheHit() throws Exception { + // Arrange — same user, same client claims on both calls + String oid = "oid-user-claims"; + String tid = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca"; + AtomicInteger callCount = new AtomicInteger(0); + when(httpClientMock.send(any(HttpRequest.class))).thenAnswer(invocation -> { + callCount.incrementAndGet(); + return createUserResponse(oid, TEST_UPN, "token-cached", tid); + }); + + UserFederatedIdentityCredentialParameters params = UserFederatedIdentityCredentialParameters + .builder(SCOPES, TEST_UPN, FAKE_ASSERTION).claimsFromClient(CLAIMS_A).build(); + + // Act + cca.acquireToken(params).get(); + assertEquals(1, callCount.get(), "First call should hit the IdP"); + cca.acquireToken(params).get(); + + // Assert — identical claims hit the cache (no second network call) + assertEquals(1, callCount.get(), "Second call with identical claims should be served from cache"); + assertEquals(1, cca.tokenCache.accessTokens.size()); + } } From d63bf9628f74561d772eec783eac7f8c0262dd2f Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 26 Jun 2026 14:43:42 -0400 Subject: [PATCH 2/3] Add claimsFromClient to Authorization Code flow Extends client-originated claims coverage to the last remaining confidential-client flow (Authorization Code / web-app code redemption), matching the generic AbstractConfidentialClientAcquireTokenParameterBuilder extension in msal-dotnet PR #5999. AuthorizationCodeParameters now carries clientClaims with the same cache-key isolation pattern as OnBehalfOfParameters (extCacheKeyHash). Wire forwarding and cache writes are already generic, so the claims are forwarded as the OAuth claims body parameter and cached per distinct value. Adds an auth-code wire test plus auth-code cases to the builder no-op and invalid-JSON tests in ClientClaimsTest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../msal4j/AuthorizationCodeParameters.java | 86 ++++++++++++++++++- .../aad/msal4j/ClientClaimsTest.java | 35 ++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthorizationCodeParameters.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthorizationCodeParameters.java index 8b430080..0894d50c 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthorizationCodeParameters.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthorizationCodeParameters.java @@ -6,6 +6,8 @@ import java.net.URI; import java.util.Map; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import static com.microsoft.aad.msal4j.ParameterValidationUtils.validateNotBlank; import static com.microsoft.aad.msal4j.ParameterValidationUtils.validateNotNull; @@ -33,10 +35,20 @@ public class AuthorizationCodeParameters implements IAcquireTokenParameters { private String tenant; + private String clientClaims; + + // Generic extended cache key components. The hash of these components isolates cache + // entries so that requests with different client-claims values do not collide. + private SortedMap cacheKeyComponents; + + // Memoized hash of cacheKeyComponents (computed once since parameters are immutable). + private String extCacheKeyHashCache; + private AuthorizationCodeParameters(String authorizationCode, URI redirectUri, Set scopes, ClaimsRequest claims, String codeVerifier, Map extraHttpHeaders, - Map extraQueryParameters, String tenant) { + Map extraQueryParameters, String tenant, + String clientClaims) { this.authorizationCode = authorizationCode; this.redirectUri = redirectUri; this.scopes = scopes; @@ -45,6 +57,10 @@ private AuthorizationCodeParameters(String authorizationCode, URI redirectUri, this.extraHttpHeaders = extraHttpHeaders; this.extraQueryParameters = extraQueryParameters; this.tenant = tenant; + this.clientClaims = clientClaims; + + // Build cache key components from any parameters that require cache isolation. + this.cacheKeyComponents = buildCacheKeyComponents(); } private static AuthorizationCodeParametersBuilder builder() { @@ -104,6 +120,50 @@ public String tenant() { return this.tenant; } + /** + * Client-originated claims set via {@link AuthorizationCodeParametersBuilder#claimsFromClient(String)}. + * Forwarded to the token endpoint as the OAuth {@code claims} parameter and used as part of the + * extended cache key so that distinct claim values are cached separately. + */ + @Override + public String clientClaims() { + return this.clientClaims; + } + + /** + * Builds the sorted map of cache key components from the parameters that require cache isolation. + * Returns null if no components are present. + */ + private SortedMap buildCacheKeyComponents() { + TreeMap components = null; + if (!StringHelper.isBlank(clientClaims)) { + components = new TreeMap<>(); + components.put("client_claims", clientClaims); + } + return components; + } + + /** + * Returns the extended cache key components for this request, if any. + * Used by {@link TokenCache} for both cache writes and reads. + */ + SortedMap cacheKeyComponents() { + return this.cacheKeyComponents; + } + + /** + * Computes the extended cache key hash from all cache key components, or an empty string when + * there are none. The result is memoized since the parameters are immutable after construction. + */ + @Override + public String computeExtCacheKeyHash() { + if (extCacheKeyHashCache != null) { + return extCacheKeyHashCache; + } + extCacheKeyHashCache = StringHelper.computeExtCacheKeyHash(cacheKeyComponents); + return extCacheKeyHashCache; + } + public static class AuthorizationCodeParametersBuilder { private String authorizationCode; private URI redirectUri; @@ -113,6 +173,7 @@ public static class AuthorizationCodeParametersBuilder { private Map extraHttpHeaders; private Map extraQueryParameters; private String tenant; + private String clientClaims; AuthorizationCodeParametersBuilder() { } @@ -193,8 +254,29 @@ public AuthorizationCodeParametersBuilder tenant(String tenant) { return this; } + /** + * Specifies client-originated claims (a raw JSON object string) to forward to the token + * endpoint as the OAuth {@code claims} request parameter. Unlike {@link #claims(ClaimsRequest)} + * (server-issued claims challenges, which bypass the cache), tokens acquired with client claims + * are cached and the cache entry is keyed on the claims value, so distinct claim values produce + * separate cache entries. Use stable, non-dynamic values to avoid cache fragmentation. + * A blank value is ignored; an invalid JSON object throws {@link MsalClientException}. + * + * @param claimsJson a valid JSON object string containing the client claims + * @return this builder instance + */ + public AuthorizationCodeParametersBuilder claimsFromClient(String claimsJson) { + if (StringHelper.isBlank(claimsJson)) { + return this; + } + + JsonHelper.validateJsonObjectFormat(claimsJson); + this.clientClaims = claimsJson; + return this; + } + public AuthorizationCodeParameters build() { - return new AuthorizationCodeParameters(this.authorizationCode, this.redirectUri, this.scopes, this.claims, this.codeVerifier, this.extraHttpHeaders, this.extraQueryParameters, this.tenant); + return new AuthorizationCodeParameters(this.authorizationCode, this.redirectUri, this.scopes, this.claims, this.codeVerifier, this.extraHttpHeaders, this.extraQueryParameters, this.tenant, this.clientClaims); } public String toString() { diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java index 42a30946..e730e5aa 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; +import java.net.URI; import java.util.Collections; import java.util.HashMap; @@ -123,6 +124,14 @@ void builders_blankClaims_areNoOp() { .build(); assertNull(fic.clientClaims()); assertEquals("", fic.computeExtCacheKeyHash()); + + AuthorizationCodeParameters ac = AuthorizationCodeParameters + .builder("code", URI.create("https://localhost/redirect")) + .scopes(Collections.singleton("scope")) + .claimsFromClient(null) + .build(); + assertNull(ac.clientClaims()); + assertEquals("", ac.computeExtCacheKeyHash()); } @Test @@ -135,6 +144,8 @@ void builders_invalidClaims_throwInvalidJson() { ManagedIdentityParameters.builder("resource").claimsFromClient("nope")).errorCode()); assertEquals(AuthenticationErrorCode.INVALID_JSON, assertThrows(MsalClientException.class, () -> UserFederatedIdentityCredentialParameters.builder(Collections.singleton("scope"), "user@contoso.com", "assertion").claimsFromClient("nope")).errorCode()); + assertEquals(AuthenticationErrorCode.INVALID_JSON, assertThrows(MsalClientException.class, () -> + AuthorizationCodeParameters.builder("code", URI.create("https://localhost/redirect")).claimsFromClient("nope")).errorCode()); } @Test @@ -274,4 +285,28 @@ void onBehalfOf_sameClaims_cacheHit() throws Exception { assertEquals(1, cca.tokenCache.accessTokens.size()); verify(httpClientMock, times(1)).send(any()); } + + // ======================================================================== + // Authorization code (confidential client / web app) — wire + // ======================================================================== + + @Test + void authorizationCode_clientClaims_sentAsClaimsBodyParam() throws Exception { + DefaultHttpClient httpClientMock = mockHttpClient(); + ConfidentialClientApplication cca = buildCca(httpClientMock); + + AuthorizationCodeParameters parameters = AuthorizationCodeParameters + .builder("auth-code-123", URI.create("https://localhost/redirect")) + .scopes(Collections.singleton("scope")) + .claimsFromClient(CLAIMS_A) + .build(); + + cca.acquireToken(parameters).get(); + + verify(httpClientMock).send(argThat(request -> { + String body = request.body(); + return body.contains("claims=") && body.contains("claimA") + && body.contains("grant_type=authorization_code"); + })); + } } From d3a8917f2aca8bad29cbe3a51a7e657dce8a3b36 Mon Sep 17 00:00:00 2001 From: Robbie Ginsburg Date: Fri, 26 Jun 2026 15:25:00 -0400 Subject: [PATCH 3/3] Address PR review feedback on client-claims - JsonHelper.validateJsonObjectFormat: reject trailing tokens after the root object (e.g. "{}{}", "{} garbage") by requiring END_DOCUMENT, and stop including the parser message in the exception so a (potentially sensitive) claims payload is never leaked. Uses a single constant message. - TokenRequestExecutor: correct the now-outdated comment listing the flows covered by the client-claims wire merge (adds authorization-code, which is shared with PublicClientApplication). - ClientCredentialParameters: remove an accidentally duplicated Javadoc block before computeExtCacheKeyHash(). - ClientClaimsTest: add trailing-token rejection and payload-not-leaked tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../msal4j/ClientCredentialParameters.java | 7 ----- .../com/microsoft/aad/msal4j/JsonHelper.java | 27 ++++++++++++------- .../aad/msal4j/TokenRequestExecutor.java | 6 +++-- .../aad/msal4j/ClientClaimsTest.java | 20 ++++++++++++++ 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java index b8c5e107..4cf26d1c 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCredentialParameters.java @@ -157,13 +157,6 @@ SortedMap cacheKeyComponents() { return this.cacheKeyComponents; } - /** - * Computes the extended cache key hash from all cache key components. - * Returns an empty string if no components are present. - *

- * The result is memoized since ClientCredentialParameters is immutable after construction. - * Used by both cache writes ({@link TokenCache}) and cache reads (silent lookup). - */ /** * Computes the extended cache key hash from all cache key components. * Returns an empty string if no components are present. diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java index 5c2a28b2..b033acd0 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java @@ -21,6 +21,12 @@ class JsonHelper { private static final Logger LOG = LoggerFactory.getLogger(JsonHelper.class); + // Constant, payload-free message for client-claims validation failures. The raw claims value + // and parser details are deliberately excluded since the payload may contain sensitive data. + private static final String INVALID_CLAIMS_OBJECT_MESSAGE = + "The claims value is not a valid JSON object. " + + "See https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter."; + private JsonHelper() { } @@ -159,23 +165,24 @@ static void validateJsonFormat(String jsonString) { /** * Validates that the supplied string is a well-formed JSON object (the root token is - * {@code {}}). Throws {@link MsalClientException} with {@link AuthenticationErrorCode#INVALID_JSON} - * otherwise. The raw value is never included in the exception message, since claims payloads may - * contain sensitive data. + * {@code {}}) and that there are no trailing tokens after it. Throws {@link MsalClientException} + * with {@link AuthenticationErrorCode#INVALID_JSON} otherwise. The raw value and the underlying + * parser message are never included in the exception message, since claims payloads may contain + * sensitive data. */ static void validateJsonObjectFormat(String jsonString) { try (JsonReader reader = JsonProviders.createReader(jsonString)) { if (reader.nextToken() != JsonToken.START_OBJECT) { - throw new MsalClientException( - "The claims value is not a valid JSON object. " + - "See https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter.", - AuthenticationErrorCode.INVALID_JSON); + throw new MsalClientException(INVALID_CLAIMS_OBJECT_MESSAGE, AuthenticationErrorCode.INVALID_JSON); } reader.skipChildren(); + // Ensure the object is the entire document; reject a valid object followed by any + // trailing tokens (e.g. "{}{}" or "{} garbage"), which is not a single JSON value. + if (reader.nextToken() != JsonToken.END_DOCUMENT) { + throw new MsalClientException(INVALID_CLAIMS_OBJECT_MESSAGE, AuthenticationErrorCode.INVALID_JSON); + } } catch (IOException e) { - throw new MsalClientException( - "The claims value is not a valid JSON object: " + e.getMessage(), - AuthenticationErrorCode.INVALID_JSON); + throw new MsalClientException(INVALID_CLAIMS_OBJECT_MESSAGE, AuthenticationErrorCode.INVALID_JSON); } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java index 81995492..8c3cc62b 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java @@ -66,8 +66,10 @@ OAuthHttpRequest createOauthHttpRequest() throws MalformedURLException { // Client-originated claims (claimsFromClient) are forwarded on the wire as a standard OAuth // "claims" parameter. They are merged here, after the capabilities/server-claims merge above, // because that logic rebuilds the "claims" param and would otherwise clobber an earlier value. - // This single point covers the confidential-client flows (client credentials, OBO, user-FIC); - // public-client and managed-identity flows return null here and are unaffected. + // This single point covers every flow whose parameters expose clientClaims(): the + // confidential-client flows (client credentials, OBO, user-FIC) and the authorization-code + // flow (web-app code redemption, on either a confidential or public client). Flows that do + // not set client claims return null/blank here and are unaffected. String clientClaims = msalRequest.requestContext().apiParameters().clientClaims(); if (!StringHelper.isBlank(clientClaims)) { if (params.get("claims") != null) { diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java index e730e5aa..5d7d95a3 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientClaimsTest.java @@ -148,6 +148,26 @@ void builders_invalidClaims_throwInvalidJson() { AuthorizationCodeParameters.builder("code", URI.create("https://localhost/redirect")).claimsFromClient("nope")).errorCode()); } + @Test + void builders_trailingTokensAfterObject_throwInvalidJson() { + // A valid object followed by any trailing tokens is not a single well-formed JSON value + // and must be rejected up front rather than forwarded on the wire. + for (String invalid : new String[]{"{\"a\":1} garbage", "{}{}", "{},123", "{} \"x\""}) { + MsalClientException ex = assertThrows(MsalClientException.class, () -> + ClientCredentialParameters.builder(Collections.singleton("scope")).claimsFromClient(invalid)); + assertEquals(AuthenticationErrorCode.INVALID_JSON, ex.errorCode()); + } + } + + @Test + void builders_invalidClaims_exceptionMessageDoesNotLeakPayload() { + // The claims payload may contain sensitive data and must never appear in the error message. + String secret = "{\"sensitive_secret_value\":"; + MsalClientException ex = assertThrows(MsalClientException.class, () -> + ClientCredentialParameters.builder(Collections.singleton("scope")).claimsFromClient(secret)); + assertFalse(ex.getMessage().contains("sensitive_secret_value")); + } + @Test void clientClaims_distinctValues_produceDistinctCacheKeyHashes() { ClientCredentialParameters a = ClientCredentialParameters