Hartmut
3222bec8a5
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>
49 lines
2.0 KiB
TypeScript
49 lines
2.0 KiB
TypeScript
// 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;
|
|
}
|