|
| 1 | +# authorizer-react: MFA Behavior Redesign UI — Design |
| 2 | + |
| 3 | +## Status |
| 4 | + |
| 5 | +Closes the gap discovered during manual testing: the component library has |
| 6 | +zero wiring for the withheld-token MFA contract shipped in authorizer#686 |
| 7 | +and authorizer-js#46 (merged into this session's work). Login/signup/passkey |
| 8 | +responses that carry `should_offer_*`/`should_show_totp_screen` flags with |
| 9 | +no `access_token` currently render nothing, or — worse, in |
| 10 | +`AuthorizerPasskeyLogin` — get treated as a successful login with a null |
| 11 | +token. This is a real defect, not unfinished polish. |
| 12 | + |
| 13 | +Supersedes authorizer-react PR #61 (closed as superseded earlier this |
| 14 | +session — built against the pre-#686 "issue token immediately, offer setup |
| 15 | +after" contract). |
| 16 | + |
| 17 | +## What already exists and is being reused, not rebuilt |
| 18 | + |
| 19 | +- **`AuthorizerMFASetup.tsx`** — a complete multi-method picker UI |
| 20 | + (TOTP/passkey/email-OTP/SMS-OTP), built in an earlier PR with the explicit |
| 21 | + stated intent of mapping onto exactly this kind of server-driven |
| 22 | + `should_offer_*` flag set. It has no Skip action and is never invoked from |
| 23 | + the login flow today — both close gaps, not a rewrite. |
| 24 | +- **`AuthorizerVerifyOtp.tsx`** — the code-entry screen for TOTP/email/SMS |
| 25 | + OTP verification, including existing lockout UI (from the older |
| 26 | + failed-attempt lockout feature, PR #53 — distinct from the new permanent |
| 27 | + `MFALockedAt` lockout this spec adds a screen for). |
| 28 | +- **`AuthorizerTOTPScanner.tsx`**, **`AuthorizerPasskeyRegister.tsx`** — |
| 29 | + existing enrollment-ceremony components, reused as-is. |
| 30 | + |
| 31 | +## Root causes, precisely |
| 32 | + |
| 33 | +1. `AuthorizerBasicAuthLogin.tsx`'s login handler (~line 96-152) only checks |
| 34 | + `should_show_totp_screen` (+ TOTP enrollment fields) and the old |
| 35 | + `should_show_email_otp_screen`/`should_show_mobile_otp_screen` flags. No |
| 36 | + reference anywhere to `should_offer_webauthn_mfa_setup`, |
| 37 | + `should_offer_email_otp_mfa_setup`, `should_offer_sms_otp_mfa_setup`, or |
| 38 | + `should_offer_webauthn_mfa_verify`. |
| 39 | +2. `AuthorizerPasskeyLogin.tsx`'s `onClick` (~line 86-115) unconditionally |
| 40 | + calls `setAuthData` whenever `res` is truthy, without checking whether |
| 41 | + `res.access_token` is actually present — a withheld-token response is |
| 42 | + silently treated as a successful login with a null token. |
| 43 | +3. `skipMfaSetup`/`lockMfa`/`emailOtpMfaSetup`/`smsOtpMfaSetup` (the 4 new |
| 44 | + SDK methods from authorizer-js#46) are referenced nowhere in this |
| 45 | + package. |
| 46 | +4. No locked-account UI exists for the new permanent `MFALockedAt` case |
| 47 | + (distinct from the existing transient failed-attempt lockout UI). |
| 48 | +5. No component wires `parseMfaRedirectParams` for the OAuth |
| 49 | + `mfa_required=1` redirect case. |
| 50 | +6. `example/` doesn't exercise any of the above, so none of it is |
| 51 | + end-to-end testable today. |
| 52 | + |
| 53 | +## Design |
| 54 | + |
| 55 | +### 1. Shared response-triage helper |
| 56 | + |
| 57 | +One function, `resolveAuthResponseStep(res: Types.AuthResponse)`, used by |
| 58 | +every entry point that can receive a withheld-token response (login, |
| 59 | +signup, passkey login, OAuth redirect). Returns a discriminated union: |
| 60 | + |
| 61 | +```typescript |
| 62 | +type AuthStep = |
| 63 | + | { kind: 'complete'; response: Types.AuthResponse } // access_token present |
| 64 | + | { kind: 'verify'; totp: boolean; webauthn: boolean; email: boolean; mobile: boolean } |
| 65 | + | { kind: 'offer'; totp: TotpEnrollment | null; webauthn: boolean; email: boolean; sms: boolean } |
| 66 | + | { kind: 'locked' }; |
| 67 | +``` |
| 68 | + |
| 69 | +`'locked'` is inferred from the login/verify call's *error* (the backend |
| 70 | +returns a `FailedPrecondition` error for a locked account, not a flag on a |
| 71 | +successful `AuthResponse` — there is no `should_show_locked_screen` field, |
| 72 | +confirmed against the backend contract), not from response fields — the |
| 73 | +triage helper's actual signature takes the `{ data, errors }` shape every |
| 74 | +SDK call already returns, and the `'locked'` case is detected from the |
| 75 | +error message/kind, not `res`. |
| 76 | + |
| 77 | +Every call site (`AuthorizerBasicAuthLogin`, `AuthorizerSignup`, |
| 78 | +`AuthorizerPasskeyLogin`, the new OAuth-redirect handler) calls this once |
| 79 | +right after its SDK call, then renders based on the returned `AuthStep` |
| 80 | +instead of hand-rolling its own flag checks. This directly fixes root cause |
| 81 | +2 (`AuthorizerPasskeyLogin` can no longer skip the access-token check) as a |
| 82 | +side effect of using the shared helper instead of ad hoc `if (res)`. |
| 83 | + |
| 84 | +### 2. `AuthorizerMFASetup` gains a "login mode" |
| 85 | + |
| 86 | +New optional prop, e.g. `loginContext?: { email?: string; phone_number?: string; state?: string; onComplete: (res: Types.AuthResponse) => void }`. |
| 87 | +When present: |
| 88 | +- A "Skip for now" action appears (absent in the existing settings-screen |
| 89 | + usage, which has no `loginContext`). |
| 90 | +- Skip calls the real `skipMfaSetup({ email, phone_number, state })` and, |
| 91 | + on success, calls `onComplete` with the returned (now-populated) |
| 92 | + `AuthResponse` — same completion path `AuthorizerBasicAuthLogin` already |
| 93 | + uses for a normal successful login. |
| 94 | +- Completing a method (TOTP scan+verify, passkey registration, email/SMS |
| 95 | + OTP send+verify) also calls `onComplete` once the corresponding |
| 96 | + `verify_otp`/`webauthn_registration_verify` call returns a token — these |
| 97 | + ceremonies already exist (`AuthorizerTOTPScanner`, |
| 98 | + `AuthorizerPasskeyRegister`); this spec doesn't change their internals, only |
| 99 | + makes sure the login-mode wrapper wires their result into `onComplete`. |
| 100 | +- When `loginContext` is absent (existing settings-screen usage): unchanged |
| 101 | + behavior, no Skip button, no `onComplete` callback — a signed-in user |
| 102 | + enrolling a second factor from account settings, not from the login gate. |
| 103 | + |
| 104 | +### 3. Real default wiring for email/SMS OTP setup |
| 105 | + |
| 106 | +`AuthorizerMFASetup`'s `onSetupMethod` currently only *delegates* to the |
| 107 | +host app for `email_otp`/`sms_otp` (per its own doc comment — "email- and |
| 108 | +SMS-OTP are... delegated to the host") since those SDK methods didn't exist |
| 109 | +yet when it was built. They exist now |
| 110 | +(`emailOtpMfaSetup`/`smsOtpMfaSetup`). Add a real internal |
| 111 | +implementation — selecting email/SMS OTP triggers the send-code call |
| 112 | +directly, then renders `AuthorizerVerifyOtp` (already built, reused as-is) |
| 113 | +for the code-entry step. `onSetupMethod` stays as an *override* escape |
| 114 | +hatch for a host that wants custom behavior, but the package now works |
| 115 | +correctly with zero host-side wiring required, closing the exact gap this |
| 116 | +session's manual testing hit. |
| 117 | + |
| 118 | +### 4. Locked-account screen |
| 119 | + |
| 120 | +New, small component: shows the backend's own error message ("contact your |
| 121 | +administrator to regain access") with no retry action — matches the |
| 122 | +existing session's established pattern (`login.go`/`webauthn.go`/etc. all |
| 123 | +already produce a clear, complete message; the UI's only job is to display |
| 124 | +it distinctly from a generic error banner, not invent new copy). |
| 125 | + |
| 126 | +### 5. OAuth redirect handling |
| 127 | + |
| 128 | +New: whatever page/component in `example/` handles the OAuth callback |
| 129 | +redirect calls `parseMfaRedirectParams(window.location.href)`. If non-null, |
| 130 | +route into the same `AuthorizerMFASetup`/`AuthorizerVerifyOtp` flow the |
| 131 | +password-login path uses (same triage helper, same components) rather than |
| 132 | +assuming a token is present. This is example-app wiring plus, if needed, a |
| 133 | +small exported helper in the library itself for a host app to reuse (follow |
| 134 | +whatever pattern `AuthorizerRoot`/`AuthorizerContext` already establish for |
| 135 | +similar cross-cutting concerns — decided at plan time by reading those |
| 136 | +files, not speculated here). |
| 137 | + |
| 138 | +### 6. `example/` app |
| 139 | + |
| 140 | +Update the example to actually mount and exercise all of the above end to |
| 141 | +end — this is what manual testing hit first, so it's explicitly in scope, |
| 142 | +not an afterthought. |
| 143 | + |
| 144 | +## Explicitly out of scope |
| 145 | + |
| 146 | +- Self-service deletion of a single non-passkey MFA method (tracked |
| 147 | + separately, see the `mfa_self_service_method_deletion_gap` project |
| 148 | + memory). |
| 149 | +- Any new visual design system, icon set, or styling beyond what |
| 150 | + `AuthorizerMFASetup`/`AuthorizerVerifyOtp` already establish. |
| 151 | + |
| 152 | +## Testing |
| 153 | + |
| 154 | +This package has no existing Jest/RTL unit-test suite for components (spot- |
| 155 | +checked: none of `AuthorizerBasicAuthLogin.tsx`/`AuthorizerMFASetup.tsx` |
| 156 | +have a corresponding `*.test.tsx` file) — manual testing via `example/` is |
| 157 | +the established verification method for this repo, confirmed at plan time |
| 158 | +before assuming otherwise. The plan's tasks each end with a concrete manual |
| 159 | +test procedure against the real linked backend + SDK, not a fabricated unit |
| 160 | +test suite this repo doesn't otherwise have. |
0 commit comments