Add review suggestions

wso2-extensions · Oct 24, 2024 · 4f3b2f0 · 4f3b2f0
1 parent 36b13b1
commit 4f3b2f0
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 13 deletions.
diff --git a/...y/organization/discovery/service/listener/OrganizationDiscoveryUserOperationListener.java b/...y/organization/discovery/service/listener/OrganizationDiscoveryUserOperationListener.java
@@ -81,7 +81,8 @@ public boolean doPreAddUserWithID(String userName, Object credential, String[] r
             if (!OrganizationManagementUtil.isOrganization(tenantDomain)) {
                 return true;
             }
-            if (claims.containsKey(CLAIM_MANAGED_ORGANIZATION)) {
+            if (claims.containsKey(CLAIM_MANAGED_ORGANIZATION)
+                    && StringUtils.isNotBlank(claims.get(CLAIM_MANAGED_ORGANIZATION))) {
                 return true;
             }
             String organizationId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getOrganizationId();

diff --git a/...ganization.discovery.service/listener/OrganizationDiscoveryUserOperationListenerTest.java b/...ganization.discovery.service/listener/OrganizationDiscoveryUserOperationListenerTest.java
@@ -21,12 +21,14 @@
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
-import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import org.wso2.carbon.context.PrivilegedCarbonContext;
+import org.wso2.carbon.identity.organization.discovery.service.internal.OrganizationDiscoveryServiceHolder;
 import org.wso2.carbon.identity.organization.discovery.service.util.TestUtils;
 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.internal.OrganizationManagementDataHolder;
 import org.wso2.carbon.user.core.UserStoreException;
 import org.wso2.carbon.user.core.UserStoreManager;
@@ -38,6 +40,7 @@
 import java.util.Map;
 
 import static org.mockito.Mockito.when;
+import static org.testng.AssertJUnit.assertFalse;
 import static org.testng.AssertJUnit.assertTrue;
 import static org.wso2.carbon.identity.organization.management.organization.user.sharing.constant.UserSharingConstants.CLAIM_MANAGED_ORGANIZATION;
 import static org.wso2.carbon.user.core.UserCoreConstants.DEFAULT_PROFILE;
@@ -79,6 +82,7 @@ public void setUp() throws Exception {
         PrivilegedCarbonContext.getThreadLocalCarbonContext().setTenantDomain(TEST_TENANT_DOMAIN);
         OrganizationManagementDataHolder.getInstance().setRealmService(realmService);
         OrganizationManagementDataHolder.getInstance().setOrganizationManager(organizationManager);
+        OrganizationDiscoveryServiceHolder.getInstance().setOrganizationManager(organizationManager);
 
         when(realmService.getTenantManager()).thenReturn(tenantManager);
         when(tenantManager.getTenantId(TEST_TENANT_DOMAIN)).thenReturn(TEST_TENANT_ID);
@@ -87,21 +91,30 @@ public void setUp() throws Exception {
         when(organizationManager.getOrganizationDepthInHierarchy(TEST_ORG_ID)).thenReturn(1);
     }
 
-    @Test
-    public void testSkipEmailDomainValidationForSharedUserCreation() throws UserStoreException {
+    @DataProvider(name = "skipEmailDomainValidationTestDataProvider")
+    public Object[][] claimsProvider() {
 
-        Map<String, String> claims = new HashMap<>();
-        claims.put(CLAIM_MANAGED_ORGANIZATION, TEST_ORG_ID);
+        return new Object[][]{
+                {new HashMap<String, String>(), false},
+                {new HashMap<String, String>() {{ put(CLAIM_MANAGED_ORGANIZATION, ""); }}, false},
+                {new HashMap<String, String>() {{ put(CLAIM_MANAGED_ORGANIZATION, TEST_ORG_ID); }}, true}
+        };
+    }
 
-        boolean result = organizationDiscoveryUserOperationListener.doPreAddUserWithID(TEST_USER, TEST_CREDENTIALS,
-                new String[]{}, claims, DEFAULT_PROFILE, userStoreManager);
+    @Test(dataProvider = "skipEmailDomainValidationTestDataProvider")
+    public void testSkipEmailDomainValidationForSharedUserCreation(Map<String, String> claims, boolean expectedResult)
+            throws UserStoreException, OrganizationManagementException {
 
-        assertTrue("Email domain validation should be skipped for shared user creation.", result);
-    }
+        when(organizationManager.resolveOrganizationId(TEST_TENANT_DOMAIN))
+                .thenThrow(new OrganizationManagementException("Validation not skipped."));
 
-    @AfterMethod
-    public void tearDown() {
+        boolean result = organizationDiscoveryUserOperationListener.doPreAddUserWithID(TEST_USER, TEST_CREDENTIALS,
+                new String[]{}, claims, DEFAULT_PROFILE, userStoreManager);
 
-        PrivilegedCarbonContext.endTenantFlow();
+        if (expectedResult) {
+            assertTrue("Email domain validation should be skipped for shared user creation.", result);
+        } else {
+            assertFalse("Expected false when CLAIM_MANAGED_ORGANIZATION claim is missing.", result);
+        }
     }
 }
diff --git a/...nts/org.wso2.carbon.identity.organization.discovery.service/src/test/resources/testng.xml b/...nts/org.wso2.carbon.identity.organization.discovery.service/src/test/resources/testng.xml
@@ -22,6 +22,7 @@
     <test name="organization-discovery-tests" preserve-order="true" parallel="false">
         <classes>
             <class name="org.wso2.carbon.identity.organization.discovery.service.OrganizationDiscoveryManagerImplTest"/>
+            <class name="org.wso2.carbon.identity.organization.discovery.service.listener.OrganizationDiscoveryUserOperationListenerTest"/>
         </classes>
     </test>
 </suite>