Skip to content

Commit

Permalink
Merge pull request #2102 from ThaminduDilshan/thamindu-multi
Browse files Browse the repository at this point in the history 
Fix for Multi attribute separator changes
  • Loading branch information
@ThaminduDilshan
ThaminduDilshan authored Jun 18, 2023
2 parents 42184c3 + 8c3f719 commit 3c0992a
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 22 deletions.
19 changes: 15 additions & 4 deletions ...oint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/token/OAuth2TokenEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.oltu.oauth2.common.message.types.GrantType;
import org.wso2.carbon.context.PrivilegedCarbonContext;
import org.wso2.carbon.identity.central.log.mgt.utils.LoggerUtils;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.oauth.client.authn.filter.OAuthClientAuthenticatorProxy;
import org.wso2.carbon.identity.oauth.common.OAuth2ErrorCodes;
import org.wso2.carbon.identity.oauth.common.OAuthConstants;
Expand Down Expand Up @@ -89,7 +90,10 @@ public Response issueAccessToken(@Context HttpServletRequest request, String pay

Map<String, List<String>> paramMap;
try {
startSuperTenantFlow();
// Start super tenant flow only if tenant qualified URLs are disabled.
if (!IdentityTenantUtil.isTenantQualifiedUrlsEnabled()) {
startSuperTenantFlow();
}
paramMap = parseJsonTokenRequest(payload);
if (LoggerUtils.isDiagnosticLogsEnabled()) {
Map<String, Object> params = new HashMap<>();
Expand All @@ -104,7 +108,9 @@ public Response issueAccessToken(@Context HttpServletRequest request, String pay
triggerOnTokenExceptionListeners(e, request, null);
throw e;
} finally {
PrivilegedCarbonContext.endTenantFlow();
if (!IdentityTenantUtil.isTenantQualifiedUrlsEnabled()) {
PrivilegedCarbonContext.endTenantFlow();
}
}
return issueAccessToken(request, paramMap);
}
Expand Down Expand Up @@ -133,7 +139,10 @@ protected Response issueAccessToken(HttpServletRequest request, Map<String, List
OAuthSystemException, InvalidRequestParentException {

try {
startSuperTenantFlow();
// Start super tenant flow only if tenant qualified URLs are disabled.
if (!IdentityTenantUtil.isTenantQualifiedUrlsEnabled()) {
startSuperTenantFlow();
}
validateRepeatedParams(request, paramMap);
HttpServletRequestWrapper httpRequest = new OAuthRequestWrapper(request, paramMap);
CarbonOAuthTokenRequest oauthRequest = buildCarbonOAuthTokenRequest(httpRequest);
Expand All @@ -157,7 +166,9 @@ protected Response issueAccessToken(HttpServletRequest request, Map<String, List
throw e;

} finally {
PrivilegedCarbonContext.endTenantFlow();
if (!IdentityTenantUtil.isTenantQualifiedUrlsEnabled()) {
PrivilegedCarbonContext.endTenantFlow();
}
}
}

Expand Down
7 changes: 3 additions & 4 deletions ....oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/util/ClaimUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ public class ClaimUtil {

private static final String SP_DIALECT = "http://wso2.org/oidc/claim";
private static final String GROUPS = "groups";
private static final String ATTRIBUTE_SEPARATOR = FrameworkUtils.getMultiAttributeSeparator();
private static final Log log = LogFactory.getLog(ClaimUtil.class);

private ClaimUtil() {
Expand Down Expand Up @@ -381,7 +380,7 @@ private static Map<ClaimMapping, String> getUserAttributesFromCache(OAuth2TokenV
*/
public static boolean isMultiValuedAttribute(String claimValue) {

return StringUtils.contains(claimValue, ATTRIBUTE_SEPARATOR);
return StringUtils.contains(claimValue, FrameworkUtils.getMultiAttributeSeparator());
}

/**
Expand All @@ -398,7 +397,7 @@ public static boolean isMultiValuedAttribute(String claimUri, String claimValue)
if (GROUPS.equals(claimUri)) {
return true;
}
return StringUtils.contains(claimValue, ATTRIBUTE_SEPARATOR);
return StringUtils.contains(claimValue, FrameworkUtils.getMultiAttributeSeparator());
}

/**
Expand All @@ -409,6 +408,6 @@ public static boolean isMultiValuedAttribute(String claimUri, String claimValue)
*/
public static String[] processMultiValuedAttribute(String claimValue) {

return claimValue.split(Pattern.quote(ATTRIBUTE_SEPARATOR));
return claimValue.split(Pattern.quote(FrameworkUtils.getMultiAttributeSeparator()));
}
}
20 changes: 20 additions & 0 deletions ....oauth/src/main/java/org/wso2/carbon/identity/oauth2/internal/OAuth2ServiceComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;
import org.wso2.carbon.context.PrivilegedCarbonContext;
import org.wso2.carbon.identity.application.authentication.framework.ApplicationAuthenticationService;
import org.wso2.carbon.identity.application.authentication.framework.AuthenticationDataPublisher;
import org.wso2.carbon.identity.application.authentication.framework.AuthenticationMethodNameTranslator;
import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants;
Expand Down Expand Up @@ -906,4 +907,23 @@ protected void unsetJWTAccessTokenClaimProvider(JWTAccessTokenClaimProvider clai
}
OAuth2ServiceComponentHolder.getInstance().removeJWTAccessTokenClaimProvider(claimProvider);
}

@Reference(
name = "identity.application.authentication.framework",
service = ApplicationAuthenticationService.class,
cardinality = ReferenceCardinality.MANDATORY,
policy = ReferencePolicy.DYNAMIC,
unbind = "unsetApplicationAuthenticationService"
)
protected void setApplicationAuthenticationService(
ApplicationAuthenticationService applicationAuthenticationService) {
/* reference ApplicationAuthenticationService service to guarantee that this component will wait until
authentication framework is started */
}

protected void unsetApplicationAuthenticationService(
ApplicationAuthenticationService applicationAuthenticationService) {
/* reference ApplicationAuthenticationService service to guarantee that this component will wait until
authentication framework is started */
}
}
4 changes: 2 additions & 2 deletions ...org/wso2/carbon/identity/oauth2/validators/JDBCPermissionBasedInternalScopeValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ public class JDBCPermissionBasedInternalScopeValidator {
private static final String ROOT = "/";
private static final String ADMIN_PERMISSION_ROOT = "/permission/admin";
private static final String EVERYONE_PERMISSION = "everyone_permission";
private static final String ATTRIBUTE_SEPARATOR = FrameworkUtils.getMultiAttributeSeparator();

/**
* Execute Internal scope Validation.
Expand Down Expand Up @@ -377,11 +376,12 @@ private String[] getAllowedPermissionsUsingRoleForNonAssociatedFederatedUsers(
*/
private List<String> getValuesOfGroupsFromUserAttributes(Map<ClaimMapping, String> userAttributes) {

String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();
if (MapUtils.isNotEmpty(userAttributes)) {
for (Map.Entry<ClaimMapping, String> entry : userAttributes.entrySet()) {
if (entry.getKey().getRemoteClaim() != null) {
if (StringUtils.equals(entry.getKey().getRemoteClaim().getClaimUri(), OAuth2Constants.GROUPS)) {
return Arrays.asList(entry.getValue().split(Pattern.quote(ATTRIBUTE_SEPARATOR)));
return Arrays.asList(entry.getValue().split(Pattern.quote(multiAttributeSeparator)));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...ty.oauth/src/main/java/org/wso2/carbon/identity/oauth2/validators/JDBCScopeValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ public class JDBCScopeValidator extends OAuth2ScopeValidator {
"retrieveRolesFromUserStoreForScopeValidation";
private static final String SCOPE_VALIDATOR_NAME = "Role based scope validator";
private static final String OPENID = "openid";
private static final String ATTRIBUTE_SEPARATOR = FrameworkUtils.getMultiAttributeSeparator();
private static final String PRESERVE_CASE_SENSITIVITY = "preservedCaseSensitive";

private static final Log log = LogFactory.getLog(JDBCScopeValidator.class);
Expand Down Expand Up @@ -352,11 +351,12 @@ private String[] getUserRolesForNotAssociatedFederatedUser(AuthenticatedUser use
*/
private List<String> getValuesOfGroupsFromUserAttributes(Map<ClaimMapping, String> userAttributes) {

String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();
if (MapUtils.isNotEmpty(userAttributes)) {
for (Map.Entry<ClaimMapping, String> entry : userAttributes.entrySet()) {
if (entry.getKey().getRemoteClaim() != null) {
if (StringUtils.equals(entry.getKey().getRemoteClaim().getClaimUri(), OAuth2Constants.GROUPS)) {
return Arrays.asList(entry.getValue().split(Pattern.quote(ATTRIBUTE_SEPARATOR)));
return Arrays.asList(entry.getValue().split(Pattern.quote(multiAttributeSeparator)));
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions ...rc/main/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ public class DefaultOIDCClaimsCallbackHandler implements CustomClaimsCallbackHan
private static final Log log = LogFactory.getLog(DefaultOIDCClaimsCallbackHandler.class);
private static final String OAUTH2 = "oauth2";
private static final String OIDC_DIALECT = "http://wso2.org/oidc/claim";
private static final String ATTRIBUTE_SEPARATOR = FrameworkUtils.getMultiAttributeSeparator();

@Override
public JWTClaimsSet handleCustomClaims(JWTClaimsSet.Builder jwtClaimsSetBuilder, OAuthTokenReqMessageContext
Expand Down Expand Up @@ -562,7 +561,8 @@ private Map<String, Object> getUserClaimsInOIDCDialect(String spTenantDomain,
userClaims.size());
}
// Map the local roles to SP defined roles.
handleServiceProviderRoleMappings(serviceProvider, ATTRIBUTE_SEPARATOR, userClaims);
handleServiceProviderRoleMappings(serviceProvider, FrameworkUtils.getMultiAttributeSeparator(),
userClaims);

// Get the user claims in oidc dialect to be returned in the id_token.
Map<String, Object> userClaimsInOIDCDialect = getUserClaimsInOIDCDialect(spTenantDomain, userClaims);
Expand Down Expand Up @@ -784,12 +784,13 @@ private JWTClaimsSet setClaimsToJwtClaimSet(JWTClaimsSet.Builder jwtClaimsSetBui
userClaimsInOIDCDialect) {

JWTClaimsSet jwtClaimsSet = jwtClaimsSetBuilder.build();
String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();
for (Map.Entry<String, Object> claimEntry : userClaimsInOIDCDialect.entrySet()) {
String claimValue = claimEntry.getValue().toString();
String claimKey = claimEntry.getKey();
if (isMultiValuedAttribute(claimKey, claimValue)) {
if (isMultiValuedAttribute(claimKey, claimValue, multiAttributeSeparator)) {
JSONArray claimValues = new JSONArray();
String[] attributeValues = claimValue.split(Pattern.quote(ATTRIBUTE_SEPARATOR));
String[] attributeValues = claimValue.split(Pattern.quote(multiAttributeSeparator));
for (String attributeValue : attributeValues) {
if (StringUtils.isNotBlank(attributeValue)) {
claimValues.add(attributeValue);
Expand Down Expand Up @@ -827,7 +828,7 @@ private boolean isLocalUser(OAuthAuthzReqMessageContext authzReqMessageContext)
return !authzReqMessageContext.getAuthorizationReqDTO().getUser().isFederatedUser();
}

private boolean isMultiValuedAttribute(String claimKey, String claimValue) {
private boolean isMultiValuedAttribute(String claimKey, String claimValue, String multiAttributeSeparator) {

// Address claim contains multi attribute separator but its not a multi valued attribute.
if (claimKey.equals(ADDRESS)) {
Expand All @@ -838,6 +839,6 @@ private boolean isMultiValuedAttribute(String claimKey, String claimValue) {
if (claimKey.equals(GROUPS)) {
return true;
}
return StringUtils.contains(claimValue, ATTRIBUTE_SEPARATOR);
return StringUtils.contains(claimValue, multiAttributeSeparator);
}
}
15 changes: 11 additions & 4 deletions ...est/java/org/wso2/carbon/identity/openidconnect/DefaultOIDCClaimsCallbackHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,6 @@ public void setUp() throws Exception {
connection = dataSource1.getConnection();
connection.createStatement().executeUpdate("RUNSCRIPT FROM '" + getFilePath(H2_SCRIPT_NAME) + "'");

mockStatic(FrameworkUtils.class);
when(FrameworkUtils.getMultiAttributeSeparator()).thenReturn(MULTI_ATTRIBUTE_SEPARATOR_DEFAULT);


RequestObjectService requestObjectService = Mockito.mock(RequestObjectService.class);
List<RequestedClaim> requestedClaims = Collections.emptyList();
when(requestObjectService.getRequestedClaimsForIDToken(anyString())).
Expand All @@ -262,13 +258,18 @@ public void setUp() throws Exception {
OpenIDConnectServiceComponentHolder.setRequestObjectService(requestObjectService);
OAuth2ServiceComponentHolder.getInstance().setScopeClaimMappingDAO(new ScopeClaimMappingDAOImpl());
}

@BeforeMethod
public void setUpBeforeMethod() throws Exception {
PrivilegedCarbonContext.startTenantFlow();
PrivilegedCarbonContext privilegedCarbonContext = PrivilegedCarbonContext.getThreadLocalCarbonContext();
privilegedCarbonContext.setTenantId(MultitenantConstants.SUPER_TENANT_ID);
privilegedCarbonContext.setTenantDomain(MultitenantConstants.SUPER_TENANT_DOMAIN_NAME);

mockStatic(FrameworkUtils.class);
when(FrameworkUtils.getMultiAttributeSeparator()).thenReturn(MULTI_ATTRIBUTE_SEPARATOR_DEFAULT);
}

public static String getFilePath(String fileName) {

if (StringUtils.isNotBlank(fileName)) {
Expand Down Expand Up @@ -316,6 +317,8 @@ public void testHandleCustomClaimsWithOAuthTokenReqMsgCtxtNoSpRequestedClaims()
ServiceProvider serviceProvider = getSpWithDefaultRequestedClaimsMappings();
mockApplicationManagementService(serviceProvider);

when(FrameworkUtils.isContinueOnClaimHandlingErrorAllowed()).thenReturn(true);

JWTClaimsSet jwtClaimsSet = getJwtClaimSet(jwtClaimsSetBuilder, requestMsgCtx);
assertNotNull(jwtClaimsSet);
assertTrue(jwtClaimsSet.getClaims().isEmpty());
Expand Down Expand Up @@ -384,6 +387,8 @@ public void testHandleCustomClaimsWithOAuthTokenReqMsgCtxtUserNotFoundInUserStor
ServiceProvider serviceProvider = getSpWithDefaultRequestedClaimsMappings();
mockApplicationManagementService(serviceProvider);

when(FrameworkUtils.isContinueOnClaimHandlingErrorAllowed()).thenReturn(true);

UserRealm userRealm = getExceptionThrowingUserRealm(new UserStoreException(USER_NOT_FOUND));
mockUserRealm(requestMsgCtx.getAuthorizedUser().toString(), userRealm);

Expand All @@ -401,6 +406,8 @@ public void testHandleCustomClaimsWithOAuthTokenReqMsgCtxtUserStoreException() t
ServiceProvider serviceProvider = getSpWithDefaultRequestedClaimsMappings();
mockApplicationManagementService(serviceProvider);

when(FrameworkUtils.isContinueOnClaimHandlingErrorAllowed()).thenReturn(true);

UserRealm userRealm = getExceptionThrowingUserRealm(new UserStoreException(""));
mockUserRealm(requestMsgCtx.getAuthorizedUser().toString(), userRealm);

Expand Down

0 comments on commit 3c0992a

Please sign in to comment.