fix(security): confirm before regenerating recovery codes

Regenerating recovery codes invalidates the current set immediately, so the
action now sits behind a confirmation dialog matching the disable-MFA pattern,
with a consequence line spelling out that the existing codes stop working. While
in the card: give the disable-step TOTP input the same digit-strip + 6-char cap
and exact-length submit guard as setup, swap the static disable-code id for a
useId value, and lift the copy-secret button to a 44px touch target on phones.
This commit is contained in:
Marc-André Bombeck committed 2026-06-27 22:23:09 +02:00
1 parent 2c1151dd40
commit 460e078925
8 files changed
+122 -23

No files matched your search

+3 -1
View File
@@ -5064,7 +5064,9 @@
"remainingOther": "{count} Wiederherstellungscodes übrig.",
"regenerate": "Codes neu erzeugen",
"regenFailed": "Wiederherstellungscodes konnten nicht neu erzeugt werden.",
"savedIt": "Ich habe sie gespeichert"
"savedIt": "Ich habe sie gespeichert",
"regenerateTitle": "Wiederherstellungscodes neu erzeugen?",
"regenerateConfirm": "Damit werden deine aktuellen Wiederherstellungscodes ersetzt. Deine bisherigen Codes funktionieren sofort nicht mehr, und der neue Satz wird nur einmal angezeigt."
},
"keys": {
"title": "Sicherheitsschlüssel",
+3 -1
View File
@@ -5064,7 +5064,9 @@
"remainingOther": "{count} recovery codes remaining.",
"regenerate": "Regenerate codes",
"regenFailed": "Could not regenerate recovery codes.",
"savedIt": "I've saved them"
"savedIt": "I've saved them",
"regenerateTitle": "Regenerate recovery codes?",
"regenerateConfirm": "This replaces your current recovery codes. Your existing codes will stop working immediately, and a new set is shown only once."
},
"keys": {
"title": "Security keys",
+3 -1
View File
@@ -5064,7 +5064,9 @@
"remainingOther": "Quedan {count} códigos de recuperación.",
"regenerate": "Regenerar códigos",
"regenFailed": "No se pudieron regenerar los códigos de recuperación.",
"savedIt": "Los he guardado"
"savedIt": "Los he guardado",
"regenerateTitle": "¿Regenerar los códigos de recuperación?",
"regenerateConfirm": "Esto reemplaza tus códigos de recuperación actuales. Tus códigos existentes dejarán de funcionar de inmediato y el nuevo conjunto se muestra solo una vez."
},
"keys": {
"title": "Llaves de seguridad",
+3 -1
View File
@@ -5064,7 +5064,9 @@
"remainingOther": "{count} codes de récupération restants.",
"regenerate": "Régénérer les codes",
"regenFailed": "Impossible de régénérer les codes de récupération.",
"savedIt": "Je les ai enregistrés"
"savedIt": "Je les ai enregistrés",
"regenerateTitle": "Régénérer les codes de récupération ?",
"regenerateConfirm": "Cela remplace vos codes de récupération actuels. Vos codes existants cesseront de fonctionner immédiatement et le nouveau jeu n'est affiché qu'une seule fois."
},
"keys": {
"title": "Clés de sécurité",
+3 -1
View File
@@ -5064,7 +5064,9 @@
"remainingOther": "{count} codici di recupero rimanenti.",
"regenerate": "Rigenera i codici",
"regenFailed": "Impossibile rigenerare i codici di recupero.",
"savedIt": "Li ho salvati"
"savedIt": "Li ho salvati",
"regenerateTitle": "Rigenerare i codici di recupero?",
"regenerateConfirm": "Questa operazione sostituisce i codici di recupero attuali. I codici esistenti smetteranno di funzionare immediatamente e il nuovo set viene mostrato una sola volta."
},
"keys": {
"title": "Chiavi di sicurezza",
+3 -1
View File
@@ -5064,7 +5064,9 @@
"remainingOther": "Pozostało {count} kodów odzyskiwania.",
"regenerate": "Wygeneruj kody ponownie",
"regenFailed": "Nie udało się ponownie wygenerować kodów odzyskiwania.",
"savedIt": "Zapisałem je"
"savedIt": "Zapisałem je",
"regenerateTitle": "Wygenerować nowe kody odzyskiwania?",
"regenerateConfirm": "Spowoduje to zastąpienie obecnych kodów odzyskiwania. Dotychczasowe kody natychmiast przestaną działać, a nowy zestaw zostanie pokazany tylko raz."
},
"keys": {
"title": "Klucze bezpieczeństwa",
@@ -0,0 +1,46 @@
/**
* v1.23 Phase-Q — `<TotpCard>` recovery-code regeneration guard.
*
* Regenerating recovery codes is destructive: the previous set stops working
* immediately. This suite pins that the regenerate action is gated behind a
* confirmation dialog (matching the disable-MFA pattern), not a bare click.
*
* 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.
*/
import { describe, expect, it } from "vitest";
import { readFileSync } from "node:fs";
import { resolve } from "node:path";
import { renderToStaticMarkup } from "react-dom/server";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { I18nProvider } from "@/lib/i18n/context";
import { TotpCard } from "../totp-card";
function render(node: React.ReactNode) {
const client = new QueryClient({
defaultOptions: { queries: { retry: false } },
});
return renderToStaticMarkup(
<I18nProvider initialLocale="en">
<QueryClientProvider client={client}>{node}</QueryClientProvider>
</I18nProvider>,
);
}
describe("<TotpCard> — recovery-code regeneration", () => {
it("SSRs the enabled card with the regenerate action", () => {
const html = render(<TotpCard enabled recoveryCodesRemaining={3} />);
expect(html).toContain("Regenerate codes");
});
it("wraps regenerate in a confirmation dialog with a consequence line", () => {
const src = readFileSync(resolve(__dirname, "../totp-card.tsx"), "utf8");
// 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");
// The mutate only fires from inside the dialog's confirm action.
expect(src).toContain("regenerate.mutate()");
});
});
@@ -74,6 +74,7 @@ export function TotpCard({
const { t } = useTranslations();
const queryClient = useQueryClient();
const codeFieldId = useId();
const disableCodeId = useId();
const [setup, setSetup] = useState<SetupData | null>(null);
const [qrDataUrl, setQrDataUrl] = useState<string | null>(null);
@@ -262,7 +263,7 @@ export function TotpCard({
type="button"
variant="outline"
size="icon"
className="h-8 w-8 shrink-0"
className="min-h-11 min-w-11 shrink-0 sm:h-8 sm:min-h-0 sm:w-8 sm:min-w-0"
onClick={copySecret}
aria-label={t("common.copy")}
>
@@ -350,18 +351,47 @@ export function TotpCard({
)}
</p>
<div className="flex flex-wrap gap-2">
<Button
type="button"
variant="outline"
className="min-h-11 sm:min-h-9"
onClick={() => regenerate.mutate()}
disabled={regenerate.isPending}
>
{regenerate.isPending && (
<Loader2 className="h-4 w-4 animate-spin motion-reduce:animate-none" />
)}
{t("settings.security.recovery.regenerate")}
</Button>
<AlertDialog>
<AlertDialogTrigger asChild>
<Button
type="button"
variant="outline"
className="min-h-11 sm:min-h-9"
disabled={regenerate.isPending}
>
{regenerate.isPending && (
<Loader2 className="h-4 w-4 animate-spin motion-reduce:animate-none" />
)}
{t("settings.security.recovery.regenerate")}
</Button>
</AlertDialogTrigger>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>
{t("settings.security.recovery.regenerateTitle")}
</AlertDialogTitle>
<AlertDialogDescription>
{t("settings.security.recovery.regenerateConfirm")}
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel>{t("common.cancel")}</AlertDialogCancel>
<AlertDialogAction
disabled={regenerate.isPending}
aria-busy={regenerate.isPending || undefined}
onClick={(e) => {
e.preventDefault();
regenerate.mutate();
}}
>
{regenerate.isPending && (
<Loader2 className="mr-1 h-3.5 w-3.5 animate-spin motion-reduce:animate-none" />
)}
{t("settings.security.recovery.regenerate")}
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
<AlertDialog>
<AlertDialogTrigger asChild>
@@ -383,13 +413,19 @@ export function TotpCard({
</AlertDialogDescription>
</AlertDialogHeader>
<div className="space-y-2">
<Label htmlFor="disable-code">
<Label htmlFor={disableCodeId}>
{t("settings.security.totp.currentCode")}
</Label>
<Input
id="disable-code"
id={disableCodeId}
value={disableCode}
onChange={(e) => setDisableCode(e.target.value)}
onChange={(e) =>
setDisableCode(
disableMethod === "totp"
? e.target.value.replace(/\D/g, "").slice(0, 6)
: e.target.value,
)
}
inputMode={disableMethod === "totp" ? "numeric" : "text"}
autoComplete="one-time-code"
placeholder={
@@ -415,7 +451,12 @@ export function TotpCard({
<AlertDialogCancel>{t("common.cancel")}</AlertDialogCancel>
<AlertDialogAction
variant="destructive"
disabled={disable.isPending || disableCode.length < 6}
disabled={
disable.isPending ||
(disableMethod === "totp"
? disableCode.length !== 6
: disableCode.length < 6)
}
aria-busy={disable.isPending || undefined}
onClick={(e) => {
e.preventDefault();