[PM-32424] Send Access Enumeration protection

[ike-kottlowski]

🎟️ Tracking

PM-32424

Client PR: bitwarden/clients#19422
SDK PR: bitwarden/sdk-internal#821

📔 Objective

Improve enumeration protection of Sends

📸 Screenshots

[github-actions]

Checkmarx One – Scan Summary & Details – 455d4801-076c-458f-b242-11428199c7db

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

[claude]

Claude finished @ike-kottlowski's task in 4m 13s —— View job

---

Code Review: PM-32424 Send Access Enumeration protection

Reviewed commits: 1cbb9c36d through fdc43dc3b (8 commits)
Files reviewed: 5 changed files (+35/-46)

Summary

This PR improves enumeration protection for email-OTP-protected Sends by unifying all error responses to return the same InvalidRequest error with the email_and_otp_required status. Previously, distinct errors were returned for invalid OTP (InvalidGrant / "email otp is invalid.") and OTP generation failure (otp_generation_failed), which could allow an attacker to distinguish between these states and narrow their attack surface.

Changes Assessed

Area	Assessment
Security	The enumeration protection logic is sound. All failure paths in SendEmailOtpRequestValidator (invalid email, OTP sent, invalid OTP, OTP generation failure) now produce identical HTTP response bodies and status codes.
Correctness	The BuildErrorResult default parameter pattern (EmailAndOtpRequired) is clean and correctly routes through the existing switch. The CultureInfo.InvariantCulture addition to string.Format for the token cache key is a good defensive practice. The OTP generation failure now appropriately logs an error instead of leaking a distinct error code.
Breaking Changes	The invalid OTP response changed from InvalidGrant to InvalidRequest with a different error description. This is intentional and coordinated with the linked client PR (bitwarden/clients#19422) and SDK PR.
Test Coverage	Unit tests, integration tests, and snapshot tests are all properly updated to verify the new uniform error behavior across all code paths.

Removed Constants

EmailOtpInvalid ("otp_invalid") and OtpGenerationFailed ("otp_generation_failed") were removed from SendAccessConstants. Verified no remaining references exist in the codebase.

Result

No issues found. This is a well-structured security improvement with good test coverage. Approving.

[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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.87%. Comparing base (fa5fde5) to head (fdc43dc).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7166      +/-   ##
==========================================
- Coverage   56.87%   56.87%   -0.01%     
==========================================
  Files        2028     2028              
  Lines       88827    88820       -7     
  Branches     7917     7918       +1     
==========================================
- Hits        50523    50518       -5     
+ Misses      36472    36471       -1     
+ Partials     1832     1831       -1     

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

[JaredSnider-Bitwarden]

Excellent work!