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),