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

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

Merged

Conversation

PasinduYeshan
Copy link
Contributor

@PasinduYeshan PasinduYeshan commented Oct 29, 2024

Proposed changes in this pull request

  • Toggle multi-email and mobile feature based on user store exclusion and support by default config.
  • Updates claims for multiple emails and mobile numbers only when these specific claims are included in the request. This approach is taken to ensure that updates to primary email and mobile numbers are not blocked by user store limitations when handling newly introduced claims for multiple emails and mobile numbers.

Additional Context: LDAP does not provide default support for the newly introduced claims - emailAddresses, verifiedEmailAddresses, mobileNumbers, and verifiedMobileNumbers. This limitation creates an issue where attempts to update even basic primary email or mobile numbers can be blocked, as the system in some scenarios tries to update these unsupported multiple-value claims. To resolve this critical issue, the system will now only attempt to update these new claims if at least one of them is explicitly present in the update request, ensuring that essential primary email/mobile updates remain functional.

Related Issues

Related PRs

@PasinduYeshan PasinduYeshan marked this pull request as draft October 29, 2024 06:06
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 90.16393% with 6 lines in your changes missing coverage. Please review.

Project coverage is 36.53%. Comparing base (a1eca1e) to head (4088369).
Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
.../org/wso2/carbon/identity/recovery/util/Utils.java 87.50% 1 Missing and 2 partials ⚠️
...overy/handler/MobileNumberVerificationHandler.java 92.30% 0 Missing and 1 partial ⚠️
...recovery/handler/UserEmailVerificationHandler.java 92.85% 0 Missing and 1 partial ⚠️
...y/recovery/signup/UserSelfRegistrationManager.java 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #872      +/-   ##
============================================
- Coverage     36.53%   36.53%   -0.01%     
- Complexity     1475     1485      +10     
============================================
  Files           267      267              
  Lines         15815    15819       +4     
  Branches       2183     2180       -3     
============================================
+ Hits           5778     5779       +1     
- Misses         9321     9326       +5     
+ Partials        716      714       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PasinduYeshan PasinduYeshan changed the title Enable disable feature by checking claim mapping existence. Toggle multi-email and mobile feature based on user store exclusion and support by default config Oct 31, 2024
@PasinduYeshan PasinduYeshan marked this pull request as ready for review October 31, 2024 15:59
@jenkins-is-staging
Copy link

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

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/11617260684
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/11617260684

@jenkins-is-staging
Copy link

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

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/11643547382
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/11643547382

@jenkins-is-staging
Copy link

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

@jenkins-is-staging
Copy link

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/11659583898
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/11659583898

@PasinduYeshan PasinduYeshan merged commit 6f41c3f into wso2-extensions:master Nov 4, 2024
5 checks passed
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.

3 participants