From b70cdcae487f7d065760560905fa7a6199f6b6be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 21 May 2026 21:28:32 +0200 Subject: [PATCH] fix(audit): drop identifier from auth.login.failed audit details The user_not_found_or_no_password branch wrote the typed email/username into AuditLog.details, leaking PII into an operator artefact. The reason field already tells the operator everything they need; the identifier adds no diagnostic value over hashing or dropping it. Per the v1.4.20 no-PII-in-operator-artefacts directive, drop the field entirely. A regression test pins the absence of both the literal identifier value and any "identifier" key in the audit-row JSON. --- .../api/auth/login/__tests__/route.test.ts | 115 ++++++++++++++++++ src/app/api/auth/login/route.ts | 5 +- 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 src/app/api/auth/login/__tests__/route.test.ts diff --git a/src/app/api/auth/login/__tests__/route.test.ts b/src/app/api/auth/login/__tests__/route.test.ts new file mode 100644 index 000000000..785b30601 --- /dev/null +++ b/src/app/api/auth/login/__tests__/route.test.ts @@ -0,0 +1,115 @@ +/** + * v1.4.43 W3-SECURITY (H-1) — `auth.login.failed` audit row must not + * leak the typed identifier (email / username) into operator + * artefacts. Per the v1.4.20 PII directive, the `reason` field is the + * only signal operators need; the identifier itself is PII and stays + * out of the row. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { findFirst: vi.fn() }, + apiToken: { create: vi.fn() }, + refreshToken: { create: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/password", () => ({ + verifyPassword: vi.fn(), +})); + +vi.mock("@/lib/auth/session", () => ({ + createSession: vi.fn().mockResolvedValue("session-id"), + setOnboardingPendingCookie: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi + .fn() + .mockResolvedValue({ allowed: true, remaining: 5, reset: 0 }), + rateLimitHeaders: vi.fn(() => ({})), +})); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/auth/hmac", () => ({ + hashToken: vi.fn((raw: string) => `hashed:${raw}`), +})); + +vi.mock("@/lib/logging/transports", () => ({ + emitIfSampled: vi.fn(), +})); + +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { POST } from "../route"; +import { prisma } from "@/lib/db"; +import { verifyPassword } from "@/lib/auth/password"; +import { auditLog } from "@/lib/auth/audit"; +import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; +import { ensureDbCompatibility } from "@/lib/db-compat"; + +const TYPED_EMAIL = "leaked-identifier@example.com"; + +function makeRequest(): NextRequest { + return new NextRequest("http://localhost/api/auth/login", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ email: TYPED_EMAIL, password: "supersecret" }), + }); +} + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(prisma.user.findFirst).mockResolvedValue(null as never); + vi.mocked(verifyPassword).mockResolvedValue(false); + vi.mocked(auditLog).mockResolvedValue(undefined as never); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: true, + remaining: 5, + reset: 0, + } as never); + vi.mocked(rateLimitHeaders).mockReturnValue({} as never); + vi.mocked(ensureDbCompatibility).mockResolvedValue(undefined); +}); + +describe("POST /api/auth/login — auth.login.failed audit row (H-1)", () => { + it("does NOT include the typed identifier in the audit details", async () => { + const res = await POST(makeRequest()); + expect(res.status).toBe(401); + + expect(auditLog).toHaveBeenCalledWith( + "auth.login.failed", + expect.objectContaining({ + details: expect.objectContaining({ + reason: "user_not_found_or_no_password", + }), + }), + ); + + // Pin the absence: neither the literal identifier string nor an + // `identifier` key may appear anywhere in the audit-row JSON. + const calls = vi.mocked(auditLog).mock.calls; + const failedCall = calls.find((c) => c[0] === "auth.login.failed"); + expect(failedCall).toBeDefined(); + const payload = failedCall![1] as { details?: Record }; + expect(payload.details).toBeDefined(); + expect(Object.keys(payload.details!)).not.toContain("identifier"); + expect(JSON.stringify(payload)).not.toContain(TYPED_EMAIL); + }); +}); diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index 1cac71665..21108ffa6 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -58,9 +58,12 @@ export const POST = apiHandler(async (request: NextRequest) => { }); if (!user || !user.passwordHash) { + // v1.4.43 W3-SECURITY (H-1): never write the typed identifier into + // the audit row — `reason` already tells the operator what + // happened, and PII must not land in operator artefacts. await auditLog("auth.login.failed", { ipAddress: ip, - details: { identifier, reason: "user_not_found_or_no_password" }, + details: { reason: "user_not_found_or_no_password" }, }); return apiError("Invalid credentials", 401); }