fix(e2e): report a 2FA-verify 5xx accurately, not as a rejected code #498

Merged

nrupard merged 1 commit from fix/BUNYIP-496-e2e-2fa-5xx-diagnostic into main

2026-08-06 21:19:37 +02:00

nrupard commented

2026-08-06 21:08:59 +02:00

Owner

Background

The E2E TOTP helper e2e/lib/login.ts:fillTotpStep submits a code and, on any rendered login error box, treats it as a rejected code: it waits out the TOTP step and retries once, then throws "TOTP rejected on both attempts ... the e2e account's enrolled 2FA secret likely does not match E2E_TOTP_SECRET, or the server clock is skewed".

But bunyip-web renders the SAME [class*="text-destructive"] error box for a server 5xx (the generic "An unexpected error occurred. Please try again later.", BUNYIP-477) as for a real code rejection ("Invalid verification code"). When POST /v1/auth/2fa/verify 5xx's (observed 2026-08-06: a user_totp decryption failure on staging - APP_ENCRYPTION_KEY rotated with an empty APP_ENCRYPTION_KEY_PREV, so TotpService::verify_code -> key_set.decrypt returns AppError::internal), readLoginError returns the generic message, the loop treats it as a rejection, wastes a step-wait retry, and throws the misleading secret/clock conclusion. The real cause (a 500) is hidden, and the retry burns 2fa_verify rate-limit budget for nothing.

Goal

fillTotpStep distinguishes a server 5xx from a real code rejection and reports the truth, so an E2E failure points at the actual cause.

Proposed approach

In fillTotpStep, after readLoginError returns a non-null message, branch on it:

If it matches the generic 5xx copy (/unexpected error|try again later/i), throw immediately with an accurate message (the code was submitted, bunyip-api 5xx'd on POST /v1/auth/2fa/verify, check the server error-log - most likely a user_totp decryption / encryption-key-state failure). Do NOT step-wait-retry: a fresh code cannot fix a server error.
Otherwise (a real "Invalid verification code"), keep the existing step-wait + fresh-code retry, then the existing exhaustion throw.

Alternatives considered

Match the exact string "Invalid verification code" as the only real rejection: rejected - brittle to copy changes. Matching the stable BUNYIP-477 generic-5xx shape and treating everything else as a rejection is more robust.
Intercept the HTTP response instead of reading the DOM error box: rejected - the helper is DOM-driven (the field auto-submits); response interception here is heavier than reading the already-rendered error.

Acceptance criteria

A 2FA-verify 5xx makes fillTotpStep throw an accurate "server error on /v1/auth/2fa/verify" diagnostic (naming the likely decryption / key cause), with no step-wait retry.
A genuine "Invalid verification code" still triggers the existing step-wait + fresh-code retry, then the existing exhaustion throw.
No new login attempts added (stays within the per-account / per-IP 2fa_verify caps).

## Background The E2E TOTP helper `e2e/lib/login.ts:fillTotpStep` submits a code and, on any rendered login error box, treats it as a rejected code: it waits out the TOTP step and retries once, then throws "TOTP rejected on both attempts ... the e2e account's enrolled 2FA secret likely does not match E2E_TOTP_SECRET, or the server clock is skewed". But bunyip-web renders the SAME `[class*="text-destructive"]` error box for a server 5xx (the generic "An unexpected error occurred. Please try again later.", BUNYIP-477) as for a real code rejection ("Invalid verification code"). When `POST /v1/auth/2fa/verify` 5xx's (observed 2026-08-06: a `user_totp` decryption failure on staging - `APP_ENCRYPTION_KEY` rotated with an empty `APP_ENCRYPTION_KEY_PREV`, so `TotpService::verify_code` -> `key_set.decrypt` returns `AppError::internal`), `readLoginError` returns the generic message, the loop treats it as a rejection, wastes a step-wait retry, and throws the misleading secret/clock conclusion. The real cause (a 500) is hidden, and the retry burns `2fa_verify` rate-limit budget for nothing. ## Goal `fillTotpStep` distinguishes a server 5xx from a real code rejection and reports the truth, so an E2E failure points at the actual cause. ## Proposed approach In `fillTotpStep`, after `readLoginError` returns a non-null message, branch on it: - If it matches the generic 5xx copy (`/unexpected error|try again later/i`), throw immediately with an accurate message (the code was submitted, bunyip-api 5xx'd on `POST /v1/auth/2fa/verify`, check the server error-log - most likely a `user_totp` decryption / encryption-key-state failure). Do NOT step-wait-retry: a fresh code cannot fix a server error. - Otherwise (a real "Invalid verification code"), keep the existing step-wait + fresh-code retry, then the existing exhaustion throw. ## Alternatives considered - Match the exact string "Invalid verification code" as the only real rejection: rejected - brittle to copy changes. Matching the stable BUNYIP-477 generic-5xx shape and treating everything else as a rejection is more robust. - Intercept the HTTP response instead of reading the DOM error box: rejected - the helper is DOM-driven (the field auto-submits); response interception here is heavier than reading the already-rendered error. ## Acceptance criteria - [ ] A 2FA-verify 5xx makes `fillTotpStep` throw an accurate "server error on /v1/auth/2fa/verify" diagnostic (naming the likely decryption / key cause), with no step-wait retry. - [ ] A genuine "Invalid verification code" still triggers the existing step-wait + fresh-code retry, then the existing exhaustion throw. - [ ] No new login attempts added (stays within the per-account / per-IP `2fa_verify` caps).

nrupard added 1 commit

2026-08-06 21:08:59 +02:00

fix(e2e): report a 2FA-verify 5xx accurately, not as a rejected code

E2E / PR gate (pull_request) Successful in 31s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 10m34s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

fbd1f006b0

fillTotpStep treated any login error box as a rejected TOTP code and, after a step-wait retry, threw "TOTP secret mismatch / clock skew". But bunyip-web renders the same error box for a server 5xx (the generic "An unexpected error occurred", BUNYIP-477) as for a real "Invalid verification code". A 2FA-verify 500 (e.g. a user_totp decryption failure) was therefore hidden behind a wrong conclusion, and the pointless retry burned 2fa_verify rate-limit budget.

Now, on a non-null error box: a match for the generic 5xx copy throws immediately with an accurate diagnostic (bunyip-api 5xx-ed on POST /v1/auth/2fa/verify; check the server error-log, likely a decryption / encryption-key-state failure) and does not retry; anything else keeps the existing step-wait + fresh-code retry.

tsc --noEmit clean.

#BUNYIP-496

nrupard scheduled this pull request to auto merge when all checks succeed

2026-08-06 21:17:20 +02:00