Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -37,6 +40,13 @@ public Map<String, Object> getClaimsMap(Map<ClaimMapping, String> userAttributes

Map<String, Object> claims = new HashMap<String, Object>();
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<String> multiValuedLocalClaimUris = null;
if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) {
String tenantDomain = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain();
multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(tenantDomain);
}
Comment on lines +46 to +49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 1

Suggested change
if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) {
String tenantDomain = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain();
multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(tenantDomain);
}
if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) {
String tenantDomain = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain();
log.debug("ConvertOnlyMultiValuedClaimsToArray is enabled. Retrieving multi-valued claims for tenant: " + tenantDomain);
multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(tenantDomain);

Comment on lines +43 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for UserInfo endpoint entry points and tenant context setup

# Find UserInfo endpoint implementations that set tenant context
rg -nP -C5 'getUserClaimsUsingTokenResponse|getClaimsMap' --type java

# Search for PrivilegedCarbonContext tenant domain setup in UserInfo paths
rg -nP -C3 'startTenantFlow|PrivilegedCarbonContext.*setTenantDomain' \
  -g '*UserInfo*.java' -g '*ClaimUtil*.java'

# Look for spTenantDomain vs thread-local tenant usage patterns
rg -nP 'getThreadLocalCarbonContext\(\)\.getTenantDomain\(\)|spTenantDomain' \
  --type java -C2 -g '*oauth*endpoint*'

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 33773


🏁 Script executed:

# Examine ClaimUtil to understand tenant domain handling
cd components/org.wso2.carbon.identity.oauth.endpoint

# Find the getClaimsFromUserStore method and surrounding code
rg -n 'getClaimsFromUserStore|spTenantDomain|userAccessingTenantDomain' src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java -A 3 -B 1

# Look at the full method to understand tenant context
echo "=== Full getUserClaimsUsingTokenResponse method ==="
sed -n '95,120p' src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java

echo "=== Full getClaimsFromUserStore method signature and start ==="
sed -n '130,180p' src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java

echo "=== Tenant flow setup in getClaimsFromUserStore ==="
sed -n '230,270p' src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 10665


🏁 Script executed:

# Check UserInfoResponseBase to see how getClaimsMap is called and if tenant context is set
fd 'UserInfoResponseBase\.java' components/ --type f | head -1 | xargs wc -l

echo "=== Looking for calls to getUserClaimsUsingTokenResponse and retrieveUserClaims ==="
rg -n 'retrieveUserClaims|getClaimsMap' components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfo*.java -B 3 -A 3

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 4338


🏁 Script executed:

# Find where the tenant flow is started in the UserInfo endpoint flow
rg -n 'startTenantFlow|PrivilegedCarbonContext' components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/ --type java -g '*Response*.java' -B 2 -A 2

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Find UserInfoResponseBase and check how it calls ClaimUtil
fd 'UserInfoResponseBase\.java' components/org.wso2.carbon.identity.oauth.endpoint --type f

# Once found, examine it for tenant context setup
find components/org.wso2.carbon.identity.oauth.endpoint -name 'UserInfoResponseBase.java' -type f -exec wc -l {} \;

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Look at AbstractUserInfoResponseBuilder or UserInfoResponseBase 
# to see the flow from endpoint to ClaimUtil
find components/org.wso2.carbon.identity.oauth.endpoint -name '*UserInfoResponse*.java' -type f | grep -E '(Base|Abstract)' | head -3

# Then check for tenant context setup in that file
for file in $(find components/org.wso2.carbon.identity.oauth.endpoint -name '*UserInfoResponse*.java' -type f | grep -E '(Base|Abstract)'); do
  echo "=== $file ==="
  wc -l "$file"
done

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 534


🏁 Script executed:

# Look for tenant flow setup before retrieveUserClaims is called
rg -n 'startTenantFlow|endTenantFlow|retrieveUserClaims' \
  components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/ \
  --type java -B 2 -A 2 | head -80

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 1908


🏁 Script executed:

# Find the actual UserInfoResponseBase (not test)
find components/org.wso2.carbon.identity.oauth.endpoint/src/main/java -name '*UserInfoResponse*.java' -type f

# Get details on these files
for file in $(find components/org.wso2.carbon.identity.oauth.endpoint/src/main/java -name '*UserInfoResponse*.java' -type f); do
  echo "=== File: $file ==="
  wc -l "$file"
done

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Look at AbstractUserInfoResponseBuilder to see the flow
find components/org.wso2.carbon.identity.oauth.endpoint/src/main/java -name 'Abstract*UserInfo*.java' -type f -exec cat {} \; | head -200

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Search for where buildResponse or retrieveUserClaims is called and check tenant context
rg -n 'buildResponse|retrieveUserClaims|buildUserInfoResponse' \
  components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/ \
  --type java -B 5 -A 10 | head -150

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 8071


🏁 Script executed:

# Find and examine AbstractUserInfoResponseBuilder
find components/org.wso2.carbon.identity.oauth.endpoint/src/main/java -name 'AbstractUserInfoResponseBuilder.java' -type f

# Get its full content - check size first
find components/org.wso2.carbon.identity.oauth.endpoint/src/main/java -name 'AbstractUserInfoResponseBuilder.java' -type f -exec wc -l {} \;

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Read AbstractUserInfoResponseBuilder
find components/org.wso2.carbon.identity.oauth.endpoint/src/main/java -name 'AbstractUserInfoResponseBuilder.java' -type f -exec cat {} \; | head -300

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Look for where retrieveUserClaims is called in AbstractUserInfoResponseBuilder
rg -n 'retrieveUserClaims|FrameworkUtils.startTenantFlow|PrivilegedCarbonContext' \
  components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/AbstractUserInfoResponseBuilder.java \
  -B 3 -A 3

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 279


🏁 Script executed:

# List all files in the user/impl directory
ls -la components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 690


🏁 Script executed:

# Find all UserInfo related classes in the main source
find components/org.wso2.carbon.identity.oauth.endpoint/src/main/java -name '*UserInfo*.java' -type f

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 974


🏁 Script executed:

# Search for where retrieveUserClaims is defined (as an abstract method or interface)
rg -n 'retrieveUserClaims' components/org.wso2.carbon.identity.oauth.endpoint/src/main/java --type java -B 2 -A 5

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 2970


🏁 Script executed:

# Search more broadly in the entire component for retrieveUserClaims definition
rg -n 'abstract.*retrieveUserClaims|protected.*retrieveUserClaims' components/org.wso2.carbon.identity.oauth.endpoint --type java -B 2 -A 5

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 2970


🏁 Script executed:

# Search in the oauth module (not just endpoint)
find components/org.wso2.carbon.identity.oauth*/src/main/java -name '*.java' -type f -exec rg -l 'abstract.*retrieveUserClaims|protected.*retrieveUserClaims' {} \;

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 507


🏁 Script executed:

# Look at UserInfoJWTResponse more completely to find its parent class
cat components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoJWTResponse.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 6591


🏁 Script executed:

# Now examine the AbstractUserInfoResponseBuilder in the oauth module
cat components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/AbstractUserInfoResponseBuilder.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 20808


Tenant domain mismatch between cache-hit and cache-miss paths.

Lines 43-49 look up multi-valued local claim URIs using the thread-local tenant domain (PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantDomain()), which is not guaranteed to match the spTenantDomain used by the cache-miss path in ClaimUtil.getClaimsFromUserStore (line 272). The cache-miss path explicitly resolves spTenantDomain from the app configuration (lines 195-202 of ClaimUtil.java), while the cache-hit path relies on whatever tenant context the caller set in the thread-local context. In shared-user or cross-tenant scenarios, these could differ, causing the same claim to be treated as multi-valued or single-valued depending on the cache path taken, violating the PR's objective of "consistent selective logic" across all UserInfo code paths.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoUserStoreClaimRetriever.java`
around lines 43 - 49, The multi-valued local claim URIs lookup in the cache-hit
path (line 46 where OAuth2Util.getMultiValuedLocalClaimUris is called) uses the
thread-local tenant domain from PrivilegedCarbonContext, but the cache-miss path
in ClaimUtil uses spTenantDomain resolved from the app configuration (lines
195-202 of ClaimUtil.java). This mismatch causes inconsistent behavior in
cross-tenant scenarios. Replace the thread-local tenant domain retrieval with
the spTenantDomain that matches the cache-miss path, ensuring
OAuth2Util.getMultiValuedLocalClaimUris receives the same tenant domain as the
ClaimUtil cache-miss path uses, so both paths treat multi-valued claims
consistently regardless of the calling context.

for (Map.Entry<ClaimMapping, String> entry : userAttributes.entrySet()) {

if (entry.getKey().getRemoteClaim() == null || IdentityCoreConstants.MULTI_ATTRIBUTE_SEPARATOR.equals(
Expand All @@ -45,10 +55,13 @@ public Map<String, Object> getClaimsMap(Map<ClaimMapping, String> 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);
Comment on lines 61 to 65

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 2

Suggested change
.getUserInfoMultiValueSupportEnabled();
if (isMultiValueSupportEnabledForUserinfoResponse &&
ClaimUtil.isMultiValuedAttribute(claimUri, claimValue)) {
ClaimUtil.isMultiValuedAttribute(claimUri, localClaimUri, claimValue,
multiValuedLocalClaimUris)) {
String[] attributeValues = ClaimUtil.processMultiValuedAttribute(claimValue);
if (isMultiValueSupportEnabledForUserinfoResponse &&
ClaimUtil.isMultiValuedAttribute(claimUri, localClaimUri, claimValue,
multiValuedLocalClaimUris)) {
if (log.isDebugEnabled()) {
log.debug("Processing multi-valued claim: " + claimUri);
}
String[] attributeValues = ClaimUtil.processMultiValuedAttribute(claimValue);

claims.put(claimUri, attributeValues);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -264,6 +265,12 @@ public static Map<String, Object> 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<String> multiValuedLocalClaimUris = null;
if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) {
multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(spTenantDomain);
}
Comment on lines +269 to +273

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 3

Suggested change
// multi-valued. A null set means the feature is off or the lookup failed, so legacy behaviour applies.
Set<String> multiValuedLocalClaimUris = null;
if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) {
multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(spTenantDomain);
}
Set<String> multiValuedLocalClaimUris = null;
if (OAuthServerConfiguration.getInstance().getConvertOnlyMultiValuedClaimsToArray()) {
if (log.isDebugEnabled()) {
log.debug("ConvertOnlyMultiValuedClaimsToArray is enabled. Resolving multi-valued local claim URIs for tenant: " + spTenantDomain);
}
multiValuedLocalClaimUris = OAuth2Util.getMultiValuedLocalClaimUris(spTenantDomain);
}

if (isNotEmpty(userClaims)) {
for (Map.Entry<String, String> entry : userClaims.entrySet()) {
//set local2sp role mappings
Expand All @@ -289,7 +296,8 @@ public static Map<String, Object> 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 {
Expand Down Expand Up @@ -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<String> 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());
}
Comment on lines +600 to +612

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 4

Suggested change
public static boolean isMultiValuedAttribute(String claimUri, String localClaimUri, String claimValue,
Set<String> 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());
}
public static boolean isMultiValuedAttribute(String claimUri, String localClaimUri, String claimValue,
Set<String> multiValuedLocalClaimUris) {
if (log.isDebugEnabled()) {
log.debug("Checking if claim is multi-valued. OIDC claim: " + claimUri + ", Local claim: " + localClaimUri);
}
/* 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) {
boolean isMultiValued = multiValuedLocalClaimUris.contains(localClaimUri);
if (log.isDebugEnabled()) {
log.debug("Using claim metadata to determine multi-valued status. Local claim " + localClaimUri + " is multi-valued: " + isMultiValued);
}
return isMultiValued;
}
return StringUtils.contains(claimValue, FrameworkUtils.getMultiAttributeSeparator());
}


/**
* Split multivalued attribute string value by attribute separator.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -562,6 +564,56 @@ public void testGetServiceProviderMappedUserRoles(List<String> 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<String> 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<String> multiValuedLocalClaimUris, boolean expected) {

try (MockedStatic<FrameworkUtils> 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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ public class OAuthServerConfiguration {
private List<String> 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;
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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());
}
Comment on lines +3950 to +3955

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 5

Suggested change
OMElement convertOnlyMultiValuedClaimsToArrayElem = openIDConnectConfigElem.getFirstChildWithName(
getQNameWithIdentityNS(ConfigElements.OPENID_CONNECT_CONVERT_ONLY_MULTI_VALUED_CLAIMS_TO_ARRAY));
if (convertOnlyMultiValuedClaimsToArrayElem != null) {
convertOnlyMultiValuedClaimsToArray = Boolean.parseBoolean(
convertOnlyMultiValuedClaimsToArrayElem.getText().trim());
}
if (convertOnlyMultiValuedClaimsToArrayElem != null) {
convertOnlyMultiValuedClaimsToArray = Boolean.parseBoolean(
convertOnlyMultiValuedClaimsToArrayElem.getText().trim());
log.info("ConvertOnlyMultiValuedClaimsToArray is set to: " + convertOnlyMultiValuedClaimsToArray);
}


OMElement userInfoResponseRemoveInternalPrefixFromRoles = openIDConnectConfigElem.getFirstChildWithName(
getQNameWithIdentityNS(ConfigElements.OPENID_CONNECT_USERINFO_REMOVE_INTERNAL_PREFIX_FROM_ROLES));
if (userInfoResponseRemoveInternalPrefixFromRoles != null) {
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> getMultiValuedLocalClaimUris(String tenantDomain) {
Comment on lines +2333 to +2334

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 6

Suggested change
*/
public static Set<String> getMultiValuedLocalClaimUris(String tenantDomain) {
public static Set<String> getMultiValuedLocalClaimUris(String tenantDomain) {
if (log.isDebugEnabled()) {
log.debug("Building multi-valued local claim URIs for tenant domain: " + 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<String> multiValuedLocalClaimUris = new HashSet<>();
for (LocalClaim localClaim : claimMetadataManagementService.getLocalClaims(tenantDomain)) {
if (Boolean.parseBoolean(localClaim.getClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY))) {
multiValuedLocalClaimUris.add(localClaim.getClaimURI());
}
}
Comment on lines +2350 to +2351

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Log Improvement Suggestion No: 7

Suggested change
}
}
}
if (log.isDebugEnabled()) {
log.debug("Found " + multiValuedLocalClaimUris.size() + " multi-valued local claims for tenant " +
"domain: " + tenantDomain);
}
return multiValuedLocalClaimUris;

return multiValuedLocalClaimUris;
} catch (ClaimMetadataException e) {
Comment on lines +2347 to +2353

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify getLocalClaims nullability contract and existing call-site assumptions (read-only).
# 1) Locate ClaimMetadataManagementService interface and getLocalClaims signature/docs.
fd -i "ClaimMetadataManagementService.java" --exec sed -n '1,240p' {}

# 2) Find implementations and inspect return behavior for getLocalClaims.
fd -i "*ClaimMetadataManagementService*.java" --exec rg -n -C3 "getLocalClaims\\s*\\(" {}

# 3) Check repository usage patterns to see whether callers assume non-null.
rg -n -C2 "getLocalClaims\\s*\\(" --type=java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 9955


🏁 Script executed:

# Find the ClaimMetadataManagementService interface definition
fd -type f -name "ClaimMetadataManagementService.java" -exec cat {} \;

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 258


🏁 Script executed:

# Examine the complete getMultiValuedLocalClaimUris method
sed -n '2330,2360p' components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 1820


🏁 Script executed:

# Check other call sites at lines 968 and 981 for comparison
sed -n '960,975p' components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java
sed -n '973,988p' components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandler.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 2238


🏁 Script executed:

# Search for any null checks on getLocalClaims results in codebase
rg -n "getLocalClaims.*null" --type=java -A 2 -B 2

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Search for ClaimMetadataManagementService interface/class with getLocalClaims method
rg -n "getLocalClaims" -A 5 -B 5 | grep -A 10 -B 10 "interface\|class.*ClaimMetadataManagementService" | head -60

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Find and examine ClaimMetadataManagementService files
find . -name "*ClaimMetadataManagementService*" -type f

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 198


🏁 Script executed:

# Look for null return in getLocalClaims implementations
rg -n "getLocalClaims" -A 10 | grep -E "(return null|return Collections|return List)" | head -20

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 226


🏁 Script executed:

# Check the complete context of the other two call sites to see if they have try-catch
sed -n '950,1000p' components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 2796


🏁 Script executed:

# Search for ClaimMetadataManagementService interface definition (likely in a separate library)
rg -n "interface ClaimMetadataManagementService" -A 20

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Look for any documentation or comments about getLocalClaims return contract
rg -n "getLocalClaims" -B 5 -A 2 | grep -E "(null|`@return`|javadoc|/**)" | head -40

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 7697


🏁 Script executed:

# Check if there are any defensive null checks elsewhere in the codebase for similar patterns
rg -n "if.*getLocalClaims.*null" --type java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Examine the complete context of both other call sites to confirm they also lack null checks
sed -n '950,1010p' components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandler.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 3460


🏁 Script executed:

# Search for all imports of LocalClaim and getLocalClaims to understand the return type
rg -n "import.*LocalClaim" --type java | head -10

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 1311


🏁 Script executed:

# Look for the actual implementation of getLocalClaims to check if it documents null returns
rg -n "public.*List.*getLocalClaims" -A 10 | head -40

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Verify if there are tests covering null return scenario for getLocalClaims
rg -n "getLocalClaims.*thenReturn.*null" --type java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Search for any null documentation or defensive code around getLocalClaims across codebase
rg -n "getLocalClaims\|getExternalClaims" -B 3 -A 3 | grep -E "(null|`@return`|List<|throws)" | head -50

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Check if getLocalClaims can return empty list vs null by examining actual usage patterns
rg -n "getLocalClaims" -A 1 | grep -E "(size|isEmpty|== null|!= null)"

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 69


🏁 Script executed:

# Look at the test setup for getLocalClaims mocks more broadly
cat components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/util/OAuth2UtilTest.java | sed -n '3120,3180p'

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 3670


🏁 Script executed:

# Verify the complete method context and exception handling around both other call sites
sed -n '940,990p' components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 2786


🏁 Script executed:

# Check what exception types are caught at those call sites
sed -n '940,1020p' components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/JWTAccessTokenOIDCClaimsHandler.java

Repository: wso2-extensions/identity-inbound-auth-oauth

Length of output: 4029


Guard against null claim lists in three locations to preserve graceful fallback semantics.

At Line 2347, the code iterates directly over claimMetadataManagementService.getLocalClaims(tenantDomain) without null protection. If that API returns null (e.g., partial service failure), this throws NullPointerException, which bypasses the ClaimMetadataException catch block and can crash token/userinfo processing instead of degrading to legacy behavior.

This same vulnerability exists at two additional locations:

  • JWTAccessTokenOIDCClaimsHandler.getMultiValuedClaimUris() (line 968)
  • DefaultOIDCClaimsCallbackHandler.getMultiValuedClaimUris() (line 981)

All three methods follow the same pattern: checking if the service itself is null (showing defensive intent), then calling getLocalClaims() directly without null protection, then catching only ClaimMetadataException. The method's documented contract states it returns null on metadata failure, indicating these methods should gracefully degrade rather than crash.

Proposed fix
     public static Set<String> 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;
             }
+            List<LocalClaim> localClaims = claimMetadataManagementService.getLocalClaims(tenantDomain);
+            if (localClaims == null) {
+                if (log.isDebugEnabled()) {
+                    log.debug("Local claim metadata is null. Falling back to legacy multi-valued claim handling for " +
+                            "tenant domain: " + tenantDomain);
+                }
+                return null;
+            }
             Set<String> multiValuedLocalClaimUris = new HashSet<>();
-            for (LocalClaim localClaim : claimMetadataManagementService.getLocalClaims(tenantDomain)) {
+            for (LocalClaim localClaim : localClaims) {
                 if (Boolean.parseBoolean(localClaim.getClaimProperty(ClaimConstants.MULTI_VALUED_PROPERTY))) {
                     multiValuedLocalClaimUris.add(localClaim.getClaimURI());
                 }
             }
             return multiValuedLocalClaimUris;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java`
around lines 2347 - 2353, Add null checks before iterating over the result of
claimMetadataManagementService.getLocalClaims() in three methods to prevent
NullPointerException when the service returns null. In the
getMultiValuedLocalClaimUris method in OAuth2Util (where the loop iterates over
claimMetadataManagementService.getLocalClaims(tenantDomain)), in
JWTAccessTokenOIDCClaimsHandler.getMultiValuedClaimUris(), and in
DefaultOIDCClaimsCallbackHandler.getMultiValuedClaimUris(), verify that
getLocalClaims() does not return null before entering the for-each loop. If null
is encountered, allow the method to gracefully return null (consistent with its
documented contract) rather than throwing an unhandled NullPointerException that
bypasses the ClaimMetadataException catch block.

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<String> getOIDCScopes(String tenantDomain) {

List<String> scopes = new ArrayList<>();
Expand Down
Loading
Loading