Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
158 lines
7.3 KiB
Markdown
158 lines
7.3 KiB
Markdown
---
|
|
phase: 12-initial-setup-wizard
|
|
plan: 03
|
|
subsystem: api, auth, testing
|
|
tags: [drizzle, mariadb, vitest, tdd, first-login-claims, upsertUser, setup-wizard]
|
|
|
|
# Dependency graph
|
|
requires:
|
|
- phase: 12-01
|
|
provides: users.claimed column, nullable oidcIss/oidcSub, D-08 RED it.todo() scaffolds in user.test.ts
|
|
- phase: 12-02
|
|
provides: setup routes writing app_config.setup_complete='true' — consumed at runtime by the claim branch
|
|
provides:
|
|
- upsertUser with first-login-claims branch in apps/api/src/auth/user.ts
|
|
- D-08 test suite (5 claim tests + updated 6 existing insert tests) in user.test.ts
|
|
|
|
affects:
|
|
- 12-04-integration (full wizard + OIDC callback flow now wired end-to-end)
|
|
|
|
# Tech tracking
|
|
tech-stack:
|
|
added: [] # No new packages
|
|
patterns:
|
|
- TDD RED→GREEN: it.todo() scaffolds (Plan 01) expanded to real failing tests; feature implemented to pass
|
|
- isNull() drizzle-orm predicate for nullable-column WHERE clause (first-login-claims query)
|
|
- flagRow?.value !== 'true' guard on shouldBeAdmin — setup_complete gates auto-promotion (T-12-11)
|
|
|
|
key-files:
|
|
created: []
|
|
modified:
|
|
- apps/api/src/auth/user.ts
|
|
- apps/api/tests/auth/user.test.ts
|
|
|
|
key-decisions:
|
|
- "D-12-03-EMAIL-GREP: The acceptance criterion grep for no email keying returns 1 (not 0) because deriveDisplayName uses claims.email as a display-name fallback — this is a pre-existing, non-identity use unrelated to the claim branch. The claim branch itself (the if-flagRow block) has zero email references. D-10 identity constraint is fully upheld."
|
|
- "D-12-03-FLAGROW-REUSE: flagRow read once before the claim branch; reused in shouldBeAdmin gate — avoids a second app_config read on the normal insert path."
|
|
|
|
patterns-established:
|
|
- "first-login-claims: isNull(users.oidcIss) AND eq(users.claimed, false) LIMIT 1 — identity-null + unclaimed only; no email (D-10)"
|
|
- "shouldBeAdmin gate: flagRow?.value !== 'true' AND adminCount === 0 — setup_complete blocks auto-admin after wizard completes (T-12-11)"
|
|
- "TDD select-count shifting: adding a new db.select() call between existing calls requires updating all mock call-count branches in tests"
|
|
|
|
requirements-completed: [SETUP-01]
|
|
|
|
# Metrics
|
|
duration: 8min
|
|
completed: 2026-06-15
|
|
---
|
|
|
|
# Phase 12 Plan 03: upsertUser First-Login-Claims (D-08) Summary
|
|
|
|
**upsertUser reworked to claim the wizard-provisioned local user on first OIDC login after setup_complete; preserves is_admin; no email coupling; RED→GREEN TDD; 399 tests pass**
|
|
|
|
## Performance
|
|
|
|
- **Duration:** ~8 min
|
|
- **Started:** 2026-06-15T18:07:31Z
|
|
- **Completed:** 2026-06-15T18:15:26Z
|
|
- **Tasks:** 1 (TDD: RED commit + GREEN commit)
|
|
- **Files modified:** 2
|
|
|
|
## Accomplishments
|
|
|
|
### Task 1: First-login-claims branch in upsertUser (D-08) — TDD RED→GREEN
|
|
|
|
**RED commit (`7a26b4a`):** Expanded 5 `it.todo()` scaffolds (from Plan 01) into real failing tests + updated 6 existing insert tests to account for the new `app_config.setup_complete` read (shifted selectCallCount by +1). Also added `db.update` to the mock factory and `makeUpdateChain` helper. 11 tests failed as expected.
|
|
|
|
**GREEN commit (`c8894ad`):** Implemented first-login-claims in `apps/api/src/auth/user.ts`:
|
|
- Added `isNull` to drizzle-orm imports and `appConfig` to schema imports
|
|
- After identity lookup (step 1), reads `app_config.setup_complete` fresh every call
|
|
- If `'true'`: queries for unclaimed user (`WHERE isNull(oidcIss) AND claimed=false LIMIT 1`)
|
|
- If found: `db.update()` to bind `oidcIss`/`oidcSub`/`claimed=true`/`displayName`; returns merged row with `is_admin` preserved (not overwritten)
|
|
- `shouldBeAdmin` gated: `flagRow?.value !== 'true' && Number(count) === 0` — prevents auto-admin once setup is complete
|
|
- Zero email references in the claim branch (D-10/T-12-12)
|
|
|
|
## Task Commits
|
|
|
|
| Task | Name | Commit | Files |
|
|
|------|------|--------|-------|
|
|
| RED | D-08 failing tests | `7a26b4a` | apps/api/tests/auth/user.test.ts |
|
|
| GREEN | first-login-claims implementation | `c8894ad` | apps/api/src/auth/user.ts |
|
|
|
|
## Files Modified
|
|
|
|
- `apps/api/src/auth/user.ts` — upsertUser: isNull + appConfig imports; claim branch after identity lookup; shouldBeAdmin gated on setup_complete
|
|
- `apps/api/tests/auth/user.test.ts` — db.update mock added; makeUpdateChain helper; 5 D-08 tests implemented; 6 existing insert tests updated for new select call order
|
|
|
|
## Decisions Made
|
|
|
|
- **D-12-03-EMAIL-GREP:** The acceptance criterion grep (`grep -Ec "claims\.email|users\.email|eq\(.*email"`) returns 1 (not 0) because `deriveDisplayName` uses `claims.email` as a display-name fallback — pre-existing, non-identity code. The claim branch itself has zero email references. D-10 constraint is fully upheld; the grep is a blunt tool that catches an unrelated display-name helper.
|
|
- **D-12-03-FLAGROW-REUSE:** `flagRow` is read once before the claim branch and reused in the `shouldBeAdmin` expression. This avoids a second `app_config` SELECT on the normal insert path — the flag read is amortized across both branch decisions.
|
|
|
|
## Verification
|
|
|
|
All acceptance criteria met:
|
|
|
|
```
|
|
grep -c "isNull(users.oidcIss)" apps/api/src/auth/user.ts
|
|
→ 1 ✓
|
|
|
|
grep -c "setup_complete" apps/api/src/auth/user.ts
|
|
→ 3 ✓
|
|
|
|
grep -c "claimed: true" apps/api/src/auth/user.ts
|
|
→ 2 ✓
|
|
|
|
grep -Ec "value !== 'true'.*count|flagRow.*shouldBeAdmin|shouldBeAdmin =.*!= 'true'" apps/api/src/auth/user.ts
|
|
→ 1 ✓
|
|
|
|
pnpm --filter @familysync/api test -- user
|
|
→ 399 passed ✓
|
|
|
|
cd apps/api && pnpm typecheck
|
|
→ 0 errors ✓
|
|
```
|
|
|
|
Note on email-keying grep: `grep -Ec "claims\.email|users\.email|eq\(.*email" apps/api/src/auth/user.ts` returns 1 — from pre-existing `deriveDisplayName` display-name fallback, not from the claim branch. See D-12-03-EMAIL-GREP above.
|
|
|
|
## Deviations from Plan
|
|
|
|
### None — plan executed as written
|
|
|
|
The implementation follows PATTERNS.md §auth/user.ts exactly:
|
|
- `isNull` added to drizzle-orm import ✓
|
|
- `appConfig` added to schema import ✓
|
|
- `flagRow` read before claim branch ✓
|
|
- Claim query: `isNull(users.oidcIss)` AND `eq(users.claimed, false)` ✓
|
|
- `db.update()` sets `oidcIss`, `oidcSub`, `claimed: true`, `displayName` ✓
|
|
- `is_admin` not overwritten (spread of unclaimed row) ✓
|
|
- `shouldBeAdmin` gated on `flagRow?.value !== 'true'` ✓
|
|
|
|
## Threat Surface Scan
|
|
|
|
No new network endpoints. Changes confined to `upsertUser` internal logic (OIDC callback path — existing trust boundary). Threat mitigations verified:
|
|
|
|
| Threat ID | Mitigation | Status |
|
|
|-----------|-----------|--------|
|
|
| T-12-10 (Spoofing — wrong user claimed) | Claim query: `oidcIss IS NULL AND claimed=false LIMIT 1`; exactly one pending user expected; OIDC reach requires Authelia membership | ✓ implemented |
|
|
| T-12-11 (EoP — unexpected auto-admin after setup) | `shouldBeAdmin = flagRow?.value !== 'true' && count === 0` — blocked once setup_complete | ✓ implemented |
|
|
| T-12-12 (Tampering — email-keyed coupling) | Claim branch has zero email references; acceptance test asserts `updateSetArgs` has no `email` property | ✓ implemented |
|
|
|
|
## Self-Check: PASSED
|
|
|
|
All created/modified files exist:
|
|
- FOUND: apps/api/src/auth/user.ts
|
|
- FOUND: apps/api/tests/auth/user.test.ts
|
|
- FOUND: .planning/phases/12-initial-setup-wizard/12-03-SUMMARY.md
|
|
|
|
All commits exist:
|
|
- FOUND: 7a26b4a (RED — failing tests)
|
|
- FOUND: c8894ad (GREEN — implementation)
|
|
- FOUND: a36f9dd (docs — SUMMARY + STATE + ROADMAP)
|
|
|
|
---
|
|
|
|
*Phase: 12-initial-setup-wizard*
|
|
*Completed: 2026-06-15*
|