From 3222bec8a52daf50f86767d29b22e1c1d20079cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Fri, 17 Apr 2026 09:11:50 +0200 Subject: [PATCH] security: atomic compare-and-swap for TOTP replay window (#43, part 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous SELECT → compare → UPDATE sequence let two concurrent login requests with the same valid 6-digit code both observe a stale lastTotpAt, both pass the in-JS replay check, and both succeed. A stolen TOTP (shoulder- surf, phishing-proxy replay) was usable twice within its 30 s window. Replace the three callsites (login authorize, self-service enable, self- service verify) with a shared consumeTotpWindow() helper: a single updateMany() expresses "window unused" as a SQL WHERE clause, so Postgres' row lock serialises concurrent writers and whichever commits second sees count=0 and is treated as a replay. Backup codes (ticket part 2) are tracked as follow-up work. Co-Authored-By: Claude Opus 4.7 --- apps/web/src/server/auth.ts | 22 +++---- packages/api/package.json | 1 + .../src/lib/__tests__/totp-consume.test.ts | 58 +++++++++++++++++++ packages/api/src/lib/totp-consume.ts | 48 +++++++++++++++ .../user-self-service-procedure-support.ts | 20 +++---- 5 files changed, 123 insertions(+), 26 deletions(-) create mode 100644 packages/api/src/lib/__tests__/totp-consume.test.ts create mode 100644 packages/api/src/lib/totp-consume.ts diff --git a/apps/web/src/server/auth.ts b/apps/web/src/server/auth.ts index c1f302c..1d18434 100644 --- a/apps/web/src/server/auth.ts +++ b/apps/web/src/server/auth.ts @@ -2,6 +2,7 @@ import { prisma } from "@capakraken/db"; import { authRateLimiter } from "@capakraken/api/middleware/rate-limit"; import { createAuditEntry } from "@capakraken/api/lib/audit"; import { logger } from "@capakraken/api/lib/logger"; +import { consumeTotpWindow } from "@capakraken/api/lib/totp-consume"; import NextAuth, { type NextAuthConfig } from "next-auth"; import Credentials from "next-auth/providers/credentials"; import { CredentialsSignin } from "next-auth"; @@ -188,15 +189,12 @@ const config = { throw new InvalidTotpError(); } - // Replay-attack prevention: reject if the same 30-second window was already used - const userWithTotp = (await prisma.user.findUnique({ - where: { id: user.id }, - select: { lastTotpAt: true }, - })) as { lastTotpAt: Date | null } | null; - if ( - userWithTotp?.lastTotpAt != null && - Date.now() - userWithTotp.lastTotpAt.getTime() < 30_000 - ) { + // Atomic replay-guard: a single UPDATE ... WHERE lastTotpAt is null + // OR older than 30 s both serialises concurrent logins (row lock) + // and expresses the "unused window" precondition in SQL. count=0 + // means another request consumed this window first → replay. + const accepted = await consumeTotpWindow(prisma, user.id); + if (!accepted) { logger.warn({ email, reason: "totp_replay" }, "TOTP replay attack blocked"); void createAuditEntry({ db: prisma, @@ -210,12 +208,6 @@ const config = { }); throw new InvalidTotpError(); } - - // Record successful TOTP use to prevent replay within the same window - await (prisma.user.update as Function)({ - where: { id: user.id }, - data: { lastTotpAt: new Date() }, - }); } // MFA enforcement: if the user's role is in requireMfaForRoles but they diff --git a/packages/api/package.json b/packages/api/package.json index fa11e02..a76cb3c 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -12,6 +12,7 @@ "./lib/reminder-scheduler": "./src/lib/reminder-scheduler.ts", "./lib/logger": "./src/lib/logger.ts", "./lib/runtime-security": "./src/lib/runtime-security.ts", + "./lib/totp-consume": "./src/lib/totp-consume.ts", "./middleware/rate-limit": "./src/middleware/rate-limit.ts" }, "scripts": { diff --git a/packages/api/src/lib/__tests__/totp-consume.test.ts b/packages/api/src/lib/__tests__/totp-consume.test.ts new file mode 100644 index 0000000..0ffcc9f --- /dev/null +++ b/packages/api/src/lib/__tests__/totp-consume.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { consumeTotpWindow } from "../totp-consume.js"; + +describe("consumeTotpWindow — atomic replay guard", () => { + let updateMany: ReturnType; + let db: { user: { updateMany: typeof updateMany } }; + + beforeEach(() => { + updateMany = vi.fn(); + db = { user: { updateMany } }; + }); + + it("returns true when the update affected a row", async () => { + updateMany.mockResolvedValue({ count: 1 }); + await expect(consumeTotpWindow(db, "user-1")).resolves.toBe(true); + }); + + it("returns false when another concurrent request already consumed the window", async () => { + updateMany.mockResolvedValue({ count: 0 }); + await expect(consumeTotpWindow(db, "user-1")).resolves.toBe(false); + }); + + it("issues a WHERE clause that only updates null or older-than-30-s rows", async () => { + updateMany.mockResolvedValue({ count: 1 }); + const now = new Date("2026-04-17T12:00:30.000Z"); + await consumeTotpWindow(db, "user-1", now); + + expect(updateMany).toHaveBeenCalledTimes(1); + const call = updateMany.mock.calls[0]![0] as { + where: { id: string; OR: Array<{ lastTotpAt: unknown }> }; + data: { lastTotpAt: Date }; + }; + expect(call.where.id).toBe("user-1"); + expect(call.where.OR).toEqual([ + { lastTotpAt: null }, + { lastTotpAt: { lt: new Date("2026-04-17T12:00:00.000Z") } }, + ]); + expect(call.data.lastTotpAt).toEqual(now); + }); + + it("simulated race: two parallel calls — exactly one wins", async () => { + // Model Postgres row-lock serialisation: the first updateMany to land + // sees count=1, the second (in the same 30-s window) sees count=0. + let served = 0; + updateMany.mockImplementation(async () => { + await new Promise((r) => setTimeout(r, 1)); + return { count: served++ === 0 ? 1 : 0 }; + }); + + const [a, b] = await Promise.all([ + consumeTotpWindow(db, "user-1"), + consumeTotpWindow(db, "user-1"), + ]); + + expect([a, b].sort()).toEqual([false, true]); + expect(updateMany).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/api/src/lib/totp-consume.ts b/packages/api/src/lib/totp-consume.ts new file mode 100644 index 0000000..110999b --- /dev/null +++ b/packages/api/src/lib/totp-consume.ts @@ -0,0 +1,48 @@ +// Atomic compare-and-swap for TOTP replay-window consumption. +// +// The old code path was: SELECT lastTotpAt → compare in JS → UPDATE. Two +// concurrent requests with the same valid 6-digit code both see a stale +// (or null) lastTotpAt, both pass the in-JS check, and both succeed. A +// stolen TOTP (shoulder-surf, phishing-proxy replay) is therefore usable +// twice within its 30 s window — the MFA design promise is violated. +// +// A single `updateMany` expresses the entire precondition in SQL: the WHERE +// clause guarantees the row has not been consumed in the last 30 s, and the +// SET sets the new timestamp. PostgreSQL's row-level lock serialises the two +// racing writes; whichever commits second sees rows-affected = 0 and the +// caller treats it as a replay. +// +// The 30 000 ms window matches the TOTP period (RFC 6238) — codes are +// validated with `window: 1` so adjacent periods are still accepted; the +// anti-replay check is the tighter per-code, per-user bound. + +// Intentionally loose structural type — Prisma's generated signature is a +// deeply-inferred generic that does not simplify to a friendly shape; we only +// need updateMany() with the documented args and a `{ count }` result. +// Keeping the internal cast isolated here means every callsite stays +// strictly typed. +interface TotpConsumeDb { + user: { + updateMany: (args: { + where: { id: string; OR: Array<{ lastTotpAt: Date | { lt: Date } | null }> }; + data: { lastTotpAt: Date }; + }) => Promise<{ count: number }>; + }; +} + +export async function consumeTotpWindow( + db: { user: { updateMany: (...args: never[]) => unknown } }, + userId: string, + now: Date = new Date(), +): Promise { + const typed = db as unknown as TotpConsumeDb; + const windowStart = new Date(now.getTime() - 30_000); + const result = await typed.user.updateMany({ + where: { + id: userId, + OR: [{ lastTotpAt: null }, { lastTotpAt: { lt: windowStart } }], + }, + data: { lastTotpAt: now }, + }); + return result.count > 0; +} diff --git a/packages/api/src/router/user-self-service-procedure-support.ts b/packages/api/src/router/user-self-service-procedure-support.ts index c53af50..ab43a44 100644 --- a/packages/api/src/router/user-self-service-procedure-support.ts +++ b/packages/api/src/router/user-self-service-procedure-support.ts @@ -5,6 +5,7 @@ import { TRPCError } from "@trpc/server"; import { z } from "zod"; import { findUniqueOrThrow } from "../db/helpers.js"; import { createAuditEntry } from "../lib/audit.js"; +import { consumeTotpWindow } from "../lib/totp-consume.js"; import { totpRateLimiter } from "../middleware/rate-limit.js"; import type { TRPCContext } from "../trpc.js"; @@ -235,8 +236,10 @@ export async function verifyAndEnableTotp( throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid TOTP token." }); } - // Replay-attack prevention: reject if the same 30-second window was already used - if (user.lastTotpAt != null && Date.now() - user.lastTotpAt.getTime() < 30_000) { + // Atomic replay-guard: single UPDATE with WHERE-guard on lastTotpAt. See + // packages/api/src/lib/totp-consume.ts for rationale. + const accepted = await consumeTotpWindow(ctx.db, user.id); + if (!accepted) { throw new TRPCError({ code: "BAD_REQUEST", message: "TOTP code already used. Wait for the next code.", @@ -245,7 +248,7 @@ export async function verifyAndEnableTotp( await (ctx.db.user.update as Function)({ where: { id: user.id }, - data: { totpEnabled: true, lastTotpAt: new Date() }, + data: { totpEnabled: true }, }); void createAuditEntry({ @@ -309,17 +312,12 @@ export async function verifyTotp( throw new TRPCError({ code: "UNAUTHORIZED", message: "Invalid TOTP token." }); } - // Replay-attack prevention: reject if the same 30-second window was already used - if (user.lastTotpAt != null && Date.now() - user.lastTotpAt.getTime() < 30_000) { + // Atomic replay-guard — see packages/api/src/lib/totp-consume.ts. + const accepted = await consumeTotpWindow(ctx.db, user.id); + if (!accepted) { throw new TRPCError({ code: "UNAUTHORIZED", message: "Invalid TOTP token." }); } - // Record successful TOTP use to prevent replay within the same window - await (ctx.db.user.update as Function)({ - where: { id: user.id }, - data: { lastTotpAt: new Date() }, - }); - return { valid: true }; }