Skip to content

Commit 1a3e34b

Browse files
committed
docs(skills): fix two real bugs in auth-server-primitives examples
1. Cookie parser truncated values containing '='. Signed cookies, JWTs, and base64-padded values all use '='. Use indexOf to split on the FIRST '=' only. 2. Login example short-circuited verifyPasswordHash on user-not-found, contradicting the prose's "same time, same error" claim — the no-user branch returned instantly while wrong-password spent ~100ms hashing, leaking account existence over the wire. Always verify against a hash; use a precomputed DUMMY_PASSWORD_HASH when the user is missing, then combine with the user-exists bit for the final ok. Same fixes in the SKILL.md and the docs companion.
1 parent 3ef6e3f commit 1a3e34b

2 files changed

Lines changed: 23 additions & 13 deletions

File tree

docs/start/framework/react/guide/authentication-server-primitives.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ export function readSessionToken(): string | null {
6161
const header = getRequestHeader('cookie')
6262
if (!header) return null
6363
for (const part of header.split(/;\s*/)) {
64-
const [name, value] = part.split('=')
65-
if (name === SESSION_COOKIE) return value ?? null
64+
// Split only on the FIRST '=' — signed/base64 values often contain '='.
65+
const eq = part.indexOf('=')
66+
if (eq === -1) continue
67+
if (part.slice(0, eq) === SESSION_COOKIE) return part.slice(eq + 1)
6668
}
6769
return null
6870
}
@@ -121,9 +123,13 @@ export const login = createServerFn({ method: 'POST' })
121123
.inputValidator(z.object({ email: z.string().email(), password: z.string() }))
122124
.handler(async ({ data }) => {
123125
const user = await db.users.findByEmail(data.email)
124-
const ok =
125-
user != null &&
126-
(await verifyPasswordHash(user.passwordHash, data.password))
126+
// Always run verifyPasswordHash — even when the user doesn't exist —
127+
// so the user-not-found branch takes the same time as wrong-password.
128+
// DUMMY_PASSWORD_HASH is a hash of any throwaway password computed once
129+
// at startup with the same algorithm/cost as real password hashes.
130+
const hashToCheck = user?.passwordHash ?? DUMMY_PASSWORD_HASH
131+
const passwordMatches = await verifyPasswordHash(hashToCheck, data.password)
132+
const ok = user != null && passwordMatches
127133
if (!ok) throw new Error('Invalid email or password')
128134

129135
// Rotate: destroy any existing session, then issue fresh.
@@ -134,7 +140,7 @@ export const login = createServerFn({ method: 'POST' })
134140
})
135141
```
136142

137-
The `Invalid email or password` message is identical for "user not found" and "wrong password". Don't leak which one it was; the constant-time `verifyPasswordHash` should also keep the timing similar.
143+
The `Invalid email or password` message is identical for "user not found" and "wrong password". The dummy-hash technique above also makes the timing identical: without it, the no-user branch returns instantly while the wrong-password branch spends ~100ms on the hash compare, leaking account existence over the wire.
138144

139145
## Logout
140146

packages/start-client-core/skills/start-core/auth-server-primitives/SKILL.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,10 @@ export function readSessionToken(): string | null {
6868
const header = getRequestHeader('cookie')
6969
if (!header) return null
7070
for (const part of header.split(/;\s*/)) {
71-
const [name, value] = part.split('=')
72-
if (name === SESSION_COOKIE) return value ?? null
71+
// Split only on the FIRST '=' — signed/base64 values often contain '='.
72+
const eq = part.indexOf('=')
73+
if (eq === -1) continue
74+
if (part.slice(0, eq) === SESSION_COOKIE) return part.slice(eq + 1)
7375
}
7476
return null
7577
}
@@ -130,11 +132,13 @@ export const login = createServerFn({ method: 'POST' })
130132
.inputValidator(z.object({ email: z.string().email(), password: z.string() }))
131133
.handler(async ({ data }) => {
132134
const user = await db.users.findByEmail(data.email)
133-
// Constant-time compare — don't short-circuit on user-not-found vs wrong-password,
134-
// both branches must take the same time and return the same error.
135-
const ok =
136-
user != null &&
137-
(await verifyPasswordHash(user.passwordHash, data.password))
135+
// Always run verifyPasswordHash — even when the user doesn't exist —
136+
// so the user-not-found branch takes the same time as wrong-password.
137+
// DUMMY_PASSWORD_HASH is a hash of any throwaway password computed once
138+
// at startup with the same algorithm/cost as real password hashes.
139+
const hashToCheck = user?.passwordHash ?? DUMMY_PASSWORD_HASH
140+
const passwordMatches = await verifyPasswordHash(hashToCheck, data.password)
141+
const ok = user != null && passwordMatches
138142
if (!ok) throw new Error('Invalid email or password')
139143

140144
// ROTATE on privilege change: destroy any existing session, then issue fresh.

0 commit comments

Comments
 (0)