[PM-9826] Remove validation from 2fa GET and mask sensitive data

[jlf0dev]

🎟️ Tracking

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

📔 Objective

Since we can't re-use TOTPs anymore, updating 2fa on accounts without a MP is currently broken. This is because we're validating the TOTP multiple times in the process (once on the GET, and once on the POST).

This removes the validation from 2fa getters, since this information mostly isn't sensitive. For the information that IS sensitive (Duo Client Secret), it masks the value before returning to user.

The TOTP is then only validated when updating the 2fa information.

Note: I also removed the validation from /get-webauthn-challenge and /send-email endpoints since they would invalidate the TOTP too early.

📸 Screenshots

See Jira for video

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[github-actions]

Checkmarx One – Scan Summary & Details – f5cb0377-c044-4db4-9a70-1519bc49c3e1

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	Privacy_Violation	/src/Api/Auth/Controllers/TwoFactorController.cs: 444	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/WebAuthnController.cs: 178	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 828	Attack Vector
	Privacy_Violation	/src/Api/Vault/Controllers/CiphersController.cs: 961	Attack Vector
	Privacy_Violation	/src/Api/Controllers/DevicesController.cs: 129	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 411	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 548	Attack Vector
	Privacy_Violation	/src/Api/Auth/Controllers/AccountsController.cs: 846	Attack Vector
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 260	Attack Vector
	Privacy_Violation	/src/Api/Controllers/DevicesController.cs: 155	Attack Vector
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 376	Attack Vector
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 429	Attack Vector
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 63	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 140	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 132	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 215	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 125	Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
	Log_Forging	/src/Api/Auth/Controllers/WebAuthnController.cs: 85	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/WebAuthnController.cs: 153	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/WebAuthnController.cs: 68	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 820	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 838	Attack Vector
	Log_Forging	/src/Api/Vault/Controllers/CiphersController.cs: 953	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 245	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 540	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 350	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 297	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 289	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 280	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 263	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 254	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/AccountsController.cs: 403	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 402	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 94	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 104	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 122	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 131	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 148	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 157	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 188	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 206	Attack Vector
	Log_Forging	/src/Api/Controllers/DevicesController.cs: 120	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 380	Attack Vector
	Log_Forging	/src/Api/Auth/Controllers/TwoFactorController.cs: 369	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 240	Attack Vector
	Log_Forging	/src/Api/Controllers/DevicesController.cs: 146	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 341	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 404	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	CSRF	/src/Billing/Controllers/StripeController.cs: 130
	CSRF	/src/Billing/Controllers/StripeController.cs: 117
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 123
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 138
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 213
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 61
	Unpinned Actions Full Length Commit SHA	/version-bump.yml: 130

[codecov]

Codecov Report

Attention: Patch coverage is 39.13043% with 14 lines in your changes missing coverage. Please review.

Project coverage is 41.58%. Comparing base (88d5a97) to head (aabbfce).
Report is 8 commits behind head on main.

Files	Patch %	Lines
src/Api/Auth/Controllers/TwoFactorController.cs	0.00%	9 Missing ⚠️
src/Core/Services/Implementations/UserService.cs	0.00%	3 Missing and 1 partial ⚠️
...ls/Response/TwoFactor/TwoFactorDuoResponseModel.cs	90.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4526      +/-   ##
==========================================
+ Coverage   41.57%   41.58%   +0.01%     
==========================================
  Files        1268     1268              
  Lines       60091    60110      +19     
  Branches     5509     5514       +5     
==========================================
+ Hits        24981    24999      +18     
+ Misses      33950    33948       -2     
- Partials     1160     1163       +3     

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

[ike-kottlowski]

I modified the bypass to check closer to the verification in the UserService to only affect users using the email OTP. This keeps the experience the same for master password users.

[trmartin4]

I defaulted the VerifySecretAsync() additional parameter to false so that we kept existing logic everywhere except the new calls from the TwoFactorController. This was identified with a failing test on VerifySecretAsync_Works

[quexten]

Nice work!