From 7a26b4aa06707ebdb238bdd31a2e6eac56e1a034 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Mon, 15 Jun 2026 14:12:50 -0400 Subject: [PATCH] test(12-03): D-08 first-login-claims failing tests (RED gate) - Expand 5 it.todo() scaffolds into real failing tests for first-login-claims - Add db.update mock to the mock factory; add makeUpdateChain helper - Update existing new-user insert tests to account for new app_config.setup_complete read (selectCallCount shift +1) - 11 tests fail: 5 D-08 claim tests + 6 existing insert tests await feature implementation Co-Authored-By: Claude Sonnet 4.6 --- apps/api/tests/auth/user.test.ts | 315 ++++++++++++++++++++++++++++--- 1 file changed, 293 insertions(+), 22 deletions(-) diff --git a/apps/api/tests/auth/user.test.ts b/apps/api/tests/auth/user.test.ts index e2738c4..627e63c 100644 --- a/apps/api/tests/auth/user.test.ts +++ b/apps/api/tests/auth/user.test.ts @@ -1,7 +1,7 @@ /** * Auth: upsertUser color round-robin + identity stability + first-login-wins is_admin * - * Tests for apps/api/src/auth/user.ts (Plan 02 + Plan 10-02) + * Tests for apps/api/src/auth/user.ts (Plan 02 + Plan 10-02 + Plan 12-03) * * Select call order for a NEW user insert (post Plan 10-02): * 1. Lookup by oidc_iss + oidc_sub (identity check) @@ -10,6 +10,13 @@ * 4. Re-fetch after insert (return full row) * * Existing-user (early-return) path remains at 1 select call (no change). + * + * Plan 12-03 additions — D-08 first-login-claims path: + * When setup_complete='true', a new oidc identity triggers claim lookup (step 1.5): + * 1. Lookup by oidc_iss + oidc_sub (no match for new identity) + * 1.5. Read app_config.setup_complete + * 1.6. If 'true': select unclaimed user (isNull(oidcIss) + claimed=false) → update + return + * 2+. Otherwise fall through to normal color / admin count / insert path */ import { describe, it, expect, vi, beforeEach } from 'vitest'; @@ -19,6 +26,7 @@ vi.mock('../../src/db/client.js', () => ({ db: { select: vi.fn(), insert: vi.fn(), + update: vi.fn(), }, })); @@ -29,6 +37,7 @@ import { upsertUser, COLOR_PALETTE } from '../../src/auth/user.js'; const mockDb = db as { select: ReturnType; insert: ReturnType; + update: ReturnType; }; // Chainable builder factory used in multiple tests @@ -52,6 +61,15 @@ function makeInsertChain(returningIdValue: { id: number }[]) { return chain; } +function makeUpdateChain() { + const chain = { + set: vi.fn(), + where: vi.fn().mockResolvedValue(undefined), + }; + chain.set.mockReturnValue(chain); + return chain; +} + describe('COLOR_PALETTE', () => { it('exports at least 4 distinct hex colors', () => { expect(COLOR_PALETTE).toBeDefined(); @@ -75,11 +93,12 @@ describe('upsertUser', () => { const iss = 'https://auth.example.com'; const sub = 'user-sub-001'; - // Select call order (new user, post Plan 10-02): + // Select call order (new user, post Plan 12-03): // 1. Lookup by iss+sub — not found - // 2. Used-colors query — no existing users → palette[0] - // 3. Zero-admin COUNT check — 0 admins → shouldBeAdmin=true - // 4. Re-fetch after insert — return the inserted row + // 2. app_config.setup_complete — not set (fallthrough to normal path) + // 3. Used-colors query — no existing users → palette[0] + // 4. Zero-admin COUNT check — 0 admins → shouldBeAdmin=true + // 5. Re-fetch after insert — return the inserted row let selectCallCount = 0; mockDb.select.mockImplementation(() => { selectCallCount++; @@ -88,12 +107,16 @@ describe('upsertUser', () => { return makeSelectChain([]); } if (selectCallCount === 2) { + // app_config.setup_complete — not set → normal insert path + return makeSelectChain([]); + } + if (selectCallCount === 3) { // Used-colors query — no existing users return { from: vi.fn().mockResolvedValue([]), }; } - if (selectCallCount === 3) { + if (selectCallCount === 4) { // Zero-admin COUNT check — 0 admins → first user becomes admin return makeSelectChain([{ count: 0 }]); } @@ -131,13 +154,17 @@ describe('upsertUser', () => { return makeSelectChain([]); // not found } if (selectCallCount === 2) { + // app_config.setup_complete — not set → normal insert path + return makeSelectChain([]); + } + if (selectCallCount === 3) { // Used-colors query — one existing user already holds palette[0], // so the next member must get the first unused color: palette[1]. return { from: vi.fn().mockResolvedValue([{ color: COLOR_PALETTE[0] }]), }; } - if (selectCallCount === 3) { + if (selectCallCount === 4) { // Zero-admin COUNT check — 1 admin already exists → shouldBeAdmin=false return makeSelectChain([{ count: 1 }]); } @@ -174,6 +201,10 @@ describe('upsertUser', () => { selectCallCount++; if (selectCallCount === 1) return makeSelectChain([]); // not found if (selectCallCount === 2) { + // app_config.setup_complete — not set → normal insert path + return makeSelectChain([]); + } + if (selectCallCount === 3) { // palette[0] and palette[2] in use; palette[1] is free return { from: vi @@ -181,7 +212,7 @@ describe('upsertUser', () => { .mockResolvedValue([{ color: COLOR_PALETTE[0] }, { color: COLOR_PALETTE[2] }]), }; } - if (selectCallCount === 3) { + if (selectCallCount === 4) { // Zero-admin COUNT check — admin exists → shouldBeAdmin=false return makeSelectChain([{ count: 1 }]); } @@ -238,10 +269,14 @@ describe('upsertUser', () => { selectCallCount++; if (selectCallCount === 1) return makeSelectChain([]); if (selectCallCount === 2) { + // app_config.setup_complete — not set → normal insert path + return makeSelectChain([]); + } + if (selectCallCount === 3) { // Used-colors query — no existing users return { from: vi.fn().mockResolvedValue([]) }; } - if (selectCallCount === 3) { + if (selectCallCount === 4) { // Zero-admin COUNT check return makeSelectChain([{ count: 0 }]); } @@ -305,10 +340,14 @@ describe('upsertUser', () => { selectCallCount++; if (selectCallCount === 1) return makeSelectChain([]); // not found if (selectCallCount === 2) { + // app_config.setup_complete — not set → normal insert path + return makeSelectChain([]); + } + if (selectCallCount === 3) { // Used-colors query — empty table return { from: vi.fn().mockResolvedValue([]) }; } - if (selectCallCount === 3) { + if (selectCallCount === 4) { // Zero-admin COUNT check — 0 admins → shouldBeAdmin=true return makeSelectChain([{ count: 0 }]); } @@ -344,10 +383,14 @@ describe('upsertUser', () => { selectCallCount++; if (selectCallCount === 1) return makeSelectChain([]); // not found if (selectCallCount === 2) { + // app_config.setup_complete — not set → normal insert path + return makeSelectChain([]); + } + if (selectCallCount === 3) { // Used-colors query — one existing user return { from: vi.fn().mockResolvedValue([{ color: COLOR_PALETTE[0] }]) }; } - if (selectCallCount === 3) { + if (selectCallCount === 4) { // Zero-admin COUNT check — 1 admin already exists → shouldBeAdmin=false return makeSelectChain([{ count: 1 }]); } @@ -400,41 +443,269 @@ describe('upsertUser', () => { }); }); -// ── Plan 12-01: D-08 first-login-claims scaffold (Wave-0 RED placeholders) ── +// ── Plan 12-03: D-08 first-login-claims (Wave-2 GREEN — full test implementations) ── // -// These tests cover the first-login-claims flow that Plan 12-02 implements in -// upsertUser. When setup_complete='true', the first OIDC login from an unknown -// iss+sub should "claim" the single unclaimed local user row (oidcIss IS NULL AND -// claimed=false), binding oidcIss/oidcSub and setting claimed=true. +// These tests cover the first-login-claims flow implemented in upsertUser. +// When setup_complete='true', the first OIDC login from an unknown iss+sub +// claims the single unclaimed local user row (oidcIss IS NULL AND claimed=false), +// binding oidcIss/oidcSub and setting claimed=true. // // Key constraints (D-08 / D-10): // - NEVER look up by email — only oidcIss+oidcSub and claimed=false // - Preserve is_admin on the claimed row (operator pre-set it in the wizard) // - Only claim when setup_complete='true' in app_config +// +// Select call order for the claim path: +// 1. Lookup by oidc_iss + oidc_sub (no match — new identity) +// 2. Read app_config.setup_complete (returns 'true') +// 3. Select unclaimed user (isNull(oidcIss) AND claimed=false) +// → db.update() to bind identity + set claimed=true +// → return merged row (is_admin preserved) -describe('upsertUser — D-08 first-login-claims (Wave-0 scaffold, Plan 12-02 implements)', () => { - it.todo( +describe('upsertUser — D-08 first-login-claims', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it( 'when setup_complete is true and an unclaimed local user exists (oidcIss IS NULL, claimed=false), ' + 'binds oidcIss+oidcSub+claimed=true and returns the updated row', + async () => { + const iss = 'https://auth.example.com'; + const sub = 'new-oidc-sub-001'; + + const unclaimedUser = { + id: 99, + oidcIss: null, + oidcSub: null, + displayName: 'Wizard User', + color: COLOR_PALETTE[0], + isAdmin: true, + claimed: false, + createdAt: new Date(), + }; + + // Select call order for claim path: + // 1. Identity lookup — no match (new iss+sub) + // 2. app_config.setup_complete — returns 'true' + // 3. Unclaimed user query — returns unclaimedUser + let selectCallCount = 0; + mockDb.select.mockImplementation(() => { + selectCallCount++; + if (selectCallCount === 1) return makeSelectChain([]); // identity lookup — no match + if (selectCallCount === 2) return makeSelectChain([{ value: 'true' }]); // setup_complete + return makeSelectChain([unclaimedUser]); // unclaimed user found + }); + + mockDb.update.mockReturnValue(makeUpdateChain()); + + const result = await upsertUser(iss, sub, 'New User'); + + // Must call update (not insert) to claim the row + expect(mockDb.update).toHaveBeenCalled(); + expect(mockDb.insert).not.toHaveBeenCalled(); + + // Returned row has new oidcIss/oidcSub and claimed=true + expect(result).toBeDefined(); + expect(result!.oidcIss).toBe(iss); + expect(result!.oidcSub).toBe(sub); + expect(result!.claimed).toBe(true); + // id matches the pre-existing unclaimed row + expect(result!.id).toBe(99); + }, ); - it.todo( + it( 'when setup_complete is true and claimed user row is found, ' + 'preserves is_admin on the claimed user (admin flag not overwritten)', + async () => { + const iss = 'https://auth.example.com'; + const sub = 'new-oidc-sub-002'; + + const unclaimedAdminUser = { + id: 100, + oidcIss: null, + oidcSub: null, + displayName: 'Admin Wizard', + color: COLOR_PALETTE[0], + isAdmin: true, // pre-set by wizard — must be preserved + claimed: false, + createdAt: new Date(), + }; + + let selectCallCount = 0; + mockDb.select.mockImplementation(() => { + selectCallCount++; + if (selectCallCount === 1) return makeSelectChain([]); // identity lookup — no match + if (selectCallCount === 2) return makeSelectChain([{ value: 'true' }]); // setup_complete + return makeSelectChain([unclaimedAdminUser]); // unclaimed admin user found + }); + + mockDb.update.mockReturnValue(makeUpdateChain()); + + const result = await upsertUser(iss, sub); + + // is_admin must be preserved from the unclaimed row — not set to false + expect(result!.isAdmin).toBe(true); + // Must have been claimed + expect(result!.claimed).toBe(true); + expect(result!.oidcIss).toBe(iss); + }, ); - it.todo( + it( 'when setup_complete is false (or unset), does NOT check for unclaimed rows — ' + 'falls through to normal insert path', + async () => { + const iss = 'https://auth.example.com'; + const sub = 'new-oidc-sub-fallthrough'; + + // Select call order for normal insert path (setup_complete NOT 'true'): + // 1. Identity lookup — no match + // 2. app_config.setup_complete — returns [] (no row → flagRow=undefined) + // 3. Used-colors query + // 4. Admin COUNT + // 5. Re-fetch after insert + let selectCallCount = 0; + mockDb.select.mockImplementation(() => { + selectCallCount++; + if (selectCallCount === 1) return makeSelectChain([]); // identity lookup + 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 — 0 → shouldBeAdmin=true + return makeSelectChain([{ count: 0 }]); + } + // Re-fetch after insert + return makeSelectChain([ + { + id: 50, + oidcIss: iss, + oidcSub: sub, + displayName: null, + color: COLOR_PALETTE[0], + isAdmin: true, + claimed: false, + createdAt: new Date(), + }, + ]); + }); + + mockDb.update.mockReturnValue(makeUpdateChain()); + mockDb.insert.mockReturnValue(makeInsertChain([{ id: 50 }])); + + const result = await upsertUser(iss, sub); + + // Normal insert path — should insert, not update (claim) + expect(mockDb.insert).toHaveBeenCalled(); + expect(result!.id).toBe(50); + // setup_complete was false → claim path never checked for unclaimed rows + // (update called at most once — could be called for displayName update on existing path, + // but here it's a new user so update should NOT be called for claiming) + // We simply confirm insert happened and a valid row returned + expect(result!.oidcIss).toBe(iss); + }, ); - it.todo( + it( 'when setup_complete is true but NO unclaimed local user exists, ' + 'falls through to normal insert path (new user row created)', + async () => { + const iss = 'https://auth.example.com'; + const sub = 'new-oidc-sub-no-unclaimed'; + + // Select call order (setup_complete=true, no unclaimed user): + // 1. Identity lookup — no match + // 2. app_config.setup_complete — returns 'true' + // 3. Unclaimed user query — returns [] (none found) + // 4. Used-colors query + // 5. Admin COUNT — admin already exists (setup is complete, claimed user is admin) + // 6. Re-fetch after insert + let selectCallCount = 0; + mockDb.select.mockImplementation(() => { + selectCallCount++; + if (selectCallCount === 1) return makeSelectChain([]); // identity lookup + if (selectCallCount === 2) return makeSelectChain([{ value: 'true' }]); // setup_complete + if (selectCallCount === 3) return makeSelectChain([]); // unclaimed user — none + if (selectCallCount === 4) { + // Used-colors query + return { from: vi.fn().mockResolvedValue([{ color: COLOR_PALETTE[0] }]) }; + } + if (selectCallCount === 5) { + // Admin COUNT — 1 admin exists (setup complete → existing admin from claim) + return makeSelectChain([{ count: 1 }]); + } + // Re-fetch after insert + return makeSelectChain([ + { + id: 60, + oidcIss: iss, + oidcSub: sub, + displayName: null, + color: COLOR_PALETTE[1], + isAdmin: false, // NOT admin because setup_complete=true && count !== 0 + claimed: false, + createdAt: new Date(), + }, + ]); + }); + + mockDb.update.mockReturnValue(makeUpdateChain()); + mockDb.insert.mockReturnValue(makeInsertChain([{ id: 60 }])); + + const result = await upsertUser(iss, sub); + + // Fell through to normal insert (no unclaimed user to claim) + expect(mockDb.insert).toHaveBeenCalled(); + expect(result!.id).toBe(60); + // setup_complete=true means shouldBeAdmin = false (even if count were 0) + const insertValues = mockDb.insert.mock.results[0]?.value?.values.mock.calls[0]?.[0]; + expect(insertValues.isAdmin).toBe(false); + }, ); - it.todo( + it( 'first-login-claims NEVER uses email as a lookup key — ' + 'identity is strictly oidcIss IS NULL AND claimed=false (D-10)', + async () => { + const iss = 'https://auth.example.com'; + const sub = 'new-oidc-sub-no-email'; + + const unclaimedUser = { + id: 101, + oidcIss: null, + oidcSub: null, + displayName: 'No Email User', + color: COLOR_PALETTE[0], + isAdmin: true, + claimed: false, + createdAt: new Date(), + }; + + let selectCallCount = 0; + mockDb.select.mockImplementation(() => { + selectCallCount++; + if (selectCallCount === 1) return makeSelectChain([]); // identity lookup + if (selectCallCount === 2) return makeSelectChain([{ value: 'true' }]); // setup_complete + return makeSelectChain([unclaimedUser]); // unclaimed user + }); + + mockDb.update.mockReturnValue(makeUpdateChain()); + + // Pass an email as displayName — it must NOT be used as a lookup key + await upsertUser(iss, sub, 'user@example.com'); + + // The update call sets must NOT contain any email-based where clause + // The update set must bind oidcIss + oidcSub; it must NOT set an email field + const updateSetArgs = mockDb.update.mock.results[0]?.value?.set.mock.calls[0]?.[0]; + expect(updateSetArgs).toBeDefined(); + expect(updateSetArgs).not.toHaveProperty('email'); + expect(updateSetArgs.oidcIss).toBe(iss); + expect(updateSetArgs.oidcSub).toBe(sub); + expect(updateSetArgs.claimed).toBe(true); + }, ); });