Skip to content

Commit

Permalink
Merge pull request #499 from AnuradhaSK/fix-admin-issue
Browse files Browse the repository at this point in the history 
Fix admin role creation issues and returning everyone role for every user in SCIM response
  • Loading branch information
@thanujalk
thanujalk authored Oct 28, 2023
2 parents fefe0e8 + 076ad9b commit 2daf1be
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 22 deletions.
47 changes: 47 additions & 0 deletions ...entity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,53 @@ public void addSCIMGroupAttributes(int tenantId, String roleName, Map<String, St
}
}

public void addSCIMRoleV2Attributes(int tenantId, String roleName, int roleAudienceRefId,
Map<String, String> attributes) throws IdentitySCIMException {

try (Connection connection = IdentityDatabaseUtil.getDBConnection(false);
PreparedStatement prepStmt = connection.prepareStatement(SQLQueries.ADD_ATTRIBUTES_WITH_AUDIENCE_SQL)) {
prepStmt.setInt(1, tenantId);
prepStmt.setString(2, roleName);
prepStmt.setInt(3, roleAudienceRefId);

for (Map.Entry<String, String> entry : attributes.entrySet()) {
if (!isExistingRoleV2Attribute(entry.getKey(), roleName, roleAudienceRefId, tenantId)) {
prepStmt.setString(4, entry.getKey());
prepStmt.setString(5, entry.getValue());
prepStmt.addBatch();
} else {
throw new IdentitySCIMException("Error when adding SCIM Attribute: " + entry.getKey() +
". An attribute with the same name already exists.");
}
}
prepStmt.executeBatch();
} catch (SQLException e) {
throw new IdentitySCIMException("Error when adding SCIM meta data for the role : " + roleName, e);
}
}

private boolean isExistingRoleV2Attribute(String attributeName, String roleName, int audienceRefId, int tenantId)
throws IdentitySCIMException {

try (Connection connection = IdentityDatabaseUtil.getDBConnection(false);
PreparedStatement prepStmt = connection.prepareStatement(
SQLQueries.CHECK_EXISTING_ATTRIBUTE_WITH_AUDIENCE_SQL)) {
prepStmt.setInt(1, tenantId);
prepStmt.setString(2, roleName);
prepStmt.setString(3, attributeName);
prepStmt.setInt(4, audienceRefId);

ResultSet resultSet = prepStmt.executeQuery();
if (resultSet.next()) {
return true;
}
} catch (SQLException e) {
throw new IdentitySCIMException("Error when reading the RoleV2 SCIM meta data from the persistence store.",
e);
}
return false;
}

/**
* Add SCIM attributes to hybrid roles created while SCIM was disabled in the user store.
*
Expand Down
6 changes: 6 additions & 0 deletions ...tity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ public class SQLQueries {
"IDN_SCIM_GROUP.ATTR_VALUE=? AND IDN_SCIM_GROUP.ATTR_NAME=?";
public static final String ADD_ATTRIBUTES_SQL =
"INSERT INTO IDN_SCIM_GROUP (TENANT_ID, ROLE_NAME, ATTR_NAME, ATTR_VALUE) VALUES (?, ?, ?, ?)";
public static final String ADD_ATTRIBUTES_WITH_AUDIENCE_SQL =
"INSERT INTO IDN_SCIM_GROUP (TENANT_ID, ROLE_NAME, AUDIENCE_REF_ID, ATTR_NAME, ATTR_VALUE) VALUES " +
"(?, ?, ?, ?, ?)";
public static final String CHECK_EXISTING_ATTRIBUTE_WITH_AUDIENCE_SQL =
"SELECT TENANT_ID, ROLE_NAME, ATTR_NAME FROM IDN_SCIM_GROUP WHERE IDN_SCIM_GROUP.TENANT_ID=? AND " +
"IDN_SCIM_GROUP.ROLE_NAME=? AND IDN_SCIM_GROUP.ATTR_NAME=? AND IDN_SCIM_GROUP.AUDIENCE_REF_ID=?";
public static final String UPDATE_ATTRIBUTES_SQL =
"UPDATE IDN_SCIM_GROUP SET UM_ATTR_VALUE=? WHERE TENANT_ID=? AND ROLE_NAME=? AND ATTR_NAME=?";
public static final String UPDATE_GROUP_NAME_SQL =
Expand Down
25 changes: 16 additions & 9 deletions ...m2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementException;
import org.wso2.carbon.identity.role.v2.mgt.core.RoleConstants;
import org.wso2.carbon.identity.role.v2.mgt.core.exception.IdentityRoleManagementException;
import org.wso2.carbon.identity.role.v2.mgt.core.util.RoleManagementUtils;
import org.wso2.carbon.identity.scim2.common.DAO.GroupDAO;
import org.wso2.carbon.identity.scim2.common.exceptions.IdentitySCIMException;
import org.wso2.carbon.identity.scim2.common.internal.SCIMCommonComponentHolder;
Expand Down Expand Up @@ -84,35 +85,41 @@ public void addMandatoryAttributes(String groupName)
}

/**
* Add admin role attributes.
* Add roleV2 SCIM metadata.
*
* @param roleName Role name.
* @throws IdentitySCIMException if any error occurs while adding admin role attributes.
*/
public void addAdminRoleMandatoryAttributes(String roleName) throws IdentitySCIMException {
public void addRoleV2MandatoryAttributes(String roleName) throws IdentitySCIMException {

Map<String, String> attributes = new HashMap<>();
String tenantDomain = IdentityTenantUtil.getTenantDomain(tenantId);
String id;
int roleAudienceRefId;
try {
id = SCIMCommonComponentHolder.getRoleManagementServiceV2().getRoleIdByName(
UserCoreUtil.removeDomainFromName(roleName), RoleConstants.ORGANIZATION,
getOrganizationId(tenantDomain), tenantDomain);
String orgId = getOrganizationId(tenantDomain);
id = SCIMCommonComponentHolder.getRoleManagementServiceV2()
.getRoleIdByName(UserCoreUtil.removeDomainFromName(roleName), RoleConstants.ORGANIZATION, orgId,
tenantDomain);
roleAudienceRefId = RoleManagementUtils.resolveAudienceRefId(RoleConstants.ORGANIZATION, orgId);
} catch (IdentityRoleManagementException e) {
throw new IdentitySCIMException("Error while resolving admin role id", e);
throw new IdentitySCIMException("Error while resolving " + roleName + " role id.", e);
}
if (StringUtils.isBlank(id)) {
id = UUID.randomUUID().toString();
throw new IdentitySCIMException("Role : " + roleName + " id not found.");
}
if (roleAudienceRefId == -1) {
throw new IdentitySCIMException("Role : " + roleName + " audience id not found.");
}
attributes.put(SCIMConstants.CommonSchemaConstants.ID_URI, id);

String createdDate = AttributeUtil.formatDateTime(Instant.now());
attributes.put(SCIMConstants.CommonSchemaConstants.CREATED_URI, createdDate);

attributes.put(SCIMConstants.CommonSchemaConstants.LAST_MODIFIED_URI, createdDate);
attributes.put(SCIMConstants.CommonSchemaConstants.LOCATION_URI, SCIMCommonUtils.getSCIMGroupURL(id));
attributes.put(SCIMConstants.CommonSchemaConstants.LOCATION_URI, SCIMCommonUtils.getSCIMRoleV2URL(id));
GroupDAO groupDAO = new GroupDAO();
groupDAO.addSCIMGroupAttributes(tenantId, roleName, attributes);
groupDAO.addSCIMRoleV2Attributes(tenantId, roleName, roleAudienceRefId, attributes);
}

/**
Expand Down
8 changes: 6 additions & 2 deletions ...cim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2986,7 +2986,9 @@ private Set<String> getRoleNamesForGroupsEndpoint(String domainName)
Set<String> scimRoles = groupHandler.listSCIMRoles();
List<String> scimDisabledHybridRoles = getSCIMDisabledHybridRoleList(roleNames, scimRoles);
if (!scimDisabledHybridRoles.isEmpty()) {
createSCIMAttributesForSCIMDisabledHybridRoles(scimDisabledHybridRoles);
if (CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME) {
createSCIMAttributesForSCIMDisabledHybridRoles(scimDisabledHybridRoles);
}
roleNames.addAll(scimDisabledHybridRoles);
}
return roleNames;
Expand All @@ -3008,7 +3010,9 @@ private Set<String> getRoleNamesForGroupsEndpoint(String domainName)
Set<String> scimRoles = groupHandler.listSCIMRoles();
List<String> scimDisabledHybridRoles = getSCIMDisabledHybridRoleList(roleNames, scimRoles);
if (!scimDisabledHybridRoles.isEmpty()) {
createSCIMAttributesForSCIMDisabledHybridRoles(scimDisabledHybridRoles);
if (CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME) {
createSCIMAttributesForSCIMDisabledHybridRoles(scimDisabledHybridRoles);
}
roleNames.addAll(scimDisabledHybridRoles);
}
return roleNames;
Expand Down
2 changes: 1 addition & 1 deletion ...mon/src/main/java/org/wso2/carbon/identity/scim2/common/internal/SCIMCommonComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ protected void activate(ComponentContext ctx) {
//Update super tenant user/group attributes.
AdminAttributeUtil.updateAdminUser(MultitenantConstants.SUPER_TENANT_ID, true);
AdminAttributeUtil.updateAdminGroup(MultitenantConstants.SUPER_TENANT_ID);

SCIMCommonUtils.updateEveryOneRoleV2MetaData(MultitenantConstants.SUPER_TENANT_ID);
if (logger.isDebugEnabled()) {
logger.debug("SCIM Common component activated successfully.");
}
Expand Down
18 changes: 14 additions & 4 deletions ...n/src/main/java/org/wso2/carbon/identity/scim2/common/listener/SCIMTenantMgtListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.identity.core.AbstractIdentityTenantMgtListener;
import org.wso2.carbon.identity.organization.management.service.OrganizationManager;
import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementException;
import org.wso2.carbon.identity.organization.management.service.util.Utils;
import org.wso2.carbon.identity.scim2.common.internal.SCIMCommonComponentHolder;
import org.wso2.carbon.identity.scim2.common.utils.AdminAttributeUtil;
import org.wso2.carbon.identity.scim2.common.utils.SCIMCommonUtils;
import org.wso2.carbon.stratos.common.exception.StratosException;
import org.wso2.carbon.user.api.Tenant;
import org.wso2.carbon.user.api.UserStoreException;
Expand All @@ -50,12 +54,16 @@ public void onTenantInitialActivation(int tenantId) throws StratosException {
try {
Tenant tenant = realmService.getTenantManager().getTenant(tenantId);
/*
If the tenant has an associated organization id, that tenant is created during an organization creation.
Therefore, no tenant admin is created inside the tenant. No need to update such admin claims.
If the tenant has an associated organization id, and if the org id satisfies isOrganization() check, that
organization creator is not inside the same organization. No need to update such admin claims.
*/
String organizationID = tenant.getAssociatedOrganizationUUID();
if (StringUtils.isNotBlank(organizationID)) {
return;
OrganizationManager organizationManager = SCIMCommonComponentHolder.getOrganizationManager();
int organizationDepth = organizationManager.getOrganizationDepthInHierarchy(organizationID);
if (organizationDepth >= Utils.getSubOrgStartLevel()) {
return;
}
}
if (log.isDebugEnabled()) {
log.debug("SCIMTenantMgtListener is fired for Tenant ID : " + tenantId);
Expand All @@ -64,7 +72,9 @@ public void onTenantInitialActivation(int tenantId) throws StratosException {
AdminAttributeUtil.updateAdminUser(tenantId, false);
// Update admin group attributes.
AdminAttributeUtil.updateAdminGroup(tenantId);
} catch (UserStoreException e) {
// Update meta data of everyone role.
SCIMCommonUtils.updateEveryOneRoleV2MetaData(tenantId);
} catch (UserStoreException | OrganizationManagementException e) {
log.error(e);
}
}
Expand Down
14 changes: 13 additions & 1 deletion ...c/main/java/org/wso2/carbon/identity/scim2/common/listener/SCIMUserOperationListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.CarbonConstants;
import org.wso2.carbon.identity.application.authentication.framework.exception.UserSessionException;
import org.wso2.carbon.identity.application.authentication.framework.store.UserSessionStore;
import org.wso2.carbon.identity.application.authentication.framework.util.FrameworkConstants;
Expand Down Expand Up @@ -58,6 +59,7 @@
import static org.wso2.carbon.identity.scim2.common.utils.SCIMCommonConstants.DATE_OF_BIRTH_LOCAL_CLAIM;
import static org.wso2.carbon.identity.scim2.common.utils.SCIMCommonConstants.DATE_OF_BIRTH_REGEX;
import static org.wso2.carbon.identity.scim2.common.utils.SCIMCommonConstants.DOB_REG_EX_VALIDATION_DEFAULT_ERROR;
import static org.wso2.carbon.identity.scim2.common.utils.SCIMCommonConstants.INTERNAL_DOMAIN;
import static org.wso2.carbon.identity.scim2.common.utils.SCIMCommonConstants.MOBILE_LOCAL_CLAIM;
import static org.wso2.carbon.identity.scim2.common.utils.SCIMCommonConstants.MOBILE_REGEX;
import static org.wso2.carbon.identity.scim2.common.utils.SCIMCommonConstants.MOBILE_REGEX_VALIDATION_DEFAULT_ERROR;
Expand Down Expand Up @@ -604,7 +606,17 @@ private boolean postAddRole(String roleName, UserStoreManager userStoreManager)
if (!scimGroupHandler.isGroupExisting(roleNameWithDomain)) {
// If no attributes - i.e: group added via mgt console, not via SCIM endpoint.
// Add META.
scimGroupHandler.addMandatoryAttributes(roleNameWithDomain);
/*
Extracting the domain name here, because resolved domainName is userstore based domains.
If the roleName passed to the method with Internal domain that will be remain as same in
roleNameWithDomain.
*/
if (INTERNAL_DOMAIN.equalsIgnoreCase(UserCoreUtil.extractDomainFromName(roleNameWithDomain)) &&
!CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME) {
scimGroupHandler.addRoleV2MandatoryAttributes(roleNameWithDomain);
} else {
scimGroupHandler.addMandatoryAttributes(roleNameWithDomain);
}
}
} catch (IdentitySCIMException e) {
throw new UserStoreException("Error retrieving group information from SCIM Tables.", e);
Expand Down
7 changes: 6 additions & 1 deletion ....common/src/main/java/org/wso2/carbon/identity/scim2/common/utils/AdminAttributeUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.CarbonConstants;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.scim2.common.exceptions.IdentitySCIMException;
Expand Down Expand Up @@ -134,7 +135,11 @@ public static void updateAdminGroup(int tenantId) {
log.debug(
"Group does not exist, setting scim attribute group value: " + roleNameWithDomain);
}
scimGroupHandler.addAdminRoleMandatoryAttributes(roleNameWithDomain);
if (CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME) {
scimGroupHandler.addMandatoryAttributes(roleNameWithDomain);
} else {
scimGroupHandler.addRoleV2MandatoryAttributes(roleNameWithDomain);
}
}

// Adding the SCIM attributes for admin group
Expand Down
23 changes: 21 additions & 2 deletions ...im2.common/src/main/java/org/wso2/carbon/identity/scim2/common/utils/SCIMCommonUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.CarbonConstants;
import org.wso2.carbon.context.PrivilegedCarbonContext;
import org.wso2.carbon.identity.application.common.model.ThreadLocalProvisioningServiceProvider;
import org.wso2.carbon.identity.application.common.util.IdentityApplicationManagementUtil;
import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataHandler;
import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService;
import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException;
Expand All @@ -37,9 +35,11 @@
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.scim2.common.cache.SCIMCustomAttributeSchemaCache;
import org.wso2.carbon.identity.scim2.common.exceptions.IdentitySCIMException;
import org.wso2.carbon.identity.scim2.common.group.SCIMGroupHandler;
import org.wso2.carbon.identity.scim2.common.internal.SCIMCommonComponentHolder;
import org.wso2.carbon.user.core.UserCoreConstants;
import org.wso2.carbon.user.core.UserStoreException;
import org.wso2.carbon.user.core.UserStoreManager;
import org.wso2.carbon.user.core.util.UserCoreUtil;
import org.wso2.carbon.utils.multitenancy.MultitenantConstants;
import org.wso2.charon3.core.attributes.SCIMCustomAttribute;
Expand Down Expand Up @@ -851,4 +851,23 @@ public static AttributeSchema buildCustomSchema(int tenantId) throws CharonExcep
throw new CharonException("Error while building scim custom schema", e);
}
}

public static void updateEveryOneRoleV2MetaData(int tenantId) {

// Handle everyone role creation also here if legacy runtime is disabled.
if (!CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME) {
try {
UserStoreManager userStoreManager = (UserStoreManager) SCIMCommonComponentHolder.getRealmService().
getTenantUserRealm(tenantId).getUserStoreManager();
String domainName = UserCoreUtil.getDomainName(userStoreManager.getRealmConfiguration());
SCIMGroupHandler scimGroupHandler = new SCIMGroupHandler(userStoreManager.getTenantId());
String everyoneRoleName = userStoreManager.getRealmConfiguration().getEveryOneRoleName();
String everyoneRoleNameWithDomain =
UserCoreUtil.addDomainToName(everyoneRoleName, domainName);
scimGroupHandler.addRoleV2MandatoryAttributes(everyoneRoleNameWithDomain);
} catch (org.wso2.carbon.user.api.UserStoreException | IdentitySCIMException e) {
log.error(e);
}
}
}
}
4 changes: 3 additions & 1 deletion ...mon/src/test/java/org/wso2/carbon/identity/scim2/common/utils/AdminAttributeUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import org.wso2.carbon.CarbonConstants;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.scim2.common.exceptions.IdentitySCIMException;
Expand Down Expand Up @@ -151,8 +152,9 @@ public void testUpdateAdminGroup(String domainName) throws Exception {
whenNew(SCIMGroupHandler.class).withAnyArguments().thenReturn(scimGroupHandler);

ArgumentCaptor<String> argument = ArgumentCaptor.forClass(String.class);
CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME = true;
adminAttributeUtil.updateAdminGroup(1);
verify(scimGroupHandler).addAdminRoleMandatoryAttributes(argument.capture());
verify(scimGroupHandler).addMandatoryAttributes(argument.capture());

assertEquals(argument.getValue(), roleNameWithDomain);
}
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@
<inbound.auth.oauth.version>6.5.3</inbound.auth.oauth.version>
<commons-collections.version>3.2.0.wso2v1</commons-collections.version>
<carbon.kernel.version>4.9.15</carbon.kernel.version>
<identity.framework.version>5.25.419</identity.framework.version>
<identity.framework.version>5.25.462</identity.framework.version>
<junit.version>4.13.1</junit.version>
<commons.lang.version>20030203.000129</commons.lang.version>
<identity.governance.version>1.8.12</identity.governance.version>
Expand Down

0 comments on commit 2daf1be

Please sign in to comment.