Improve the group role separation logic

components/org.wso2.carbon.identity.scim2.common/pom.xml

@@ -356,4 +356,4 @@
         </plugins>
     </build>
 
-</project>
+</project>

components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/utils/AdminAttributeUtil.java

@@ -1,12 +1,12 @@
 /*
- * Copyright (c) 2017, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+ * Copyright (c) 2017-2024, WSO2 LLC. (http://www.wso2.com).
  *
- * WSO2 Inc. licenses this file to you under the Apache License,
+ * WSO2 LLC. licenses this file to you under the Apache License,
  * Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
@@ -129,8 +129,12 @@ public static void updateAdminGroup(int tenantId) {
                 // UserCore Util functionality does not append primary domain.
                 roleNameWithDomain = SCIMCommonUtils.getGroupNameWithDomain(roleNameWithDomain);
                 try {
-                    //Validate the SCIM IS is avaialble for Groups.
-                    if (!scimGroupHandler.isGroupExisting(roleNameWithDomain)) {
+                    // Validate the SCIM ID is available for Groups.
+                    if (!scimGroupHandler.isGroupExisting(roleNameWithDomain) &&
+                            ((AbstractUserStoreManager) userStoreManager).isRoleAndGroupSeparationEnabled()) {
+                        // Adding the SCIM attributes to internal roles in user core (ex. Internal/admin).
+                        // This admin role is introduced after the role and group separation was introduced.
+                        // These are mapped to roles in SCIM
                         if (log.isDebugEnabled()) {
                             log.debug(
                                     "Group does not exist, setting scim attribute group value: " + roleNameWithDomain);
@@ -143,17 +147,18 @@ public static void updateAdminGroup(int tenantId) {
                     }
 
                     // Adding the SCIM attributes for admin group
-                    if (((AbstractUserStoreManager) userStoreManager).isRoleAndGroupSeparationEnabled()) {
-                        String groupNameWithDomain = getAdminGroupName(adminRoleName, domainName);
-                        // Validate the SCIM ID is available for groups.
-                        if (userStoreManager.isExistingRole(groupNameWithDomain) && !scimGroupHandler
-                                .isGroupExisting(groupNameWithDomain)) {
-                            if (log.isDebugEnabled()) {
-                                log.debug("Group does not exist, setting scim attributes for group: "
-                                        + groupNameWithDomain);
-                            }
-                            scimGroupHandler.addMandatoryAttributes(groupNameWithDomain);
+                    String groupNameWithDomain = getAdminGroupName(adminRoleName, domainName);
+                    // Validate the SCIM ID is available for groups.
+                    if (userStoreManager.isExistingRole(groupNameWithDomain) && !scimGroupHandler
+                            .isGroupExisting(groupNameWithDomain)) {
+                        // Adding the SCIM attributes to userstore roles in user core (ex. PRIMARY/admin).
+                        // This admin role was available before the role and group separation was introduced.
+                        // These are mapped to groups in SCIM
+                        if (log.isDebugEnabled()) {
+                            log.debug("Group does not exist, setting scim attributes for group: "
+                                    + groupNameWithDomain);
                         }
+                        scimGroupHandler.addMandatoryAttributes(groupNameWithDomain);
                     }
                 } catch (IdentitySCIMException e) {
                     throw new UserStoreException(

components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/utils/AdminAttributeUtilTest.java

@@ -1,12 +1,12 @@
 /*
- * Copyright (c) 2017, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+ * Copyright (c) 2017-2024, WSO2 LLC. (http://www.wso2.com).
  *
- * WSO2 Inc. licenses this file to you under the Apache License,
+ * WSO2 LLC. licenses this file to you under the Apache License,
  * Version 2.0 (the "License"); you may not use this file except
  * in compliance with the License.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
@@ -25,32 +25,24 @@
 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;
-import org.wso2.carbon.identity.scim2.common.group.SCIMGroupHandler;
 import org.wso2.carbon.identity.scim2.common.internal.SCIMCommonComponentHolder;
 import org.wso2.carbon.stratos.common.util.ClaimsMgtUtil;
-import org.wso2.carbon.user.api.RealmConfiguration;
 import org.wso2.carbon.user.api.UserRealm;
 import org.wso2.carbon.user.api.UserStoreException;
 import org.wso2.carbon.user.core.UserStoreManager;
 import org.wso2.carbon.user.core.service.RealmService;
 import org.wso2.carbon.user.core.util.UserCoreUtil;
-import org.wso2.charon3.core.schema.SCIMConstants;
 
 import java.util.Map;
 
 import static org.mockito.Matchers.anyInt;
-import static org.mockito.Matchers.anyObject;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.verify;
 import static org.powermock.api.mockito.PowerMockito.mockStatic;
 import static org.powermock.api.mockito.PowerMockito.when;
-import static org.powermock.api.mockito.PowerMockito.whenNew;
-import static org.testng.Assert.assertEquals;
 
 @PrepareForTest({SCIMCommonComponentHolder.class, ClaimsMgtUtil.class, IdentityTenantUtil.class, UserCoreUtil.class,
         IdentityUtil.class, SCIMCommonUtils.class, AdminAttributeUtil.class})
@@ -59,18 +51,12 @@ public class AdminAttributeUtilTest extends PowerMockTestCase {
     @Mock
     RealmService realmService;
 
-    @Mock
-    RealmConfiguration realmConfiguration;
-
     @Mock
     UserRealm userRealm;
 
     @Mock
     UserStoreManager userStoreManager;
 
-    @Mock
-    SCIMGroupHandler scimGroupHandler;
-
     AdminAttributeUtil adminAttributeUtil;
 
     @BeforeMethod
@@ -119,72 +105,4 @@ public void testUpdateAdminUser1() throws Exception {
         adminAttributeUtil.updateAdminUser(1, true);
         verify(userStoreManager.isSCIMEnabled());
     }
-
-    @DataProvider(name = "testUpdateAdminGroupData")
-    public Object[][] testUpdateAdminGroupData() {
-        return new Object[][]{
-                {"testDomain"},
-                {null}
-        };
-    }
-
-    @Test(dataProvider = "testUpdateAdminGroupData")
-    public void testUpdateAdminGroup(String domainName) throws Exception {
-        String roleNameWithDomain = "TESTDOMAIN/admin";
-
-        mockStatic(SCIMCommonComponentHolder.class);
-        mockStatic(ClaimsMgtUtil.class);
-        mockStatic(IdentityTenantUtil.class);
-        mockStatic(UserCoreUtil.class);
-        mockStatic(IdentityUtil.class);
-        mockStatic(SCIMCommonUtils.class);
-        when(SCIMCommonComponentHolder.getRealmService()).thenReturn(realmService);
-        when(realmService.getTenantUserRealm(anyInt())).thenReturn(userRealm);
-        when(userRealm.getUserStoreManager()).thenReturn(userStoreManager);
-        when(userStoreManager.isSCIMEnabled()).thenReturn(true);
-        when(userStoreManager.getTenantId()).thenReturn(1);
-        when(userStoreManager.getRealmConfiguration()).thenReturn(realmConfiguration);
-        when(realmConfiguration.getAdminRoleName()).thenReturn("admin");
-        when(UserCoreUtil.getDomainName((RealmConfiguration) anyObject())).thenReturn(domainName);
-        when(IdentityUtil.getPrimaryDomainName()).thenReturn("TESTDOMAIN");
-        when(UserCoreUtil.addDomainToName(anyString(), anyString())).thenReturn(roleNameWithDomain);
-        when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn(roleNameWithDomain);
-        whenNew(SCIMGroupHandler.class).withAnyArguments().thenReturn(scimGroupHandler);
-
-        ArgumentCaptor<String> argument = ArgumentCaptor.forClass(String.class);
-        CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME = true;
-        adminAttributeUtil.updateAdminGroup(1);
-        verify(scimGroupHandler).addMandatoryAttributes(argument.capture());
-
-        assertEquals(argument.getValue(), roleNameWithDomain);
-    }
-
-    @Test(expectedExceptions = IdentitySCIMException.class)
-    public void testUpdateAdminGroup1() throws Exception {
-        String roleNameWithDomain = "TESTDOMAIN/admin";
-
-        mockStatic(SCIMCommonComponentHolder.class);
-        mockStatic(ClaimsMgtUtil.class);
-        mockStatic(IdentityTenantUtil.class);
-        mockStatic(UserCoreUtil.class);
-        mockStatic(IdentityUtil.class);
-        mockStatic(SCIMCommonUtils.class);
-        when(SCIMCommonComponentHolder.getRealmService()).thenReturn(realmService);
-        when(realmService.getTenantUserRealm(anyInt())).thenReturn(userRealm);
-        when(userRealm.getUserStoreManager()).thenReturn(userStoreManager);
-        when(userStoreManager.isSCIMEnabled()).thenReturn(true);
-        when(userStoreManager.getTenantId()).thenReturn(1);
-        when(userStoreManager.getRealmConfiguration()).thenReturn(realmConfiguration);
-        when(realmConfiguration.getAdminRoleName()).thenReturn("admin");
-        when(UserCoreUtil.getDomainName((RealmConfiguration) anyObject())).thenReturn("testDomain");
-        when(IdentityUtil.getPrimaryDomainName()).thenReturn("TESTDOMAIN");
-        when(UserCoreUtil.addDomainToName(anyString(), anyString())).thenReturn(roleNameWithDomain);
-        when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn(roleNameWithDomain);
-        whenNew(SCIMGroupHandler.class).withAnyArguments().thenReturn(scimGroupHandler);
-        when(scimGroupHandler.isGroupExisting(anyString())).thenThrow(new IdentitySCIMException("testException"));
-
-        adminAttributeUtil.updateAdminGroup(1);
-        verify(scimGroupHandler.isGroupExisting(anyString()));
-    }
-
 }

components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/utils/AdminAttributeUtilTestForGroup.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright (c) 2024, 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
+ * in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.wso2.carbon.identity.scim2.common.utils;
+
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.testng.PowerMockTestCase;
+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;
+import org.wso2.carbon.identity.scim2.common.group.SCIMGroupHandler;
+import org.wso2.carbon.identity.scim2.common.internal.SCIMCommonComponentHolder;
+import org.wso2.carbon.stratos.common.util.ClaimsMgtUtil;
+import org.wso2.carbon.user.api.RealmConfiguration;
+import org.wso2.carbon.user.api.UserRealm;
+import org.wso2.carbon.user.core.common.AbstractUserStoreManager;
+import org.wso2.carbon.user.core.service.RealmService;
+import org.wso2.carbon.user.core.util.UserCoreUtil;
+
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.verify;
+import static org.powermock.api.mockito.PowerMockito.mockStatic;
+import static org.powermock.api.mockito.PowerMockito.when;
+import static org.powermock.api.mockito.PowerMockito.whenNew;
+import static org.testng.Assert.assertEquals;
+
+@PrepareForTest({SCIMCommonComponentHolder.class, ClaimsMgtUtil.class, IdentityTenantUtil.class, UserCoreUtil.class,
+        IdentityUtil.class, SCIMCommonUtils.class, AdminAttributeUtil.class})
+public class AdminAttributeUtilTestForGroup extends PowerMockTestCase {
+
+    @Mock
+    RealmService realmService;
+
+    @Mock
+    RealmConfiguration realmConfiguration;
+
+    @Mock
+    UserRealm userRealm;
+
+    @Mock
+    AbstractUserStoreManager userStoreManager;
+
+    @Mock
+    SCIMGroupHandler scimGroupHandler;
+
+    AdminAttributeUtil adminAttributeUtil;
+
+    @BeforeMethod
+    public void setUp() throws Exception {
+        adminAttributeUtil = new AdminAttributeUtil();
+    }
+
+    @DataProvider(name = "testUpdateAdminUserData")
+    public Object[][] testUpdateAdminUserData() {
+        return new Object[][]{
+                {true},
+                {false}
+        };
+    }
+
+    @DataProvider(name = "testUpdateAdminGroupData")
+    public Object[][] testUpdateAdminGroupData() {
+        return new Object[][]{
+                {"testDomain"},
+                {null}
+        };
+    }
+
+    @Test(dataProvider = "testUpdateAdminGroupData")
+    public void testUpdateAdminGroup(String domainName) throws Exception {
+        String roleNameWithDomain = "TESTDOMAIN/admin";
+
+        mockStatic(SCIMCommonComponentHolder.class);
+        mockStatic(ClaimsMgtUtil.class);
+        mockStatic(IdentityTenantUtil.class);
+        mockStatic(UserCoreUtil.class);
+        mockStatic(IdentityUtil.class);
+        mockStatic(SCIMCommonUtils.class);
+        when(SCIMCommonComponentHolder.getRealmService()).thenReturn(realmService);
+        when(realmService.getTenantUserRealm(anyInt())).thenReturn(userRealm);
+        when(userRealm.getUserStoreManager()).thenReturn(userStoreManager);
+        when(userStoreManager.isSCIMEnabled()).thenReturn(true);
+        when(userStoreManager.getTenantId()).thenReturn(1);
+        when(userStoreManager.getRealmConfiguration()).thenReturn(realmConfiguration);
+        when(userStoreManager.isRoleAndGroupSeparationEnabled()).thenReturn(true);
+        when(realmConfiguration.getAdminRoleName()).thenReturn("admin");
+        when(UserCoreUtil.getDomainName((RealmConfiguration) anyObject())).thenReturn(domainName);
+        when(IdentityUtil.getPrimaryDomainName()).thenReturn("TESTDOMAIN");
+        when(UserCoreUtil.addDomainToName(anyString(), anyString())).thenReturn(roleNameWithDomain);
+        when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn(roleNameWithDomain);
+        whenNew(SCIMGroupHandler.class).withAnyArguments().thenReturn(scimGroupHandler);
+
+        ArgumentCaptor<String> argument = ArgumentCaptor.forClass(String.class);
+        CarbonConstants.ENABLE_LEGACY_AUTHZ_RUNTIME = true;
+        adminAttributeUtil.updateAdminGroup(1);
+        verify(scimGroupHandler).addMandatoryAttributes(argument.capture());
+
+        assertEquals(argument.getValue(), roleNameWithDomain);
+    }
+
+    @Test(expectedExceptions = IdentitySCIMException.class)
+    public void testUpdateAdminGroup1() throws Exception {
+        String roleNameWithDomain = "TESTDOMAIN/admin";
+
+        mockStatic(SCIMCommonComponentHolder.class);
+        mockStatic(ClaimsMgtUtil.class);
+        mockStatic(IdentityTenantUtil.class);
+        mockStatic(UserCoreUtil.class);
+        mockStatic(IdentityUtil.class);
+        mockStatic(SCIMCommonUtils.class);
+        when(SCIMCommonComponentHolder.getRealmService()).thenReturn(realmService);
+        when(realmService.getTenantUserRealm(anyInt())).thenReturn(userRealm);
+        when(userRealm.getUserStoreManager()).thenReturn(userStoreManager);
+        when(userStoreManager.isSCIMEnabled()).thenReturn(true);
+        when(userStoreManager.getTenantId()).thenReturn(1);
+        when(userStoreManager.getRealmConfiguration()).thenReturn(realmConfiguration);
+        when(realmConfiguration.getAdminRoleName()).thenReturn("admin");
+        when(UserCoreUtil.getDomainName((RealmConfiguration) anyObject())).thenReturn("testDomain");
+        when(IdentityUtil.getPrimaryDomainName()).thenReturn("TESTDOMAIN");
+        when(UserCoreUtil.addDomainToName(anyString(), anyString())).thenReturn(roleNameWithDomain);
+        when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn(roleNameWithDomain);
+        whenNew(SCIMGroupHandler.class).withAnyArguments().thenReturn(scimGroupHandler);
+        when(scimGroupHandler.isGroupExisting(anyString())).thenThrow(new IdentitySCIMException("testException"));
+
+        adminAttributeUtil.updateAdminGroup(1);
+        verify(scimGroupHandler.isGroupExisting(anyString()));
+    }
+
+}

components/org.wso2.carbon.identity.scim2.common/src/test/resources/testng.xml

@@ -1,17 +1,19 @@
 <!--
-  ~ Copyright (c) 2017, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+  ~ Copyright (c) 2017-2024, WSO2 LLC. (http://www.wso2.com).
   ~
-  ~ Licensed under the Apache License, Version 2.0 (the "License");
-  ~ you may not use this file except in compliance with the License.
+  ~ WSO2 LLC. licenses this file to you under the Apache License,
+  ~ Version 2.0 (the "License"); you may not use this file except
+  ~ in compliance with the License.
   ~ You may obtain a copy of the License at
   ~
-  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~ http://www.apache.org/licenses/LICENSE-2.0
   ~
-  ~ Unless required by applicable law or agreed to in writing, software
-  ~ distributed under the License is distributed on an "AS IS" BASIS,
-  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-  ~ See the License for the specific language governing permissions and
-  ~ limitations under the License.
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied.  See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
   -->
 <!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
 
@@ -27,6 +29,7 @@
             <class name="org.wso2.carbon.identity.scim2.common.listener.SCIMUserOperationListenerTest"/>
             <class name="org.wso2.carbon.identity.scim2.common.impl.IdentitySCIMManagerTest"/>
             <class name="org.wso2.carbon.identity.scim2.common.utils.AdminAttributeUtilTest"/>
+            <class name="org.wso2.carbon.identity.scim2.common.utils.AdminAttributeUtilTestForGroup"/>
             <class name="org.wso2.carbon.identity.scim2.common.impl.SCIMUserManagerTest"/>
             <class name="org.wso2.carbon.identity.scim2.common.impl.SCIMRoleManagerTest"/>
             <class name="org.wso2.carbon.identity.scim2.common.impl.IdentityResourceURLBuilderTest"/>