[PM-21179] Add interface to check if user is enrolled in account recovery

[r-tome]

🎟️ Tracking

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

📔 Objective

Centralizes the logic for determining whether an organization user is enrolled in account recovery by introducing two methods on OrganizationUser:

• Static IsValidResetPasswordKey(string?)
• Instance IsEnrolledInAccountRecovery()

This removes scattered string.IsNullOrEmpty / string.IsNullOrWhiteSpace checks across the codebase and replaces them with a single consistent implementation.
It also fixes an inconsistency where some call sites treated whitespace-only keys as valid (IsNullOrEmpty) while others did not (IsNullOrWhiteSpace).

[claude]

Claude finished @r-tome's task in 3m 19s —— View job

---

Pull Request Review: PM-21179 Add interface to check if user is enrolled in account recovery

Summary

This PR centralizes the validation of reset password keys into the OrganizationUser entity, replacing scattered string.IsNullOrEmpty and string.IsNullOrWhiteSpace calls across the codebase with two new methods:

• OrganizationUser.IsValidResetPasswordKey(string?) -- static method for validating reset password keys from any source
• OrganizationUser.IsEnrolledInAccountRecovery() -- instance method for checking enrollment status on an OrganizationUser entity

Assessment

Area	Status
Security	No concerns -- this is a security improvement; whitespace-only keys are now consistently rejected
Correctness	No issues found
Breaking Changes	None
Performance	No impact
Test Coverage	Thorough -- all modified components have corresponding test additions

Key Observations

Behavioral consistency fix: Several call sites previously used string.IsNullOrEmpty (which accepts whitespace-only strings as valid), while others used string.IsNullOrWhiteSpace. The centralized method standardizes on IsNullOrWhiteSpace, which is the correct behavior -- a whitespace-only string should never be considered a valid reset password key. The affected call sites are in OrganizationUserResponseModel, AdminRecoverAccountCommand, OrganizationUserUserDetailsQuery, UserService, and UserDecryptionOptionsBuilder.

Complete migration verified: No remaining instances of the old string.IsNullOrEmpty(*.ResetPasswordKey) or string.IsNullOrWhiteSpace(*.ResetPasswordKey) patterns exist in the source code. All validation checks have been migrated to use the new centralized methods.

Client compatibility properly handled: The OrganizationUserRotationValidator correctly maintains backward compatibility with a known client-side bug (PM-31001) where clients send "" as a reset password key during rotation. The validation only checks for null (not empty/whitespace) on incoming rotation keys, with a clear TODO comment documenting the intended migration path once PM-31001 is resolved on the client side. This addresses the feedback from the existing review thread.

Clean design: The static method appropriately handles validation for DTOs and request models that carry a ResetPasswordKey property but are not OrganizationUser entities, while the instance method provides a domain-expressive way to check enrollment status on actual entity instances.

Incremental Changes Review (post-initial feedback)

The following commits were added after the initial review:

• 716b78f1 -- Addressed @quexten's feedback about the client-side bug (PM-31001). Changed OrganizationUserRotationValidator to check only for null on incoming rotation keys instead of using IsValidResetPasswordKey(), preserving backward compatibility. Added well-documented TODO comments and updated tests accordingly. This change is correct.
• beb9eafd -- Trivial BOM character removal from test file header.
• Two merge commits from main (no conflicts in PR-relevant code).

Verdict

Approve -- This is a well-structured refactoring that improves consistency, fixes a subtle validation gap (whitespace-only keys), and provides comprehensive test coverage. The incremental changes properly address the client compatibility concern raised during review. No issues found.

[codecov]

Codecov Report

❌ Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.75%. Comparing base (9eccb00) to head (8dfe4af).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...Console/Controllers/OrganizationUsersController.cs	50.00%	0 Missing and 1 partial ⚠️
...nse/Organizations/OrganizationUserResponseModel.cs	50.00%	1 Missing ⚠️
...anizationUsers/OrganizationUserUserDetailsQuery.cs	50.00%	0 Missing and 1 partial ⚠️
.../Reports/ReportFeatures/MemberAccessReportQuery.cs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6993      +/-   ##
==========================================
+ Coverage   60.72%   60.75%   +0.03%     
==========================================
  Files        2013     2013              
  Lines       88125    88129       +4     
  Branches     7848     7848              
==========================================
+ Hits        53512    53545      +33     
+ Misses      32712    32679      -33     
- Partials     1901     1905       +4     

☔ 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.

[github-actions]

Checkmarx One – Scan Summary & Details – 8644088c-3e7e-43ff-89d1-3c2b21f4edc3

Great job! No new security vulnerabilities introduced in this pull request

[quexten]

Looks like this may re-introduce a regression to key-rotation, since clients are still sending empty strings. Your approach is the correct one, but is blocked by this client mis-behavior.

[Patrick-Pimentel-Bitwarden]

👍 Auth changes are minimal

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud