From 36410831ed19892ba6bd2b4ffc20e6a0a11c911b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 04:49:11 +0200 Subject: [PATCH] fix(settings): close a confirmation when its request settles The confirming click on both alert dialogs in the TOTP card calls preventDefault(), which is what keeps the dialog up and its button disabled while the request is in flight. Nothing took the dialog down again, so a refused rotation left the confirmation open over the card, and the card is the only place the refusal explains itself: the step-up message painted behind the overlay, inside the aria-hidden region the modal creates, with the confirm button live again. A success hid it, because the fresh-codes panel replaces the branch the dialog lives in and it unmounted anyway. Both dialogs are controlled now, and each mutation closes its own when the request settles, whichever way it went. The disable arm carried the same shape and a mistyped six-digit code there is routine, so it is fixed with it. The card test pins the sentence a step-up refusal shows and the wiring that takes the dialog down; what a person sees is asserted against a real browser in e2e/mfa-step-up-journey.spec.ts. --- .../__tests__/totp-card.test.tsx | 68 ++++++++++++++++++- .../settings/security-section/totp-card.tsx | 38 +++++++++-- 2 files changed, 98 insertions(+), 8 deletions(-) diff --git a/src/components/settings/security-section/__tests__/totp-card.test.tsx b/src/components/settings/security-section/__tests__/totp-card.test.tsx index 9b778c031..8e82079c6 100644 --- a/src/components/settings/security-section/__tests__/totp-card.test.tsx +++ b/src/components/settings/security-section/__tests__/totp-card.test.tsx @@ -8,6 +8,26 @@ * Project convention is SSR-only tests (no `@testing-library/react`); the * dialog's open-state behaviour is Radix's own, so we assert the source wires * the trigger + consequence copy rather than driving a portal at runtime. + * + * ## The refused mutation + * + * A refusal used to leave the confirmation up: the confirming click + * `preventDefault()`s Radix's close so the request can stay in flight behind a + * disabled button, and nothing took the dialog down again. On success that was + * invisible — the fresh-codes panel replaces the branch the dialog lives in, + * so it unmounted anyway — but a refused rotation (or a mistyped code at + * disable) left an overlay covering the one place the card explains itself. + * + * Two halves are pinned below: the sentence a step-up refusal puts on the + * card, called directly; and the wiring that takes the dialog down when the + * request settles, read off the source. The second half is structural because + * the DOM is out of reach here — what a person actually sees, a closed dialog + * over a visible message, is asserted in `e2e/mfa-step-up-journey.spec.ts` + * against a real browser. + * + * Mutation check: drop either `onSettled` and the wiring case goes red; drop + * an `open=` binding and the controlled case goes red; return `err.message` + * from `describeError`'s step-up arm and the sentence case goes red. */ import { describe, expect, it } from "vitest"; import { readFileSync } from "node:fs"; @@ -15,8 +35,12 @@ import { resolve } from "node:path"; import { renderToStaticMarkup } from "react-dom/server"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { ApiError } from "@/lib/api/api-fetch"; import { I18nProvider } from "@/lib/i18n/context"; -import { TotpCard } from "../totp-card"; +import en from "@/../messages/en.json"; +import { describeError, TotpCard } from "../totp-card"; + +const STEP_UP_SENTENCE = en.settings.security.stepUpRequired; function render(node: React.ReactNode) { const client = new QueryClient({ @@ -29,6 +53,10 @@ function render(node: React.ReactNode) { ); } +function source(): string { + return readFileSync(resolve(__dirname, "../totp-card.tsx"), "utf8"); +} + describe(" — recovery-code regeneration", () => { it("SSRs the enabled card with the regenerate action", () => { const html = render(); @@ -36,7 +64,7 @@ describe(" — recovery-code regeneration", () => { }); it("wraps regenerate in a confirmation dialog with a consequence line", () => { - const src = readFileSync(resolve(__dirname, "../totp-card.tsx"), "utf8"); + const src = source(); // The regenerate action is a dialog trigger, not a bare mutate-on-click. expect(src).toContain("settings.security.recovery.regenerateTitle"); expect(src).toContain("settings.security.recovery.regenerateConfirm"); @@ -44,3 +72,39 @@ describe(" — recovery-code regeneration", () => { expect(src).toContain("regenerate.mutate()"); }); }); + +describe(" — a refused mutation", () => { + it("puts the step-up sentence on the card, not the server's prose", () => { + const refusal = new ApiError( + "Recent second-factor verification required", + 401, + { errorCode: "auth.stepup.required" }, + ); + + const shown = describeError(refusal, "fallback", STEP_UP_SENTENCE); + + expect(shown).toBe(STEP_UP_SENTENCE); + expect(shown).not.toContain("second-factor verification required"); + }); + + it("closes each confirmation when its request settles", () => { + const src = source(); + // Whatever the answer was — the refusal is the case that needs it, and a + // handler that only closed on success would leave the refusal covered. + expect(src).toContain("onSettled: () => setRegenerateOpen(false)"); + expect(src).toContain("onSettled: () => setDisableOpen(false)"); + }); + + it("leaves no confirmation for Radix to close on its own", () => { + const src = source(); + // Every dialog on this card is controlled, so none of them can be left + // holding an overlay after a `preventDefault()`ed confirm. + const dialogs = src.match(/]/g); + const controlled = src.match(/ setDisableOpen(false), }); const regenerate = useMutation({ @@ -183,6 +205,7 @@ export function TotpCard({ t("settings.security.stepUpRequired"), ), ), + onSettled: () => setRegenerateOpen(false), }); async function copySecret() { @@ -356,7 +379,10 @@ export function TotpCard({ )}

- +