Skip to content

Commit

Permalink
Merge pull request #489 from PasinduYeshan/block-role
Browse files Browse the repository at this point in the history
Block role creation for sub organizations.
  • Loading branch information
sadilchamishka authored Oct 20, 2023
2 parents 628a4c7 + 7bb31df commit cf266e8
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 9 deletions.
8 changes: 7 additions & 1 deletion components/org.wso2.carbon.identity.scim2.common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@
<artifactId>org.wso2.carbon.identity.testutil</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.wso2.carbon.identity.organization.management.core</groupId>
<artifactId>org.wso2.carbon.identity.organization.management.service</artifactId>
</dependency>
<dependency>
<groupId>commons-lang</groupId>
<artifactId>commons-lang</artifactId>
Expand Down Expand Up @@ -217,7 +221,9 @@
version="${carbon.identity.framework.imp.pkg.version.range}",
org.wso2.carbon.identity.role.mgt.core.*;
version="${carbon.identity.framework.imp.pkg.version.range}",
org.wso2.carbon.user.mgt.*;version="${carbon.identity.framework.imp.pkg.version.range}"
org.wso2.carbon.user.mgt.*;version="${carbon.identity.framework.imp.pkg.version.range}",
org.wso2.carbon.identity.organization.management.service.*;
version="${org.wso2.carbon.identity.organization.management.core.version.range}",
</Import-Package>
<Export-Package>
!org.wso2.carbon.identity.scim2.common.internal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@

import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.context.PrivilegedCarbonContext;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementException;
import org.wso2.carbon.identity.organization.management.service.util.OrganizationManagementUtil;
import org.wso2.carbon.identity.role.mgt.core.GroupBasicInfo;
import org.wso2.carbon.identity.role.mgt.core.IdentityRoleManagementException;
import org.wso2.carbon.identity.role.mgt.core.RoleBasicInfo;
Expand Down Expand Up @@ -99,6 +100,11 @@ public Role createRole(Role role) throws CharonException, ConflictException, Bad
log.debug("Creating role: " + role.getDisplayName());
}
try {
if (!isRoleModificationAllowedForTenant(tenantDomain)) {
throw new BadRequestException("Role creation is not allowed for organizations.",
ResponseCodeConstants.INVALID_VALUE);
}

// Check if the role already exists.
if (roleManagementService.isExistingRole(role.getId(), tenantDomain)) {
String error = "Role with name: " + role.getDisplayName() + " already exists in the tenantDomain: "
Expand Down Expand Up @@ -951,4 +957,13 @@ private boolean isUsersAttributeRequired(Map<String, Boolean> requiredAttributes
}
return false;
}

private boolean isRoleModificationAllowedForTenant(String tenantDomain) throws CharonException {

try {
return !OrganizationManagementUtil.isOrganization(tenantDomain);
} catch (OrganizationManagementException e) {
throw new CharonException("Error while checking whether the tenant is an organization.", e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementException;
import org.wso2.carbon.identity.organization.management.service.util.OrganizationManagementUtil;
import org.wso2.carbon.identity.role.mgt.core.GroupBasicInfo;
import org.wso2.carbon.identity.role.mgt.core.IdentityRoleManagementClientException;
import org.wso2.carbon.identity.role.mgt.core.IdentityRoleManagementException;
Expand Down Expand Up @@ -85,7 +87,7 @@
/**
* Contains the unit test cases for SCIMRoleManager.
*/
@PrepareForTest({SCIMCommonUtils.class})
@PrepareForTest({SCIMCommonUtils.class, OrganizationManagementUtil.class})
public class SCIMRoleManagerTest extends PowerMockTestCase {

private static final String SAMPLE_TENANT_DOMAIN = "carbon.super";
Expand Down Expand Up @@ -129,6 +131,7 @@ public void setUpClass() {
public void setUpMethod() {

mockStatic(SCIMCommonUtils.class);
mockStatic(OrganizationManagementUtil.class);
when(SCIMCommonUtils.getSCIMRoleURL(nullable(String.class))).thenReturn(DUMMY_SCIM_URL);
when(mockRoleManagementService.getSystemRoles()).thenReturn(SYSTEM_ROLES);
}
Expand All @@ -144,30 +147,35 @@ public Object[][] dataProviderForCreateRoleExistingRole() {

@Test(dataProvider = "dataProviderForCreateRoleExistingRole")
public void testCreateRoleExistingRole(String roleId, String roleDisplayName)
throws IdentityRoleManagementException, BadRequestException, CharonException {
throws IdentityRoleManagementException, BadRequestException, CharonException,
OrganizationManagementException {

Role role = getDummyRole(roleId, roleDisplayName);
when(mockRoleManagementService.isExistingRole(anyString(), anyString())).thenReturn(true);
when(OrganizationManagementUtil.isOrganization(anyString())).thenReturn(false);
SCIMRoleManager scimRoleManager = new SCIMRoleManager(mockRoleManagementService, SAMPLE_TENANT_DOMAIN2);
assertThrows(ConflictException.class, () -> scimRoleManager.createRole(role));
}

@Test
public void testCreateRoleAddRoleExistingRoleName()
throws BadRequestException, CharonException, IdentityRoleManagementException {
throws BadRequestException, CharonException, IdentityRoleManagementException,
OrganizationManagementException {

Role role = getDummyRole(SAMPLE_VALID_ROLE_ID2, SAMPLE_EXISTING_ROLE_NAME);
when(mockRoleManagementService.addRole(anyString(), anyListOf(String.class), anyListOf(String.class),
anyListOf(String.class), anyString())).thenThrow(
new IdentityRoleManagementException(ROLE_ALREADY_EXISTS.getCode(),
"Role already exist for the role name: " + SAMPLE_EXISTING_ROLE_NAME));
when(OrganizationManagementUtil.isOrganization(anyString())).thenReturn(false);
SCIMRoleManager scimRoleManager = new SCIMRoleManager(mockRoleManagementService, SAMPLE_TENANT_DOMAIN);
assertThrows(ConflictException.class, () -> scimRoleManager.createRole(role));
}

@Test
public void testCreateRoleAddRoleInvalidRoleName()
throws BadRequestException, CharonException, IdentityRoleManagementException {
throws BadRequestException, CharonException, IdentityRoleManagementException,
OrganizationManagementException {

Role role = getDummyRole(SAMPLE_VALID_ROLE_ID, SAMPLE_INVALID_ROLE_NAME);

Expand All @@ -177,6 +185,7 @@ public void testCreateRoleAddRoleInvalidRoleName()
String.format("Invalid role name: %s. Role names with the prefix: %s, is not allowed"
+ " to be created from externally in the system.", SAMPLE_INVALID_ROLE_NAME,
UserCoreConstants.INTERNAL_SYSTEM_ROLE_PREFIX)));
when(OrganizationManagementUtil.isOrganization(anyString())).thenReturn(false);
SCIMRoleManager scimRoleManager = new SCIMRoleManager(mockRoleManagementService, SAMPLE_TENANT_DOMAIN);
assertThrows(BadRequestException.class, () -> scimRoleManager.createRole(role));
}
Expand All @@ -193,13 +202,15 @@ public Object[][] dataProviderForCreateRoleUnexpectedServerError() {
@Test(dataProvider = "dataProviderForCreateRoleUnexpectedServerError")
public void testCreateRoleUnexpectedServerError(String roleId, String roleDisplayName, String tenantDomain,
String sError)
throws BadRequestException, CharonException, IdentityRoleManagementException {
throws BadRequestException, CharonException, IdentityRoleManagementException,
OrganizationManagementException {

Role role = getDummyRole(roleId, roleDisplayName);
when(mockRoleManagementService.addRole(anyString(), anyListOf(String.class), anyListOf(String.class),
anyListOf(String.class), anyString())).
thenThrow(unExpectedErrorThrower(tenantDomain, sError,
"Error while creating the role: %s in the tenantDomain: %s", roleDisplayName));
when(OrganizationManagementUtil.isOrganization(anyString())).thenReturn(false);
SCIMRoleManager scimRoleManager = new SCIMRoleManager(mockRoleManagementService, tenantDomain);
assertThrows(CharonException.class, () -> scimRoleManager.createRole(role));
}
Expand All @@ -220,11 +231,13 @@ public Object[][] dataProviderForCreateRolePositive() {

@Test(dataProvider = "dataProviderForCreateRolePositive")
public void testCreateRolePositive(String roleId, String roleDisplayName, String tenantDomain)
throws IdentityRoleManagementException, BadRequestException, CharonException, ConflictException {
throws IdentityRoleManagementException, BadRequestException, CharonException, ConflictException,
OrganizationManagementException {

Role role = getDummyRole(roleId, roleDisplayName);
when(mockRoleManagementService.addRole(nullable(String.class), anyListOf(String.class), anyListOf(String.class),
anyListOf(String.class), anyString())).thenReturn(new RoleBasicInfo(roleId, roleDisplayName));
when(OrganizationManagementUtil.isOrganization(anyString())).thenReturn(false);
SCIMRoleManager scimRoleManager = new SCIMRoleManager(mockRoleManagementService, tenantDomain);
Role createdRole = scimRoleManager.createRole(role);
assertEquals(createdRole.getDisplayName(), roleDisplayName);
Expand Down
10 changes: 9 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,11 @@
<scope>test</scope>
<version>${identity.framework.version}</version>
</dependency>

<dependency>
<groupId>org.wso2.carbon.identity.organization.management.core</groupId>
<artifactId>org.wso2.carbon.identity.organization.management.service</artifactId>
<version>${org.wso2.carbon.identity.organization.management.core.version}</version>
</dependency>
<dependency>
<groupId>org.wso2.carbon.identity.inbound.provisioning.scim2</groupId>
<artifactId>org.wso2.carbon.identity.scim2.common</artifactId>
Expand Down Expand Up @@ -263,6 +267,8 @@
<commons.lang.version>20030203.000129</commons.lang.version>
<identity.governance.version>1.8.12</identity.governance.version>
<charon.version>4.0.10</charon.version>
<org.wso2.carbon.identity.organization.management.core.version>1.0.70
</org.wso2.carbon.identity.organization.management.core.version>

<!--Maven Plugin Version-->
<maven.compiler.plugin.version>2.3.1</maven.compiler.plugin.version>
Expand Down Expand Up @@ -291,6 +297,8 @@
<scim.common.imp.pkg.version.range>[5.0.0, 6.0.0)</scim.common.imp.pkg.version.range>
<carbon.identity.framework.imp.pkg.version.range>[5.20.118, 7.0.0)
</carbon.identity.framework.imp.pkg.version.range>
<org.wso2.carbon.identity.organization.management.core.version.range>[1.0.0, 2.0.0)
</org.wso2.carbon.identity.organization.management.core.version.range>

<org.slf4j.verison>1.7.21</org.slf4j.verison>
<testng.version>6.9.10</testng.version>
Expand Down

0 comments on commit cf266e8

Please sign in to comment.