mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
feat(security): require fresh verification before destructive account actions
Account deletion and the keep-account data reset now demand a recent second-factor re-verification for accounts that have a second factor active; single-factor accounts are unaffected and keep the typed- confirmation-only contract. The gate runs through requireFreshMfaIfEnrolled, which is cookie-only by construction — a Bearer transport for an MFA-enrolled account cannot satisfy step-up and surfaces the auth.stepup.required envelope so the UI can launch a re-verification flow.
This commit is contained in:
1 parent
6d053c3667
commit
01689f77bc
4 files changed
+166
-4
No files matched your search
@@ -3,7 +3,11 @@ import { auditLog } from "@/lib/auth/audit";
|
||||
import { apiSuccess, apiError, getClientIp } from "@/lib/api-response";
|
||||
import { destroyAllSessions } from "@/lib/auth/session";
|
||||
import { NextRequest } from "next/server";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiHandler,
|
||||
requireFreshMfaIfEnrolled,
|
||||
MFA_STEP_UP_MAX_AGE_SECONDS,
|
||||
} from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
@@ -11,9 +15,14 @@ export const dynamic = "force-dynamic";
|
||||
/**
|
||||
* Permanently delete the user account and ALL associated data.
|
||||
* Cascading deletes in the schema handle all related records.
|
||||
*
|
||||
* v1.23 — for an account with a second factor active this requires a fresh
|
||||
* step-up (within MFA_STEP_UP_MAX_AGE_SECONDS) in addition to the typed
|
||||
* confirmation, so a hijacked live session cannot erase the record. Accounts
|
||||
* without MFA are unaffected and keep the typed-confirmation-only contract.
|
||||
*/
|
||||
export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
const { user } = await requireAuth();
|
||||
const { user } = await requireFreshMfaIfEnrolled(MFA_STEP_UP_MAX_AGE_SECONDS);
|
||||
|
||||
let confirm = "";
|
||||
try {
|
||||
|
||||
@@ -2,7 +2,11 @@ import { prisma } from "@/lib/db";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { apiSuccess, apiError, getClientIp } from "@/lib/api-response";
|
||||
import { NextRequest } from "next/server";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiHandler,
|
||||
requireFreshMfaIfEnrolled,
|
||||
MFA_STEP_UP_MAX_AGE_SECONDS,
|
||||
} from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { invalidateUserData } from "@/lib/cache/invalidate";
|
||||
|
||||
@@ -10,9 +14,13 @@ export const dynamic = "force-dynamic";
|
||||
|
||||
/**
|
||||
* Delete all user-owned health/integration data while keeping the account.
|
||||
*
|
||||
* v1.23 — gated by a fresh step-up for MFA-enrolled accounts (see the account
|
||||
* deletion route); single-factor accounts keep the typed-confirmation-only
|
||||
* contract.
|
||||
*/
|
||||
export const DELETE = apiHandler(async (request: NextRequest) => {
|
||||
const { user } = await requireAuth();
|
||||
const { user } = await requireFreshMfaIfEnrolled(MFA_STEP_UP_MAX_AGE_SECONDS);
|
||||
|
||||
let confirm = "";
|
||||
try {
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
// Mirror require-fresh-mfa.test.ts mocking so the api-handler module loads
|
||||
// without a live DB / request scope.
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
session: { findUnique: vi.fn() },
|
||||
apiToken: { findUnique: vi.fn(), update: vi.fn() },
|
||||
user: { findUnique: vi.fn() },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn(() => "hash") }));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
|
||||
const headersGet = vi.fn<(name: string) => string | null>();
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: headersGet })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import {
|
||||
requireFreshMfaIfEnrolled,
|
||||
StepUpRequiredError,
|
||||
HttpError,
|
||||
MFA_STEP_UP_MAX_AGE_SECONDS,
|
||||
} from "../api-handler";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { headers } from "next/headers";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
|
||||
const MFA_USER = {
|
||||
id: "user-1",
|
||||
role: "USER" as const,
|
||||
username: "u",
|
||||
totpConfirmedAt: new Date("2020-01-01"),
|
||||
};
|
||||
const PLAIN_USER = { ...MFA_USER, totpConfirmedAt: null };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
headersGet.mockReturnValue(null);
|
||||
// resetAllMocks wipes the inline mock factory implementation too; restore
|
||||
// the headers() wrapper so requireAuth's Bearer branch can read the header.
|
||||
vi.mocked(headers).mockImplementation(
|
||||
async () => ({ get: headersGet }) as never,
|
||||
);
|
||||
// resetAllMocks wipes the factory mockResolvedValue; auditLog is awaited
|
||||
// via `.catch(...)` in the Bearer path, so it must return a thenable.
|
||||
vi.mocked(auditLog).mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
function mockCookieSession(user: unknown) {
|
||||
vi.mocked(getSession).mockResolvedValue({
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 1e6) },
|
||||
user,
|
||||
} as never);
|
||||
}
|
||||
|
||||
describe("requireFreshMfaIfEnrolled", () => {
|
||||
it("passes a non-MFA user straight through without a freshness read", async () => {
|
||||
mockCookieSession(PLAIN_USER);
|
||||
const ctx = await requireFreshMfaIfEnrolled(MFA_STEP_UP_MAX_AGE_SECONDS);
|
||||
expect(ctx.user.id).toBe("user-1");
|
||||
expect(prisma.session.findUnique).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("passes an MFA user with a fresh verification", async () => {
|
||||
mockCookieSession(MFA_USER);
|
||||
vi.mocked(prisma.session.findUnique).mockResolvedValue({
|
||||
mfaVerifiedAt: new Date(Date.now() - 60_000),
|
||||
} as never);
|
||||
const ctx = await requireFreshMfaIfEnrolled(MFA_STEP_UP_MAX_AGE_SECONDS);
|
||||
expect(ctx.user.id).toBe("user-1");
|
||||
});
|
||||
|
||||
it("blocks an MFA user without a fresh verification", async () => {
|
||||
mockCookieSession(MFA_USER);
|
||||
vi.mocked(prisma.session.findUnique).mockResolvedValue({
|
||||
mfaVerifiedAt: null,
|
||||
} as never);
|
||||
await expect(
|
||||
requireFreshMfaIfEnrolled(MFA_STEP_UP_MAX_AGE_SECONDS),
|
||||
).rejects.toBeInstanceOf(StepUpRequiredError);
|
||||
});
|
||||
|
||||
it("a Bearer token can never satisfy step-up for an MFA-enrolled account", async () => {
|
||||
// requireAuth resolves the Bearer caller (no cookie), but the step-up read
|
||||
// is cookie-only — getSession returns null inside requireFreshMfa.
|
||||
vi.mocked(getSession).mockResolvedValue(null as never);
|
||||
headersGet.mockImplementation((n) =>
|
||||
n.toLowerCase() === "authorization" ? "Bearer hlk_xyz" : null,
|
||||
);
|
||||
vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({
|
||||
id: "tok-1",
|
||||
userId: "user-1",
|
||||
permissions: ["*"],
|
||||
revoked: false,
|
||||
expiresAt: null,
|
||||
} as never);
|
||||
vi.mocked(prisma.user.findUnique).mockResolvedValue(MFA_USER as never);
|
||||
vi.mocked(prisma.apiToken.update).mockResolvedValue({} as never);
|
||||
|
||||
await expect(
|
||||
requireFreshMfaIfEnrolled(MFA_STEP_UP_MAX_AGE_SECONDS),
|
||||
).rejects.toBeInstanceOf(HttpError);
|
||||
});
|
||||
});
|
||||
@@ -557,6 +557,34 @@ export async function requireFreshMfa(
|
||||
return { ...sessionData, mfaVerifiedAt: verifiedAt };
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.23 — conditional step-up for destructive account actions.
|
||||
*
|
||||
* Resolves the caller with the standard `requireAuth()` (cookie OR Bearer).
|
||||
* For an account WITHOUT a confirmed second factor the caller passes straight
|
||||
* through — a single-factor user is intentionally unaffected, so account
|
||||
* deletion / data reset keeps its existing typed-confirmation-only contract.
|
||||
* For an account WITH MFA active (`totpConfirmedAt` set) it additionally runs
|
||||
* `requireFreshMfa`, which is cookie-only by construction: an MFA-enrolled
|
||||
* account's Bearer transport carries no `mfaVerifiedAt` and therefore cannot
|
||||
* satisfy step-up, surfacing `StepUpRequiredError` (401,
|
||||
* `errorCode: "auth.stepup.required"`) so the UI launches a re-verification.
|
||||
*
|
||||
* Gating only the MFA-enrolled cohort keeps the boundary structural: a
|
||||
* hijacked live cookie session for an MFA user cannot nuke the record without
|
||||
* a fresh factor, while users who never opted into MFA are not forced through
|
||||
* a flow they have no way to complete.
|
||||
*/
|
||||
export async function requireFreshMfaIfEnrolled(
|
||||
maxAgeSeconds: number,
|
||||
): Promise<AuthContext> {
|
||||
const auth = await requireAuth();
|
||||
if (auth.user.totpConfirmedAt) {
|
||||
await requireFreshMfa(maxAgeSeconds);
|
||||
}
|
||||
return auth;
|
||||
}
|
||||
|
||||
/**
|
||||
* Report unhandled errors to GlitchTip (fire-and-forget).
|
||||
* Uses dynamic import to avoid circular dependencies and startup cost.
|
||||
|
||||
Reference in new issue
Block a user