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.

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

Support storing multiple mobile numbers and email addresses per user #857

Merged
merged 59 commits into from
Oct 8, 2024

Conversation

PasinduYeshan
Copy link
Contributor

@PasinduYeshan PasinduYeshan commented Sep 10, 2024

Proposed changes in this pull request

$subject.

This feature will be disabled by default for the time being, however with the following config in deployment.toml, the multiple support for email addresses and mobile numbers can be enabled.

[identity_mgt.user_claim_update]
enable_multiple_emails_and_mobile_numbers = true

verifiedEmailAddresses and verifiedMobileNumbers claims can only be updated when Enable user email verification on update and the above config are both enabled.

Multiple email addresses support

Users will be able to store multiple email addresses in newly introduced custom claim http://wso2.org/claims/emailAddresses and multiple verified email addresses in http://wso2.org/claims/verifiedEmailAddresses claim.

Multiple mobile numbers support

Users will be able to store multiple mobile numbers in newly introduced custom claim http://wso2.org/claims/mobileNumbers and multiple verified mobile numbers in http://wso2.org/claims/verifiedMobileNumbers claim.

Changes after Reference PR

1. Handling Primary Email/Mobile Updates When Multi-attribute Support is Disabled

  • Previous Behavior: When multi-attribute support was disabled, changing the primary email or mobile number skipped verification if the new number/email was already in the verified list.
  • Resolution: The verification logic has been refined. Now, if multi-attribute support is disabled, existing verified numbers or emails are not considered, ensuring proper verification steps. This logic has been moved inside the multi-attribute-specific flow.

2. Ensuring Proper Error Handling with Disabled Verification & Enabled Multi-attribute Support

  • Previous Behavior: When verification was disabled but multi-attribute support was enabled, updating the primary mobile number with a new, unlisted number threw an error.
  • Resolution: The system now automatically adds the new mobile number to the mobile numbers list, preventing the error and allowing the update to proceed smoothly.

3. Automatic Addition of Primary Number to Verified Lists with Enabled Multi-attribute & Verification

  • Previous Behavior: The existing primary mobile number was not being added to the mobileNumbers and verifiedMobileNumbers lists during updates.
  • Resolution: The old primary mobile number is now automatically added to both the mobileNumbers and verifiedMobileNumbers lists, ensuring the data remains consistent.

4. Improved Handling of New Primary Number with Enabled Multi-attribute & Verification

  • Previous Behavior: When adding a new number as the primary mobile number, an error was thrown due to the number not being recognized in the verified mobile numbers lists.
  • Resolution: The new primary number is now added to the mobileNumbers and verifiedMobileNumbers lists, without throwing an error ensuring a smoother transition.

5. Streamlining Verification Logic for Pre-verified Mobile Numbers

  • Previous Behavior: When updating a mobile number already in the verified list, the system did not follow the newly introduced SKIP_ON_ALREADY_VERIFIED_EMAIL_ADDRESSES logic but used the SKIP_ON_INAPPLICABLE_CLAIMS rule.
  • Resolution: The verification logic has been moved to the appropriate flow, ensuring that the system properly handles pre-verified numbers based on the correct logic.

6. Add Unit test for following classes.

  • MobileNumberVerificationHandler
  • UserEmailVerificationHandler
  • UserSelfRegistrationManager
  • Utils

Related Git Issue

Referenced PRs

lashinijay and others added 22 commits April 19, 2024 14:49
…ple-mobile-number-support

# Conflicts:
#	components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/IdentityRecoveryConstants.java
#	components/org.wso2.carbon.identity.recovery/src/main/java/org/wso2/carbon/identity/recovery/signup/UserSelfRegistrationManager.java
…/wso2/carbon/identity/recovery/connector/UserClaimUpdateConfigImplTest.java
@PasinduYeshan PasinduYeshan changed the title Add Support storing multiple mobile numbers and email addresses per user Sep 10, 2024
lashinijay
lashinijay previously approved these changes Sep 27, 2024
@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/11128314508

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/11128314508
Status: success

Copy link

@jenkins-is-staging jenkins-is-staging left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/11128314508

sadilchamishka
sadilchamishka previously approved these changes Oct 2, 2024
* This thread local variable is used to stop verification pending mobile number from being set as the primary
* mobile number when only updating the verifiedMobileNumbers claim.
*/
private static ThreadLocal<Boolean> isOnlyVerifiedMobileNumbersUpdated = new ThreadLocal<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have final access modifier as we are not assigning a value to the variable but rather updating states of the thread local (Not mandatory to have)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, the ThreadLocal unsetting logic was added to the PRE_ADD_USER_CLAIM events before the value change.

@jenkins-is-staging
Copy link

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/11228740568

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/11228740568
Status: success

Copy link

@jenkins-is-staging jenkins-is-staging left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/11228740568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants