Skip to content

[PM-35201] Enhance AdminRecoverAccountValidator to include Accepted status#7579

Merged
JaredScar merged 4 commits intomainfrom
ac/pm-35201-extend-account-recovery-to-accepted-status
May 7, 2026
Merged

[PM-35201] Enhance AdminRecoverAccountValidator to include Accepted status#7579
JaredScar merged 4 commits intomainfrom
ac/pm-35201-extend-account-recovery-to-accepted-status

Conversation

@JaredScar
Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-35201

📔 Objective

Extends the states for password recovery to support the Accepted status

…rganization users

- Updated validation logic to allow organization users with Accepted status to reset their passwords or two-factor authentication.
- Added unit tests to cover scenarios for Accepted users, ensuring correct validation behavior for account recovery requests.
@JaredScar JaredScar requested a review from a team as a code owner May 4, 2026 20:06
@JaredScar JaredScar requested a review from JimmyVo16 May 4, 2026 20:06
@JaredScar JaredScar added the ai-review Request a Claude code review label May 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR extends AdminRecoverAccountValidator (v2) to allow Accepted organization users (in addition to Confirmed and Revoked) to undergo admin-initiated account recovery, provided they are enrolled in account recovery (ResetPasswordKey set, typically via auto-enrollment during invitation acceptance). The change is small, surgical, and parallels the existing Revoked allowance. The inline code comment was updated to reflect the new state set, and the new xUnit tests mirror the established Revoked patterns covering the success paths (master password reset, two-factor reset) and the not-enrolled error path. The recovery flow only updates User.Key (account-level), so absence of OrganizationUser.Key for Accepted members does not affect correctness.

Code Review Details

No findings.

Previous concerns about missing using directives in the test file (NSubstitute, Xunit, Bit.Core.Test.AdminConsole.AutoFixture) have been resolved in subsequent commits and verified locally by the author.

Copy link
Copy Markdown
Contributor

@JimmyVo16 JimmyVo16 left a comment

Choose a reason for hiding this comment

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

Looks good. Just need to fix the tests. It looks like there are some missing references.

… Xunit

- Removed unused AutoFixture import and added NSubstitute and Xunit for improved testing capabilities.
- Prepared the test file for enhanced unit testing of account recovery validation logic.
@JaredScar JaredScar requested a review from JimmyVo16 May 7, 2026 16:38
@JaredScar JaredScar requested a review from JimmyVo16 May 7, 2026 18:11
@JaredScar JaredScar enabled auto-merge (squash) May 7, 2026 18:16
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

@JaredScar JaredScar merged commit dd19dd8 into main May 7, 2026
41 checks passed
@JaredScar JaredScar deleted the ac/pm-35201-extend-account-recovery-to-accepted-status branch May 7, 2026 18:32
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.84%. Comparing base (d25553e) to head (e91e253).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7579   +/-   ##
=======================================
  Coverage   59.83%   59.84%           
=======================================
  Files        2103     2103           
  Lines       92788    92789    +1     
  Branches     8266     8266           
=======================================
+ Hits        55524    55525    +1     
  Misses      35294    35294           
  Partials     1970     1970           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants