Skip to content

Commit

Permalink
Only update multi emails mobiles claims when they are present in request
Browse files Browse the repository at this point in the history
  • Loading branch information
@PasinduYeshan
PasinduYeshan committed Nov 1, 2024
1 parent f67cbcb commit 7cfce1c
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 106 deletions.
43 changes: 14 additions & 29 deletions .../main/java/org/wso2/carbon/identity/recovery/handler/MobileNumberVerificationHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,30 +122,6 @@ public void handleEvent(Event event) throws IdentityEventException {
}
claims.remove(IdentityRecoveryConstants.VERIFY_MOBILE_CLAIM);
claims.remove(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM);

if (supportMultipleMobileNumbers) {
if (claims.containsKey(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM) &&
!claims.get(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM).isEmpty()) {
String mobileNumber = claims.get(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM);
List<String> exisitingAllNumbersList = Utils.getMultiValuedClaim(userStoreManager, user,
IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM);
List<String> updatedAllNumbersList =
claims.containsKey(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM)
? getListOfMobileNumbersFromString(
claims.get(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM))
: exisitingAllNumbersList;
if (!updatedAllNumbersList.contains(mobileNumber)) {
updatedAllNumbersList.add(mobileNumber);
claims.put(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM,
String.join(FrameworkUtils.getMultiAttributeSeparator(), updatedAllNumbersList));
}
}
} else {
// Multiple mobile numbers per user support is disabled.
log.debug("Supporting multiple mobile numbers per user is disabled.");
claims.remove(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM);
claims.remove(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM);
}
return;
}

Expand Down Expand Up @@ -329,6 +305,13 @@ private void preSetUserClaimOnMobileNumberUpdate(Map<String, String> claims, Use

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

// Update multiple mobile numbers only if they’re in the claims map.
// This avoids issues with updating the primary mobile number due to user store limitations on multiple
// mobile numbers.
boolean shouldUpdateMultiMobilesRelatedClaims =
claims.containsKey(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM) ||
claims.containsKey(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM);
String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();

String mobileNumber = claims.get(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM);
Expand Down Expand Up @@ -366,10 +349,12 @@ private void preSetUserClaimOnMobileNumberUpdate(Map<String, String> claims, Use
updatedVerifiedNumbersList.removeIf(number -> !updatedAllNumbersList.contains(number));
}

claims.put(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM,
String.join(multiAttributeSeparator, updatedAllNumbersList));
claims.put(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM,
String.join(multiAttributeSeparator, updatedVerifiedNumbersList));
if (shouldUpdateMultiMobilesRelatedClaims) {
claims.put(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM,
String.join(multiAttributeSeparator, updatedAllNumbersList));
claims.put(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM,
String.join(multiAttributeSeparator, updatedVerifiedNumbersList));
}
} else {
updatedAllNumbersList = new ArrayList<>();
claims.remove(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM);
Expand Down Expand Up @@ -412,7 +397,7 @@ private void preSetUserClaimOnMobileNumberUpdate(Map<String, String> claims, Use
.SkipMobileNumberVerificationOnUpdateStates.SKIP_ON_EXISTING_MOBILE_NUM.toString());
invalidatePendingMobileVerification(user, userStoreManager, claims);

if (supportMultipleMobileNumbers) {
if (supportMultipleMobileNumbers && shouldUpdateMultiMobilesRelatedClaims) {
if (!updatedVerifiedNumbersList.contains(existingMobileNumber)) {
updatedVerifiedNumbersList.add(existingMobileNumber);
claims.put(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM,
Expand Down
44 changes: 15 additions & 29 deletions ...src/main/java/org/wso2/carbon/identity/recovery/handler/UserEmailVerificationHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,30 +132,7 @@ public void handleEvent(Event event) throws IdentityEventException {
}
invalidatePendingEmailVerification(user, userStoreManager, claims);
}

if (supportMultipleEmails) {
// Drop the verified email addresses claim as verification on update is not enabled.
claims.remove(IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM);

if (claims.containsKey(IdentityRecoveryConstants.EMAIL_ADDRESS_CLAIM) &&
!claims.get(IdentityRecoveryConstants.EMAIL_ADDRESS_CLAIM).isEmpty()) {

String email = claims.get(IdentityRecoveryConstants.EMAIL_ADDRESS_CLAIM);
List<String> existingAllEmailAddresses = Utils.getMultiValuedClaim(userStoreManager, user,
IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM);

List<String> updatedAllEmailAddresses = claims.containsKey(
IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM)
? getListOfEmailAddressesFromString(
claims.get(IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM))
: existingAllEmailAddresses;
if (!updatedAllEmailAddresses.contains(email)) {
updatedAllEmailAddresses.add(email);
claims.put(IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM,
String.join(FrameworkUtils.getMultiAttributeSeparator(), updatedAllEmailAddresses));
}
}
}
claims.remove(IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM);
claims.remove(IdentityRecoveryConstants.VERIFY_EMAIL_CLIAM);
}
}
Expand Down Expand Up @@ -571,6 +548,13 @@ private void preSetUserClaimsOnEmailUpdate(Map<String, String> claims, UserStore

boolean supportMultipleEmails =
Utils.isMultiEmailsAndMobileNumbersPerUserEnabled(user.getTenantDomain(), user.getUserStoreDomain());
// Update multiple email address related claims only if they’re in the claims map.
// This avoids issues with updating the primary email address due to user store limitations on multiple
// email addresses.
boolean shouldUpdateMultiMobilesRelatedClaims =
claims.containsKey(IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM) ||
claims.containsKey(IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM);

String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();

String emailAddress = claims.get(IdentityRecoveryConstants.EMAIL_ADDRESS_CLAIM);
Expand Down Expand Up @@ -613,10 +597,12 @@ private void preSetUserClaimsOnEmailUpdate(Map<String, String> claims, UserStore
final List<String> tempUpdatedAllEmailAddresses = new ArrayList<>(updatedAllEmailAddresses);
updatedVerifiedEmailAddresses.removeIf(number -> !tempUpdatedAllEmailAddresses.contains(number));

claims.put(IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM,
StringUtils.join(updatedVerifiedEmailAddresses, multiAttributeSeparator));
claims.put(IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM,
StringUtils.join(updatedAllEmailAddresses, multiAttributeSeparator));
if (shouldUpdateMultiMobilesRelatedClaims) {
claims.put(IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM,
StringUtils.join(updatedVerifiedEmailAddresses, multiAttributeSeparator));
claims.put(IdentityRecoveryConstants.EMAIL_ADDRESSES_CLAIM,
StringUtils.join(updatedAllEmailAddresses, multiAttributeSeparator));
}
} else {
/*
email addresses and verified email addresses should not be updated when support for multiple email
Expand Down Expand Up @@ -657,7 +643,7 @@ private void preSetUserClaimsOnEmailUpdate(Map<String, String> claims, UserStore
.SkipEmailVerificationOnUpdateStates.SKIP_ON_EXISTING_EMAIL.toString());
invalidatePendingEmailVerification(user, userStoreManager, claims);

if (supportMultipleEmails) {
if (supportMultipleEmails && shouldUpdateMultiMobilesRelatedClaims) {
if (!updatedVerifiedEmailAddresses.contains(existingEmail)) {
updatedVerifiedEmailAddresses.add(existingEmail);
claims.put(IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM,
Expand Down
23 changes: 11 additions & 12 deletions ...y/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,10 @@ private UserRecoveryData validateSelfRegistrationCode(String code, String verifi
if (StringUtils.isNotBlank(pendingEmailClaimValue)) {
eventProperties.put(IdentityEventConstants.EventProperty.VERIFIED_EMAIL, pendingEmailClaimValue);
userClaims.put(IdentityRecoveryConstants.EMAIL_ADDRESS_PENDING_VALUE_CLAIM, StringUtils.EMPTY);
if (supportMultipleEmailsAndMobileNumbers) {
// Only update verified email addresses claim if the recovery scenario is
// EMAIL_VERIFICATION_ON_VERIFIED_LIST_UPDATE.
if (RecoveryScenarios.EMAIL_VERIFICATION_ON_VERIFIED_LIST_UPDATE.equals(
recoveryData.getRecoveryScenario()) && supportMultipleEmailsAndMobileNumbers) {
try {
List<String> verifiedEmails = Utils.getMultiValuedClaim(userStoreManager, user,
IdentityRecoveryConstants.VERIFIED_EMAIL_ADDRESSES_CLAIM);
Expand All @@ -797,9 +800,7 @@ private UserRecoveryData validateSelfRegistrationCode(String code, String verifi
throw new IdentityRecoveryServerException("Error occurred while obtaining existing claim " +
"value for the user : " + user.getUserName(), e);
}
}
if (!RecoveryScenarios.EMAIL_VERIFICATION_ON_VERIFIED_LIST_UPDATE
.equals(recoveryData.getRecoveryScenario())) {
} else {
userClaims.put(IdentityRecoveryConstants.EMAIL_ADDRESS_CLAIM, pendingEmailClaimValue);
}
// Todo passes when email address is properly set here.
Expand All @@ -810,7 +811,8 @@ private UserRecoveryData validateSelfRegistrationCode(String code, String verifi
if (RecoverySteps.VERIFY_MOBILE_NUMBER.equals(recoveryData.getRecoveryStep())) {
String pendingMobileClaimValue = recoveryData.getRemainingSetIds();
if (StringUtils.isNotBlank(pendingMobileClaimValue)) {
if (supportMultipleEmailsAndMobileNumbers) {
if (RecoveryScenarios.MOBILE_VERIFICATION_ON_VERIFIED_LIST_UPDATE.equals(
recoveryData.getRecoveryScenario()) && supportMultipleEmailsAndMobileNumbers) {
try {
List<String> existingVerifiedMobileNumbersList = Utils.getMultiValuedClaim(userStoreManager,
user, IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM);
Expand All @@ -836,9 +838,7 @@ private UserRecoveryData validateSelfRegistrationCode(String code, String verifi
throw new IdentityRecoveryServerException("Error occurred while obtaining existing claim " +
"value for the user : " + user.getUserName(), e);
}
}
if (!RecoveryScenarios.MOBILE_VERIFICATION_ON_VERIFIED_LIST_UPDATE
.equals(recoveryData.getRecoveryScenario())) {
} else {
userClaims.put(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM, pendingMobileClaimValue);
}
userClaims.put(IdentityRecoveryConstants.MOBILE_NUMBER_PENDING_VALUE_CLAIM, StringUtils.EMPTY);
Expand Down Expand Up @@ -1000,7 +1000,8 @@ public void confirmVerificationCodeMe(String code, Map<String, String> propertie
Verifying whether user is trying to add a mobile number to http://wso2.org/claims/verifedMobileNumbers
claim.
*/
if (supportMultipleEmailsAndMobileNumbers) {
if (RecoveryScenarios.MOBILE_VERIFICATION_ON_VERIFIED_LIST_UPDATE.equals(
recoveryData.getRecoveryScenario()) && supportMultipleEmailsAndMobileNumbers) {
try {
String multiAttributeSeparator = FrameworkUtils.getMultiAttributeSeparator();
List<String> existingVerifiedMobileNumbersList = Utils.getMultiValuedClaim(userStoreManager,
Expand All @@ -1027,9 +1028,7 @@ public void confirmVerificationCodeMe(String code, Map<String, String> propertie
throw new IdentityRecoveryServerException("Error occurred while obtaining existing claim " +
"value for the user : " + user.getUserName(), e);
}
}
if (!RecoveryScenarios.MOBILE_VERIFICATION_ON_VERIFIED_LIST_UPDATE
.equals(recoveryData.getRecoveryScenario())) {
} else {
userClaims.put(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM, pendingMobileNumberClaimValue);
}
userClaims.put(NotificationChannels.SMS_CHANNEL.getVerifiedClaimUrl(), Boolean.TRUE.toString());
Expand Down
44 changes: 21 additions & 23 deletions ...t/java/org/wso2/carbon/identity/recovery/handler/MobileNumberVerificationHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,40 +211,23 @@ public void testHandleEventVerificationDisabledMultiAttributeEnabled()
List<String> allMobileNumbers = new ArrayList<>(Arrays.asList(EXISTING_NUMBER_1, EXISTING_NUMBER_2));
mockExistingNumbersList(allMobileNumbers);

// Expectation: New mobile number should be added to the mobile numbers claim.
// Expectation: New mobile number should be added to the mobile number claim.
mobileNumberVerificationHandler.handleEvent(event);
Map<String, String> userClaims = getUserClaimsFromEvent(event);
Assert.assertTrue(StringUtils.contains(
userClaims.get(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM), NEW_MOBILE_NUMBER));
userClaims.get(IdentityRecoveryConstants.MOBILE_NUMBER_CLAIM), NEW_MOBILE_NUMBER));

// Case 2: Send mobile numbers claim with mobile number claim.
// Case 2: Updated verified mobile numbers list.
Event event2 = createEvent(IdentityEventConstants.Event.PRE_SET_USER_CLAIM, null,
null, EXISTING_NUMBER_1, NEW_MOBILE_NUMBER);
mockVerificationPendingMobileNumber();
mockUtilMethods(false, true, false);

// New primary mobile number is not included in existing all mobile numbers list.
mockExistingNumbersList(null);

// Expectation: New mobile number should be added to the mobile numbers claim.
mobileNumberVerificationHandler.handleEvent(event2);
Map<String, String> userClaims2 = getUserClaimsFromEvent(event2);
Assert.assertTrue(StringUtils.contains(
userClaims2.get(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM), NEW_MOBILE_NUMBER));
Assert.assertTrue(StringUtils.contains(
userClaims2.get(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM), EXISTING_NUMBER_1));

// Case 3: Updated verified mobile numbers list.
Event event3 = createEvent(IdentityEventConstants.Event.PRE_SET_USER_CLAIM, null,
NEW_MOBILE_NUMBER, null, null);
mockVerificationPendingMobileNumber();

// Expectation: Verified mobile number claim should be removed from user claims.
mobileNumberVerificationHandler.handleEvent(event3);
verify(userRecoveryDataStore, times(3)).invalidate(any(),
mobileNumberVerificationHandler.handleEvent(event2);
verify(userRecoveryDataStore, times(2)).invalidate(any(),
eq(RecoveryScenarios.MOBILE_VERIFICATION_ON_UPDATE),
eq(RecoverySteps.VERIFY_MOBILE_NUMBER));
Map<String, String> userClaims3 = getUserClaimsFromEvent(event3);
Map<String, String> userClaims3 = getUserClaimsFromEvent(event2);
Assert.assertFalse(userClaims3.containsKey(IdentityRecoveryConstants.VERIFIED_MOBILE_NUMBERS_CLAIM));
}

Expand Down Expand Up @@ -383,6 +366,21 @@ public void testUpdatePrimaryMobileNotInVerifiedList() throws Exception {
Map<String, String> userClaims = getUserClaimsFromEvent(event);
Assert.assertEquals(userClaims.get(IdentityRecoveryConstants.MOBILE_NUMBER_PENDING_VALUE_CLAIM),
NEW_MOBILE_NUMBER);
// Multiple mobile numbers related claims should only be updated when they are present in the request.
Assert.assertFalse(userClaims.containsKey(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM));

// Case 2: Send mobileNumbers claim in the request.
Event event2 = createEvent(IdentityEventConstants.Event.PRE_SET_USER_CLAIMS, null,
null, EXISTING_NUMBER_1, NEW_MOBILE_NUMBER);
mockExistingVerifiedNumbersList(new ArrayList<>(Arrays.asList(EXISTING_NUMBER_1)));
mockExistingNumbersList(new ArrayList<>(Arrays.asList(EXISTING_NUMBER_1)));
mockVerificationPendingMobileNumber();

mobileNumberVerificationHandler.handleEvent(event2);
Map<String, String> userClaims2 = getUserClaimsFromEvent(event2);
Assert.assertEquals(userClaims2.get(IdentityRecoveryConstants.MOBILE_NUMBER_PENDING_VALUE_CLAIM),
NEW_MOBILE_NUMBER);
Assert.assertTrue(userClaims2.containsKey(IdentityRecoveryConstants.MOBILE_NUMBERS_CLAIM));
}

@Test(description = "Verification enabled, Multi-attribute enabled, Update primary mobile in verified list")
Expand Down
Loading

0 comments on commit 7cfce1c

Please sign in to comment.