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 usernames recovery when claims are not unique #859

Merged
merged 14 commits into from
Oct 2, 2024

Conversation

KD23243
Copy link
Contributor

@KD23243 KD23243 commented Sep 13, 2024

Proposed changes in this pull request

  • Implement a feature that allows users to recover multiple usernames associated with the same claims during the recovery process.
  • Send multiple account recovery emails with all the usernames linked to the provided email address to help users identify their accounts.
  • Add support for non-unique user recovery by introducing the following configuration in the deployment.toml
[identity_mgt.username_recovery.non_unique_user]
enabled = true

When this configuration is enabled, the system will send multiple recovery notifications with all usernames associated with the identified users.

Follow up actions

  • Update relevant documentation and user guides to explain how the new recovery process works.

Related Git Issue

Related PRs

Checklist

General

  • Is this PR explained thoroughly? All code changes must be accounted for in the PR description.
  • Is the PR labeled correctly?

Functionality

  • Are all requirements met? Compare implemented functionality with the requirements specification.
  • Does the UI work as expected? There should be no Javascript errors in the console; all resources should load. There should be no unexpected errors. Deliberately try to break the feature to find out if there are corner cases that are not handled.

Code

  • Do you fully understand the introduced changes to the code? If not ask for clarification, it might uncover ways to solve a problem in a more elegant and efficient way.

Tests

  • Are there sufficient test cases? Ensure that all components are tested individually; models, forms, and serializers should be tested in isolation even if a test for a view covers these components.
  • If this is a new feature or a significant change to an existing feature? has the manual testing spreadsheet been updated with instructions for manual testing?

Security

  • Confirm this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.
  • Are all UI and API inputs run through forms or serializers?
  • Are all external inputs validated and sanitized appropriately?
  • Does all branching logic have a default case?
  • Does this solution handle outliers and edge cases gracefully?
  • Are all external communications secured and restricted to SSL?

@KD23243 KD23243 marked this pull request as draft September 15, 2024 14:23
@KD23243 KD23243 marked this pull request as ready for review September 18, 2024 10:38
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 72.30769% with 18 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@29524ef). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ernal/service/impl/UserAccountRecoveryManager.java 66.66% 10 Missing and 4 partials ⚠️
...ice/impl/username/UsernameRecoveryManagerImpl.java 82.60% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master     #859   +/-   ##
=========================================
  Coverage          ?   24.21%           
  Complexity        ?      996           
=========================================
  Files             ?      267           
  Lines             ?    15478           
  Branches          ?     2094           
=========================================
  Hits              ?     3748           
  Misses            ?    11256           
  Partials          ?      474           

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

@KD23243
Copy link
Contributor Author

KD23243 commented Oct 1, 2024

The changed functionality has been tested with WSO2IS-7.1.0-m2.

Both flows (with and without the configuration) are working as expected. For unique users, the functionality remains the same, and recovery emails are sent to their registered addresses. When the configuration is enabled, the system also sends multiple recovery notifications for non-unique users. Hence the behaviour matching the expected functionality.

@ashensw ashensw merged commit b914f08 into wso2-extensions:master Oct 2, 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