Admin Portal - Fix GetTwoFactorProviders wrong exception type handling on Organization and User

[JaredSnider-Bitwarden]

🎟️ Tracking

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

📔 Objective

Organization.GetTwoFactorProviders and User.GetTwoFactorProviders both catch JsonException intending to return null on malformed JSON (per their doc comments). But Organization.cs and User.cs each import System.Text.Json, so the catch binds to System.Text.Json.JsonException — while JsonHelpers.LegacyDeserialize uses Newtonsoft under the hood and throws Newtonsoft.Json.JsonException (or its JsonReaderException / JsonSerializationException subclasses), an unrelated type. The catch never matched, so any row with an invalid TwoFactorProviders value took down the admin console Organizations view (via Admin/Controllers/OrganizationsController) and would take down the Users view the same way (via TwoFactorIsEnabledQuery fanned out by Admin/Controllers/UsersController).

Observed locally: integration-test rows seeded via AutoFixture left "TwoFactorProviders" + Guid.NewGuid() in the column (AutoFixture's default string generator prefixes with the property name), which is not valid JSON.

Fix: catch Newtonsoft.Json.JsonException (base of both reader and serialization exceptions) on both entities so the methods degrade to null as intended, drop the now-unused System.Text.Json using, and add matching theories on OrganizationTests and UserTests covering pure garbage, the AutoFixture pattern, malformed JSON, and a schema mismatch.

📸 Screenshots

Before:

After:

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

The core fix is correct: JsonHelpers.LegacyDeserialize delegates to Newtonsoft.Json.JsonConvert.DeserializeObject, which throws JsonReaderException / JsonSerializationException — both derived from Newtonsoft.Json.JsonException — so the new catch clauses on User and Organization now match, and this aligns both entities with the pre-existing OrganizationUserUserDetails.GetTwoFactorProviders. Neither entity retains any other System.Text.Json reference after the using removals, and every existing caller of GetTwoFactorProviders() already handles null because the empty-column path always returned it. The new ILogger<TwoFactorIsEnabledQuery> parameter resolves through the existing AddScoped registration, no call site constructs the query manually, all its tests use SutProvider, and the warn log records only the user id — never the raw column — so the zero-knowledge and logging rules hold.

Code Review Details

• ❓ : Unparseable Organization.TwoFactorProviders now degrades org-level 2FA silently, with no equivalent to the user-side warn log
  • src/Core/AdminConsole/Entities/Organization.cs:482

Previously raised and closed by the author: the login-path fail-open on User (providers == null treated as "no providers"). That was the pre-existing contract at the GetEnabledTwoFactorProviders layer, the column is not user-writable, and corrupt rows are now observable via the new log — deferring to that decision.

[codecov]

Codecov Report

❌ Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.56%. Comparing base (2e3db8d) to head (b622daf).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...torAuth/Implementations/TwoFactorIsEnabledQuery.cs	53.84%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8234      +/-   ##
==========================================
+ Coverage   63.29%   68.56%   +5.27%     
==========================================
  Files        2383     2399      +16     
  Lines      103921   103955      +34     
  Branches     9403     9409       +6     
==========================================
+ Hits        65778    71280    +5502     
+ Misses      35899    30325    -5574     
- Partials     2244     2350     +106     

☔ View full report in Codecov by Harness.
📢 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.