From ed65cb79943a7b3787f71a6844cf08be3c44ebeb Mon Sep 17 00:00:00 2001 From: Pasindu Yeshan Date: Mon, 15 Jun 2026 23:17:33 +0530 Subject: [PATCH] Introduce selective multi-valued claim handling in JWT and UserInfo based on claim metadata Add opt-in config ConvertOnlyMultiValuedClaimsToArray (default false) so that claim values containing commas are emitted as a JSON array only when the claim's local-claim metadata has multiValued=true. When the flag is off (default), legacy comma-split behavior is preserved unchanged. Special cases retained: groups always array, address never array. JWT/ID-token path: OAuthServerConfiguration parses the flag; DefaultOIDCClaimsCallbackHandler resolves the set of multi-valued OIDC claim URIs per tenant via the claim metadata service and gates the array/string decision on that set. JWTAccessTokenOIDCClaimsHandler (the access-token claims-separation path) applies the same selective logic so JWT access tokens honour the flag too. UserInfo path: ClaimUtil.isMultiValuedAttribute overloaded to accept the local-claim-URI-keyed set; UserInfoUserStoreClaimRetriever uses the same set on the cache-hit path. OAuth2Util holds the shared helper. All paths degrade gracefully to legacy comma-split on ClaimMetadataException or an unavailable claim metadata service. Tests: DefaultOIDCClaimsCallbackHandlerMultiValuedClaimTest and JWTAccessTokenOIDCClaimsHandlerMultiValuedClaimTest (JWT/ID-token + access-token paths), OAuth2UtilTest and ClaimUtilTest additions (UserInfo path) covering flag off/on, single vs multi-valued, groups/address special cases, and graceful fallback. --- .../impl/UserInfoUserStoreClaimRetriever.java | 15 +- .../oauth/endpoint/util/ClaimUtil.java | 38 ++- .../oauth/endpoint/util/ClaimUtilTest.java | 52 ++++ .../config/OAuthServerConfiguration.java | 22 ++ .../identity/oauth2/util/OAuth2Util.java | 37 +++ .../DefaultOIDCClaimsCallbackHandler.java | 72 +++++- .../JWTAccessTokenOIDCClaimsHandler.java | 69 +++++- .../identity/oauth2/util/OAuth2UtilTest.java | 73 ++++++ ...msCallbackHandlerMultiValuedClaimTest.java | 223 +++++++++++++++++ ...OIDCClaimsHandlerMultiValuedClaimTest.java | 225 ++++++++++++++++++ .../src/test/resources/testng.xml | 2 + 11 files changed, 816 insertions(+), 12 deletions(-) create mode 100644 components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandlerMultiValuedClaimTest.java create mode 100644 components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandlerMultiValuedClaimTest.java diff --git a/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoUserStoreClaimRetriever.java b/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoUserStoreClaimRetriever.java index 1040378c1d6..4d9120b1973 100644 --- a/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoUserStoreClaimRetriever.java +++ b/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoUserStoreClaimRetriever.java @@ -18,14 +18,17 @@ package org.wso2.carbon.identity.oauth.endpoint.user.impl; import org.apache.commons.collections.MapUtils; +import org.wso2.carbon.context.PrivilegedCarbonContext; import org.wso2.carbon.identity.application.common.model.ClaimMapping; import org.wso2.carbon.identity.core.util.IdentityCoreConstants; import org.wso2.carbon.identity.oauth.config.OAuthServerConfiguration; import org.wso2.carbon.identity.oauth.endpoint.util.ClaimUtil; import org.wso2.carbon.identity.oauth.user.UserInfoClaimRetriever; +import org.wso2.carbon.identity.oauth2.util.OAuth2Util; import java.util.HashMap; import java.util.Map; +import java.util.Set; /** * Retrieving claims from the user store for the given claims dialect @@ -37,6 +40,13 @@ public Map getClaimsMap(Map userAttributes Map claims = new HashMap(); if (MapUtils.isNotEmpty(userAttributes)) { + // When ConvertOnlyMultiValuedClaimsToArray is enabled, resolve local claim URIs flagged multi-valued. + // A null set means the feature is off or the lookup failed, so legacy behaviour applies. + Set multiValuedLocalClaimUris = null; + if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) { + String tenantDomain = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain(); + multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(tenantDomain); + } for (Map.Entry entry : userAttributes.entrySet()) { if (entry.getKey().getRemoteClaim() == null || IdentityCoreConstants.MULTI_ATTRIBUTE_SEPARATOR.equals( @@ -45,10 +55,13 @@ public Map getClaimsMap(Map userAttributes } String claimValue = entry.getValue(); String claimUri = entry.getKey().getRemoteClaim().getClaimUri(); + String localClaimUri = entry.getKey().getLocalClaim() != null ? + entry.getKey().getLocalClaim().getClaimUri() : null; boolean isMultiValueSupportEnabledForUserinfoResponse = OAuthServerConfiguration.getInstance() .getUserInfoMultiValueSupportEnabled(); if (isMultiValueSupportEnabledForUserinfoResponse && - ClaimUtil.isMultiValuedAttribute(claimUri, claimValue)) { + ClaimUtil.isMultiValuedAttribute(claimUri, localClaimUri, claimValue, + multiValuedLocalClaimUris)) { String[] attributeValues = ClaimUtil.processMultiValuedAttribute(claimValue); claims.put(claimUri, attributeValues); } else { diff --git a/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java b/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java index 41914059bb6..535247f677a 100644 --- a/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java +++ b/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java @@ -69,6 +69,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Pattern; import static org.apache.commons.collections.MapUtils.isEmpty; @@ -264,6 +265,12 @@ public static Map getClaimsFromUserStore(String authorizedUserNa userId, realm, claimURIList, isImpersonatedUser); } + // When ConvertOnlyMultiValuedClaimsToArray is enabled, resolve (once) local claim URIs flagged + // multi-valued. A null set means the feature is off or the lookup failed, so legacy behaviour applies. + Set multiValuedLocalClaimUris = null; + if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) { + multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(spTenantDomain); + } if (isNotEmpty(userClaims)) { for (Map.Entry entry : userClaims.entrySet()) { //set local2sp role mappings @@ -289,7 +296,8 @@ public static Map getClaimsFromUserStore(String authorizedUserNa boolean isMultiValueSupportEnabledForUserinfoResponse = OAuthServerConfiguration .getInstance().getUserInfoMultiValueSupportEnabled(); if (isMultiValueSupportEnabledForUserinfoResponse && - isMultiValuedAttribute(oidcClaimUri, claimValue)) { + isMultiValuedAttribute(oidcClaimUri, entry.getKey(), claimValue, + multiValuedLocalClaimUris)) { String[] attributeValues = processMultiValuedAttribute(claimValue); mappedAppClaims.put(oidcClaimUri, attributeValues); } else { @@ -575,6 +583,34 @@ public static boolean isMultiValuedAttribute(String claimUri, String claimValue) return StringUtils.contains(claimValue, FrameworkUtils.getMultiAttributeSeparator()); } + /** + * Checks whether a user value is multivalued or not, honouring selective multi-valued claim handling. + * When {@code multiValuedLocalClaimUris} is non-null (i.e. the {@code ConvertOnlyMultiValuedClaimsToArray} + * configuration is enabled and the claim metadata was resolved), only claims whose local claim URI is flagged + * as multi-valued in claim metadata are treated as arrays. The {@code groups} claim is always treated as an + * array. A null set falls back to the legacy separator-based behaviour. + * + * @param claimUri OIDC dialect claim URI (short name). + * @param localClaimUri Local claim URI mapped to the OIDC claim. + * @param claimValue Claim value. + * @param multiValuedLocalClaimUris Set of local claim URIs flagged as multi-valued, or {@code null} for legacy + * behaviour. + * @return Whether the claim should be emitted as a multi-valued (array) attribute. + */ + public static boolean isMultiValuedAttribute(String claimUri, String localClaimUri, String claimValue, + Set multiValuedLocalClaimUris) { + + /* To format the groups claim to always return as an array, we should consider single + group as multi value attribute. */ + if (GROUPS.equals(claimUri)) { + return true; + } + if (multiValuedLocalClaimUris != null) { + return multiValuedLocalClaimUris.contains(localClaimUri); + } + return StringUtils.contains(claimValue, FrameworkUtils.getMultiAttributeSeparator()); + } + /** * Split multivalued attribute string value by attribute separator. * diff --git a/components/org.wso2.carbon.identity.oauth.endpoint/src/test/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtilTest.java b/components/org.wso2.carbon.identity.oauth.endpoint/src/test/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtilTest.java index 4ce6cfdf779..eb02313e17b 100644 --- a/components/org.wso2.carbon.identity.oauth.endpoint/src/test/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtilTest.java +++ b/components/org.wso2.carbon.identity.oauth.endpoint/src/test/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtilTest.java @@ -66,8 +66,10 @@ import java.lang.reflect.Field; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -562,6 +564,56 @@ public void testGetServiceProviderMappedUserRoles(List locallyMappedUser Assert.assertEquals(returned, expected, "Invalid returned value"); } + private static final String GROUPS_CLAIM_URI = "groups"; + private static final String GIVEN_NAME_OIDC_URI = "given_name"; + private static final String GIVEN_NAME_LOCAL_URI = "http://wso2.org/claims/givenname"; + private static final String MULTI_TEST_OIDC_URI = "multi_test"; + private static final String MULTI_TEST_LOCAL_URI = "http://wso2.org/claims/multiTest"; + + @DataProvider(name = "selectiveMultiValuedAttributeProvider") + public Object[][] selectiveMultiValuedAttributeProvider() { + + Set multiValuedLocalClaimUris = new HashSet<>(); + multiValuedLocalClaimUris.add(MULTI_TEST_LOCAL_URI); + + return new Object[][]{ + // claimUri, localClaimUri, claimValue, multiValuedLocalClaimUris (null = legacy), expectedMultiValued. + + // Scenario (d): groups is always treated as an array, regardless of flag/value. + {GROUPS_CLAIM_URI, "http://wso2.org/claims/groups", "admin", null, true}, + {GROUPS_CLAIM_URI, "http://wso2.org/claims/groups", "admin", multiValuedLocalClaimUris, true}, + + // Scenario (c): flag ON + claim flagged multiValued=true -> array, even without separator. + {MULTI_TEST_OIDC_URI, MULTI_TEST_LOCAL_URI, "a", multiValuedLocalClaimUris, true}, + {MULTI_TEST_OIDC_URI, MULTI_TEST_LOCAL_URI, "a,b,c,d", multiValuedLocalClaimUris, true}, + + // Scenario (b): flag ON + claim flagged multiValued=false with comma value -> plain string (no array). + {GIVEN_NAME_OIDC_URI, GIVEN_NAME_LOCAL_URI, "a,b,c,d", multiValuedLocalClaimUris, false}, + // Flag ON + non-flagged claim, no separator -> still not an array. + {GIVEN_NAME_OIDC_URI, GIVEN_NAME_LOCAL_URI, "a", multiValuedLocalClaimUris, false}, + + // Scenario (a): flag OFF (null set) -> legacy comma-split behaviour preserved. + {GIVEN_NAME_OIDC_URI, GIVEN_NAME_LOCAL_URI, "a,b,c,d", null, true}, + {GIVEN_NAME_OIDC_URI, GIVEN_NAME_LOCAL_URI, "a", null, false}, + // Legacy fallback applies to a flagged claim's local URI when the set is null (flag off). + {MULTI_TEST_OIDC_URI, MULTI_TEST_LOCAL_URI, "a", null, false}, + }; + } + + @Test(dataProvider = "selectiveMultiValuedAttributeProvider") + public void testIsMultiValuedAttributeSelective(String claimUri, String localClaimUri, String claimValue, + Set multiValuedLocalClaimUris, boolean expected) { + + try (MockedStatic frameworkUtils = mockStatic(FrameworkUtils.class)) { + frameworkUtils.when(FrameworkUtils::getMultiAttributeSeparator).thenReturn(CLAIM_SEPARATOR); + boolean actual = ClaimUtil.isMultiValuedAttribute(claimUri, localClaimUri, claimValue, + multiValuedLocalClaimUris); + Assert.assertEquals(actual, expected, + "Unexpected multi-valued resolution for claim '" + claimUri + "' (local '" + localClaimUri + + "') with value '" + claimValue + "' and selectiveSet=" + multiValuedLocalClaimUris); + } + } + @Test public void testResolveRoleClaimForImpersonatedSubOrgUser() throws Exception { diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/config/OAuthServerConfiguration.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/config/OAuthServerConfiguration.java index 9e27ae34a14..ba88ddfd02b 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/config/OAuthServerConfiguration.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/config/OAuthServerConfiguration.java @@ -246,6 +246,7 @@ public class OAuthServerConfiguration { private List supportedIdTokenEncryptionMethods = new ArrayList<>(); private String userInfoJWTSignatureAlgorithm = "SHA256withRSA"; private boolean userInfoMultiValueSupportEnabled = true; + private boolean convertOnlyMultiValuedClaimsToArray = false; private boolean userInfoRemoveInternalPrefixFromRoles = false; private boolean isReturnOnlyAppAssociatedRolesInUserInfo = false; private boolean isReturnOnlyAppAssociatedRolesInJWTToken = false; @@ -1826,6 +1827,18 @@ public boolean getUserInfoMultiValueSupportEnabled() { return userInfoMultiValueSupportEnabled; } + /** + * Returns whether only claims flagged as multi-valued in claim metadata should be converted to a JSON array + * when building JWT access tokens and ID tokens. When disabled (default), the legacy behaviour applies where + * any claim value containing the multi attribute separator is split into an array. + * + * @return True if only multi-valued claims (per claim metadata) should be converted to an array. + */ + public boolean getConvertOnlyMultiValuedClaimsToArray() { + + return convertOnlyMultiValuedClaimsToArray; + } + /** * Returns whether Internal prefix should be removed from the roles claim of the userinfo response. * @@ -3934,6 +3947,13 @@ private void parseOpenIDConnectConfig(OMElement oauthConfigElem) { userInfoMultiValueSupportEnabledElem.getText().trim()); } + OMElement convertOnlyMultiValuedClaimsToArrayElem = openIDConnectConfigElem.getFirstChildWithName( + getQNameWithIdentityNS(ConfigElements.OPENID_CONNECT_CONVERT_ONLY_MULTI_VALUED_CLAIMS_TO_ARRAY)); + if (convertOnlyMultiValuedClaimsToArrayElem != null) { + convertOnlyMultiValuedClaimsToArray = Boolean.parseBoolean( + convertOnlyMultiValuedClaimsToArrayElem.getText().trim()); + } + OMElement userInfoResponseRemoveInternalPrefixFromRoles = openIDConnectConfigElem.getFirstChildWithName( getQNameWithIdentityNS(ConfigElements.OPENID_CONNECT_USERINFO_REMOVE_INTERNAL_PREFIX_FROM_ROLES)); if (userInfoResponseRemoveInternalPrefixFromRoles != null) { @@ -4681,6 +4701,8 @@ private class ConfigElements { public static final String OPENID_CONNECT_USERINFO_JWT_SIGNATURE_ALGORITHM = "UserInfoJWTSignatureAlgorithm"; public static final String OPENID_CONNECT_USERINFO_MULTI_VALUE_SUPPORT_ENABLED = "UserInfoMultiValueSupportEnabled"; + public static final String OPENID_CONNECT_CONVERT_ONLY_MULTI_VALUED_CLAIMS_TO_ARRAY = + "ConvertOnlyMultiValuedClaimsToArray"; public static final String OPENID_CONNECT_USERINFO_REMOVE_INTERNAL_PREFIX_FROM_ROLES = "UserInfoRemoveInternalPrefixFromRoles"; private static final String OPENID_CONNECT_RETURN_APP_ROLES_IN_USERINFO = diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java index 0953e1ff656..9558d8c6e61 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java @@ -93,6 +93,8 @@ import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService; import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException; import org.wso2.carbon.identity.claim.metadata.mgt.model.ExternalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants; import org.wso2.carbon.identity.consent.server.configs.mgt.exceptions.ConsentServerConfigsMgtException; import org.wso2.carbon.identity.consent.server.configs.mgt.services.ConsentServerConfigsManagementService; import org.wso2.carbon.identity.core.IdentityKeyStoreResolver; @@ -2320,6 +2322,41 @@ public static void initiateOIDCScopes(int tenantId) { } } + /** + * Build the set of local claim URIs that are flagged as multi-valued in claim metadata for the given tenant. + * Used when the {@code ConvertOnlyMultiValuedClaimsToArray} configuration is enabled so that only claims + * explicitly flagged as multi-valued are emitted as arrays (e.g. in the UserInfo response). + * + * @param tenantDomain Tenant domain to resolve claim metadata for. + * @return Set of local claim URIs flagged as multi-valued, or {@code null} if the metadata could not be + * resolved (so that callers fall back to legacy separator-based multi-valued claim handling). + */ + public static Set getMultiValuedLocalClaimUris(String tenantDomain) { + + try { + ClaimMetadataManagementService claimMetadataManagementService = + OAuth2ServiceComponentHolder.getInstance().getClaimMetadataManagementService(); + if (claimMetadataManagementService == null) { + if (log.isDebugEnabled()) { + log.debug("ClaimMetadataManagementService is not available. Falling back to legacy multi-valued " + + "claim handling for tenant domain: " + tenantDomain); + } + return null; + } + Set multiValuedLocalClaimUris = new HashSet<>(); + for (LocalClaim localClaim : claimMetadataManagementService.getLocalClaims(tenantDomain)) { + if (Boolean.parseBoolean(localClaim.getClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY))) { + multiValuedLocalClaimUris.add(localClaim.getClaimURI()); + } + } + return multiValuedLocalClaimUris; + } catch (ClaimMetadataException e) { + log.error("Error while retrieving claim metadata to determine multi-valued claims for tenant domain: " + + tenantDomain + ". Falling back to legacy multi-valued claim handling.", e); + return null; + } + } + public static List getOIDCScopes(String tenantDomain) { List scopes = new ArrayList<>(); diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandler.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandler.java index 31a08f385f0..fbb18f91856 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandler.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandler.java @@ -37,6 +37,11 @@ import org.wso2.carbon.identity.application.mgt.ApplicationManagementService; import org.wso2.carbon.identity.base.IdentityConstants; import org.wso2.carbon.identity.base.IdentityException; +import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService; +import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException; +import org.wso2.carbon.identity.claim.metadata.mgt.model.ExternalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants; import org.wso2.carbon.identity.core.util.IdentityUtil; import org.wso2.carbon.identity.oauth.cache.AuthorizationGrantCache; import org.wso2.carbon.identity.oauth.cache.AuthorizationGrantCacheEntry; @@ -68,8 +73,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Pattern; import static org.apache.commons.collections.MapUtils.isEmpty; @@ -99,7 +106,8 @@ public JWTClaimsSet handleCustomClaims(JWTClaimsSet.Builder jwtClaimsSetBuilder, try { Map userClaimsInOIDCDialect = getUserClaimsInOIDCDialect(tokenReqMessageContext); tokenReqMessageContext.addProperty(ID_TOKEN_USER_CLAIMS_PROP_KEY, userClaimsInOIDCDialect.keySet()); - return setClaimsToJwtClaimSet(jwtClaimsSetBuilder, userClaimsInOIDCDialect); + return setClaimsToJwtClaimSet(jwtClaimsSetBuilder, userClaimsInOIDCDialect, + getServiceProviderTenantDomain(tokenReqMessageContext)); } catch (OAuthSystemException e) { if (log.isDebugEnabled()) { log.debug("Error occurred while adding claims of user: " + tokenReqMessageContext.getAuthorizedUser() + @@ -116,7 +124,8 @@ public JWTClaimsSet handleCustomClaims(JWTClaimsSet.Builder jwtClaimsSet, try { Map userClaimsInOIDCDialect = getUserClaimsInOIDCDialect(authzReqMessageContext); - return setClaimsToJwtClaimSet(jwtClaimsSet, userClaimsInOIDCDialect); + return setClaimsToJwtClaimSet(jwtClaimsSet, userClaimsInOIDCDialect, + getServiceProviderTenantDomain(authzReqMessageContext)); } catch (OAuthSystemException e) { log.error("Error occurred while adding claims of user: " + authzReqMessageContext.getAuthorizationReqDTO().getUser() + " to the JWTClaimSet used to " + @@ -870,14 +879,20 @@ private Map getUserAttributesFromCacheUsingCode(String aut * @param userClaimsInOIDCDialect */ private JWTClaimsSet setClaimsToJwtClaimSet(JWTClaimsSet.Builder jwtClaimsSetBuilder, Map - userClaimsInOIDCDialect) { + userClaimsInOIDCDialect, String spTenantDomain) { JWTClaimsSet jwtClaimsSet = jwtClaimsSetBuilder.build(); String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator(); + // When ConvertOnlyMultiValuedClaimsToArray is enabled, resolve (once) the claims flagged multi-valued in + // metadata. A null set means the feature is off or the lookup failed, so legacy behaviour applies. + Set multiValuedClaimUris = null; + if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) { + multiValuedClaimUris = getMultiValuedClaimUris(spTenantDomain); + } for (Map.Entry claimEntry : userClaimsInOIDCDialect.entrySet()) { String claimValue = claimEntry.getValue().toString(); String claimKey = claimEntry.getKey(); - if (isMultiValuedAttribute(claimKey, claimValue, multiAttributeSeparator)) { + if (isMultiValuedAttribute(claimKey, claimValue, multiAttributeSeparator, multiValuedClaimUris)) { JSONArray claimValues = new JSONArray(); String[] attributeValues = claimValue.split(Pattern.quote(multiAttributeSeparator)); for (String attributeValue : attributeValues) { @@ -922,7 +937,8 @@ private boolean isLocalUser(OAuthAuthzReqMessageContext authzReqMessageContext) return !authzReqMessageContext.getAuthorizationReqDTO().getUser().isFederatedUser(); } - private boolean isMultiValuedAttribute(String claimKey, String claimValue, String multiAttributeSeparator) { + private boolean isMultiValuedAttribute(String claimKey, String claimValue, String multiAttributeSeparator, + Set multiValuedClaimUris) { // Address claim contains multi attribute separator but its not a multi valued attribute. if (claimKey.equals(ADDRESS)) { @@ -933,9 +949,55 @@ private boolean isMultiValuedAttribute(String claimKey, String claimValue, Strin if (claimKey.equals(GROUPS)) { return true; } + // Feature on: only claims flagged multi-valued in metadata are arrays. Null set -> legacy fallback. + if (multiValuedClaimUris != null) { + return multiValuedClaimUris.contains(claimKey); + } return StringUtils.contains(claimValue, multiAttributeSeparator); } + /** + * Build the set of OIDC dialect claim URIs that map to local claims flagged as multi-valued in claim metadata + * for the given tenant. Used when the {@code ConvertOnlyMultiValuedClaimsToArray} configuration is enabled. + * + * @param tenantDomain Tenant domain of the service provider. + * @return Set of OIDC claim URIs flagged as multi-valued, or {@code null} if the metadata could not be resolved + * (so that the caller falls back to legacy multi-valued claim handling). + */ + private Set getMultiValuedClaimUris(String tenantDomain) { + + try { + ClaimMetadataManagementService claimMetadataManagementService = + OpenIDConnectServiceComponentHolder.getInstance().getClaimMetadataManagementService(); + if (claimMetadataManagementService == null) { + if (log.isDebugEnabled()) { + log.debug("ClaimMetadataManagementService is not available. Falling back to legacy multi-valued " + + "claim handling for tenant domain: " + tenantDomain); + } + return null; + } + // Build a map of local claim URI -> multiValued flag. + Map localClaimMultiValuedMap = new HashMap<>(); + for (LocalClaim localClaim : claimMetadataManagementService.getLocalClaims(tenantDomain)) { + localClaimMultiValuedMap.put(localClaim.getClaimURI(), Boolean.parseBoolean( + localClaim.getClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY))); + } + // Map each OIDC dialect claim to its local claim and collect the ones flagged multi-valued. + Set multiValuedClaimUris = new HashSet<>(); + for (ExternalClaim oidcClaim : claimMetadataManagementService.getExternalClaims( + OAuthConstants.OIDC_DIALECT, tenantDomain)) { + if (Boolean.TRUE.equals(localClaimMultiValuedMap.get(oidcClaim.getMappedLocalClaim()))) { + multiValuedClaimUris.add(oidcClaim.getClaimURI()); + } + } + return multiValuedClaimUris; + } catch (ClaimMetadataException e) { + log.error("Error while retrieving claim metadata to determine multi-valued claims for tenant domain: " + + tenantDomain + ". Falling back to legacy multi-valued claim handling.", e); + return null; + } + } + private boolean isApiBasedAuthFlow(String accessToken, String authorizationCode) { if (StringUtils.isNotEmpty(authorizationCode)) { diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java index af9aa963fcd..56fd86f6ec4 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java @@ -37,7 +37,11 @@ import org.wso2.carbon.identity.base.IdentityConstants; import org.wso2.carbon.identity.base.IdentityException; import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataHandler; +import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService; import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException; +import org.wso2.carbon.identity.claim.metadata.mgt.model.ExternalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants; import org.wso2.carbon.identity.core.util.IdentityTenantUtil; import org.wso2.carbon.identity.core.util.IdentityUtil; import org.wso2.carbon.identity.oauth.cache.AuthorizationGrantCache; @@ -70,9 +74,11 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -100,7 +106,7 @@ public JWTClaimsSet handleCustomClaims(JWTClaimsSet.Builder builder, OAuthTokenR throws IdentityOAuth2Exception { Map claims = getUserClaimsInOIDCDialect(request); - return setClaimsToJwtClaimSet(builder, claims); + return setClaimsToJwtClaimSet(builder, claims, getServiceProviderTenantDomain(request)); } @Override @@ -112,7 +118,7 @@ public JWTClaimsSet handleCustomClaims(JWTClaimsSet.Builder builder, OAuthAuthzR to manage user attributes for the access token. */ Map claims = getUserClaimsInOIDCDialect(request); - return setClaimsToJwtClaimSet(builder, claims); + return setClaimsToJwtClaimSet(builder, claims, getServiceProviderTenantDomain(request)); } /** @@ -877,14 +883,20 @@ private Map getOIDCToLocalClaimMappings(String tenantDomain) thr * @return JWTClaimSet with claims. */ private JWTClaimsSet setClaimsToJwtClaimSet(JWTClaimsSet.Builder jwtClaimsSetBuilder, Map - userClaimsInOIDCDialect) { + userClaimsInOIDCDialect, String spTenantDomain) { JWTClaimsSet jwtClaimsSet = jwtClaimsSetBuilder.build(); String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator(); + // When ConvertOnlyMultiValuedClaimsToArray is enabled, resolve (once) the claims flagged multi-valued in + // metadata. A null set means the feature is off or the lookup failed, so legacy behaviour applies. + Set multiValuedClaimUris = null; + if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) { + multiValuedClaimUris = getMultiValuedClaimUris(spTenantDomain); + } for (Map.Entry claimEntry : userClaimsInOIDCDialect.entrySet()) { String claimValue = claimEntry.getValue().toString(); String claimKey = claimEntry.getKey(); - if (isMultiValuedAttribute(claimKey, claimValue, multiAttributeSeparator)) { + if (isMultiValuedAttribute(claimKey, claimValue, multiAttributeSeparator, multiValuedClaimUris)) { JSONArray claimValues = new JSONArray(); String[] attributeValues = claimValue.split(Pattern.quote(multiAttributeSeparator)); for (String attributeValue : attributeValues) { @@ -912,7 +924,8 @@ private JWTClaimsSet setClaimsToJwtClaimSet(JWTClaimsSet.Builder jwtClaimsSetBui * @param multiAttributeSeparator Multi attribute separator. * @return True if claim value is multi valued, false otherwise. */ - private boolean isMultiValuedAttribute(String claimKey, String claimValue, String multiAttributeSeparator) { + private boolean isMultiValuedAttribute(String claimKey, String claimValue, String multiAttributeSeparator, + Set multiValuedClaimUris) { // Address claim contains multi attribute separator but its not a multi valued attribute. if (claimKey.equals(ADDRESS)) { @@ -923,9 +936,55 @@ private boolean isMultiValuedAttribute(String claimKey, String claimValue, Strin if (claimKey.equals(GROUPS)) { return true; } + // Feature on: only claims flagged multi-valued in metadata are arrays. Null set -> legacy fallback. + if (multiValuedClaimUris != null) { + return multiValuedClaimUris.contains(claimKey); + } return StringUtils.contains(claimValue, multiAttributeSeparator); } + /** + * Build the set of OIDC dialect claim URIs that map to local claims flagged as multi-valued in claim metadata + * for the given tenant. Used when the {@code ConvertOnlyMultiValuedClaimsToArray} configuration is enabled. + * + * @param tenantDomain Tenant domain of the service provider. + * @return Set of OIDC claim URIs flagged as multi-valued, or {@code null} if the metadata could not be resolved + * (so that the caller falls back to legacy multi-valued claim handling). + */ + private Set getMultiValuedClaimUris(String tenantDomain) { + + try { + ClaimMetadataManagementService claimMetadataManagementService = + OpenIDConnectServiceComponentHolder.getInstance().getClaimMetadataManagementService(); + if (claimMetadataManagementService == null) { + if (log.isDebugEnabled()) { + log.debug("ClaimMetadataManagementService is not available. Falling back to legacy multi-valued " + + "claim handling for tenant domain: " + tenantDomain); + } + return null; + } + // Build a map of local claim URI -> multiValued flag. + Map localClaimMultiValuedMap = new HashMap<>(); + for (LocalClaim localClaim : claimMetadataManagementService.getLocalClaims(tenantDomain)) { + localClaimMultiValuedMap.put(localClaim.getClaimURI(), Boolean.parseBoolean( + localClaim.getClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY))); + } + // Map each OIDC dialect claim to its local claim and collect the ones flagged multi-valued. + Set multiValuedClaimUris = new HashSet<>(); + for (ExternalClaim oidcClaim : claimMetadataManagementService.getExternalClaims( + OIDC_DIALECT, tenantDomain)) { + if (Boolean.TRUE.equals(localClaimMultiValuedMap.get(oidcClaim.getMappedLocalClaim()))) { + multiValuedClaimUris.add(oidcClaim.getClaimURI()); + } + } + return multiValuedClaimUris; + } catch (ClaimMetadataException e) { + log.error("Error while retrieving claim metadata to determine multi-valued claims for tenant domain: " + + tenantDomain + ". Falling back to legacy multi-valued claim handling.", e); + return null; + } + } + /** * Get access token claims. * diff --git a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/util/OAuth2UtilTest.java b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/util/OAuth2UtilTest.java index 5661944d80d..56c0582fabf 100644 --- a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/util/OAuth2UtilTest.java +++ b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/util/OAuth2UtilTest.java @@ -65,6 +65,8 @@ import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService; import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException; import org.wso2.carbon.identity.claim.metadata.mgt.model.ExternalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants; import org.wso2.carbon.identity.common.testng.WithCarbonHome; import org.wso2.carbon.identity.core.IdentityKeyStoreResolver; import org.wso2.carbon.identity.core.ServiceURLBuilder; @@ -3106,6 +3108,77 @@ public void testInitiateOIDCScopes(List scopeClaimsList, List mockedHolder = + mockStatic(OAuth2ServiceComponentHolder.class)) { + + OAuth2ServiceComponentHolder holder = mock(OAuth2ServiceComponentHolder.class); + ClaimMetadataManagementService claimService = mock(ClaimMetadataManagementService.class); + mockedHolder.when(OAuth2ServiceComponentHolder::getInstance).thenReturn(holder); + when(holder.getClaimMetadataManagementService()).thenReturn(claimService); + + LocalClaim multiValuedClaim = new LocalClaim("http://wso2.org/claims/multiTest"); + multiValuedClaim.setClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY, "true"); + LocalClaim singleValuedClaim = new LocalClaim("http://wso2.org/claims/givenname"); + singleValuedClaim.setClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY, "false"); + LocalClaim noPropertyClaim = new LocalClaim("http://wso2.org/claims/lastname"); + + List localClaims = new ArrayList<>(); + localClaims.add(multiValuedClaim); + localClaims.add(singleValuedClaim); + localClaims.add(noPropertyClaim); + when(claimService.getLocalClaims(SUPER_TENANT_DOMAIN_NAME)).thenReturn(localClaims); + + Set result = OAuth2Util.getMultiValuedLocalClaimUris(SUPER_TENANT_DOMAIN_NAME); + + assertNotNull(result, "Expected a non-null set when claim metadata is resolved."); + assertEquals(result.size(), 1, "Only the multiValued=true claim should be returned."); + assertTrue(result.contains("http://wso2.org/claims/multiTest")); + assertFalse(result.contains("http://wso2.org/claims/givenname")); + assertFalse(result.contains("http://wso2.org/claims/lastname")); + } + } + + @Test(description = "Issue #7138: getMultiValuedLocalClaimUris falls back to null (legacy behaviour) when the " + + "ClaimMetadataManagementService is unavailable.") + public void testGetMultiValuedLocalClaimUrisWithNullService() { + + try (MockedStatic mockedHolder = + mockStatic(OAuth2ServiceComponentHolder.class)) { + + OAuth2ServiceComponentHolder holder = mock(OAuth2ServiceComponentHolder.class); + mockedHolder.when(OAuth2ServiceComponentHolder::getInstance).thenReturn(holder); + when(holder.getClaimMetadataManagementService()).thenReturn(null); + + Set result = OAuth2Util.getMultiValuedLocalClaimUris(SUPER_TENANT_DOMAIN_NAME); + + assertNull(result, "Expected null (legacy fallback) when the claim metadata service is unavailable."); + } + } + + @Test(description = "Issue #7138: getMultiValuedLocalClaimUris falls back to null (legacy behaviour) and does not " + + "fail token issuance when claim metadata lookup throws.") + public void testGetMultiValuedLocalClaimUrisWithClaimMetadataException() throws Exception { + + try (MockedStatic mockedHolder = + mockStatic(OAuth2ServiceComponentHolder.class)) { + + OAuth2ServiceComponentHolder holder = mock(OAuth2ServiceComponentHolder.class); + ClaimMetadataManagementService claimService = mock(ClaimMetadataManagementService.class); + mockedHolder.when(OAuth2ServiceComponentHolder::getInstance).thenReturn(holder); + when(holder.getClaimMetadataManagementService()).thenReturn(claimService); + when(claimService.getLocalClaims(SUPER_TENANT_DOMAIN_NAME)) + .thenThrow(new ClaimMetadataException("error while reading claim metadata")); + + Set result = OAuth2Util.getMultiValuedLocalClaimUris(SUPER_TENANT_DOMAIN_NAME); + + assertNull(result, "Expected null (graceful legacy fallback) on ClaimMetadataException."); + } + } + @DataProvider(name = "initiateOIDCScopesDataProvider") public Object[][] initiateOIDCScopesDataProvider() { diff --git a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandlerMultiValuedClaimTest.java b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandlerMultiValuedClaimTest.java new file mode 100644 index 00000000000..2d94997c3a1 --- /dev/null +++ b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandlerMultiValuedClaimTest.java @@ -0,0 +1,223 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.wso2.carbon.identity.openidconnect; + +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; +import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService; +import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException; +import org.wso2.carbon.identity.claim.metadata.mgt.model.ExternalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants; +import org.wso2.carbon.identity.oauth.common.OAuthConstants; +import org.wso2.carbon.identity.openidconnect.internal.OpenIDConnectServiceComponentHolder; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; + +/** + * Tests selective multi-valued claim handling on the JWT/ID-token path + * ({@link DefaultOIDCClaimsCallbackHandler}), gated by {@code ConvertOnlyMultiValuedClaimsToArray}. + * Exercises the private {@code isMultiValuedAttribute} and {@code getMultiValuedClaimUris} methods. + */ +@Listeners(MockitoTestNGListener.class) +public class DefaultOIDCClaimsCallbackHandlerMultiValuedClaimTest { + + private static final String SEPARATOR = ","; + private static final String TENANT_DOMAIN = "carbon.super"; + + private static final String GIVEN_NAME = "given_name"; + private static final String MULTI_TEST = "multi_test"; + private static final String ADDRESS = "address"; + private static final String GROUPS = "groups"; + + private static final String LOCAL_GIVEN_NAME_URI = "http://wso2.org/claims/givenname"; + private static final String LOCAL_MULTI_TEST_URI = "http://wso2.org/claims/multiTest"; + + @Mock + private ClaimMetadataManagementService claimMetadataManagementService; + + private DefaultOIDCClaimsCallbackHandler handler; + private ClaimMetadataManagementService originalService; + + @BeforeMethod + public void setUp() { + + handler = new DefaultOIDCClaimsCallbackHandler(); + // Preserve whatever (if anything) the holder already had so we don't pollute sibling tests. + originalService = OpenIDConnectServiceComponentHolder.getInstance().getClaimMetadataManagementService(); + } + + @AfterMethod + public void tearDown() { + + OpenIDConnectServiceComponentHolder.getInstance().setClaimMetadataManagementService(originalService); + } + + /** + * (a) Flag OFF: a {@code null} multi-valued set must preserve the legacy behaviour — a value + * containing the separator is treated as multi-valued (array), a value without it is not. + */ + @Test + public void testLegacyBehaviourWhenFeatureDisabled() throws Exception { + + assertTrue(invokeIsMultiValued(GIVEN_NAME, "a,b,c,d", null), + "Legacy path must split a comma-containing value into an array when the feature is off."); + assertFalse(invokeIsMultiValued(GIVEN_NAME, "singleValue", null), + "Legacy path must keep a value without the separator as a plain string."); + } + + /** + * (b) Flag ON + claim NOT flagged multi-valued: even though the value contains commas it must + * be emitted as a plain string (not array). + */ + @Test + public void testSingleValuedClaimWithCommaRemainsStringWhenFeatureEnabled() throws Exception { + + Set multiValued = new HashSet<>(); + multiValued.add(MULTI_TEST); // given_name is intentionally absent. + assertFalse(invokeIsMultiValued(GIVEN_NAME, "a,b,c,d", multiValued), + "A non-multi-valued claim with a comma value must stay a string when the feature is on."); + } + + /** + * (c) Flag ON + claim flagged multi-valued: emitted as an array. + */ + @Test + public void testMultiValuedClaimBecomesArrayWhenFeatureEnabled() throws Exception { + + Set multiValued = new HashSet<>(); + multiValued.add(MULTI_TEST); + assertTrue(invokeIsMultiValued(MULTI_TEST, "a,b,c,d", multiValued), + "A claim flagged multi-valued must be emitted as an array when the feature is on."); + } + + /** + * (d) Special cases must be preserved regardless of the feature flag: {@code address} is never + * an array (even with a separator) and {@code groups} is always an array. + */ + @Test(dataProvider = "specialCaseProvider") + public void testSpecialCaseClaimsPreserved(Set multiValuedSet) throws Exception { + + assertFalse(invokeIsMultiValued(ADDRESS, "country,street,province", multiValuedSet), + "address must never be treated as a multi-valued (array) attribute."); + assertTrue(invokeIsMultiValued(GROUPS, "admin", multiValuedSet), + "groups must always be treated as a multi-valued (array) attribute."); + } + + @DataProvider(name = "specialCaseProvider") + public Object[][] specialCaseProvider() { + + Set enabledEmpty = new HashSet<>(); // feature on, nothing flagged. + Set enabledWithGroups = new HashSet<>(); + enabledWithGroups.add(GROUPS); + return new Object[][]{ + {null}, // feature off (legacy). + {enabledEmpty}, // feature on, groups/address not in set. + {enabledWithGroups} + }; + } + + /** + * Resolving the metadata-derived set must return only the OIDC claim URIs whose mapped local + * claim is flagged {@code multiValued=true}. + */ + @Test + public void testGetMultiValuedClaimUrisResolvesFlaggedClaims() throws Exception { + + LocalClaim multiValuedLocal = new LocalClaim(LOCAL_MULTI_TEST_URI); + multiValuedLocal.setClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY, "true"); + LocalClaim singleValuedLocal = new LocalClaim(LOCAL_GIVEN_NAME_URI); + singleValuedLocal.setClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY, "false"); + + ExternalClaim multiTestOidc = + new ExternalClaim(OAuthConstants.OIDC_DIALECT, MULTI_TEST, LOCAL_MULTI_TEST_URI); + ExternalClaim givenNameOidc = + new ExternalClaim(OAuthConstants.OIDC_DIALECT, GIVEN_NAME, LOCAL_GIVEN_NAME_URI); + + when(claimMetadataManagementService.getLocalClaims(TENANT_DOMAIN)) + .thenReturn(Arrays.asList(multiValuedLocal, singleValuedLocal)); + when(claimMetadataManagementService.getExternalClaims(OAuthConstants.OIDC_DIALECT, TENANT_DOMAIN)) + .thenReturn(Arrays.asList(multiTestOidc, givenNameOidc)); + OpenIDConnectServiceComponentHolder.getInstance() + .setClaimMetadataManagementService(claimMetadataManagementService); + + Set resolved = invokeGetMultiValuedClaimUris(TENANT_DOMAIN); + assertEquals(resolved, new HashSet<>(java.util.Collections.singletonList(MULTI_TEST)), + "Only OIDC claims mapped to a multiValued=true local claim must be returned."); + } + + /** + * (e) Graceful fallback: when the claim metadata service is unavailable the method must return + * {@code null} (legacy fallback) rather than fail token issuance. + */ + @Test + public void testGetMultiValuedClaimUrisReturnsNullWhenServiceUnavailable() throws Exception { + + OpenIDConnectServiceComponentHolder.getInstance().setClaimMetadataManagementService(null); + assertNull(invokeGetMultiValuedClaimUris(TENANT_DOMAIN), + "A missing claim metadata service must fall back to legacy handling (null set)."); + } + + /** + * (e) Graceful fallback: a {@link ClaimMetadataException} during lookup must be swallowed and + * mapped to {@code null} (legacy fallback), so token issuance never fails on metadata errors. + */ + @Test + public void testGetMultiValuedClaimUrisReturnsNullOnClaimMetadataException() throws Exception { + + when(claimMetadataManagementService.getLocalClaims(TENANT_DOMAIN)) + .thenThrow(new ClaimMetadataException("Simulated claim metadata failure.")); + OpenIDConnectServiceComponentHolder.getInstance() + .setClaimMetadataManagementService(claimMetadataManagementService); + + assertNull(invokeGetMultiValuedClaimUris(TENANT_DOMAIN), + "A ClaimMetadataException must be handled gracefully and fall back to legacy handling."); + } + + private boolean invokeIsMultiValued(String claimKey, String claimValue, Set multiValuedClaimUris) + throws Exception { + + Method method = DefaultOIDCClaimsCallbackHandler.class.getDeclaredMethod( + "isMultiValuedAttribute", String.class, String.class, String.class, Set.class); + method.setAccessible(true); + return (boolean) method.invoke(handler, claimKey, claimValue, SEPARATOR, multiValuedClaimUris); + } + + @SuppressWarnings("unchecked") + private Set invokeGetMultiValuedClaimUris(String tenantDomain) throws Exception { + + Method method = DefaultOIDCClaimsCallbackHandler.class.getDeclaredMethod( + "getMultiValuedClaimUris", String.class); + method.setAccessible(true); + return (Set) method.invoke(handler, tenantDomain); + } +} diff --git a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandlerMultiValuedClaimTest.java b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandlerMultiValuedClaimTest.java new file mode 100644 index 00000000000..693cc2fb87c --- /dev/null +++ b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandlerMultiValuedClaimTest.java @@ -0,0 +1,225 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.wso2.carbon.identity.openidconnect; + +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; +import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService; +import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException; +import org.wso2.carbon.identity.claim.metadata.mgt.model.ExternalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim; +import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants; +import org.wso2.carbon.identity.oauth.common.OAuthConstants; +import org.wso2.carbon.identity.openidconnect.internal.OpenIDConnectServiceComponentHolder; + +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; + +/** + * Tests selective multi-valued claim handling on the JWT access token path + * ({@link JWTAccessTokenOIDCClaimsHandler}), gated by {@code ConvertOnlyMultiValuedClaimsToArray}. + * Exercises the private {@code isMultiValuedAttribute} and {@code getMultiValuedClaimUris} methods. + */ +@Listeners(MockitoTestNGListener.class) +public class JWTAccessTokenOIDCClaimsHandlerMultiValuedClaimTest { + + private static final String SEPARATOR = ","; + private static final String TENANT_DOMAIN = "carbon.super"; + + private static final String GIVEN_NAME = "given_name"; + private static final String MULTI_TEST = "multi_test"; + private static final String ADDRESS = "address"; + private static final String GROUPS = "groups"; + + private static final String LOCAL_GIVEN_NAME_URI = "http://wso2.org/claims/givenname"; + private static final String LOCAL_MULTI_TEST_URI = "http://wso2.org/claims/multiTest"; + + @Mock + private ClaimMetadataManagementService claimMetadataManagementService; + + private JWTAccessTokenOIDCClaimsHandler handler; + private ClaimMetadataManagementService originalService; + + @BeforeMethod + public void setUp() { + + handler = new JWTAccessTokenOIDCClaimsHandler(); + // Preserve whatever (if anything) the holder already had so we don't pollute sibling tests. + originalService = OpenIDConnectServiceComponentHolder.getInstance().getClaimMetadataManagementService(); + } + + @AfterMethod + public void tearDown() { + + OpenIDConnectServiceComponentHolder.getInstance().setClaimMetadataManagementService(originalService); + } + + /** + * (a) Flag OFF: a {@code null} multi-valued set must preserve the legacy behaviour — a value + * containing the separator is treated as multi-valued (array), a value without it is not. + */ + @Test + public void testLegacyBehaviourWhenFeatureDisabled() throws Exception { + + assertTrue(invokeIsMultiValued(GIVEN_NAME, "a,b,c,d", null), + "Legacy path must split a comma-containing value into an array when the feature is off."); + assertFalse(invokeIsMultiValued(GIVEN_NAME, "singleValue", null), + "Legacy path must keep a value without the separator as a plain string."); + } + + /** + * (b) Flag ON + claim NOT flagged multi-valued: even though the value contains commas it must be + * emitted as a plain string (not array) in the JWT access token. + */ + @Test + public void testSingleValuedClaimWithCommaRemainsStringWhenFeatureEnabled() throws Exception { + + Set multiValued = new HashSet<>(); + multiValued.add(MULTI_TEST); // given_name is intentionally absent. + assertFalse(invokeIsMultiValued(GIVEN_NAME, "a,b,c,d", multiValued), + "A non-multi-valued claim with a comma value must stay a string in the JWT access token " + + "when the feature is on."); + } + + /** + * (c) Flag ON + claim flagged multi-valued: emitted as an array. + */ + @Test + public void testMultiValuedClaimBecomesArrayWhenFeatureEnabled() throws Exception { + + Set multiValued = new HashSet<>(); + multiValued.add(MULTI_TEST); + assertTrue(invokeIsMultiValued(MULTI_TEST, "a,b,c,d", multiValued), + "A claim flagged multi-valued must be emitted as an array when the feature is on."); + } + + /** + * (d) Special cases must be preserved regardless of the feature flag: {@code address} is never an + * array (even with a separator) and {@code groups} is always an array. + */ + @Test(dataProvider = "specialCaseProvider") + public void testSpecialCaseClaimsPreserved(Set multiValuedSet) throws Exception { + + assertFalse(invokeIsMultiValued(ADDRESS, "country,street,province", multiValuedSet), + "address must never be treated as a multi-valued (array) attribute."); + assertTrue(invokeIsMultiValued(GROUPS, "admin", multiValuedSet), + "groups must always be treated as a multi-valued (array) attribute."); + } + + @DataProvider(name = "specialCaseProvider") + public Object[][] specialCaseProvider() { + + Set enabledEmpty = new HashSet<>(); // feature on, nothing flagged. + Set enabledWithGroups = new HashSet<>(); + enabledWithGroups.add(GROUPS); + return new Object[][]{ + {null}, // feature off (legacy). + {enabledEmpty}, // feature on, groups/address not in set. + {enabledWithGroups} + }; + } + + /** + * Resolving the metadata-derived set must return only the OIDC claim URIs whose mapped local claim + * is flagged {@code multiValued=true}. + */ + @Test + public void testGetMultiValuedClaimUrisResolvesFlaggedClaims() throws Exception { + + LocalClaim multiValuedLocal = new LocalClaim(LOCAL_MULTI_TEST_URI); + multiValuedLocal.setClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY, "true"); + LocalClaim singleValuedLocal = new LocalClaim(LOCAL_GIVEN_NAME_URI); + singleValuedLocal.setClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY, "false"); + + ExternalClaim multiTestOidc = + new ExternalClaim(OAuthConstants.OIDC_DIALECT, MULTI_TEST, LOCAL_MULTI_TEST_URI); + ExternalClaim givenNameOidc = + new ExternalClaim(OAuthConstants.OIDC_DIALECT, GIVEN_NAME, LOCAL_GIVEN_NAME_URI); + + when(claimMetadataManagementService.getLocalClaims(TENANT_DOMAIN)) + .thenReturn(Arrays.asList(multiValuedLocal, singleValuedLocal)); + when(claimMetadataManagementService.getExternalClaims(OAuthConstants.OIDC_DIALECT, TENANT_DOMAIN)) + .thenReturn(Arrays.asList(multiTestOidc, givenNameOidc)); + OpenIDConnectServiceComponentHolder.getInstance() + .setClaimMetadataManagementService(claimMetadataManagementService); + + Set resolved = invokeGetMultiValuedClaimUris(TENANT_DOMAIN); + assertEquals(resolved, new HashSet<>(Collections.singletonList(MULTI_TEST)), + "Only OIDC claims mapped to a multiValued=true local claim must be returned."); + } + + /** + * (e) Graceful fallback: when the claim metadata service is unavailable the method must return + * {@code null} (legacy fallback) rather than fail token issuance. + */ + @Test + public void testGetMultiValuedClaimUrisReturnsNullWhenServiceUnavailable() throws Exception { + + OpenIDConnectServiceComponentHolder.getInstance().setClaimMetadataManagementService(null); + assertNull(invokeGetMultiValuedClaimUris(TENANT_DOMAIN), + "A missing claim metadata service must fall back to legacy handling (null set)."); + } + + /** + * (e) Graceful fallback: a {@link ClaimMetadataException} during lookup must be swallowed and + * mapped to {@code null} (legacy fallback), so token issuance never fails on metadata errors. + */ + @Test + public void testGetMultiValuedClaimUrisReturnsNullOnClaimMetadataException() throws Exception { + + when(claimMetadataManagementService.getLocalClaims(TENANT_DOMAIN)) + .thenThrow(new ClaimMetadataException("Simulated claim metadata failure.")); + OpenIDConnectServiceComponentHolder.getInstance() + .setClaimMetadataManagementService(claimMetadataManagementService); + + assertNull(invokeGetMultiValuedClaimUris(TENANT_DOMAIN), + "A ClaimMetadataException must be handled gracefully and fall back to legacy handling."); + } + + private boolean invokeIsMultiValued(String claimKey, String claimValue, Set multiValuedClaimUris) + throws Exception { + + Method method = JWTAccessTokenOIDCClaimsHandler.class.getDeclaredMethod( + "isMultiValuedAttribute", String.class, String.class, String.class, Set.class); + method.setAccessible(true); + return (boolean) method.invoke(handler, claimKey, claimValue, SEPARATOR, multiValuedClaimUris); + } + + @SuppressWarnings("unchecked") + private Set invokeGetMultiValuedClaimUris(String tenantDomain) throws Exception { + + Method method = JWTAccessTokenOIDCClaimsHandler.class.getDeclaredMethod( + "getMultiValuedClaimUris", String.class); + method.setAccessible(true); + return (Set) method.invoke(handler, tenantDomain); + } +} diff --git a/components/org.wso2.carbon.identity.oauth/src/test/resources/testng.xml b/components/org.wso2.carbon.identity.oauth/src/test/resources/testng.xml index 1d28a78fd98..97873a569a1 100755 --- a/components/org.wso2.carbon.identity.oauth/src/test/resources/testng.xml +++ b/components/org.wso2.carbon.identity.oauth/src/test/resources/testng.xml @@ -143,6 +143,8 @@ + +