diff --git a/apps/api/src/auth/user.ts b/apps/api/src/auth/user.ts index aacd2e1..2de9e42 100644 --- a/apps/api/src/auth/user.ts +++ b/apps/api/src/auth/user.ts @@ -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(); diff --git a/apps/api/src/routes/setup.ts b/apps/api/src/routes/setup.ts index 5d5ec8f..737c597 100644 --- a/apps/api/src/routes/setup.ts +++ b/apps/api/src/routes/setup.ts @@ -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) { diff --git a/apps/api/tests/auth/user.test.ts b/apps/api/tests/auth/user.test.ts index 627e63c..82ab413 100644 --- a/apps/api/tests/auth/user.test.ts +++ b/apps/api/tests/auth/user.test.ts @@ -441,6 +441,55 @@ describe('upsertUser', () => { // select must only have been called once (identity lookup, then early-return) expect(mockDb.select).toHaveBeenCalledTimes(1); }); + + // WR-01: upsertUser's fresh OIDC insert must set claimed=true. + // An OIDC-created user is identity-bound at insert time and must NOT be born + // with claimed=false, which would make it indistinguishable from a pending + // wizard bootstrap user (oidcIss IS NULL AND claimed=false) in the TOCTOU guard. + it('WR-01: fresh OIDC insert sets claimed=true (OIDC user is never a pending wizard user)', async () => { + const iss = 'https://auth.example.com'; + const sub = 'sub-wr01-claimed'; + + let selectCallCount = 0; + mockDb.select.mockImplementation(() => { + selectCallCount++; + if (selectCallCount === 1) return makeSelectChain([]); // identity lookup — not found + if (selectCallCount === 2) return makeSelectChain([]); // setup_complete — not set + if (selectCallCount === 3) { + // Used-colors query + return { from: vi.fn().mockResolvedValue([]) }; + } + if (selectCallCount === 4) { + // Admin COUNT + return makeSelectChain([{ count: 0 }]); + } + // Re-fetch after insert + return makeSelectChain([ + { + id: 20, + oidcIss: iss, + oidcSub: sub, + displayName: null, + color: COLOR_PALETTE[0], + isAdmin: true, + claimed: true, + createdAt: new Date(), + }, + ]); + }); + + mockDb.insert.mockReturnValue(makeInsertChain([{ id: 20 }])); + + await upsertUser(iss, sub); + + // The insert values must include claimed: true + const insertValues = mockDb.insert.mock.results[0]?.value?.values.mock.calls[0]?.[0]; + expect(insertValues).toBeDefined(); + expect(insertValues.claimed).toBe(true); + // And it must carry a real oidcIss (not null — distinguishable from wizard row) + expect(insertValues.oidcIss).toBe(iss); + expect(insertValues.oidcSub).toBe(sub); + }); }); // ── Plan 12-03: D-08 first-login-claims (Wave-2 GREEN — full test implementations) ── diff --git a/apps/api/tests/routes/setup.test.ts b/apps/api/tests/routes/setup.test.ts index ebe0fc5..0942c6e 100644 --- a/apps/api/tests/routes/setup.test.ts +++ b/apps/api/tests/routes/setup.test.ts @@ -478,6 +478,39 @@ describe('POST /api/setup/credential', () => { const body = JSON.parse(bodyText) as { error: string }; expect(body.error).toBe('Invalid request'); }); + + // WR-01: TOCTOU guard must ignore OIDC users that happen to have claimed=false + // (oidcIss IS NOT NULL). Only wizard bootstrap users (oidcIss IS NULL AND claimed=false) + // must trigger the DUPLICATE_UNCLAIMED guard. This prevents a partially-bootstrapped + // instance (where an OIDC user somehow exists pre-setup) from permanently blocking + // the wizard credential step with 409. + it('WR-01: TOCTOU guard ignores claimed=false OIDC users (oidcIss NOT NULL) — credential step still succeeds', async () => { + // Seed an OIDC user with claimed=false to simulate the latent bug scenario. + // After the WR-01 fix upsertUser always inserts claimed=true for OIDC users, but + // this tests that the guard's WHERE clause is narrowed correctly for defense-in-depth. + await db.insert(users).values({ + oidcIss: 'https://auth.test.setup', + oidcSub: `sub-wr01-oidc-claimed-false-${Date.now()}`, + displayName: 'OIDC User With claimed=false', + color: '#4A90D9', + isAdmin: false, + claimed: false, // legacy/hypothetical — oidcIss is NOT NULL + }); + + mockValidateCredentialShouldThrow = false; + + const app = await getApp(); + const res = await app.fetch( + jsonRequest('POST', '/api/setup/credential', { + fastmailEmail: 'operator@fastmail.com', + appPassword: 'valid-app-password-wr01', + }), + ); + // Must succeed — the OIDC-with-oidcIss row must NOT block the wizard + expect(res.status).toBe(200); + const body = (await res.json()) as { ok: boolean }; + expect(body.ok).toBe(true); + }); }); // ===========================================================================