From c44eed365d0d684e7c8ba21951c9853b2f5cf1e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 27 Jun 2026 21:50:16 +0200 Subject: [PATCH] fix(security): revoke other sessions when the second factor is removed Removing the second factor is a security-state change, so it should not leave stale credentials behind. The disable endpoint now drops every other web session, every native refresh token, and every trusted device, keeping only the caller's current session. The lockout-recovery CLI does the same out of band, clearing all sessions and trusted devices for the account. --- scripts/disable-mfa.ts | 11 +++ .../me/mfa/disable/__tests__/route.test.ts | 89 +++++++++++++++++++ src/app/api/auth/me/mfa/disable/route.ts | 10 ++- 3 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 src/app/api/auth/me/mfa/disable/__tests__/route.test.ts diff --git a/scripts/disable-mfa.ts b/scripts/disable-mfa.ts index 456ccae76..ba232baa6 100644 --- a/scripts/disable-mfa.ts +++ b/scripts/disable-mfa.ts @@ -68,6 +68,17 @@ async function main(): Promise { }, }); await tx.mfaRecoveryCode.deleteMany({ where: { userId: user.id } }); + // v1.23 (M-review L3) — removing the second factor is a security-state + // change. Out-of-band there is no "current" session to keep, so drop every + // session, every native refresh token, and every "remember this device" + // trusted device for the account: a stolen session or trusted-device cookie + // must not survive the lockout-recovery reset. + await tx.session.deleteMany({ where: { userId: user.id } }); + await tx.refreshToken.updateMany({ + where: { userId: user.id, revokedAt: null }, + data: { revokedAt: new Date() }, + }); + await tx.trustedDevice.deleteMany({ where: { userId: user.id } }); await tx.auditLog.create({ data: { action: "auth.mfa.disabled", diff --git a/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts b/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts new file mode 100644 index 000000000..dfd88440c --- /dev/null +++ b/src/app/api/auth/me/mfa/disable/__tests__/route.test.ts @@ -0,0 +1,89 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/api-handler", async () => { + const actual = + await vi.importActual( + "@/lib/api-handler", + ); + return { + ...actual, + requireFreshMfa: vi.fn(), + }; +}); + +vi.mock("@/lib/db", () => ({ + prisma: { + $transaction: vi.fn(), + user: { update: vi.fn() }, + mfaRecoveryCode: { deleteMany: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/session", () => ({ + destroyOtherSessions: vi.fn().mockResolvedValue({ sessionsRevoked: 2 }), +})); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn().mockResolvedValue({ allowed: true }), + rateLimitHeaders: vi.fn(() => ({})), +})); + +vi.mock("@/lib/auth/mfa/verify-factor", () => ({ + verifyMfaFactor: vi.fn(), +})); + +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); + +import { POST } from "../route"; +import { requireFreshMfa } from "@/lib/api-handler"; +import { prisma } from "@/lib/db"; +import { destroyOtherSessions } from "@/lib/auth/session"; +import { verifyMfaFactor } from "@/lib/auth/mfa/verify-factor"; + +function req() { + return new Request("http://localhost/api/auth/me/mfa/disable", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ method: "totp", code: "123456" }), + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(requireFreshMfa).mockResolvedValue({ + user: { id: "u1" }, + session: { id: "sess-current" }, + } as never); + vi.mocked(verifyMfaFactor).mockResolvedValue({ ok: true } as never); + vi.mocked(prisma.$transaction).mockImplementation(async (cb: unknown) => { + if (typeof cb === "function") { + return (cb as (tx: unknown) => unknown)({ + user: { update: vi.fn() }, + mfaRecoveryCode: { deleteMany: vi.fn() }, + }); + } + return undefined; + }); +}); + +describe("POST /api/auth/me/mfa/disable", () => { + it("revokes other sessions + trusted devices (via destroyOtherSessions), keeping the current one", async () => { + const res = await POST(req()); + expect(res.status).toBe(200); + expect(destroyOtherSessions).toHaveBeenCalledWith("u1", "sess-current"); + }); + + it("rejects a wrong current factor without revoking anything", async () => { + vi.mocked(verifyMfaFactor).mockResolvedValue({ + ok: false, + replay: false, + } as never); + const res = await POST(req()); + expect(res.status).toBe(401); + expect(destroyOtherSessions).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/auth/me/mfa/disable/route.ts b/src/app/api/auth/me/mfa/disable/route.ts index 9595de572..d5ab54a18 100644 --- a/src/app/api/auth/me/mfa/disable/route.ts +++ b/src/app/api/auth/me/mfa/disable/route.ts @@ -25,6 +25,7 @@ import { import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; import { prisma } from "@/lib/db"; +import { destroyOtherSessions } from "@/lib/auth/session"; import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; import { verifyMfaFactor } from "@/lib/auth/mfa/verify-factor"; import { mfaDisableSchema } from "@/lib/validations/mfa"; @@ -37,7 +38,7 @@ const DISABLE_WINDOW_MS = 15 * 60 * 1000; export const POST = apiHandler(async (req: Request) => { // Step-up gate first — throws StepUpRequiredError (401 + errorCode) if the // session is not freshly second-factor-verified. - const { user } = await requireFreshMfa(MFA_STEP_UP_MAX_AGE_SECONDS); + const { user, session } = await requireFreshMfa(MFA_STEP_UP_MAX_AGE_SECONDS); const rl = await checkRateLimit( `mfa:disable:${user.id}`, @@ -94,6 +95,13 @@ export const POST = apiHandler(async (req: Request) => { await tx.mfaRecoveryCode.deleteMany({ where: { userId: user.id } }); }); + // v1.23 (M-review L3) — removing the second factor is a security-state + // change: drop every OTHER web session, every native refresh token, and + // every "remember this device" trusted device (all handled by + // destroyOtherSessions), keeping only the caller's current session. A stale + // session or a trusted-device cookie must not survive the factor's removal. + await destroyOtherSessions(user.id, session.id); + await auditLog("auth.mfa.disabled", { userId: user.id, ipAddress: getClientIp(req),