fix(auth): race-proof username uniqueness in PATCH /profile
Problem: the profile-update handler pre-checked username uniqueness with a SELECT followed by an UPDATE outside any transaction. Two concurrent PATCHes setting the same slug would both pass the SELECT (no conflict yet), then one of the UPDATEs would hit the underlying UNIQUE constraint and surface as an unhandled SqliteError → 500. Fix: drop the racy pre-check entirely. The UNIQUE constraint on users.username (column-level on fresh DBs, partial unique index on migrated DBs) is the source of truth. Wrap the UPDATE in try/catch and convert SQLITE_CONSTRAINT_UNIQUE into a clean 409 username_taken response. Same "push the invariant down to the database" pattern as the recent first-user-auto-admin race fix. Surfaced by the username-uniqueness review.
This commit is contained in:
parent
fbe37109a4
commit
d68859d68b
1 changed files with 17 additions and 9 deletions
|
|
@ -306,14 +306,11 @@ authRoutes.patch('/profile', requireAuth, async (c) => {
|
|||
detail: 'lowercase a-z, 0-9, _ or -; 2-31 characters; must start with a letter or digit',
|
||||
}, 400);
|
||||
}
|
||||
if (next !== null) {
|
||||
// Pre-check uniqueness so we can return a clear 409 instead of the
|
||||
// SQLite UNIQUE-constraint error bubbling up as a 500.
|
||||
const taken = db.prepare(
|
||||
'SELECT 1 FROM users WHERE username = ? AND id <> ?',
|
||||
).get(next, userId);
|
||||
if (taken) return c.json({ error: 'username_taken' }, 409);
|
||||
}
|
||||
// Uniqueness is enforced by the UNIQUE constraint on users.username
|
||||
// (column-level for fresh DBs, partial unique index for migrated DBs).
|
||||
// We rely on it directly rather than pre-checking — a pre-check would
|
||||
// be racy (two concurrent PATCHes could both pass it). The constraint
|
||||
// catch below converts SQLITE_CONSTRAINT_UNIQUE into a clean 409.
|
||||
updates.push('username = ?');
|
||||
params.push(next);
|
||||
}
|
||||
|
|
@ -333,7 +330,18 @@ authRoutes.patch('/profile', requireAuth, async (c) => {
|
|||
}
|
||||
|
||||
params.push(userId);
|
||||
db.prepare(`UPDATE users SET ${updates.join(', ')} WHERE id = ?`).run(...params);
|
||||
try {
|
||||
db.prepare(`UPDATE users SET ${updates.join(', ')} WHERE id = ?`).run(...params);
|
||||
} catch (err) {
|
||||
// SQLite throws an SqliteError with a message containing "UNIQUE
|
||||
// constraint failed: users.username" (column-level constraint) or
|
||||
// ".users_username_idx" (the partial unique index used by migrated DBs).
|
||||
// Either way the user-facing meaning is the same — slug taken.
|
||||
if (err instanceof Error && /UNIQUE constraint failed.*username/i.test(err.message)) {
|
||||
return c.json({ error: 'username_taken' }, 409);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const me = loadMe(userId);
|
||||
if (!me) return c.json({ error: 'internal_error' }, 500);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue