fix(journal/auth): atomic magic-token consume to close TOCTOU
verifyLoginCode, verifyMagicToken, and verifyEmailChange previously did SELECT WHERE used_at IS NULL → UPDATE … SET used_at = now. Two concurrent verifications could both pass the SELECT and both succeed, accepting the same single-use token twice. Collapsed each to a single UPDATE … WHERE … RETURNING * statement. Postgres serializes row-level locks within an UPDATE, so exactly one concurrent caller observes a returned row; the rest see an empty array and get \"Invalid or expired\". The token is also marked used as part of the same statement — no second write needed. verifyEmailChange's tertiary email-availability check now runs *after* the consume; we keep the original semantics where the token is burned on a clash (the previous code explicitly did the same with a separate UPDATE). No behavior change on the happy path. Closes a credential-reuse window that mattered most for the 6-digit login codes (small search space, more likely to race). Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6113b66846
commit
f22bec5a13
1 changed files with 23 additions and 26 deletions
|
|
@ -279,9 +279,13 @@ export async function createMagicToken(email: string): Promise<{ token: string;
|
||||||
export async function verifyLoginCode(email: string, code: string): Promise<string> {
|
export async function verifyLoginCode(email: string, code: string): Promise<string> {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
|
||||||
|
// Consume atomically: a single UPDATE…RETURNING ensures only one
|
||||||
|
// concurrent request wins. The old select-then-update sequence was a
|
||||||
|
// TOCTOU window where two clients could both see `used_at IS NULL`
|
||||||
|
// and both succeed.
|
||||||
const [record] = await db
|
const [record] = await db
|
||||||
.select()
|
.update(magicTokens)
|
||||||
.from(magicTokens)
|
.set({ usedAt: new Date() })
|
||||||
.where(
|
.where(
|
||||||
and(
|
and(
|
||||||
eq(magicTokens.email, email),
|
eq(magicTokens.email, email),
|
||||||
|
|
@ -290,15 +294,11 @@ export async function verifyLoginCode(email: string, code: string): Promise<stri
|
||||||
gt(magicTokens.expiresAt, new Date()),
|
gt(magicTokens.expiresAt, new Date()),
|
||||||
isNull(magicTokens.usedAt),
|
isNull(magicTokens.usedAt),
|
||||||
),
|
),
|
||||||
);
|
)
|
||||||
|
.returning();
|
||||||
|
|
||||||
if (!record) throw new Error("Invalid or expired code");
|
if (!record) throw new Error("Invalid or expired code");
|
||||||
|
|
||||||
await db
|
|
||||||
.update(magicTokens)
|
|
||||||
.set({ usedAt: new Date() })
|
|
||||||
.where(eq(magicTokens.id, record.id));
|
|
||||||
|
|
||||||
const [user] = await db.select().from(users).where(eq(users.email, record.email));
|
const [user] = await db.select().from(users).where(eq(users.email, record.email));
|
||||||
if (!user) throw new Error("User not found");
|
if (!user) throw new Error("User not found");
|
||||||
|
|
||||||
|
|
@ -332,9 +332,10 @@ export async function initiateEmailChange(userId: string, newEmail: string): Pro
|
||||||
export async function verifyMagicToken(token: string): Promise<string> {
|
export async function verifyMagicToken(token: string): Promise<string> {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
|
||||||
|
// Atomic consume — see verifyLoginCode for rationale.
|
||||||
const [record] = await db
|
const [record] = await db
|
||||||
.select()
|
.update(magicTokens)
|
||||||
.from(magicTokens)
|
.set({ usedAt: new Date() })
|
||||||
.where(
|
.where(
|
||||||
and(
|
and(
|
||||||
eq(magicTokens.token, token),
|
eq(magicTokens.token, token),
|
||||||
|
|
@ -342,15 +343,11 @@ export async function verifyMagicToken(token: string): Promise<string> {
|
||||||
gt(magicTokens.expiresAt, new Date()),
|
gt(magicTokens.expiresAt, new Date()),
|
||||||
isNull(magicTokens.usedAt),
|
isNull(magicTokens.usedAt),
|
||||||
),
|
),
|
||||||
);
|
)
|
||||||
|
.returning();
|
||||||
|
|
||||||
if (!record) throw new Error("Invalid or expired magic link");
|
if (!record) throw new Error("Invalid or expired magic link");
|
||||||
|
|
||||||
await db
|
|
||||||
.update(magicTokens)
|
|
||||||
.set({ usedAt: new Date() })
|
|
||||||
.where(eq(magicTokens.id, record.id));
|
|
||||||
|
|
||||||
const [user] = await db.select().from(users).where(eq(users.email, record.email));
|
const [user] = await db.select().from(users).where(eq(users.email, record.email));
|
||||||
if (!user) throw new Error("User not found");
|
if (!user) throw new Error("User not found");
|
||||||
|
|
||||||
|
|
@ -360,9 +357,14 @@ export async function verifyMagicToken(token: string): Promise<string> {
|
||||||
export async function verifyEmailChange(token: string, userId: string): Promise<string> {
|
export async function verifyEmailChange(token: string, userId: string): Promise<string> {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
|
||||||
|
// Atomic consume — see verifyLoginCode for rationale. The token is
|
||||||
|
// marked used regardless of whether the email is still available;
|
||||||
|
// re-checking availability after the consume keeps the token
|
||||||
|
// single-use even if the new email was claimed in between (the
|
||||||
|
// original implementation also marked it used in that case).
|
||||||
const [record] = await db
|
const [record] = await db
|
||||||
.select()
|
.update(magicTokens)
|
||||||
.from(magicTokens)
|
.set({ usedAt: new Date() })
|
||||||
.where(
|
.where(
|
||||||
and(
|
and(
|
||||||
eq(magicTokens.token, token),
|
eq(magicTokens.token, token),
|
||||||
|
|
@ -370,25 +372,20 @@ export async function verifyEmailChange(token: string, userId: string): Promise<
|
||||||
gt(magicTokens.expiresAt, new Date()),
|
gt(magicTokens.expiresAt, new Date()),
|
||||||
isNull(magicTokens.usedAt),
|
isNull(magicTokens.usedAt),
|
||||||
),
|
),
|
||||||
);
|
)
|
||||||
|
.returning();
|
||||||
|
|
||||||
if (!record) throw new Error("Invalid or expired verification link");
|
if (!record) throw new Error("Invalid or expired verification link");
|
||||||
|
|
||||||
const newEmail = record.email;
|
const newEmail = record.email;
|
||||||
|
|
||||||
// Re-check email availability at verification time — someone may have
|
// Re-check email availability at verification time — someone may have
|
||||||
// registered with this email between initiation and verification
|
// registered with this email between initiation and verification.
|
||||||
const [existing] = await db.select().from(users).where(eq(users.email, newEmail));
|
const [existing] = await db.select().from(users).where(eq(users.email, newEmail));
|
||||||
if (existing) {
|
if (existing) {
|
||||||
await db.update(magicTokens).set({ usedAt: new Date() }).where(eq(magicTokens.id, record.id));
|
|
||||||
throw new Error("This email is now in use by another account");
|
throw new Error("This email is now in use by another account");
|
||||||
}
|
}
|
||||||
|
|
||||||
await db
|
|
||||||
.update(magicTokens)
|
|
||||||
.set({ usedAt: new Date() })
|
|
||||||
.where(eq(magicTokens.id, record.id));
|
|
||||||
|
|
||||||
await db
|
await db
|
||||||
.update(users)
|
.update(users)
|
||||||
.set({ email: newEmail })
|
.set({ email: newEmail })
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue