From cbda7c38d1dbdc89cc820deade364d2b0cb57d45 Mon Sep 17 00:00:00 2001 From: KaveeshaPiumini Date: Wed, 10 Jul 2024 12:29:39 +0530 Subject: [PATCH 1/4] Fixed NPE in self-registering a user to an invalid user realm through API #20480 --- .../wso2/carbon/identity/recovery/util/Utils.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java index 50ca856066..d2630fffc8 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java @@ -65,6 +65,7 @@ import org.wso2.carbon.user.api.UserStoreManager; import org.wso2.carbon.user.core.UserCoreConstants; import org.wso2.carbon.user.core.UserRealm; +import org.wso2.carbon.user.core.UserStoreClientException; import org.wso2.carbon.user.core.common.AbstractUserStoreManager; import org.wso2.carbon.user.core.constants.UserCoreErrorConstants; import org.wso2.carbon.user.core.service.RealmService; @@ -827,12 +828,19 @@ private static RealmConfiguration getRealmConfiguration(User user) throws Identi try { userStoreManager = IdentityRecoveryServiceDataHolder.getInstance().getRealmService(). getTenantUserRealm(tenantId).getUserStoreManager(); + if (((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getSecondaryUserStoreManager( + user.getUserStoreDomain()) != null) { + return ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getSecondaryUserStoreManager( + user.getUserStoreDomain()).getRealmConfiguration(); + } else { + throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_DOMAIN_VIOLATED, + user.getUserStoreDomain(), + new UserStoreClientException("Invalid Domain Name: " + user.getUserStoreDomain())); + } } catch (UserStoreException userStoreException) { throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_UNEXPECTED, null, userStoreException); } - return ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager) - .getSecondaryUserStoreManager(user.getUserStoreDomain()).getRealmConfiguration(); } /** From 4e1212a4e99e4a908d6f844e322a3ad7f09c1cb5 Mon Sep 17 00:00:00 2001 From: KaveeshaPiumini Date: Thu, 11 Jul 2024 14:17:35 +0530 Subject: [PATCH 2/4] Refactor code --- .../carbon/identity/recovery/util/Utils.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java index d2630fffc8..537964b947 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java @@ -824,19 +824,18 @@ public static void checkPasswordPatternViolation(UserStoreException exception, U private static RealmConfiguration getRealmConfiguration(User user) throws IdentityRecoveryClientException { int tenantId = IdentityTenantUtil.getTenantId(user.getTenantDomain()); - UserStoreManager userStoreManager; try { - userStoreManager = IdentityRecoveryServiceDataHolder.getInstance().getRealmService(). + UserStoreManager userStoreManager = IdentityRecoveryServiceDataHolder.getInstance().getRealmService(). getTenantUserRealm(tenantId).getUserStoreManager(); - if (((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getSecondaryUserStoreManager( - user.getUserStoreDomain()) != null) { - return ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getSecondaryUserStoreManager( - user.getUserStoreDomain()).getRealmConfiguration(); - } else { - throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_DOMAIN_VIOLATED, - user.getUserStoreDomain(), - new UserStoreClientException("Invalid Domain Name: " + user.getUserStoreDomain())); + userStoreManager = ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getSecondaryUserStoreManager( + user.getUserStoreDomain()); + if (userStoreManager != null) { + return ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getRealmConfiguration(); } + throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_DOMAIN_VIOLATED, + user.getUserStoreDomain(), + new UserStoreClientException("Invalid Domain Name: " + user.getUserStoreDomain())); + } catch (UserStoreException userStoreException) { throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_UNEXPECTED, null, userStoreException); From ace5130227d757b1cf1c2c20673dc1058bd125b7 Mon Sep 17 00:00:00 2001 From: KaveeshaPiumini Date: Fri, 12 Jul 2024 14:42:44 +0530 Subject: [PATCH 3/4] Add unit test --- .../carbon/identity/recovery/util/Utils.java | 2 +- .../identity/recovery/util/UtilsTest.java | 125 ++++++++++++++++++ .../src/test/resources/testng.xml | 1 + 3 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java index 537964b947..478111a458 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java @@ -834,7 +834,7 @@ private static RealmConfiguration getRealmConfiguration(User user) throws Identi } throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_DOMAIN_VIOLATED, user.getUserStoreDomain(), - new UserStoreClientException("Invalid Domain Name: " + user.getUserStoreDomain())); + new UserStoreClientException("Invalid domain " + user.getUserStoreDomain() + " provided.")); } catch (UserStoreException userStoreException) { throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_UNEXPECTED, diff --git a/components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java b/components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java new file mode 100644 index 0000000000..0e7905d181 --- /dev/null +++ b/components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java @@ -0,0 +1,125 @@ +/* + * 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.recovery.util; + +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import org.wso2.carbon.identity.application.common.model.User; +import org.wso2.carbon.identity.common.testng.WithCarbonHome; +import org.wso2.carbon.identity.core.util.IdentityTenantUtil; +import org.wso2.carbon.identity.recovery.IdentityRecoveryClientException; +import org.wso2.carbon.identity.recovery.IdentityRecoveryConstants; +import org.wso2.carbon.identity.recovery.internal.IdentityRecoveryServiceDataHolder; +import org.wso2.carbon.user.api.UserStoreException; +import org.wso2.carbon.user.core.UserRealm; +import org.wso2.carbon.user.core.UserStoreManager; +import org.wso2.carbon.user.core.service.RealmService; + +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; + +@WithCarbonHome +public class UtilsTest { + + private static final String TENANT_DOMAIN = "test.com"; + private static final int TENANT_ID = 123; + private static final String USER_NAME = "testUser"; + private static final String USER_STORE_DOMAIN = "TEST"; + @Mock + private UserStoreManager userStoreManager; + @Mock + private UserRealm userRealm; + @Mock + private RealmService realmService; + @Mock + private IdentityRecoveryServiceDataHolder identityRecoveryServiceDataHolder; + + private MockedStatic mockedStaticIdentityTenantUtil; + private MockedStatic mockedStaticUserStoreManager; + private MockedStatic mockedIdentityRecoveryServiceDataHolder; + + @BeforeMethod + public void setUp() { + + MockitoAnnotations.openMocks(this); + } + + @BeforeClass + public void beforeTest() { + + mockedStaticIdentityTenantUtil = mockStatic(IdentityTenantUtil.class); + mockedStaticUserStoreManager = mockStatic(UserStoreManager.class); + mockedIdentityRecoveryServiceDataHolder = Mockito.mockStatic(IdentityRecoveryServiceDataHolder.class); + } + + @AfterClass + public void afterTest() { + + mockedStaticIdentityTenantUtil.close(); + mockedStaticUserStoreManager.close(); + mockedIdentityRecoveryServiceDataHolder.close(); + } + + @DataProvider(name = "CheckPasswordPatternViolation") + public Object[][] CheckPasswordPatternViolation() { + + return new Object[][]{ + {new UserStoreException("Invalid Domain Name"), createUser()} + }; + } + + private static User createUser() { + + User user = new User(); + user.setUserName(USER_NAME); + user.setTenantDomain(TENANT_DOMAIN); + user.setUserStoreDomain(USER_STORE_DOMAIN); + return user; + } + + @Test(dataProvider = "CheckPasswordPatternViolation", expectedExceptions = IdentityRecoveryClientException.class) + public void testCheckPasswordPatternViolation(UserStoreException exception, User user) throws Exception { + + when(IdentityTenantUtil.getTenantId(user.getTenantDomain())).thenReturn(TENANT_ID); + mockedIdentityRecoveryServiceDataHolder.when(IdentityRecoveryServiceDataHolder::getInstance).thenReturn( + identityRecoveryServiceDataHolder); + when(identityRecoveryServiceDataHolder.getRealmService()).thenReturn(realmService); + when(realmService.getTenantUserRealm(TENANT_ID)).thenReturn(userRealm); + when(userRealm.getUserStoreManager()).thenReturn(userStoreManager); + when(userStoreManager.getSecondaryUserStoreManager(USER_STORE_DOMAIN)).thenReturn(null); + + try { + Utils.checkPasswordPatternViolation(exception, user); + } catch (IdentityRecoveryClientException e) { + assertEquals(e.getErrorCode(), + IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_DOMAIN_VIOLATED.getCode()); + assertEquals(e.getMessage(), "Invalid domain " + user.getUserStoreDomain() + " provided."); + throw e; + } + + } +} diff --git a/components/org.wso2.carbon.identity.recovery/src/test/resources/testng.xml b/components/org.wso2.carbon.identity.recovery/src/test/resources/testng.xml index 7664b3acf5..7c8f7107ff 100644 --- a/components/org.wso2.carbon.identity.recovery/src/test/resources/testng.xml +++ b/components/org.wso2.carbon.identity.recovery/src/test/resources/testng.xml @@ -27,6 +27,7 @@ + From efe29a8f70924608d9a8131ef48dbfe9d37ae22d Mon Sep 17 00:00:00 2001 From: KaveeshaPiumini Date: Mon, 22 Jul 2024 11:28:06 +0530 Subject: [PATCH 4/4] Refactor test and improve method --- .../carbon/identity/recovery/util/Utils.java | 12 ++++++--- .../identity/recovery/util/UtilsTest.java | 27 ++++++------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java index 478111a458..bb5598d602 100644 --- a/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java +++ b/components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java @@ -92,6 +92,7 @@ import java.util.regex.Pattern; import static org.wso2.carbon.identity.auth.attribute.handler.AuthAttributeHandlerConstants.ErrorMessages.ERROR_CODE_AUTH_ATTRIBUTE_HANDLER_NOT_FOUND; +import static org.wso2.carbon.identity.core.util.IdentityUtil.getPrimaryDomainName; import static org.wso2.carbon.identity.recovery.IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_REGISTRATION_OPTION; import static org.wso2.carbon.identity.recovery.IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_INVALID_USER_ATTRIBUTES_FOR_REGISTRATION; import static org.wso2.carbon.identity.recovery.IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_UNEXPECTED_ERROR_VALIDATING_ATTRIBUTES; @@ -827,15 +828,18 @@ private static RealmConfiguration getRealmConfiguration(User user) throws Identi try { UserStoreManager userStoreManager = IdentityRecoveryServiceDataHolder.getInstance().getRealmService(). getTenantUserRealm(tenantId).getUserStoreManager(); - userStoreManager = ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getSecondaryUserStoreManager( - user.getUserStoreDomain()); - if (userStoreManager != null) { + if (StringUtils.equals(user.getUserStoreDomain(), getPrimaryDomainName())) { + return ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getRealmConfiguration(); + } + UserStoreManager secondaryUserStoreManager = + ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getSecondaryUserStoreManager( + user.getUserStoreDomain()); + if (secondaryUserStoreManager != null) { return ((org.wso2.carbon.user.core.UserStoreManager) userStoreManager).getRealmConfiguration(); } throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_DOMAIN_VIOLATED, user.getUserStoreDomain(), new UserStoreClientException("Invalid domain " + user.getUserStoreDomain() + " provided.")); - } catch (UserStoreException userStoreException) { throw Utils.handleClientException(IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_UNEXPECTED, null, userStoreException); diff --git a/components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java b/components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java index 0e7905d181..a1e4c25c34 100644 --- a/components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java +++ b/components/org.wso2.carbon.identity.recovery/src/test/java/org/wso2/carbon/identity/recovery/util/UtilsTest.java @@ -25,11 +25,10 @@ import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; -import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import org.wso2.carbon.identity.application.common.model.User; -import org.wso2.carbon.identity.common.testng.WithCarbonHome; import org.wso2.carbon.identity.core.util.IdentityTenantUtil; +import org.wso2.carbon.identity.core.util.IdentityUtil; import org.wso2.carbon.identity.recovery.IdentityRecoveryClientException; import org.wso2.carbon.identity.recovery.IdentityRecoveryConstants; import org.wso2.carbon.identity.recovery.internal.IdentityRecoveryServiceDataHolder; @@ -42,7 +41,6 @@ import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; -@WithCarbonHome public class UtilsTest { private static final String TENANT_DOMAIN = "test.com"; @@ -61,6 +59,7 @@ public class UtilsTest { private MockedStatic mockedStaticIdentityTenantUtil; private MockedStatic mockedStaticUserStoreManager; private MockedStatic mockedIdentityRecoveryServiceDataHolder; + private MockedStatic mockedIdentityUtil; @BeforeMethod public void setUp() { @@ -74,6 +73,7 @@ public void beforeTest() { mockedStaticIdentityTenantUtil = mockStatic(IdentityTenantUtil.class); mockedStaticUserStoreManager = mockStatic(UserStoreManager.class); mockedIdentityRecoveryServiceDataHolder = Mockito.mockStatic(IdentityRecoveryServiceDataHolder.class); + mockedIdentityUtil = mockStatic(IdentityUtil.class); } @AfterClass @@ -82,27 +82,16 @@ public void afterTest() { mockedStaticIdentityTenantUtil.close(); mockedStaticUserStoreManager.close(); mockedIdentityRecoveryServiceDataHolder.close(); + mockedIdentityUtil.close(); } - @DataProvider(name = "CheckPasswordPatternViolation") - public Object[][] CheckPasswordPatternViolation() { - - return new Object[][]{ - {new UserStoreException("Invalid Domain Name"), createUser()} - }; - } - - private static User createUser() { + @Test(expectedExceptions = IdentityRecoveryClientException.class) + public void testCheckPasswordPatternViolationForInvalidDomain() throws Exception { User user = new User(); user.setUserName(USER_NAME); user.setTenantDomain(TENANT_DOMAIN); user.setUserStoreDomain(USER_STORE_DOMAIN); - return user; - } - - @Test(dataProvider = "CheckPasswordPatternViolation", expectedExceptions = IdentityRecoveryClientException.class) - public void testCheckPasswordPatternViolation(UserStoreException exception, User user) throws Exception { when(IdentityTenantUtil.getTenantId(user.getTenantDomain())).thenReturn(TENANT_ID); mockedIdentityRecoveryServiceDataHolder.when(IdentityRecoveryServiceDataHolder::getInstance).thenReturn( @@ -110,16 +99,16 @@ public void testCheckPasswordPatternViolation(UserStoreException exception, User when(identityRecoveryServiceDataHolder.getRealmService()).thenReturn(realmService); when(realmService.getTenantUserRealm(TENANT_ID)).thenReturn(userRealm); when(userRealm.getUserStoreManager()).thenReturn(userStoreManager); + mockedIdentityUtil.when(IdentityUtil::getPrimaryDomainName).thenReturn("PRIMARY"); when(userStoreManager.getSecondaryUserStoreManager(USER_STORE_DOMAIN)).thenReturn(null); try { - Utils.checkPasswordPatternViolation(exception, user); + Utils.checkPasswordPatternViolation(new UserStoreException("Invalid Domain Name"), user); } catch (IdentityRecoveryClientException e) { assertEquals(e.getErrorCode(), IdentityRecoveryConstants.ErrorMessages.ERROR_CODE_DOMAIN_VIOLATED.getCode()); assertEquals(e.getMessage(), "Invalid domain " + user.getUserStoreDomain() + " provided."); throw e; } - } }