fix(12): WR-01 narrow TOCTOU guard and set claimed=true for OIDC inserts
- apps/api/src/auth/user.ts: upsertUser step-5 insert now sets claimed=true for all OIDC-created users. An identity-bound OIDC user is never a pending wizard bootstrap user; explicit claimed=true prevents ambiguity with the (oidcIss IS NULL AND claimed=false) sentinel used by the TOCTOU guard and isSetupLocked. First-login-claims path is unaffected (it updates a pre-existing oidcIss=null row; this change only touches the fresh insert). - apps/api/src/routes/setup.ts: TOCTOU guard in POST /credential now queries WHERE oidc_iss IS NULL AND claimed = false FOR UPDATE, matching the exact definition of a pending wizard bootstrap user. This provides defense-in-depth against any future path that could produce claimed=false OIDC rows. - apps/api/tests/auth/user.test.ts: new WR-01 test asserts that the fresh OIDC insert sets claimed=true in the values passed to db.insert(). - apps/api/tests/routes/setup.test.ts: new WR-01 integration test seeds an OIDC user with claimed=false (oidcIss NOT NULL) and verifies POST /credential still succeeds (guard ignores the OIDC row, only counts local wizard rows). All 402 API tests, 253 PWA tests, and typecheck pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
22d1581484
commit
687f9dc9fa
@@ -156,6 +156,11 @@ export async function upsertUser(oidcIss: string, oidcSub: string, displayName?:
|
||||
const shouldBeAdmin = flagRow?.value !== 'true' && Number(count) === 0;
|
||||
|
||||
// 5. Insert new user row
|
||||
// claimed=true: an OIDC-created user is identity-bound at insert time and is
|
||||
// never a pending wizard bootstrap user. Setting this explicitly prevents the
|
||||
// TOCTOU guard in POST /credential (which counts WHERE oidc_iss IS NULL AND
|
||||
// claimed = false) from ever treating a fresh OIDC insert as an unclaimed
|
||||
// wizard row (WR-01).
|
||||
// mysql2 has no RETURNING clause — use $returningId() then re-select
|
||||
const [inserted] = await db
|
||||
.insert(users)
|
||||
@@ -165,6 +170,7 @@ export async function upsertUser(oidcIss: string, oidcSub: string, displayName?:
|
||||
displayName: displayName ?? null,
|
||||
color,
|
||||
isAdmin: shouldBeAdmin,
|
||||
claimed: true,
|
||||
})
|
||||
.$returningId();
|
||||
|
||||
|
||||
@@ -259,11 +259,15 @@ setupRouter.post('/credential', zValidator('json', credentialSchema, noEchoHook)
|
||||
let localUser: typeof users.$inferSelect | undefined;
|
||||
try {
|
||||
localUser = await db.transaction(async (tx) => {
|
||||
// Serialise: at most one unclaimed admin row may exist (WR-02).
|
||||
// Serialise: at most one unclaimed wizard bootstrap row may exist (WR-02).
|
||||
// The filter is (oidc_iss IS NULL AND claimed = false) — the precise definition
|
||||
// of a "pending wizard bootstrap user" — so that OIDC-created rows (which are
|
||||
// born with claimed=true after WR-01 fix, but could theoretically be claimed=false
|
||||
// on legacy data) are never counted here (WR-01 defense-in-depth).
|
||||
// Cast through unknown — Drizzle mysql2 execute() returns [rows, fields] for SELECTs;
|
||||
// the generic type parameter on execute() is not sufficient to type the result correctly.
|
||||
const countRows = (await tx.execute(
|
||||
sql`SELECT COUNT(*) AS count FROM users WHERE claimed = false FOR UPDATE`,
|
||||
sql`SELECT COUNT(*) AS count FROM users WHERE oidc_iss IS NULL AND claimed = false FOR UPDATE`,
|
||||
)) as unknown as [{ count: string | number }[], unknown];
|
||||
const unclaimedCount = Number(countRows[0][0]?.count ?? 0);
|
||||
if (unclaimedCount > 0) {
|
||||
|
||||
Reference in New Issue
Block a user