Auth/PM-3813 - 2FA Management Endpoints - User Verification Refactor#21385
Auth/PM-3813 - 2FA Management Endpoints - User Verification Refactor#21385JaredSnider-Bitwarden wants to merge 3 commits into
Conversation
Mirror the server-side per-provider model rewrite on the client: - Existing PUT/DELETE setup request models drop SecretVerificationRequest inheritance and become standalone with explicit userVerificationToken fields. two-factor-email.request keeps its inheritance because it also serves the login flow. - New per-provider delete request models (YubiKey, Duo, Email, OrganizationDuo, WebAuthn delete-all) — token-only shape mirroring the server. - Authenticator delete request file/class renamed from disable-two-factor-authenticator to delete-two-factor-authenticator to match server naming. - New TwoFactorWebAuthnChallengeResponse wrapper around the FIDO2 options + minted token (replaces the bare ChallengeResponse payload from get-webauthn-challenge). - Response models for Duo, Email, WebAuthn, and YubiKey gain userVerificationToken. - Obsolete two-factor-provider.request deleted (no remaining consumers after the disable-model rewrite).
Both TwoFactorApiService and TwoFactorService (abstractions and implementations) pick up: - New methods deleteTwoFactorYubiKey, deleteTwoFactorDuo, deleteTwoFactorEmail, deleteTwoFactorOrganizationDuo, and deleteTwoFactorWebAuthnAll routing to the corresponding per-provider DELETE endpoints. - Removal of legacy putTwoFactorDisable and putTwoFactorOrganizationDisable (server endpoints are gone). - Updated return type on getTwoFactorWebAuthnChallenge to the new TwoFactorWebAuthnChallengeResponse wrapper. DefaultTwoFactorApiService spec: legacy put*Disable tests removed, five new deleteTwoFactor* tests added, WebAuthn challenge test asserts the new wrapper.
Per-provider setup components (Authenticator, YubiKey, Duo, Email, WebAuthn) instantiate request models directly, cache the user verification token from the GET response, and thread it through every PUT / DELETE / setup-POST call. Each component implements its own disableMethod against the appropriate per-provider DELETE endpoint: - Authenticator, YubiKey, Email each call their corresponding deleteTwoFactor* method. - Duo branches on organizationId between deleteTwoFactorDuo and deleteTwoFactorOrganizationDuo (the component is shared between personal Duo and OrgDuo setup). - WebAuthn's "Disable All Keys" button calls deleteTwoFactorWebAuthnAll (single round-trip; server-side handles the wipe atomically). Per-credential remove continues to use deleteTwoFactorWebAuthn. - WebAuthn challenge consumer reads options + userVerificationToken from the new wrapper response. Base setup component disableMethod becomes protected abstract — every subclass provides its own override. Parent settings page stops rendering the lapsed-premium-only secondary "Disable" button; the standard "Manage" button is now enabled for lapsed-premium users on already-enrolled premium providers, so the same GET → DELETE flow handles them.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the 2FA management refactor that replaces the generic Code Review DetailsNo blocking findings. One item worth confirming during QA (not posted inline — depends on the companion server change in PM-38137): this PR removes |
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #21385 +/- ##
==========================================
- Coverage 49.27% 49.19% -0.08%
==========================================
Files 4056 4079 +23
Lines 127010 127859 +849
Branches 19371 19568 +197
==========================================
+ Hits 62583 62905 +322
- Misses 59803 60300 +497
- Partials 4624 4654 +30 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-38137
📔 Objective
TODO
📸 Screenshots
TODO