security: atomic compare-and-swap for TOTP replay window (#43, part 1)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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": {
|
||||
|
||||
@@ -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<typeof vi.fn>;
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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<boolean> {
|
||||
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;
|
||||
}
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user