Skip to content

Commit

Permalink
Merge pull request #2674 from sadilchamishka/improve-userinfo-respons…
Browse files Browse the repository at this point in the history 
…e-for-roles

Improve user info response to return roles without internal domain appended
  • Loading branch information
@sadilchamishka
sadilchamishka authored Jan 22, 2025
2 parents b21c233 + 8559bcf commit cecf1b2
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 1 deletion.
12 changes: 12 additions & 0 deletions ...va/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoJSONResponseBuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.wso2.carbon.identity.core.persistence.JDBCPersistenceManager;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.oauth.cache.AuthorizationGrantCache;
import org.wso2.carbon.identity.oauth.common.OAuthConstants;
import org.wso2.carbon.identity.oauth.config.OAuthServerConfiguration;
import org.wso2.carbon.identity.oauth.endpoint.util.ClaimUtil;
import org.wso2.carbon.identity.oauth.tokenprocessor.DefaultTokenProvider;
Expand All @@ -49,6 +50,7 @@

import java.sql.Connection;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -70,6 +72,7 @@
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
import static org.wso2.carbon.base.MultitenantConstants.SUPER_TENANT_DOMAIN_NAME;
import static org.wso2.carbon.identity.oauth2.Oauth2ScopeConstants.INTERNAL_ROLE_PREFIX;

/**
* This class contains tests for UserInfoJSONResponseBuilder.
Expand Down Expand Up @@ -161,6 +164,10 @@ public void testGetResponseString(Map<String, Object> inputClaims,
authenticatedUser.setAuthenticatedSubjectIdentifier(AUTHORIZED_USER_ID);
mockAccessTokenDOInOAuth2Util(authenticatedUser, oAuth2Util);

if (Arrays.asList(requestedScopes).contains(OAuthConstants.OIDCClaims.ROLES)) {
when(mockOAuthServerConfiguration.isUserInfoResponseRemoveInternalPrefixFromRoles())
.thenReturn(true);
}
String responseString =
userInfoJSONResponseBuilder.getResponseString(
getTokenResponseDTO(AUTHORIZED_USER_FULL_QUALIFIED, requestedScopes));
Expand All @@ -170,6 +177,11 @@ public void testGetResponseString(Map<String, Object> inputClaims,
assertFalse(claimsInResponse.isEmpty());
assertNotNull(claimsInResponse.get(sub));

if (claimsInResponse.containsKey(OAuth2Util.OIDC_ROLE_CLAIM_URI)) {
Object[] rolesArray = (Object[]) claimsInResponse.get(OAuth2Util.OIDC_ROLE_CLAIM_URI);
assertFalse(rolesArray[0].toString().startsWith(INTERNAL_ROLE_PREFIX));
}

for (Map.Entry<String, Object> expectClaimEntry : expectedClaims.entrySet()) {
assertTrue(claimsInResponse.containsKey(expectClaimEntry.getKey()));
assertNotNull(claimsInResponse.get(expectClaimEntry.getKey()));
Expand Down
11 changes: 11 additions & 0 deletions ...test/java/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoResponseBaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,10 @@ protected Object[][] getOidcScopeFilterTestData() {
final Map<String, Object> expectedClaimMapForCustomScope = new HashMap<>();
expectedClaimMapForCustomScope.put(firstName, FIRST_NAME_VALUE);

Map<String, Object> userClaimsMap1 = new HashMap<>();
userClaimsMap1.put(OAuth2Util.OIDC_ROLE_CLAIM_URI, new String[]{"Internal/test_role1",
"Internal/test_role2"});

return new Object[][]{
// Input User Claims,
// Map<"openid", ("first_name","username","last_name")>
Expand All @@ -494,6 +498,13 @@ protected Object[][] getOidcScopeFilterTestData() {
false,
OIDC_SCOPE_ARRAY,
Collections.emptyMap()
},
{
userClaimsMap1,
Collections.emptyMap(),
false,
new String[]{OIDC_SCOPE, "roles"},
Collections.emptyMap()
}
};
}
Expand Down
12 changes: 12 additions & 0 deletions components/org.wso2.carbon.identity.oauth.endpoint/src/test/resources/dbscripts/scope.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ INSERT INTO IDN_OIDC_SCOPE_CLAIM_MAPPING (ID, SCOPE_ID, EXTERNAL_CLAIM_ID,TENAN
(3, 2,3, 1);
INSERT INTO IDN_CLAIM (ID, DIALECT_ID, CLAIM_URI,TENANT_ID) VALUES
(4, 1,'email', 1);
INSERT INTO IDN_OAUTH2_SCOPE (SCOPE_ID, NAME, DISPLAY_NAME, TENANT_ID, SCOPE_TYPE) VALUES
(3, 'roles', 'roles', 1, 'OIDC');
INSERT INTO IDN_CLAIM (ID, DIALECT_ID, CLAIM_URI,TENANT_ID) VALUES
(22, 1,'roles', 1);
INSERT INTO IDN_OIDC_SCOPE_CLAIM_MAPPING (ID, SCOPE_ID, EXTERNAL_CLAIM_ID,TENANT_ID) VALUES
(22, 3, 22, 1);
INSERT INTO IDN_OIDC_SCOPE_CLAIM_MAPPING (ID, SCOPE_ID, EXTERNAL_CLAIM_ID,TENANT_ID) VALUES
(4, 2,4, 1);
INSERT INTO IDN_CLAIM (ID, DIALECT_ID, CLAIM_URI,TENANT_ID) VALUES
Expand Down Expand Up @@ -102,6 +108,12 @@ INSERT INTO IDN_OIDC_SCOPE_CLAIM_MAPPING (ID, SCOPE_ID, EXTERNAL_CLAIM_ID,TENAN
(16, 11,16, -1234);
INSERT INTO IDN_CLAIM (ID, DIALECT_ID, CLAIM_URI,TENANT_ID) VALUES
(17, 5,'email', -1234);
INSERT INTO IDN_OAUTH2_SCOPE (SCOPE_ID, NAME, DISPLAY_NAME, TENANT_ID, SCOPE_TYPE) VALUES
(12, 'roles', 'roles', -1234, 'OIDC');
INSERT INTO IDN_CLAIM (ID, DIALECT_ID, CLAIM_URI,TENANT_ID) VALUES
(23, 5,'roles', -1234);
INSERT INTO IDN_OIDC_SCOPE_CLAIM_MAPPING (ID, SCOPE_ID, EXTERNAL_CLAIM_ID,TENANT_ID) VALUES
(23, 12, 23, -1234);
INSERT INTO IDN_OIDC_SCOPE_CLAIM_MAPPING (ID, SCOPE_ID, EXTERNAL_CLAIM_ID,TENANT_ID) VALUES
(17, 11,17, -1234);
INSERT INTO IDN_CLAIM (ID, DIALECT_ID, CLAIM_URI,TENANT_ID) VALUES
Expand Down
24 changes: 23 additions & 1 deletion ...y.oauth/src/main/java/org/wso2/carbon/identity/oauth/config/OAuthServerConfiguration.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013-2024, WSO2 LLC. (http://www.wso2.com).
* Copyright (c) 2013-2025, WSO2 LLC. (http://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
Expand Down Expand Up @@ -233,6 +233,8 @@ public class OAuthServerConfiguration {
private List<String> supportedIdTokenEncryptionMethods = new ArrayList<>();
private String userInfoJWTSignatureAlgorithm = "SHA256withRSA";
private boolean userInfoMultiValueSupportEnabled = true;
private boolean userInfoRemoveInternalPrefixFromRoles = false;

private String authContextTTL = "15L";
// property added to fix IDENTITY-4551 in backward compatible manner
private boolean useMultiValueSeparatorForAuthContextToken = true;
Expand Down Expand Up @@ -1575,6 +1577,16 @@ public boolean getUserInfoMultiValueSupportEnabled() {
return userInfoMultiValueSupportEnabled;
}

/**
* Returns whether Internal prefix should be removed from the roles claim of the userinfo response.
*
* @return True if Internal prefix value should be removed from the role claim of userinfo response.
*/
public boolean isUserInfoResponseRemoveInternalPrefixFromRoles() {

return userInfoRemoveInternalPrefixFromRoles;
}

public String getConsumerDialectURI() {
return consumerDialectURI;
}
Expand Down Expand Up @@ -3503,6 +3515,14 @@ private void parseOpenIDConnectConfig(OMElement oauthConfigElem) {
userInfoMultiValueSupportEnabled = Boolean.parseBoolean(
userInfoMultiValueSupportEnabledElem.getText().trim());
}

OMElement userInfoResponseRemoveInternalPrefixFromRoles = openIDConnectConfigElem.getFirstChildWithName(
getQNameWithIdentityNS(ConfigElements.OPENID_CONNECT_USERINFO_REMOVE_INTERNAL_PREFIX_FROM_ROLES));
if (userInfoResponseRemoveInternalPrefixFromRoles != null) {
userInfoRemoveInternalPrefixFromRoles =
Boolean.parseBoolean(userInfoResponseRemoveInternalPrefixFromRoles.getText().trim());
}

if (openIDConnectConfigElem.getFirstChildWithName(
getQNameWithIdentityNS(ConfigElements.OPENID_CONNECT_SIGN_JWT_WITH_SP_KEY)) != null) {
isJWTSignedWithSPKey = Boolean.parseBoolean(openIDConnectConfigElem.getFirstChildWithName(
Expand Down Expand Up @@ -4132,6 +4152,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_USERINFO_REMOVE_INTERNAL_PREFIX_FROM_ROLES =
"UserInfoRemoveInternalPrefixFromRoles";
public static final String OPENID_CONNECT_SIGN_JWT_WITH_SP_KEY = "SignJWTWithSPKey";
public static final String OPENID_CONNECT_IDTOKEN_CUSTOM_CLAIM_CALLBACK_HANDLER =
"IDTokenCustomClaimsCallBackHandler";
Expand Down
31 changes: 31 additions & 0 deletions ...src/main/java/org/wso2/carbon/identity/openidconnect/AbstractUserInfoResponseBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
import org.wso2.carbon.identity.application.common.model.ServiceProvider;
import org.wso2.carbon.identity.application.common.util.IdentityApplicationConstants;
import org.wso2.carbon.identity.application.mgt.ApplicationManagementService;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.oauth.cache.AuthorizationGrantCache;
import org.wso2.carbon.identity.oauth.cache.AuthorizationGrantCacheEntry;
import org.wso2.carbon.identity.oauth.cache.AuthorizationGrantCacheKey;
import org.wso2.carbon.identity.oauth.common.exception.InvalidOAuthClientException;
import org.wso2.carbon.identity.oauth.config.OAuthServerConfiguration;
import org.wso2.carbon.identity.oauth.dao.OAuthAppDO;
import org.wso2.carbon.identity.oauth.user.UserInfoEndpointException;
import org.wso2.carbon.identity.oauth.user.UserInfoResponseBuilder;
Expand All @@ -40,6 +42,8 @@
import org.wso2.carbon.identity.oauth2.util.OAuth2Util;
import org.wso2.carbon.identity.openidconnect.internal.OpenIDConnectServiceComponentHolder;
import org.wso2.carbon.identity.openidconnect.model.RequestedClaim;
import org.wso2.carbon.user.core.UserCoreConstants;
import org.wso2.carbon.user.core.util.UserCoreUtil;

import java.util.ArrayList;
import java.util.HashMap;
Expand All @@ -50,6 +54,7 @@
import static org.apache.commons.collections.CollectionUtils.isNotEmpty;
import static org.apache.commons.lang.StringUtils.isNotEmpty;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OAuth20Params.USERINFO;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OIDCClaims.ROLES;

/**
* Abstract user info response builder.
Expand All @@ -76,6 +81,9 @@ public String getResponseString(OAuth2TokenValidationResponseDTO tokenResponse)
Map<String, Object> userClaims = retrieveUserClaims(tokenResponse);
Map<String, Object> filteredUserClaims = filterOIDCClaims(tokenResponse, clientId, spTenantDomain, userClaims);

// Handle roles claim.
handleRolesClaim(filteredUserClaims);

// Handle subject claim.
String subjectClaim = getSubjectClaim(userClaims, clientId, spTenantDomain, tokenResponse);
subjectClaim = getOIDCSubjectClaim(clientId, spTenantDomain, subjectClaim);
Expand All @@ -84,6 +92,29 @@ public String getResponseString(OAuth2TokenValidationResponseDTO tokenResponse)
return buildResponse(tokenResponse, spTenantDomain, filteredUserClaims);
}

private void handleRolesClaim(Map<String, Object> filteredUserClaims) {

// This check is added for the backward compatibility of userinfo response.
if (!OAuthServerConfiguration.getInstance().isUserInfoResponseRemoveInternalPrefixFromRoles()) {
return;
}

if (!(filteredUserClaims.get(ROLES) instanceof String[])) {
return;
}
String[] roles = (String[]) filteredUserClaims.get(ROLES);
if (roles == null) {
return;
}
for (int i = 0; i < roles.length; i++) {
String role = roles[i];
if (UserCoreConstants.INTERNAL_DOMAIN.equalsIgnoreCase(IdentityUtil.extractDomainFromName(role))) {
String domainRemovedRole = UserCoreUtil.removeDomainFromName(role);
roles[i] = domainRemovedRole;
}
}
}

private String getOIDCSubjectClaim(String clientId, String spTenantDomain, String subjectClaim)
throws UserInfoEndpointException {

Expand Down

0 comments on commit cecf1b2

Please sign in to comment.