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

Toggle multi-email and mobile feature based on user store exclusion and support by default config #872

Merged
merged 8 commits into from
Nov 4, 2024
Merged

Toggle multi-email and mobile feature based on user store exclusion and support by default config #872

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cbbd23e
Enable disable feature by checking claim mapping existence.
PasinduYeshan Oct 29, 2024
95942fb
Add user store exclusion claim config property logic
PasinduYeshan Oct 31, 2024
f67cbcb
Fix test related to user store exclusion claim config
PasinduYeshan Oct 31, 2024
7cfce1c
Only update multi emails mobiles claims when they are present in request
PasinduYeshan Nov 1, 2024
e86142e
Add more tests
PasinduYeshan Nov 1, 2024
cea323d
Improve tests
PasinduYeshan Nov 1, 2024
4ee528d
use ClaimConstant
PasinduYeshan Nov 4, 2024
4088369
Update framework version
PasinduYeshan Nov 4, 2024
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
6 changes: 4 additions & 2 deletions .../main/java/org/wso2/carbon/identity/recovery/handler/MobileNumberVerificationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ public void handleEvent(Event event) throws IdentityEventException {
claims = new HashMap<>();
}

boolean supportMultipleMobileNumbers = Utils.isMultiEmailsAndMobileNumbersPerUserEnabled();
boolean supportMultipleMobileNumbers =
Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(user.getTenantDomain(), user.getUserStoreDomain());

boolean enable = isMobileVerificationOnUpdateEnabled(user.getTenantDomain());

Expand Down Expand Up @@ -326,7 +327,8 @@ private void preSetUserClaimOnMobileNumberUpdate(Map<String, String> claims, Use
Utils.unsetThreadLocalToSkipSendingSmsOtpVerificationOnUpdate();
}

boolean supportMultipleMobileNumbers = Utils.isMultiEmailsAndMobileNumbersPerUserEnabled();
boolean supportMultipleMobileNumbers =
Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(user.getTenantDomain(), user.getUserStoreDomain());
String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();

String mobileNumber = claims.get(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM);
Expand Down
6 changes: 4 additions & 2 deletions ...src/main/java/org/wso2/carbon/identity/recovery/handler/UserEmailVerificationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ public void handleEvent(Event event) throws IdentityEventException {
claims = new HashMap<>();
}

boolean supportMultipleEmails = Utils.isMultiEmailsAndMobileNumbersPerUserEnabled();
boolean supportMultipleEmails =
Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(user.getTenantDomain(), user.getUserStoreDomain());

boolean enable = false;

Expand Down Expand Up @@ -568,7 +569,8 @@ private void preSetUserClaimsOnEmailUpdate(Map<String, String> claims, UserStore
Utils.unsetThreadLocalToSkipSendingEmailVerificationOnUpdate();
}

boolean supportMultipleEmails = Utils.isMultiEmailsAndMobileNumbersPerUserEnabled();
boolean supportMultipleEmails =
Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(user.getTenantDomain(), user.getUserStoreDomain());
String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();

String emailAddress = claims.get(IdentityRecoveryConstants.EMAIL_ADDRESS_CLAIM);
Expand Down
6 changes: 4 additions & 2 deletions ...y/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,8 @@ private UserRecoveryData validateSelfRegistrationCode(String code, String verifi
HashMap<String, String> userClaims = getClaimsListToUpdate(user, verifiedChannelType,
externallyVerifiedClaim, recoveryData.getRecoveryScenario().toString());

boolean supportMultipleEmailsAndMobileNumbers = Utils.isMultiEmailsAndMobileNumbersPerUserEnabled();
boolean supportMultipleEmailsAndMobileNumbers =
Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(user.getTenantDomain(), user.getUserStoreDomain());
String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();

if (RecoverySteps.VERIFY_EMAIL.equals(recoveryData.getRecoveryStep())) {
Expand Down Expand Up @@ -990,7 +991,8 @@ public void confirmVerificationCodeMe(String code, Map<String, String> propertie
UserStoreManager userStoreManager = getUserStoreManager(user);
HashMap<String, String> userClaims = new HashMap<>();

boolean supportMultipleEmailsAndMobileNumbers = Utils.isMultiEmailsAndMobileNumbersPerUserEnabled();
boolean supportMultipleEmailsAndMobileNumbers =
Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(user.getTenantDomain(), user.getUserStoreDomain());

String pendingMobileNumberClaimValue = recoveryData.getRemainingSetIds();
if (StringUtils.isNotBlank(pendingMobileNumberClaimValue)) {
Expand Down
70 changes: 66 additions & 4 deletions ....carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@
import org.wso2.carbon.identity.auth.attribute.handler.model.ValidationResult;
import org.wso2.carbon.identity.base.IdentityException;
import org.wso2.carbon.identity.central.log.mgt.utils.LoggerUtils;
import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException;
import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim;
import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.event.IdentityEventConstants;
Expand Down Expand Up @@ -108,6 +111,7 @@

private static final Log AUDIT_LOG = CarbonConstants.AUDIT_LOG;
private static final Log log = LogFactory.getLog(Utils.class);
public static final String EXCLUDED_USER_STORE_DOMAINS_CLAIM_PROPERTY_NAME = "ExcludedUserStoreDomains";

//This is used to pass the arbitrary properties from self user manager to self user handler
private static ThreadLocal<org.wso2.carbon.identity.recovery.model.Property[]> arbitraryProperties = new
Expand Down Expand Up @@ -1393,14 +1397,72 @@
}

/**
* Check whether the supporting multiple email addresses and mobile numbers per user is enabled.
* Check whether the supporting multiple email addresses and mobile numbers per user feature is enabled.
*
* @param tenantDomain Tenant domain.
* @param userStoreDomain User store domain.
* @return True if the config is set to true, false otherwise.
*/
public static boolean isMultiEmailsAndMobileNumbersPerUserEnabled() {
public static boolean isMultiEmailsAndMobileNumbersPerUserEnabled(String tenantDomain, String userStoreDomain) {

return Boolean.parseBoolean(IdentityUtil.getProperty(
IdentityRecoveryConstants.ConnectorConfig.SUPPORT_MULTI_EMAILS_AND_MOBILE_NUMBERS_PER_USER));
if (!Boolean.parseBoolean(IdentityUtil.getProperty(
IdentityRecoveryConstants.ConnectorConfig.SUPPORT_MULTI_EMAILS_AND_MOBILE_NUMBERS_PER_USER))) {
return false;
}

if (StringUtils.isBlank(tenantDomain) || StringUtils.isBlank(userStoreDomain)) {
return false;

Check warning on line 1414 in components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java

View check run for this annotation

Codecov / codecov/patch

components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/util/Utils.java#L1414 

Added line #L1414 was not covered by tests
}

try {
List<LocalClaim> localClaims =
IdentityRecoveryServiceDataHolder.getInstance().getClaimMetadataManagementService()
.getLocalClaims(tenantDomain);

List<String> requiredClaims = Arrays.asList(
IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM,
IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM,
IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM,
IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM);

// Check if all required claims are valid for the user store.
return requiredClaims.stream().allMatch(claimUri ->
isClaimSupportedForUserStore(localClaims, claimUri, userStoreDomain));
} catch (ClaimMetadataException e) {
log.error("Error while retrieving multiple emails and mobiles config.", e);
return false;
}
}

/**
* Check if a claim is supported and not excluded for a specific user store.
*
* @param localClaims List of local claims.
* @param claimUri URI of the claim to check.
* @param userStoreDomain User store domain to validate against.
* @return True if claim is supported and not excluded.
*/
private static boolean isClaimSupportedForUserStore(List<LocalClaim> localClaims, String claimUri,
String userStoreDomain) {

return localClaims.stream()
.filter(claim -> claimUri.equals(claim.getClaimURI()))
.anyMatch(claim -> {
Map<String, String> properties = claim.getClaimProperties();

// Check if claim is supported by default.
boolean isSupported = Boolean.parseBoolean(
properties.getOrDefault(ClaimConstants.SUPPORTED_BY_DEFAULT_PROPERTY,
Boolean.FALSE.toString()));

// Check if user store is not in excluded list.
String excludedUserStoreDomains = properties.get(EXCLUDED_USER_STORE_DOMAINS_CLAIM_PROPERTY_NAME);
boolean isNotExcluded = StringUtils.isBlank(excludedUserStoreDomains) ||
!Arrays.asList(excludedUserStoreDomains.toUpperCase().split(","))
.contains(userStoreDomain.toUpperCase());

return isSupported && isNotExcluded;
});
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ...t/java/org/wso2/carbon/identity/recovery/handler/MobileNumberVerificationHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,8 +575,8 @@ private static Map<String, String> getUserClaimsFromEvent(Event event2) {
private void mockUtilMethods(boolean mobileVerificationEnabled, boolean multiAttributeEnabled,
boolean useVerifyClaimEnabled) {

mockedUtils.when(
Utils::isMultiEmailsAndMobileNumbersPerUserEnabled).thenReturn(multiAttributeEnabled);
mockedUtils.when(() -> Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(anyString(), anyString()))
.thenReturn(multiAttributeEnabled);
mockedUtils.when(Utils::isUseVerifyClaimEnabled).thenReturn(useVerifyClaimEnabled);
mockedUtils.when(() -> Utils.getConnectorConfig(
eq(IdentityRecoveryConstants.ConnectorConfig.ENABLE_MOBILE_NUM_VERIFICATION_ON_UPDATE),
Expand Down
4 changes: 2 additions & 2 deletions ...test/java/org/wso2/carbon/identity/recovery/handler/UserEmailVerificationHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,8 +762,8 @@ private void mockPendingVerificationEmail(String pendingEmail) throws UserStoreE
private void mockUtilMethods(boolean emailVerificationEnabled, boolean multiAttributeEnabled,
boolean userVerifyClaimEnabled, boolean notificationOnEmailUpdate) {

mockedUtils.when(
Utils::isMultiEmailsAndMobileNumbersPerUserEnabled).thenReturn(multiAttributeEnabled);
mockedUtils.when(() -> Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(anyString(), anyString()))
.thenReturn(multiAttributeEnabled);
mockedUtils.when(Utils::isUseVerifyClaimEnabled).thenReturn(userVerifyClaimEnabled);
mockGetConnectorConfig(IdentityRecoveryConstants.ConnectorConfig.ENABLE_EMAIL_VERIFICATION_ON_UPDATE,
emailVerificationEnabled);
Expand Down
57 changes: 47 additions & 10 deletions ...c/test/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@
import org.wso2.carbon.identity.auth.attribute.handler.exception.AuthAttributeHandlerException;
import org.wso2.carbon.identity.auth.attribute.handler.model.ValidationResult;
import org.wso2.carbon.identity.base.IdentityException;
import org.wso2.carbon.identity.claim.metadata.mgt.ClaimMetadataManagementService;
import org.wso2.carbon.identity.claim.metadata.mgt.exception.ClaimMetadataException;
import org.wso2.carbon.identity.claim.metadata.mgt.model.LocalClaim;
import org.wso2.carbon.identity.claim.metadata.mgt.util.ClaimConstants;
import org.wso2.carbon.identity.common.testng.WithCarbonHome;
import org.wso2.carbon.identity.consent.mgt.services.ConsentUtilityService;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
Expand Down Expand Up @@ -97,9 +101,11 @@
import org.wso2.carbon.utils.multitenancy.MultitenantUtils;

import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;

import static org.mockito.ArgumentMatchers.any;
Expand All @@ -111,6 +117,7 @@
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
Expand Down Expand Up @@ -627,7 +634,7 @@ public void testAttributeVerificationFailures(String scenario, Property[] proper

@Test
public void testConfirmVerificationCodeMe()
throws IdentityRecoveryException, UserStoreException {
throws IdentityRecoveryException, UserStoreException, ClaimMetadataException {

// Case 1: Multiple email and mobile per user is enabled.
String verificationPendingMobileNumber = "0700000000";
Expand Down Expand Up @@ -691,7 +698,7 @@ public void testConfirmVerificationCodeMe()

@Test
public void testConfirmVerificationCodeMeVerificationOnVerifiedListUpdate()
throws IdentityRecoveryException, UserStoreException {
throws IdentityRecoveryException, UserStoreException, ClaimMetadataException {

// Case 1: Recovery Scenario - MOBILE_VERIFICATION_ON_VERIFIED_LIST_UPDATE.
String verificationPendingMobileNumber = "0700000000";
Expand Down Expand Up @@ -736,7 +743,7 @@ public void testConfirmVerificationCodeMeVerificationOnVerifiedListUpdate()

@Test(expectedExceptions = IdentityRecoveryServerException.class)
public void testConfirmVerificationCodeMeUserStoreException()
throws IdentityRecoveryException, UserStoreException {
throws IdentityRecoveryException, UserStoreException, ClaimMetadataException {

// Case 3: Throws user store exception while getting user claim values.
String verificationPendingMobileNumber = "0700000000";
Expand All @@ -758,7 +765,7 @@ public void testConfirmVerificationCodeMeUserStoreException()

@Test
public void testGetConfirmedSelfRegisteredUserVerifyEmail()
throws IdentityRecoveryException, UserStoreException, IdentityGovernanceException {
throws IdentityRecoveryException, UserStoreException, IdentityGovernanceException, ClaimMetadataException {

String verifiedChannelType = NotificationChannels.EMAIL_CHANNEL.getChannelType();
String verifiedChannelClaim = "http://wso2.org/claims/emailaddress";
Expand Down Expand Up @@ -859,7 +866,8 @@ public void testGetConfirmedSelfRegisteredUserVerifyMobile()
when(identityGovernanceService.getConfiguration(any(), anyString())).thenReturn(testProperties);

try (MockedStatic<Utils> mockedUtils = mockStatic(Utils.class)) {
mockedUtils.when(Utils::isMultiEmailsAndMobileNumbersPerUserEnabled).thenReturn(true);
mockedUtils.when(() -> Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(anyString(), anyString()))
.thenReturn(true);
mockedUtils.when(() -> Utils.getConnectorConfig(
eq(IdentityRecoveryConstants.ConnectorConfig.ENABLE_MOBILE_VERIFICATION_BY_PRIVILEGED_USER),
anyString()))
Expand Down Expand Up @@ -889,7 +897,8 @@ public void testGetConfirmedSelfRegisteredUserVerifyMobile()
// Case 2: External Verified Channel type.
verifiedChannelType = NotificationChannels.EXTERNAL_CHANNEL.getChannelType();
try (MockedStatic<Utils> mockedUtils = mockStatic(Utils.class)) {
mockedUtils.when(Utils::isMultiEmailsAndMobileNumbersPerUserEnabled).thenReturn(true);
mockedUtils.when(() -> Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(anyString(), anyString()))
.thenReturn(true);
mockedUtils.when(() -> Utils.getConnectorConfig(
eq(IdentityRecoveryConstants.ConnectorConfig.ENABLE_MOBILE_VERIFICATION_BY_PRIVILEGED_USER),
anyString()))
Expand All @@ -911,7 +920,8 @@ public void testGetConfirmedSelfRegisteredUserVerifyMobile()

// Case 3: Throws user store exception while getting user claim values.
try (MockedStatic<Utils> mockedUtils = mockStatic(Utils.class)) {
mockedUtils.when(Utils::isMultiEmailsAndMobileNumbersPerUserEnabled).thenReturn(true);
mockedUtils.when(() -> Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(anyString(), anyString()))
.thenReturn(true);
mockedUtils.when(() -> Utils.getConnectorConfig(
eq(IdentityRecoveryConstants.ConnectorConfig.ENABLE_MOBILE_VERIFICATION_BY_PRIVILEGED_USER),
anyString()))
Expand All @@ -935,7 +945,8 @@ public void testGetConfirmedSelfRegisteredUserVerifyMobile()
verifiedChannelType = NotificationChannels.SMS_CHANNEL.getChannelType();
verifiedChannelClaim = "http://wso2.org/claims/invalid";
try (MockedStatic<Utils> mockedUtils = mockStatic(Utils.class)) {
mockedUtils.when(Utils::isMultiEmailsAndMobileNumbersPerUserEnabled).thenReturn(true);
mockedUtils.when(() -> Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(anyString(), anyString()))
.thenReturn(true);
mockedUtils.when(() -> Utils.getConnectorConfig(
eq(IdentityRecoveryConstants.ConnectorConfig.ENABLE_MOBILE_VERIFICATION_BY_PRIVILEGED_USER),
anyString()))
Expand All @@ -952,7 +963,7 @@ public void testGetConfirmedSelfRegisteredUserVerifyMobile()

@Test
public void testGetConfirmedSelfRegisteredUserConfirmSignUp()
throws IdentityRecoveryException, UserStoreException, IdentityGovernanceException {
throws IdentityRecoveryException, UserStoreException, IdentityGovernanceException, ClaimMetadataException {

String verifiedChannelType = NotificationChannels.EMAIL_CHANNEL.getChannelType();
String verifiedChannelClaim = "http://wso2.org/claims/emailaddress";
Expand Down Expand Up @@ -1562,11 +1573,37 @@ private User getUser() {
return user;
}

private void mockMultiAttributeEnabled(Boolean isEnabled) {
private void mockMultiAttributeEnabled(Boolean isEnabled) throws ClaimMetadataException {

mockedIdentityUtil.when(() -> IdentityUtil.getProperty(
eq(IdentityRecoveryConstants.ConnectorConfig.SUPPORT_MULTI_EMAILS_AND_MOBILE_NUMBERS_PER_USER)))
.thenReturn(isEnabled.toString());
if (!isEnabled) return;
// Mock ClaimMetadataManagementService.
ClaimMetadataManagementService claimMetadataManagementService = mock(ClaimMetadataManagementService.class);
when(identityRecoveryServiceDataHolder.getClaimMetadataManagementService())
.thenReturn(claimMetadataManagementService);

List<LocalClaim> localClaims = new ArrayList<>();
Map<String, String> claimProperties = new HashMap<>();
claimProperties.put(ClaimConstants.SUPPORTED_BY_DEFAULT_PROPERTY, Boolean.TRUE.toString());

LocalClaim mobileNumbersClaim = new LocalClaim(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM);
mobileNumbersClaim.setClaimProperties(claimProperties);
LocalClaim verifiedMobileNumbersClaim = new LocalClaim(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM);
verifiedMobileNumbersClaim.setClaimProperties(claimProperties);
LocalClaim emailAddressesClaim = new LocalClaim(IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM);
emailAddressesClaim.setClaimProperties(claimProperties);
LocalClaim verifiedEmailAddressesClaim =
new LocalClaim(IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM);
verifiedEmailAddressesClaim.setClaimProperties(claimProperties);

localClaims.add(verifiedMobileNumbersClaim);
localClaims.add(mobileNumbersClaim);
localClaims.add(emailAddressesClaim);
localClaims.add(verifiedEmailAddressesClaim);

doReturn(localClaims).when(claimMetadataManagementService).getLocalClaims(TEST_TENANT_DOMAIN_NAME);
}

private void mockGetUserClaimValue(String claimUri, String claimValue) throws UserStoreException {
Expand Down
Loading
Loading