Skip to content
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.

Sign up for GitHub

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

Improve user info response to return roles without internal domain appended #2674

Merged
merged 2 commits into from
Jan 22, 2025
Merged

Improve user info response to return roles without internal domain appended #2674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d11413
Improve userinfo response to return roles without internal domain app…
sadilchamishka Jan 20, 2025
8559bcf
Add unit test
sadilchamishka Jan 21, 2025
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
12 changes: 12 additions & 0 deletions ...va/org/wso2/carbon/identity/oauth/endpoint/user/impl/UserInfoJSONResponseBuilderTest.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading