mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 14:40:52 +00:00
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.
This commit is contained in:
1 parent
94c3cdea96
commit
c44eed365d
3 files changed
+109
-1
No files matched your search
@@ -0,0 +1,89 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@/lib/api-handler", async () => {
|
||||
const actual =
|
||||
await vi.importActual<typeof import("@/lib/api-handler")>(
|
||||
"@/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();
|
||||
});
|
||||
});
|
||||
@@ -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),
|
||||
|
||||
Reference in new issue
Block a user