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

Jump to bottom

Fix admin role creation issues and returning everyone role for every user in SCIM response #499

Merged
merged 2 commits into from
Oct 28, 2023
Merged

Fix admin role creation issues and returning everyone role for every user in SCIM response #499

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
27f93cf
fix admin role creation issue
AnuradhaSK Oct 27, 2023
076ad9b
bump framework
AnuradhaSK Oct 27, 2023
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
47 changes: 47 additions & 0 deletions ...entity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading