-
Notifications
You must be signed in to change notification settings - Fork 426
Introduce selective multi-valued claim handling in JWT and UserInfo based on claim metadata #3255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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<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
+43
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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.javaRepository: 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 3Repository: 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 2Repository: 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"
doneRepository: 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 -80Repository: 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"
doneRepository: 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 -200Repository: 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 -150Repository: 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 -300Repository: 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 3Repository: 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 fRepository: 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 5Repository: 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 5Repository: 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.javaRepository: 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.javaRepository: 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 ( 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| for (Map.Entry<ClaimMapping, String> entry : userAttributes.entrySet()) { | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (entry.getKey().getRemoteClaim() == null || IdentityCoreConstants.MULTI_ATTRIBUTE_SEPARATOR.equals( | ||||||||||||||||||||||||||||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log Improvement Suggestion No: 2
Suggested change
|
||||||||||||||||||||||||||||
| claims.put(claimUri, attributeValues); | ||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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<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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log Improvement Suggestion No: 3
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isNotEmpty(userClaims)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (Map.Entry<String, String> entry : userClaims.entrySet()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //set local2sp role mappings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log Improvement Suggestion No: 4
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Split multivalued attribute string value by attribute separator. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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; | ||||||||||||||||||||||||
|
|
@@ -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()); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+3950
to
+3955
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log Improvement Suggestion No: 5
Suggested change
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| 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 = | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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<String> getMultiValuedLocalClaimUris(String tenantDomain) { | ||||||||||||||||||
|
Comment on lines
+2333
to
+2334
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log Improvement Suggestion No: 6
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log Improvement Suggestion No: 7
Suggested change
|
||||||||||||||||||
| return multiValuedLocalClaimUris; | ||||||||||||||||||
| } catch (ClaimMetadataException e) { | ||||||||||||||||||
|
Comment on lines
+2347
to
+2353
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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=javaRepository: 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.javaRepository: 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.javaRepository: 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 2Repository: 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 -60Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: # Find and examine ClaimMetadataManagementService files
find . -name "*ClaimMetadataManagementService*" -type fRepository: 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 -20Repository: 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.javaRepository: 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 20Repository: 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 -40Repository: 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 javaRepository: 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.javaRepository: 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 -10Repository: 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 -40Repository: 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 javaRepository: 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 -50Repository: 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.javaRepository: 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.javaRepository: 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 This same vulnerability exists at two additional locations:
All three methods follow the same pattern: checking if the service itself is null (showing defensive intent), then calling 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 |
||||||||||||||||||
| 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<>(); | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
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