From b24fbbfde7e8caa1d86473ceb4711943c9cc5fc7 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sat, 13 Jun 2026 13:57:51 -0400 Subject: [PATCH] docs(10): create phase plan (4 plans, 4 waves) for admin-role-settings - 10-01 v1.1 DB foundation migration + dev-bypass admin seed - 10-02 requireAdmin guard + first-login-wins + /api/me extension (TDD) - 10-03 adminRouter credentials/shared-calendar + member self-service (TDD) - 10-04 PWA /admin route + nav gating + CredentialSheet + SetupBanner - filled 10-VALIDATION Per-Task Verification Map (Nyquist compliant) - finalized ROADMAP Phase 10 plan list --- .planning/ROADMAP.md | 7 +- .../10-admin-role-settings/10-01-PLAN.md | 192 +++++++++++++++ .../10-admin-role-settings/10-02-PLAN.md | 204 ++++++++++++++++ .../10-admin-role-settings/10-03-PLAN.md | 225 ++++++++++++++++++ .../10-admin-role-settings/10-04-PLAN.md | 205 ++++++++++++++++ .../10-admin-role-settings/10-VALIDATION.md | 68 ++++-- 6 files changed, 876 insertions(+), 25 deletions(-) create mode 100644 .planning/phases/10-admin-role-settings/10-01-PLAN.md create mode 100644 .planning/phases/10-admin-role-settings/10-02-PLAN.md create mode 100644 .planning/phases/10-admin-role-settings/10-03-PLAN.md create mode 100644 .planning/phases/10-admin-role-settings/10-04-PLAN.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 5366af2..87e155e 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -170,7 +170,12 @@ Plans: **Folded-in scope** (from backlog 999.5, self-service member onboarding): the credential surface this phase builds is the same one a member needs on first login. Expose a `needsProviderSetup` signal (member has no `member_credentials` row) and let a member enter/validate (CalDAV PROPFIND) + encrypt their **own** Fastmail app password — the self-service counterpart of the admin-managed flow, sharing the validation/encryption/initial-sync path. Non-technical-friendly instructions (link to Fastmail's app-password page, required Calendars/CalDAV scope) are the hard UX constraint. Member-scoped: a member can only set their own credential; never log/echo the password. -**Plans**: TBD +**Plans**: 4 plans (4 waves) +Plans: +- [ ] 10-01-PLAN.md — v1.1 DB foundation migration (is_admin, provider_type+unique, reminder_lead_minutes, app_config) + dev-bypass admin seed +- [ ] 10-02-PLAN.md — requireAdmin guard + first-login-wins bootstrap + /api/me isAdmin/needsProviderSetup (TDD) +- [ ] 10-03-PLAN.md — adminRouter (members/credentials/calendars/shared) + member self-service credential, validate→encrypt→sync (TDD) +- [ ] 10-04-PLAN.md — PWA /admin route + nav gating + CredentialSheet + SetupBanner (playwright-cli verified) **UI hint**: yes ### Phase 11: Per-Event Reminders diff --git a/.planning/phases/10-admin-role-settings/10-01-PLAN.md b/.planning/phases/10-admin-role-settings/10-01-PLAN.md new file mode 100644 index 0000000..946b015 --- /dev/null +++ b/.planning/phases/10-admin-role-settings/10-01-PLAN.md @@ -0,0 +1,192 @@ +--- +phase: 10-admin-role-settings +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - apps/api/src/db/schema.ts + - apps/api/src/db/migrations/0001_v1_1_foundation.sql + - apps/api/src/db/migrations/meta/_journal.json + - apps/pwa/e2e/global-setup.ts +autonomous: true +requirements: [ADMIN-01, ADMIN-02, ADMIN-03] +must_haves: + truths: + - "The live dev MariaDB has users.is_admin (boolean, default false), member_credentials.provider_type (varchar, default 'caldav'), member_credentials UNIQUE(user_id), calendar_events.reminder_lead_minutes (int, nullable), and an app_config table" + - "Running db:generate then db:migrate applies the migration with no destructive (DROP/TRUNCATE) statement" + - "The e2e dev-bypass user (id=1) exists in the users table with is_admin=true so requireAdmin admits it" + artifacts: + - path: "apps/api/src/db/schema.ts" + provides: "v1.1 schema: users.isAdmin, memberCredentials.providerType + unique(user_id), calendarEvents.reminderLeadMinutes, appConfig table" + contains: "appConfig" + - path: "apps/api/src/db/migrations/0001_v1_1_foundation.sql" + provides: "generated ALTER/CREATE DDL for the v1.1 bundle" + contains: "is_admin" + - path: "apps/pwa/e2e/global-setup.ts" + provides: "seeds users row id=1 with is_admin=true for the dev-bypass admin UI verification path" + contains: "is_admin" + key_links: + - from: "apps/api/src/db/schema.ts" + to: "apps/api/src/db/migrations/0001_v1_1_foundation.sql" + via: "drizzle-kit generate" + pattern: "is_admin" + - from: "apps/pwa/e2e/global-setup.ts" + to: "users table" + via: "INSERT seed of id=1 is_admin=true" + pattern: "is_admin" +--- + + +Ship the v1.1 DB foundation migration that all of Phase 10 (and Phases 11/12 downstream) build on: add `users.is_admin`, `member_credentials.provider_type` (the generic provider discriminator, D-04) + a `UNIQUE(user_id)` constraint (D-05), `calendar_events.reminder_lead_minutes`, and a new `app_config` table — in one drizzle-kit `generate`+`migrate` migration (NEVER `push`). Seed the dev-bypass user (id=1) as an admin (D-01 dev note) so local/e2e admin-UI verification works. + +Purpose: Every subsequent Phase 10 plan reads these columns (requireAdmin reads `is_admin`, the credential routes read `provider_type` and rely on the per-user UNIQUE for upsert, `/api/me` reads `is_admin`). `reminder_lead_minutes` is created-now / consumed by Phase 11; `app_config.setup_complete` is created-now / consumed by Phase 12. This plan is the head of the wave chain. +Output: Edited `schema.ts`, a generated `0001_v1_1_foundation.sql` migration file (committed artifact) + updated `_journal.json`, applied to the live dev DB, and a seeded admin row for the e2e dev-bypass user. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/10-admin-role-settings/10-CONTEXT.md +@.planning/phases/10-admin-role-settings/10-RESEARCH.md +@.planning/phases/10-admin-role-settings/10-PATTERNS.md + + + + + + Task 1: Extend schema.ts with the v1.1 column/table bundle + apps/api/src/db/schema.ts + + - apps/api/src/db/schema.ts (the file being modified — read the full file; `users` ~line 34, `memberCredentials` lines 55–68, `calendarEvents` ~lines 120–130 incl. `allDay` line 127, `calendars.isShared` line 89, `pushSubscriptions` lines 236–257 for the single-table pattern, the `unique`/`index` import + usage) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/api/src/db/schema.ts` (concrete column excerpts + line numbers: copy `allDay` boolean pattern for `is_admin`, `fastmailEmail` varchar pattern for `provider_type`, `pushSubscriptions` table pattern for `app_config`, `calendars` unique pattern for `uniq_member_credential_user`) + - .planning/phases/10-admin-role-settings/10-RESEARCH.md §Pattern 6 (provider discriminator, D-04) + §Code Examples "Drizzle Upsert Pattern" schema note (UNIQUE(user_id) rationale) + + + In apps/api/src/db/schema.ts make exactly four additive changes, matching the existing column idiom (per 10-PATTERNS.md excerpts — do NOT inline new code styles): + 1. `users` table: add `isAdmin: boolean('is_admin').default(false).notNull()` (copy the `allDay` boolean idiom). + 2. `memberCredentials` table: add `providerType: varchar('provider_type', { length: 64 }).notNull().default('caldav')` — the generic provider discriminator per D-04 (generic provider shape, Fastmail/CalDAV the only implemented provider; default 'caldav' for existing rows; no second provider built here). And add `unique('uniq_member_credential_user').on(t.userId)` to the table's index array (keep the existing `idx_member_credentials_user_id` index) — this enforces one-credential-per-member per D-05 and enables `onDuplicateKeyUpdate` upsert. + 3. `calendarEvents` table: add `reminderLeadMinutes: int('reminder_lead_minutes')` (nullable — no `.notNull()`; created-now / consumed by Phase 11). + 4. New `appConfig` table (export `const appConfig`), following the single-table `pushSubscriptions` idiom: a single key/value config — `key: varchar('key', { length: 128 }).primaryKey()`, `value: text('value')` (nullable), `updatedAt: timestamp('updated_at').defaultNow().onUpdateNow()`. This holds `setup_complete` (created-now / consumed by Phase 12 — do NOT add setup_complete gating logic here, only the table). Add a `setup_complete` semantics comment so Phase 12 can read/write the `setup_complete` key without a reshape. + Do NOT touch `calendars.is_shared` (already exists, line 89). Do NOT change crypto, encrypted_password, or any existing column. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/api exec tsc --noEmit 2>&1 | tail -5 + + + - `apps/api/src/db/schema.ts` contains `is_admin`, `provider_type`, `uniq_member_credential_user`, `reminder_lead_minutes`, and an exported `appConfig` table with `key`/`value`/`updated_at`. + - `grep -c "export const appConfig" apps/api/src/db/schema.ts` returns 1. + - `pnpm --filter @familysync/api exec tsc --noEmit` exits 0 (types compile — note this passes WITHOUT the migration, because Drizzle types come from schema.ts; column existence is verified in Task 2). + - `is_admin` uses `.default(false).notNull()`; `provider_type` uses `.notNull().default('caldav')`; `reminder_lead_minutes` is nullable (no `.notNull()`). + + schema.ts holds all four v1.1 additions, typechecks clean, no existing column altered. + + + + Task 2: [BLOCKING] Generate + migrate the v1.1 migration against the live dev DB + apps/api/src/db/migrations/0001_v1_1_foundation.sql, apps/api/src/db/migrations/meta/_journal.json + + - apps/api/src/db/migrations/0000_baseline.sql (existing migration format — `--> statement-breakpoint` between DDL statements; the format the generated file must follow) + - apps/api/drizzle.config.ts (migration output dir + env-driven DB credentials) + - apps/api/package.json (the `db:generate` / `db:migrate` scripts) + - .planning/phases/10-admin-role-settings/10-RESEARCH.md §Pattern 3 (generate+migrate workflow, exact commands, DB_HOST=127.0.0.1 dev override) + §Pitfall 4 (why never push) + - MEMORY note [[drizzle-mariadb-push-unsafe]] context in 10-CONTEXT.md Claude's Discretion (generate+migrate, never push) + + + BLOCKING — this must run AFTER Task 1 (schema.ts complete) and BEFORE any plan that reads the new columns. Bring up the dev MariaDB if not already bound on 3306 (per [[dev-stack-bringup]]: dev compose override exposes 3306). Then: + 1. Run `pnpm --filter @familysync/api db:generate` to produce `apps/api/src/db/migrations/0001_v1_1_foundation.sql` (drizzle-kit names it; the actual filename may differ — commit whatever drizzle-kit emits as the next sequential migration) and update `meta/_journal.json`. DO NOT hand-write the SQL. + 2. INSPECT the generated SQL: it MUST be only `ALTER TABLE ... ADD COLUMN` / `ADD UNIQUE` / `CREATE TABLE` statements (additive). If it contains any `DROP TABLE`, `DROP COLUMN`, or `TRUNCATE`, STOP — that is the false-destructive-diff trap; do NOT apply it, and do NOT fall back to `db:push`. Re-derive from schema.ts. + 3. Apply with `DB_HOST=127.0.0.1` (+ dev DB_USER/DB_PASSWORD/DB_NAME from .env): `cd apps/api && set -a; source ../../.env; set +a; DB_HOST=127.0.0.1 pnpm db:migrate`. NEVER `db:push`. + 4. Verify the live columns/table exist via a mysql2 query (not just tsc): assert `is_admin` on `users`, `provider_type` + the unique index on `member_credentials`, `reminder_lead_minutes` on `calendar_events`, and the `app_config` table. + + + cd /home/luc/Projects/familysync && set -a; source .env 2>/dev/null; set +a; DB_HOST=127.0.0.1 node -e "const m=require('mysql2/promise');(async()=>{const c=await m.createConnection({host:'127.0.0.1',port:Number(process.env.DB_PORT||3306),user:process.env.DB_USER||'familysync',password:process.env.DB_PASSWORD||'',database:process.env.DB_NAME||'familysync'});const[u]=await c.query(\"SHOW COLUMNS FROM users LIKE 'is_admin'\");const[mc]=await c.query(\"SHOW COLUMNS FROM member_credentials LIKE 'provider_type'\");const[ce]=await c.query(\"SHOW COLUMNS FROM calendar_events LIKE 'reminder_lead_minutes'\");const[ac]=await c.query(\"SHOW TABLES LIKE 'app_config'\");if(u.length&&mc.length&&ce.length&&ac.length){console.log('MIGRATION OK');process.exit(0)}console.error('MISSING',{u:u.length,mc:mc.length,ce:ce.length,ac:ac.length});process.exit(1)})().catch(e=>{console.error(e.message);process.exit(1)})" + + + - A new migration `.sql` file exists under `apps/api/src/db/migrations/` (sequential after `0000_baseline.sql`) and `meta/_journal.json` references it. + - The generated SQL contains NO `DROP TABLE`, `DROP COLUMN`, or `TRUNCATE` statement: `grep -v '^--' | grep -ciE 'drop (table|column)|truncate'` returns 0. + - The live dev MariaDB query above prints `MIGRATION OK` and exits 0 — `users.is_admin`, `member_credentials.provider_type`, the `member_credentials` unique on `user_id`, `calendar_events.reminder_lead_minutes`, and the `app_config` table all exist. + - `db:push` was NOT run (no push in command history for this task). + + The v1.1 migration is generated (additive-only), committed, and applied to the live dev DB; all new columns/table verified present by a real DB query. + + + + Task 3: Seed the dev-bypass user (id=1) as admin in the e2e global-setup + apps/pwa/e2e/global-setup.ts + + - apps/pwa/e2e/global-setup.ts (the file being modified — read the full file; the FK-checks-off TRUNCATE block, the `INSERT IGNORE INTO calendars (id, user_id, ...)` idempotent pattern lines 113–117, the mysql2 connection setup) + - apps/api/src/auth/devBypass.ts (DEV_USER id=1, displayName 'Dev User', color '#4A90D9' — the seed row must match this identity so /api/me dev path and the seeded DB row agree) + - apps/api/src/db/schema.ts users table (oidc_iss / oidc_sub NOT NULL, color NOT NULL — the seed INSERT must supply non-null values for the required columns) + - .planning/phases/10-admin-role-settings/10-RESEARCH.md §Pattern 5 "DEV_AUTH_BYPASS user-1 admin acquisition" (seed approach recommended) + §Pitfall 3 (why requireAdmin 403s without this seed) + + + The dev-bypass path injects DEV_USER (id=1) WITHOUT a DB upsert, so the `users` table has no row for id=1 — `requireAdmin` (Plan 02) does a DB lookup and would 403 the bypass admin UI locally and in e2e. Fix the seed per the D-01 dev note: in `apps/pwa/e2e/global-setup.ts`, inside the seed block (after the FK-checks-on, mirroring the existing `INSERT IGNORE INTO calendars` idempotent idiom), add an idempotent seed of the dev user row: `INSERT INTO users (id, oidc_iss, oidc_sub, display_name, color, is_admin) VALUES (1, 'dev-bypass', 'dev-user-1', 'Dev User', '#4A90D9', true) ON DUPLICATE KEY UPDATE is_admin=true`. Supply placeholder non-null oidc_iss/oidc_sub values (the bypass path never reads them; the row only needs to satisfy the NOT NULL constraints and carry is_admin=true). Keep it idempotent so re-runs converge. Do not change the existing TRUNCATE/event/list seeds. Add a comment that this row gives the dev-bypass admin UI a real `is_admin=true` row for requireAdmin's DB lookup. + + + cd /home/luc/Projects/familysync && grep -c "is_admin" apps/pwa/e2e/global-setup.ts + + + - `apps/pwa/e2e/global-setup.ts` contains an `INSERT INTO users` ... `is_admin` seed for id=1 with `ON DUPLICATE KEY UPDATE` (idempotent). + - `grep -c "is_admin" apps/pwa/e2e/global-setup.ts` returns >= 1. + - The seed supplies non-null `oidc_iss`, `oidc_sub`, and `color` (satisfies users NOT NULL constraints). + - The existing calendar/event/list seeds are unchanged (the `INSERT IGNORE INTO calendars` and `Seeded Test Event` anchors still present). + + global-setup seeds users id=1 with is_admin=true idempotently; the dev-bypass admin UI path now has a DB row requireAdmin will admit. + + + + + +This plan creates the following new symbols/files (excluded from drift verification — they do not exist before this plan): +- `users.is_admin` column (boolean, default false) +- `member_credentials.provider_type` column (varchar(64), default 'caldav') — D-04 generic provider discriminator +- `member_credentials` UNIQUE constraint `uniq_member_credential_user` on `user_id` +- `calendar_events.reminder_lead_minutes` column (int, nullable) — consumed by Phase 11 +- `app_config` table (`key` PK, `value`, `updated_at`) — `setup_complete` key consumed by Phase 12 +- exported `appConfig` Drizzle table in `apps/api/src/db/schema.ts` +- generated migration SQL file `apps/api/src/db/migrations/0001_v1_1_foundation.sql` (or drizzle-kit's emitted sequential name) + `_journal.json` entry +- seeded `users` row id=1 with `is_admin=true` in `apps/pwa/e2e/global-setup.ts` + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| schema.ts → live MariaDB | DDL applied to a populated production-shaped DB; a wrong (destructive) migration could drop/truncate live data | +| e2e seed → DB | global-setup TRUNCATEs + INSERTs against whatever DB_* points at | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-10-01 | Tampering | drizzle-kit migration on populated MariaDB | mitigate | Task 2 uses generate+migrate (never push); blocks on any DROP/TRUNCATE in the generated SQL (acceptance grep == 0) and verifies columns via a live DB query, not just tsc (false-positive trap) | +| T-10-02 | Tampering | e2e global-setup TRUNCATE against wrong DB | accept | Pre-existing fail-closed guards (NODE_ENV=production refusal + DEV_AUTH_BYPASS=true requirement) already gate the seed; this plan only adds an idempotent INSERT, no new TRUNCATE surface | +| T-10-03 | Elevation of Privilege | dev-bypass user gains is_admin | accept | Seed grants is_admin=true ONLY to id=1 ONLY in the dev/e2e bypass DB (guarded by DEV_AUTH_BYPASS + non-production); production users acquire is_admin via first-login-wins (Plan 02), never via this seed | +| T-10-SC | Tampering | npm/pip/cargo installs | mitigate | No new packages installed this phase (RESEARCH Package Legitimacy Audit: all already in lockfile); no install task, so no [ASSUMED]/[SUS] checkpoint needed | + + + +- `pnpm --filter @familysync/api exec tsc --noEmit` passes (schema typechecks). +- The live DB query in Task 2 prints `MIGRATION OK`. +- The generated migration SQL is additive-only (no DROP/TRUNCATE). +- `grep -c is_admin apps/pwa/e2e/global-setup.ts` >= 1. + + + +- All four v1.1 schema items exist in the live dev MariaDB (verified by query, not types). +- Migration applied via generate+migrate; SQL committed; journal updated; no push used. +- Dev-bypass user id=1 seeded as admin so downstream requireAdmin verification works. +- Supports phase Success Criterion 5 (DB migration in place for downstream phases). + + + +Create `.planning/phases/10-admin-role-settings/10-01-SUMMARY.md` when done. + diff --git a/.planning/phases/10-admin-role-settings/10-02-PLAN.md b/.planning/phases/10-admin-role-settings/10-02-PLAN.md new file mode 100644 index 0000000..1cdc14e --- /dev/null +++ b/.planning/phases/10-admin-role-settings/10-02-PLAN.md @@ -0,0 +1,204 @@ +--- +phase: 10-admin-role-settings +plan: 02 +type: tdd +wave: 2 +depends_on: ["10-01"] +files_modified: + - apps/api/src/lib/requireAdmin.ts + - apps/api/tests/lib/requireAdmin.test.ts + - apps/api/src/auth/user.ts + - apps/api/tests/auth/user.test.ts + - apps/api/src/routes/me.ts + - apps/api/tests/routes/me.test.ts +autonomous: true +requirements: [ADMIN-03] +must_haves: + truths: + - "requireAdmin returns 403 for an authenticated non-admin user and calls next() for an admin user (role read from the DB, never a client flag)" + - "On first login when zero admins exist, upsertUser flags the new user is_admin=true; subsequent users are normal members" + - "GET /api/me returns isAdmin and needsProviderSetup for both the dev-bypass path and the OIDC path" + artifacts: + - path: "apps/api/src/lib/requireAdmin.ts" + provides: "MiddlewareHandler that reads c.get('user').id, looks up users.is_admin in the DB, 403s non-admins" + exports: ["requireAdmin"] + min_lines: 15 + - path: "apps/api/src/auth/user.ts" + provides: "upsertUser extended with first-login-wins is_admin bootstrap (zero-admins → first user is admin)" + contains: "isAdmin" + - path: "apps/api/src/routes/me.ts" + provides: "/api/me response extended with isAdmin + needsProviderSetup (both dev-bypass and OIDC paths)" + contains: "needsProviderSetup" + key_links: + - from: "apps/api/src/lib/requireAdmin.ts" + to: "users.is_admin" + via: "Drizzle select where eq(users.id, userId)" + pattern: "users\\.isAdmin" + - from: "apps/api/src/routes/me.ts" + to: "member_credentials" + via: "needsProviderSetup = no member_credentials row for the user" + pattern: "memberCredentials" +--- + + +Build the server-side admin role primitives that ADMIN-03 depends on: the `requireAdmin` MiddlewareHandler (DB-backed role check, always server-enforced), the first-login-wins `is_admin` bootstrap in `upsertUser` (D-01), and the `/api/me` extension exposing `isAdmin` + `needsProviderSetup` (D-03) for both the dev-bypass and OIDC code paths. TDD: each behavior has a defined input→output contract, so write the failing test first. + +Purpose: `requireAdmin` is the single security boundary for every `/api/admin/*` route (Plan 03 mounts it). `isAdmin` on `/api/me` drives PWA nav gating (Plan 04, UX-only). `needsProviderSetup` drives the member self-service banner (Plan 04). First-login-wins is written so Phase 12 can later tighten it to "first login after setup_complete" without a rewrite. +Output: New `requireAdmin.ts` + tests, extended `user.ts` + tests, extended `me.ts` + tests. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/10-admin-role-settings/10-CONTEXT.md +@.planning/phases/10-admin-role-settings/10-RESEARCH.md +@.planning/phases/10-admin-role-settings/10-PATTERNS.md +@.planning/phases/10-admin-role-settings/10-01-SUMMARY.md + + + + + + Task 1: requireAdmin middleware (RED→GREEN→REFACTOR) + apps/api/src/lib/requireAdmin.ts, apps/api/tests/lib/requireAdmin.test.ts + + - apps/api/src/auth/devBypass.ts (MiddlewareHandler signature + the `c.get('user')`/`c.set('user', DEV_USER)` pattern + the `ContextVariableMap` augmentation, line ~46; the side-effect import idiom) + - apps/api/src/lib/ (sibling lib modules — e.g. listAccess.ts — for the lib-file import/style conventions) + - apps/api/tests/lib/ + apps/api/tests/auth/devBypass.test.ts (existing test idioms: how a Hono app/middleware is exercised, how c.get('user') is stubbed, how DB is reached in tests per [[api-integration-test-db]]) + - apps/api/src/db/schema.ts users.isAdmin (from Plan 01) + apps/api/src/db/client.ts (the `db` export) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/api/src/lib/requireAdmin.ts` (the exact MiddlewareHandler shape, the DB lookup excerpt, the `import '../auth/devBypass.js'` side-effect import) + 10-RESEARCH.md §Pattern 1 (Pitfall 9) + §Pitfall 3 + + + - Test (RED): an authenticated user whose DB row has is_admin=false → requireAdmin responds 403 `{ error: 'Forbidden' }` and does NOT call next(). + - Test: an authenticated user whose DB row has is_admin=true → requireAdmin calls next() (request proceeds). + - Test: no resolved user on context (c.get('user') undefined) → 403 (never throws). + - Test: the role is read from the DB (users.is_admin), NOT from any value on c.get('user') — a context user object claiming isAdmin=true but with a non-admin DB row is still 403 (defence: bypass only skips OIDC, not the DB check). + + + Create `apps/api/src/lib/requireAdmin.ts` exporting `requireAdmin: MiddlewareHandler` (per 10-PATTERNS.md excerpt): read `c.get('user')?.id`; if no id → `c.json({ error: 'Forbidden' }, 403)`; else `db.select({ isAdmin: users.isAdmin }).from(users).where(eq(users.id, userId)).limit(1)`; if `!row?.isAdmin` → 403; else `await next()`. Include the `import '../auth/devBypass.js'` side-effect import for the ContextVariableMap augmentation. NEVER log the user object or any credential. Write `apps/api/tests/lib/requireAdmin.test.ts` FIRST (the four behaviors above), confirm RED, then implement to GREEN. Follow the real-DB test conventions in [[api-integration-test-db]] (tests live in tests/, DB_HOST=127.0.0.1 override) if the test exercises the live DB; otherwise stub the db module. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- requireAdmin 2>&1 | tail -15 + + + - `apps/api/src/lib/requireAdmin.ts` exports `requireAdmin` typed as `MiddlewareHandler`. + - The 403 response body is `{ error: 'Forbidden' }` (HTTP 403) for a non-admin authenticated user. + - The role decision reads `users.isAdmin` from the DB (`grep -q "users.isAdmin" apps/api/src/lib/requireAdmin.ts`); it does NOT branch on a property of `c.get('user')` other than `.id`. + - No `console.log`/`console.error` of the user object or credentials in the file. + - `pnpm --filter @familysync/api test -- requireAdmin` passes all four cases. + + requireAdmin guard exists, DB-backed, 403s non-admins, tests green. + + + + Task 2: First-login-wins is_admin bootstrap in upsertUser (RED→GREEN→REFACTOR) + apps/api/src/auth/user.ts, apps/api/tests/auth/user.test.ts + + - apps/api/src/auth/user.ts (the file being modified — the full `upsertUser` function lines 76–128: the existing-row early-return path, the color assignment, the INSERT `.values({...}).$returningId()` block lines 112–122) + - apps/api/tests/auth/user.test.ts (existing upsertUser test idioms — how it seeds/asserts DB state, the real-DB test setup) + - apps/api/src/db/schema.ts users.isAdmin (from Plan 01) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/api/src/auth/user.ts` (the zero-admin COUNT check + `isAdmin: shouldBeAdmin` in `.values()`, the `import { sql }` addition) + 10-RESEARCH.md §Pattern 5 (Phase-12-safe first-login-wins, the "zero admins exist" check that P12 tightens to "after setup_complete") + + + - Test (RED): upsertUser inserting a brand-new user when the users table has ZERO admins → the inserted row has is_admin=true. + - Test: upsertUser inserting a new user when an admin already exists → the inserted row has is_admin=false. + - Test: upsertUser for an EXISTING user (oidc_iss+oidc_sub already present) → is_admin is NOT changed by the upsert (the early-return path is untouched; promotion/demotion is not this function's job). + + + In `apps/api/src/auth/user.ts`, before the INSERT in `upsertUser` (after color assignment), add a zero-admin check (per 10-PATTERNS.md excerpt): `db.select({ count: sql\`COUNT(*)\` }).from(users).where(eq(users.isAdmin, true))`; `shouldBeAdmin = Number(count) === 0`; pass `isAdmin: shouldBeAdmin` in the INSERT `.values({...})`. Add `import { sql } from 'drizzle-orm'` if absent. Leave the existing-user early-return path unchanged (do NOT toggle is_admin for existing users). Add a comment marking this as the Phase-12 hook point: "first user when zero admins exist (D-01); Phase 12 tightens to first user after app_config.setup_complete". Write the three test cases in `apps/api/tests/auth/user.test.ts` FIRST, confirm RED, implement to GREEN. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- user 2>&1 | tail -15 + + + - A new user inserted with zero pre-existing admins has `is_admin=true`; with an existing admin, `is_admin=false`. + - The existing-user early-return path does not modify is_admin (test asserts unchanged). + - `grep -q "isAdmin" apps/api/src/auth/user.ts` and the INSERT `.values()` includes `isAdmin`. + - A comment in `user.ts` names the Phase-12 tightening hook (first login after setup_complete). + - `pnpm --filter @familysync/api test -- user` passes all cases. + + First-login-wins bootstrap writes is_admin on first insert, member-count-agnostic, Phase-12-safe, tests green. + + + + Task 3: Extend /api/me with isAdmin + needsProviderSetup (RED→GREEN→REFACTOR) + apps/api/src/routes/me.ts, apps/api/tests/routes/me.test.ts + + - apps/api/src/routes/me.ts (the file being modified — the dev-bypass short-circuit lines 31–43 returning `{ user: { id, displayName, color } }`, the OIDC path lines 60–75 returning the resolved user; both must add isAdmin + needsProviderSetup) + - apps/api/tests/routes/me.test.ts (existing /api/me test idioms — both dev-bypass and OIDC response assertions) + - apps/api/src/db/schema.ts users.isAdmin + memberCredentials (from Plan 01) + apps/api/src/db/client.ts (`db`) + - apps/api/src/auth/user.ts (the resolved `user` shape returned by upsertUser — confirm it now carries isAdmin after Task 2; if not selected, me.ts must select users.isAdmin itself) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/api/src/routes/me.ts` (the needsProviderSetup lookup excerpt: `db.select({id: memberCredentials.id}).from(memberCredentials).where(eq(memberCredentials.userId, userId)).limit(1)` → `needsProviderSetup = !cred`) + 10-RESEARCH.md §Code Examples "/api/me Response Extension" + §Open Questions #3 (needsProviderSetup lives on /api/me) + + + - Test (RED): dev-bypass path (DEV_USER id=1) → response `user` includes `isAdmin` (looked up from the DB row for id=1, NOT hardcoded) and `needsProviderSetup` (true iff no member_credentials row for id=1). + - Test: OIDC path → response `user` includes `isAdmin` (from the resolved users row) and `needsProviderSetup` (member_credentials existence for that user id). + - Test: a user WITH a member_credentials row → needsProviderSetup=false; a user WITHOUT one → needsProviderSetup=true. + + + In `apps/api/src/routes/me.ts`, extend BOTH response paths to include `isAdmin` and `needsProviderSetup`. The dev-bypass path currently short-circuits without a DB lookup — it MUST now query `users.isAdmin` for id=1 (same lookup as requireAdmin) rather than hardcoding, and compute `needsProviderSetup` via the member_credentials existence check (per 10-PATTERNS.md excerpt). The OIDC path uses the resolved user's isAdmin + the same member_credentials existence check. Add the `eq`/`db`/`users`/`memberCredentials` imports as needed. Do NOT add the self-service `/api/me/credential` POST endpoint here — that belongs to Plan 03. Write the three test cases in `apps/api/tests/routes/me.test.ts` FIRST, confirm RED, implement to GREEN. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- me 2>&1 | tail -15 + + + - `GET /api/me` response `user` object contains `isAdmin` (boolean) and `needsProviderSetup` (boolean) on BOTH the dev-bypass and OIDC paths. + - The dev-bypass path's isAdmin is read from the DB (`grep -q "memberCredentials" apps/api/src/routes/me.ts` and isAdmin not hardcoded `true`/`false` in the dev path). + - needsProviderSetup is true exactly when no member_credentials row exists for the user. + - No `/api/me/credential` POST route added in this plan. + - `pnpm --filter @familysync/api test -- me` passes all cases. + + /api/me exposes isAdmin + needsProviderSetup on both paths, DB-backed, tests green. + + + + + +New symbols/files created by this plan (excluded from drift verification): +- `apps/api/src/lib/requireAdmin.ts` exporting `requireAdmin` (MiddlewareHandler) +- `apps/api/tests/lib/requireAdmin.test.ts` +- first-login-wins `is_admin` bootstrap branch in `upsertUser` (`apps/api/src/auth/user.ts`) +- `isAdmin` + `needsProviderSetup` fields on the `/api/me` response (`apps/api/src/routes/me.ts`) +- test additions in `apps/api/tests/auth/user.test.ts` and `apps/api/tests/routes/me.test.ts` + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| client → /api/admin/* (via requireAdmin) | untrusted authenticated request must be proven admin server-side before any admin handler runs | +| /api/me → browser | the isAdmin flag crosses to the client for UX gating only; never the security boundary | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-10-04 | Elevation of Privilege | non-admin invoking admin-gated logic | mitigate | requireAdmin reads users.is_admin from the DB and 403s non-admins (Task 1); decision never trusts a client-supplied or context-attached isAdmin claim, only the DB row | +| T-10-05 | Elevation of Privilege | DEV_AUTH_BYPASS bypasses the role check | mitigate | requireAdmin and /api/me both DB-lookup users.is_admin even on the bypass path (the bypass skips OIDC, not the DB check); the bypass admin row comes only from the guarded dev seed (Plan 01 Task 3) | +| T-10-06 | Spoofing/EoP | client trusting its own isAdmin to reach admin features | mitigate | isAdmin on /api/me is documented and used as UX-only; the server-side 403 (requireAdmin, Plan 03) is the real boundary on every /api/admin/* request | +| T-10-07 | Information Disclosure | logging the resolved user / claims | mitigate | requireAdmin and me.ts must not console.log the user object or any credential (acceptance grep) | +| T-10-SC | Tampering | npm/pip/cargo installs | mitigate | No new packages this phase (RESEARCH Package Legitimacy Audit); no install task | + + + +- `pnpm --filter @familysync/api test -- requireAdmin && pnpm --filter @familysync/api test -- user && pnpm --filter @familysync/api test -- me` all pass. +- `pnpm --filter @familysync/api exec tsc --noEmit` passes (per [[vitest-passes-tsc-fails]], run tsc separately — vitest stays green on type errors). +- `grep -q "users.isAdmin" apps/api/src/lib/requireAdmin.ts`. + + + +- requireAdmin 403s authenticated non-admins, admits admins, DB-backed (ADMIN-03 server enforcement; supports Success Criteria 1 & 4). +- First-login-wins writes is_admin on first insert, role-agnostic / member-count-agnostic (Success Criterion 4). +- /api/me exposes isAdmin + needsProviderSetup on both paths for downstream PWA gating + self-service. + + + +Create `.planning/phases/10-admin-role-settings/10-02-SUMMARY.md` when done. + diff --git a/.planning/phases/10-admin-role-settings/10-03-PLAN.md b/.planning/phases/10-admin-role-settings/10-03-PLAN.md new file mode 100644 index 0000000..12146cb --- /dev/null +++ b/.planning/phases/10-admin-role-settings/10-03-PLAN.md @@ -0,0 +1,225 @@ +--- +phase: 10-admin-role-settings +plan: 03 +type: tdd +wave: 3 +depends_on: ["10-01", "10-02"] +files_modified: + - apps/api/src/broker/outboxWorker.ts + - apps/api/src/routes/admin.ts + - apps/api/src/routes/me.ts + - apps/api/src/index.ts + - apps/api/tests/routes/admin.test.ts +autonomous: true +requirements: [ADMIN-01, ADMIN-02, ADMIN-03] +must_haves: + truths: + - "GET /api/admin/members returns 403 for a non-admin authenticated user and a member+credential-status list for an admin" + - "POST /api/admin/credentials validates against CalDAV (PROPFIND), 400 on bad credential with NO submitted password in the body, 200 + encrypted store on success; never logs/echoes the password" + - "PUT /api/admin/calendars/:id/shared sets exactly one calendar is_shared=1 and clears any prior shared calendar" + - "POST /api/me/credential sets only the current user's credential (ignores any userId in the body); a non-admin cannot POST /api/admin/credentials" + artifacts: + - path: "apps/api/src/routes/admin.ts" + provides: "adminRouter guarded by requireAdmin (.use('*', ...) first); GET /members, POST /credentials, GET /calendars, PUT /calendars/:id/shared" + exports: ["adminRouter"] + min_lines: 60 + - path: "apps/api/src/routes/me.ts" + provides: "POST /api/me/credential member-scoped self-service (currentUserId only)" + contains: "credential" + - path: "apps/api/src/index.ts" + provides: "app.route('/api/admin', adminRouter) mounted in the existing auth band" + contains: "adminRouter" + key_links: + - from: "apps/api/src/routes/admin.ts" + to: "requireAdmin" + via: "adminRouter.use('*', requireAdmin) as the first statement (Pitfall 9)" + pattern: "adminRouter\\.use\\('\\*', requireAdmin\\)" + - from: "apps/api/src/routes/admin.ts" + to: "broker validate→encrypt→sync" + via: "createFastmailClient + fetchCalendars, encryptPassword, exported triggerTargetedResync/loadClientForUser" + pattern: "encryptPassword" + - from: "apps/api/src/index.ts" + to: "adminRouter" + via: "app.route('/api/admin', adminRouter)" + pattern: "app.route\\('/api/admin'" +--- + + +Build the admin API surface (ADMIN-01 credential rotation + ADMIN-02 shared-calendar designation), gated by `requireAdmin` (ADMIN-03), plus the member-scoped self-service credential endpoint (D-07), all sharing one validate→encrypt→initial-sync path. Promote the broker's private resync helpers to exports so both the admin and self-service paths reuse them. TDD: the credential and guard contracts have precise input→output behavior (403 / 400-no-echo / 200), so write the failing tests first. + +Purpose: This is the single shared credential + shared-calendar surface (`/api/admin/credentials`, `/api/admin/calendars/:id/shared`) — Phase 12 MUST reuse it, not duplicate it into `/api/setup/*`. The self-service endpoint is the member-scoped counterpart of admin rotation. +Output: Exported broker helpers, new `admin.ts` router, the `/api/me/credential` self-service endpoint, the index.ts mount, and integration tests covering the Pitfall 7 (no-echo) and Pitfall 9 (403) hard checks. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/10-admin-role-settings/10-CONTEXT.md +@.planning/phases/10-admin-role-settings/10-RESEARCH.md +@.planning/phases/10-admin-role-settings/10-PATTERNS.md +@.planning/phases/10-admin-role-settings/10-01-SUMMARY.md +@.planning/phases/10-admin-role-settings/10-02-SUMMARY.md +@apps/api/src/lib/requireAdmin.ts + + + + + + Task 1: Promote broker resync helpers to exports + apps/api/src/broker/outboxWorker.ts + + - apps/api/src/broker/outboxWorker.ts (the file being modified — `loadClientForUser` lines 271–288, `triggerTargetedResync` lines 302–348 incl. the `client.fetchCalendars()` + `syncCalendar` loop ~line 331; confirm neither is exported yet) + - apps/api/src/broker/client.ts (createFastmailClient — the CalDAV client the helpers build on) + apps/api/src/broker/sync.ts (syncCalendar signature) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/api/src/broker/outboxWorker.ts` (add `export` to both functions; the post-credential-save full sync uses loadClientForUser → fetchCalendars → syncCalendar per davCal) + 10-RESEARCH.md §Pattern 4 + §Open Questions #2 (full per-member poll after a fresh credential save — no known calendarUrl yet) + Assumptions A1 + + + Add the `export` keyword to `loadClientForUser` and `triggerTargetedResync` in `apps/api/src/broker/outboxWorker.ts` so the admin + self-service credential routes can reuse them (per 10-PATTERNS.md). Do NOT change their bodies or the outbox drain cycle (A1: standalone fetch+sync helpers, no coupling to the drain loop). After a FRESH credential save there is no known calendarUrl, so the credential routes will call `loadClientForUser(userId)` → `client.fetchCalendars()` → `syncCalendar(...)` per returned DAV calendar (the full per-member poll, mirroring poller.ts) rather than `triggerTargetedResync` — but export both for flexibility. Confirm existing broker tests still pass (no behavior change). NEVER reintroduce node-cron ([[node-cron-skips-in-long-running-process]]) — these helpers are setInterval-driven callers' utilities, untouched. + + + cd /home/luc/Projects/familysync && grep -E "^export (async )?function (loadClientForUser|triggerTargetedResync)" apps/api/src/broker/outboxWorker.ts && pnpm --filter @familysync/api test -- outbox 2>&1 | tail -8 + + + - `grep -cE "^export (async )?function (loadClientForUser|triggerTargetedResync)" apps/api/src/broker/outboxWorker.ts` returns 2. + - The function bodies are unchanged (only `export` prepended) — `git diff` shows only the two `export` keyword additions. + - `pnpm --filter @familysync/api test -- outbox` still passes (no regression to the drain cycle). + + Both broker resync helpers are exported, bodies unchanged, broker tests green. + + + + Task 2: adminRouter — guard + members + credentials + shared-calendar (RED→GREEN→REFACTOR) + apps/api/src/routes/admin.ts, apps/api/src/index.ts, apps/api/tests/routes/admin.test.ts + + - apps/api/src/routes/push.ts (closest analog: Hono sub-router, zValidator, resolveUserId, ContextVariableMap side-effect import; the subscribeSchema zValidator usage lines ~60–67) + - apps/api/src/routes/events.ts (Drizzle leftJoin + where SELECT lines 165–177; the 400-not-422 zValidator convention; onDuplicateKeyUpdate upsert idiom) + - apps/api/src/lib/requireAdmin.ts (from Plan 02 — the guard to apply .use('*', requireAdmin) FIRST) + - apps/api/src/broker/crypto.ts (encryptPassword — reuse verbatim, AES-256-GCM, never log the return) + apps/api/src/broker/client.ts (createFastmailClient + the fetchCalendars PROPFIND validation signal) + apps/api/src/broker/outboxWorker.ts (the helpers exported in Task 1) + - apps/api/src/db/schema.ts users / memberCredentials (provider_type + uniq_member_credential_user from Plan 01) / calendars.isShared (line 89) + - apps/api/src/index.ts (route mount block lines 67–72; the devAuthBypass→oidcAuthMiddleware band already covers /api/*; mount adminRouter AFTER the existing routes) + - apps/api/tests/routes/push.test.ts (integration test idioms: import `app` (NOT adminRouter directly — Pitfall 9), how dev-bypass admin vs non-admin is exercised, the real-DB test setup per [[api-integration-test-db]]) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/api/src/routes/admin.ts` + §`apps/api/src/index.ts` (router+guard pattern, credentialSchema, noEchoHook, SELECT/upsert/exclusive-is_shared excerpts) + 10-RESEARCH.md §Pattern 1 (Pitfall 9), §Pattern 2 (Pitfall 7 no-echo hook), §Pattern 7 (exclusive is_shared), §Pitfall 1/2/5/6, §UI-SPEC Surface 2/5 (member-row + picker shapes the GET responses must feed) + + + - Test (RED, Pitfall 9): GET /api/admin/members as a non-admin authenticated user → 403. As an admin → 200 with a list of members each carrying credential status (has credential / not). Integration test imports `app`, never adminRouter directly. + - Test (Pitfall 7): POST /api/admin/credentials with an INVALID app password (CalDAV PROPFIND fails) → 400, and the response body contains NONE of the submitted password value (assert the exact submitted string is absent from the body) and no Zod `received`/`issues`/`value` field. + - Test: POST /api/admin/credentials with a VALID credential (CalDAV PROPFIND succeeds) → 200; the stored member_credentials.encrypted_password is NOT the plaintext (encryptPassword applied); response never echoes the password; initial sync is triggered (fire-and-forget). + - Test (Pitfall 9): POST /api/admin/credentials as a non-admin → 403. + - Test (ADMIN-02, Pitfall 7-adjacent): PUT /api/admin/calendars/:id/shared as admin → exactly one calendar has is_shared=1 afterward (the target), any prior shared calendar cleared. As non-admin → 403. + - Test: GET /api/admin/calendars as admin → 200 list of synced calendars (id, name, is_shared). As non-admin → 403. + + + Create `apps/api/src/routes/admin.ts` exporting `adminRouter = new Hono()` with `adminRouter.use('*', requireAdmin)` as the VERY FIRST statement (Pitfall 9). Add the side-effect import `'../auth/devBypass.js'`. Routes (paths are planner's call per D — use these): + - `GET /members`: SELECT users LEFT JOIN member_credentials → return id, displayName, color, hasCredential (boolean). Feeds UI-SPEC Surface 2. + - `POST /credentials`: `zValidator('json', credentialSchema, noEchoHook)` where credentialSchema = `{ userId: number().int().positive(), providerType: literal('caldav'), fastmailEmail: string().email().max(256), appPassword: string().min(1).max(500) }` and noEchoHook returns `c.json({ error: 'Invalid request' }, 400)` (NEVER `c.json(result.error, ...)`). Handler: validate via `createFastmailClient(email, appPassword)` + `client.fetchCalendars()` (throws on auth failure → return 400 generic, NO password in body); on success `encryptPassword(appPassword)` → upsert member_credentials via `onDuplicateKeyUpdate` (uses the Plan-01 UNIQUE(user_id)) with providerType 'caldav'; then fire-and-forget the initial full per-member sync (loadClientForUser → fetchCalendars → syncCalendar per davCal) and return 200. NEVER `console.log` the body or `c.req.valid('json')`. + - `GET /calendars`: SELECT calendars (id, displayName, isShared). Feeds UI-SPEC Surface 5. + - `PUT /calendars/:id/shared`: exclusive update (Pattern 7) — `db.update(calendars).set({isShared:false}).where(eq(calendars.isShared,true))` then `db.update(calendars).set({isShared:true}).where(eq(calendars.id, targetId))` (D-06 single-select). Return 200. + Mount in `apps/api/src/index.ts`: `app.route('/api/admin', adminRouter)` after the existing route block (no extra app-level middleware — the guard lives inside the router). Write `apps/api/tests/routes/admin.test.ts` FIRST with all six behaviors (import `app`), confirm RED, implement to GREEN. Mock/stub CalDAV (createFastmailClient/fetchCalendars) for the validation outcomes to avoid live Fastmail calls in CI (per [[dev-data-user1-no-calendars]] — route-mocks for credential paths). + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- admin 2>&1 | tail -20 + + + - `apps/api/src/routes/admin.ts` first statement after router creation is `adminRouter.use('*', requireAdmin)` — `grep -nA1 "new Hono()" apps/api/src/routes/admin.ts` shows the `.use('*', requireAdmin)` immediately after. + - `apps/api/src/index.ts` contains `app.route('/api/admin', adminRouter)`. + - GET /api/admin/members returns 403 for a non-admin authenticated user (integration test importing `app`). + - A 400 response from POST /api/admin/credentials with a bad credential contains NO submitted password value and no Zod `received`/`issues` field (test asserts the exact submitted string absent). + - On a valid credential, the persisted member_credentials.encrypted_password != the plaintext (encryptPassword applied) and 200 is returned. + - After PUT /api/admin/calendars/:id/shared, exactly one calendar row has is_shared=1. + - No `console.log`/`console.error` of request bodies in `apps/api/src/routes/admin.ts` (`grep -ciE "console\.(log|error)\(.*(body|valid|password)" apps/api/src/routes/admin.ts` returns 0). + - `pnpm --filter @familysync/api test -- admin` passes all six behaviors. + + adminRouter exists, guard-first, mounted; members/credentials/calendars/shared routes behave per contract; no-echo + 403 hard checks green. + + + + Task 3: Member-scoped self-service credential endpoint POST /api/me/credential (RED→GREEN→REFACTOR) + apps/api/src/routes/me.ts, apps/api/tests/routes/admin.test.ts + + - apps/api/src/routes/me.ts (the file being modified — the meRouter export, the resolveUserId/dev-bypass + OIDC user resolution already present; the isAdmin/needsProviderSetup response added in Plan 02) + - apps/api/src/routes/admin.ts (from Task 2 — reuse the SAME credentialSchema shape minus userId, the SAME noEchoHook, the SAME validate→encrypt→sync sequence; extract any shared helper rather than duplicating the validate/encrypt logic) + - apps/api/src/broker/crypto.ts + client.ts + outboxWorker.ts (the shared path) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §Shared Patterns "resolveUserId" + 10-RESEARCH.md §Pattern 4, §Pitfall 6 (cross-member write — endpoint MUST use currentUserId from session, NEVER a body userId), §Architectural Responsibility Map (needsProviderSetup) + §UI-SPEC Surface 4 (self-service onboarding) + + + - Test (RED, Pitfall 6): POST /api/me/credential as user A with a body that includes `userId` for user B → the credential is written to user A (currentUserId), NOT user B; the body userId is ignored. + - Test: POST /api/me/credential with a valid credential → 200, stored encrypted for the current user, needsProviderSetup becomes false on the next /api/me; initial sync triggered. + - Test (Pitfall 7): POST /api/me/credential with a bad credential → 400 generic, no password echoed. + - Test: the endpoint does NOT require admin (a normal member can set their own credential) but is still behind the auth guard (unauthenticated → 401 from the outer band). + + + Add `POST /credential` to the meRouter in `apps/api/src/routes/me.ts` (final path `/api/me/credential`), member-scoped. Schema = the admin credentialSchema WITHOUT `userId` (`{ providerType: literal('caldav'), fastmailEmail, appPassword }`) + the SAME `noEchoHook`. The handler resolves `currentUserId` via the existing resolveUserId/dev-bypass pattern and ALWAYS writes to that id — it MUST NOT read a userId from the body (Pitfall 6). Reuse the SAME validate→encrypt→initial-sync path as admin Task 2 (extract a shared helper, e.g. `validateEncryptAndStoreCredential(userId, email, password)`, to avoid divergence — D-07 "identical path"). Add the self-service test cases to `apps/api/tests/routes/admin.test.ts` (or a sibling me-credential test — planner's call; keep them with the credential-surface tests). Write tests FIRST, confirm RED, implement to GREEN. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- credential 2>&1 | tail -15 + + + - `apps/api/src/routes/me.ts` adds a `POST /credential` route; the final mounted path is `/api/me/credential`. + - A POST to /api/me/credential with a body `userId` for another user writes ONLY to the current session user (test proves the other user's credential is untouched). + - The self-service path reuses the same validate→encrypt→store logic as the admin path (shared helper; no duplicated encrypt/PROPFIND block) — `grep` shows a single shared function called by both routes. + - A bad credential returns 400 generic with no echoed password. + - A normal (non-admin) member can succeed on /api/me/credential (no requireAdmin on this route). + - `pnpm --filter @familysync/api test -- credential` passes all cases. + + Member self-service credential endpoint exists, member-scoped (no cross-member write), shares the admin validate→encrypt→sync path, tests green. + + + + + +New symbols/files created by this plan (excluded from drift verification): +- `export` on `loadClientForUser` + `triggerTargetedResync` in `apps/api/src/broker/outboxWorker.ts` +- `apps/api/src/routes/admin.ts` exporting `adminRouter` with `GET /members`, `POST /credentials`, `GET /calendars`, `PUT /calendars/:id/shared` +- `requireAdmin` applied as `adminRouter.use('*', requireAdmin)` (consumes the Plan-02 guard) +- `app.route('/api/admin', adminRouter)` mount in `apps/api/src/index.ts` +- `POST /api/me/credential` member-scoped self-service endpoint in `apps/api/src/routes/me.ts` +- shared `validateEncryptAndStoreCredential` helper (admin + self-service) +- `apps/api/tests/routes/admin.test.ts` (+ self-service credential test cases) + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| client → /api/admin/* | untrusted authenticated request; must pass requireAdmin before any handler | +| client → /api/me/credential | authenticated member request; must be confined to the caller's own credential row | +| API → Fastmail CalDAV | the submitted app password leaves the trust boundary only to validate (PROPFIND); it must never be logged or echoed back to the client | +| app password → MariaDB | plaintext must be AES-256-GCM encrypted before any DB write | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-10-08 | Elevation of Privilege | non-admin hitting /api/admin/* | mitigate | adminRouter.use('*', requireAdmin) FIRST (Pitfall 9); integration tests import `app` and assert 403 on every admin route for a non-admin | +| T-10-09 | Information Disclosure | app password echoed in a Zod/validation error | mitigate | noEchoHook returns `{ error: 'Invalid request' }` with no result.error; test asserts the submitted password string is absent from any 400 body (Pitfall 7) | +| T-10-10 | Information Disclosure | app password logged | mitigate | No console.log of body/valid()/password in admin.ts or the me credential route (acceptance grep == 0) | +| T-10-11 | Information Disclosure | plaintext credential at rest | mitigate | encryptPassword (AES-256-GCM via crypto.ts) applied before the DB write; test asserts stored value != plaintext; no new crypto written | +| T-10-12 | Elevation of Privilege / IDOR | member self-service writes another member's credential | mitigate | /api/me/credential always uses currentUserId from the session and ignores any body userId (Pitfall 6); test proves the other user's row is untouched | +| T-10-13 | IDOR | admin rotating an arbitrary member's credential | accept | D-05 explicitly allows an admin to rotate ANY member's credential; this is gated by requireAdmin and is the intended capability (the self-service path remains member-scoped) | +| T-10-SC | Tampering | npm/pip/cargo installs | mitigate | No new packages this phase (RESEARCH Package Legitimacy Audit); no install task | + + + +- `pnpm --filter @familysync/api test -- admin && pnpm --filter @familysync/api test -- credential && pnpm --filter @familysync/api test -- outbox` all pass. +- `pnpm --filter @familysync/api exec tsc --noEmit` passes (run tsc separately per [[vitest-passes-tsc-fails]]). +- `grep -A1 "new Hono()" apps/api/src/routes/admin.ts` shows `.use('*', requireAdmin)` first. +- `grep -ciE "console\.(log|error)\(.*(body|valid|password)" apps/api/src/routes/admin.ts` == 0. + + + +- ADMIN-01: admin can rotate any member's credential, CalDAV-validated, encrypted, never echoed/logged (Success Criterion 2). +- ADMIN-02: admin sets exactly one shared calendar via the API (Success Criterion 3). +- ADMIN-03: every /api/admin/* route 403s non-admins (Success Criterion 1); guard inside the sub-router (Pitfall 9). +- D-07: member self-service credential, member-scoped, same path. +- Single shared surface — no /api/setup/* duplication (Phase 12 reuses these routes). + + + +Create `.planning/phases/10-admin-role-settings/10-03-SUMMARY.md` when done. + diff --git a/.planning/phases/10-admin-role-settings/10-04-PLAN.md b/.planning/phases/10-admin-role-settings/10-04-PLAN.md new file mode 100644 index 0000000..5a8058a --- /dev/null +++ b/.planning/phases/10-admin-role-settings/10-04-PLAN.md @@ -0,0 +1,205 @@ +--- +phase: 10-admin-role-settings +plan: 04 +type: execute +wave: 4 +depends_on: ["10-02", "10-03"] +files_modified: + - apps/pwa/src/api/client.ts + - apps/pwa/src/routes/AdminPage.tsx + - apps/pwa/src/components/CredentialSheet.tsx + - apps/pwa/src/components/SetupBanner.tsx + - apps/pwa/src/components/AppNav.tsx + - apps/pwa/src/components/BottomTabBar.tsx + - apps/pwa/src/App.tsx + - apps/pwa/e2e/admin.spec.ts +autonomous: true +requirements: [ADMIN-01, ADMIN-02, ADMIN-03] +must_haves: + truths: + - "An admin sees an Admin nav entry, reaches /admin, can list members + credential status, rotate a member credential via the sheet, and pick the shared calendar" + - "A non-admin never sees the Admin nav entry and is redirected from /admin to /calendar" + - "A member with needsProviderSetup=true sees the self-service SetupBanner and can add their own credential via the same sheet (member-scoped)" + artifacts: + - path: "apps/pwa/src/routes/AdminPage.tsx" + provides: "/admin page: Members section + Shared-Calendar picker, wired to /api/admin/*" + min_lines: 60 + - path: "apps/pwa/src/components/CredentialSheet.tsx" + provides: "shared credential bottom sheet (admin rotation + self-service), password never pre-filled, CalDAV validation states" + min_lines: 50 + - path: "apps/pwa/src/components/SetupBanner.tsx" + provides: "needsProviderSetup self-service onboarding banner (no dismiss; clears on save)" + min_lines: 20 + - path: "apps/pwa/src/api/client.ts" + provides: "MeUser.isAdmin + needsProviderSetup; fetchAdminMembers/saveCredential/fetchAdminCalendars/setSharedCalendar/saveMyCredential" + contains: "isAdmin" + key_links: + - from: "apps/pwa/src/App.tsx" + to: "AdminPage / Navigate redirect" + via: "meQuery.data.user.isAdmin gate on the /admin Route" + pattern: "isAdmin" + - from: "apps/pwa/src/components/AppNav.tsx" + to: "/admin NavLink" + via: "conditional render on isAdmin (ShieldCheck icon)" + pattern: "ShieldCheck" + - from: "apps/pwa/src/components/CredentialSheet.tsx" + to: "/api/admin/credentials | /api/me/credential" + via: "TanStack mutation, invalidates ['admin','members'] + ['me']" + pattern: "invalidateQueries" +--- + + +Build the React PWA admin surfaces per the 10-UI-SPEC contract: extend the `/api/me` client type + add admin/self-service fetchers, add the gated `/admin` route (D-02: dedicated gated route, not an extension of SettingsSheet) + conditional nav entries, build the shared CredentialSheet (admin rotation + member self-service) and the needsProviderSetup SetupBanner. Verify the route guard and nav gating in a real Chromium browser with playwright-cli (the desktop-Chromium-drivable behaviors), not a human checkpoint. + +Purpose: This is the user-facing half of ADMIN-01/02/03 + the D-07 self-service onboarding. The client `isAdmin` flag is consumed for UX gating only per D-03 (server already enforces 403 on every /api/admin/* route from Plan 03); `needsProviderSetup` drives the self-service banner. +Output: Extended client.ts, AdminPage.tsx, CredentialSheet.tsx, SetupBanner.tsx, nav-entry edits, the /admin route in App.tsx, and an e2e spec. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/10-admin-role-settings/10-CONTEXT.md +@.planning/phases/10-admin-role-settings/10-UI-SPEC.md +@.planning/phases/10-admin-role-settings/10-PATTERNS.md +@.planning/phases/10-admin-role-settings/10-02-SUMMARY.md +@.planning/phases/10-admin-role-settings/10-03-SUMMARY.md + + + + + + Task 1: Extend client.ts — MeUser fields + admin/self-service fetchers + apps/pwa/src/api/client.ts + + - apps/pwa/src/api/client.ts (the file being modified — `MeUser` interface lines 62–66, `handleAuthResponse` lines 51–58, the `createEvent` fetch pattern lines 221–233 with `credentials:'include'`, `redirect:'manual'`, `SessionExpiredError` handling) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/pwa/src/api/client.ts` (the MeUser field additions + the fetch-function pattern to copy from createEvent for each admin/me endpoint) + §Shared Patterns "handleAuthResponse + redirect:'manual'" + - .planning/phases/10-admin-role-settings/10-03-SUMMARY.md (the exact /api/admin/* + /api/me/credential request/response shapes shipped by Plan 03 — match them) + - .planning/phases/10-admin-role-settings/10-UI-SPEC.md Surface 2 (member-row fields) + Surface 5 (calendar-picker fields) + + + In `apps/pwa/src/api/client.ts`: (1) add `isAdmin: boolean` and `needsProviderSetup: boolean` to the `MeUser` interface (D-03: client consumes isAdmin for UX gating only). (2) Add typed fetch functions matching the Plan-03 contracts, each following the `createEvent` idiom (`credentials:'include'`, `redirect:'manual'`, `handleAuthResponse(res, label)`): `fetchAdminMembers()` → GET /api/admin/members; `saveCredential(payload)` → POST /api/admin/credentials (admin, includes userId); `fetchAdminCalendars()` → GET /api/admin/calendars; `setSharedCalendar(calendarId)` → PUT /api/admin/calendars/:id/shared; `saveMyCredential(payload)` → POST /api/me/credential (self-service, NO userId). Define request/response TS types matching the Plan-03 SUMMARY shapes. NEVER store or log the app password client-side beyond the in-flight request body. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/pwa exec tsc --noEmit 2>&1 | tail -8 + + + - `MeUser` includes `isAdmin: boolean` and `needsProviderSetup: boolean` (`grep -c "isAdmin\|needsProviderSetup" apps/pwa/src/api/client.ts` >= 2). + - All five fetchers exist and use `credentials:'include'` + `redirect:'manual'` + `handleAuthResponse`. + - `saveMyCredential`'s payload type has NO `userId` field; `saveCredential`'s does. + - `pnpm --filter @familysync/pwa exec tsc --noEmit` exits 0. + + client.ts exposes the new MeUser flags + five typed admin/self-service fetchers, typechecks clean. + + + + Task 2: CredentialSheet + SetupBanner components + apps/pwa/src/components/CredentialSheet.tsx, apps/pwa/src/components/SetupBanner.tsx + + - apps/pwa/src/components/SettingsSheet.tsx (EXACT analog for CredentialSheet — the bottom-sheet pattern: role="dialog", aria-modal, Escape handler lines 52–76, focus-on-open, backdrop zIndex 300 / sheet zIndex 301 / borderRadius 12px 12px 0 0 / padding var(--space-6), focus-return-to-trigger) + - apps/pwa/src/components/PermissionDeniedBanner.tsx (analog for SetupBanner — conditional banner rendered at App level, role/aria-live pattern) + - apps/pwa/src/components/DeleteConfirmationDialog.tsx (the minHeight:48px confirm-button precedent referenced in UI-SPEC) + - apps/pwa/src/api/client.ts (saveCredential + saveMyCredential from Task 1; SessionExpiredError) + - .planning/phases/10-admin-role-settings/10-UI-SPEC.md Surface 3 (credential sheet: heading variants, member subtitle, password field type="password"/autocomplete="new-password"/never pre-filled, helper text + Fastmail app-password link target="_blank" rel="noopener noreferrer", "Validating against CalDAV…" Loader2 spinner, failure copy, Save/Cancel actions) + Surface 4 (self-service banner: KeyRound icon, copy, "Set up now" CTA, no X/dismiss) + the Copywriting Contract (exact strings) + Accessibility Contracts + Color/Typography/Spacing (all via var(--token), 44px touch targets) + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/pwa/src/components/CredentialSheet.tsx` + §`apps/pwa/src/components/SetupBanner.tsx` (structural + mutation + style excerpts) + §Shared Patterns "CSS token inline style pattern" + + + Build `CredentialSheet.tsx` (shared by admin rotation AND self-service per D-07) following the SettingsSheet bottom-sheet pattern: props for mode (admin-rotate / admin-add / self-service), target member (admin) or current user (self-service), open/close. Render the heading variant per UI-SPEC Copywriting Contract ("Rotate Credential" / "Add Credential" / "Add your calendar credential"), the member-name subtitle, a `type="password" autoComplete="new-password"` field NEVER pre-filled, the helper text with the Fastmail app-password link (exact URL + copy from UI-SPEC, opens in new tab), the "Validating against CalDAV…" inline state (Loader2 size 16) during the mutation, the failure error copy on a CalDAV 400, and Cancel (ghost) + Save Credential (accent-filled) actions. Use a TanStack `useMutation` that calls `saveCredential` (admin) or `saveMyCredential` (self-service) and on success invalidates `['admin','members']` + `['me']` (so needsProviderSetup refreshes) and closes the sheet. Build `SetupBanner.tsx` following PermissionDeniedBanner: render only when `meQuery.data?.user.needsProviderSetup === true`, `role="status" aria-live="polite"`, KeyRound icon, the exact heading/body/CTA copy, "Set up now" opening the CredentialSheet in self-service mode; NO dismiss button (it clears when needsProviderSetup becomes false). All styling via `var(--token)`; every interactive element minWidth/minHeight 44px. Never log/echo the password. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/pwa exec tsc --noEmit 2>&1 | tail -8 && grep -l "autocomplete=\"new-password\"\|autoComplete=\"new-password\"" apps/pwa/src/components/CredentialSheet.tsx + + + - `CredentialSheet.tsx` uses `role="dialog"`, `aria-modal`, an Escape handler, and a `type="password"` input with `autoComplete="new-password"` that is never pre-filled (no `value={existingPassword}` from any fetched source). + - The helper text contains the Fastmail app-password link with `target="_blank"` and `rel="noopener noreferrer"`. + - The success mutation invalidates both `['admin','members']` and `['me']` (`grep -c "invalidateQueries" apps/pwa/src/components/CredentialSheet.tsx` >= 2). + - `SetupBanner.tsx` renders conditionally on `needsProviderSetup`, uses `role="status"`/`aria-live`, and has NO dismiss/X button. + - Exact UI-SPEC Copywriting Contract strings are present (e.g. "Set up your calendar", "Validating against CalDAV…", "Save Credential"). + - No hard-coded color/spacing px except the 44px/48px touch-target minimums; values reference `var(--...)`. + - `pnpm --filter @familysync/pwa exec tsc --noEmit` exits 0. + + CredentialSheet (admin + self-service) and SetupBanner match the UI-SPEC contract, accessible, token-styled, typecheck clean. + + + + Task 3: /admin route + AdminPage + conditional nav entries, with playwright-cli verification + apps/pwa/src/routes/AdminPage.tsx, apps/pwa/src/App.tsx, apps/pwa/src/components/AppNav.tsx, apps/pwa/src/components/BottomTabBar.tsx, apps/pwa/e2e/admin.spec.ts + + - apps/pwa/src/App.tsx (the Routes block lines 121–127, the meQuery lines 63–68, the content-area style lines 99–102, where PermissionDeniedBanner-style banners mount — SetupBanner mounts here too) + - apps/pwa/src/routes/ListsIndex.tsx (page-level component analog for AdminPage: TanStack Query + sections layout) + - apps/pwa/src/components/AppNav.tsx (NavLink + Lucide pattern lines 14–15, the DesktopNav section) + apps/pwa/src/components/BottomTabBar.tsx (tab pattern lines 27–98) — add the conditional Admin entry (ShieldCheck) to both + - apps/pwa/e2e/calendar.spec.ts + layout.spec.ts + e2e/README.md (existing spec idioms, selectors, the dev-bypass admin user seeded in global-setup as id=1 is_admin=true from Plan 01) + - .claude/skills/playwright-cli/SKILL.md (how to drive the global playwright-cli binary for the browser verification) + - .planning/phases/10-admin-role-settings/10-UI-SPEC.md Surface 1 (/admin page: AppNav persistent, ShieldCheck size 18, content maxWidth 640px centered desktop, var(--space-12) vertical padding, "Admin Settings" heading) + Surface 2 (Members section) + Surface 5 (Shared Calendar picker: radio group, "Currently shared" label, two-tap Save, empty state) + Copywriting Contract + - .planning/phases/10-admin-role-settings/10-PATTERNS.md §`apps/pwa/src/routes/AdminPage.tsx` + §`apps/pwa/src/App.tsx` + §Shared Patterns "NavLink + Lucide icon" + + + Build `AdminPage.tsx` (analog ListsIndex): "Admin Settings" heading (18px/600), a MEMBERS section listing members from `fetchAdminMembers` (avatar swatch + name + credential status badge per UI-SPEC Surface 2 + a "Rotate"/"Add credential" button opening CredentialSheet in admin mode for that member), and a SHARED CALENDAR section (Surface 5) using `fetchAdminCalendars`: an exclusive single-select radio group (D-06), the "Currently shared" label on the active one, a two-tap "Save" button (disabled until selection differs) calling `setSharedCalendar`, and the empty state ("No calendars synced yet") when none synced. Desktop: centered column maxWidth 640px. In `App.tsx`: add the `/admin` Route (D-02: new dedicated gated route) gated by `meQuery.data?.user.isAdmin ? : ` — the client redirect is UX-only per D-03 (the server-side 403 from Plan 03 is the real boundary); add a loading gate so an in-flight meQuery doesn't flash-redirect (planner's call per 10-PATTERNS.md note); mount `` above the calendar content (renders only on needsProviderSetup). In `AppNav.tsx` (desktop) and `BottomTabBar.tsx` (mobile): add an Admin entry (ShieldCheck icon, `aria-label="Admin settings"`) rendered ONLY when `meQuery.data?.user.isAdmin === true` (D-03 UX gating). Write `apps/pwa/e2e/admin.spec.ts`: with the dev-bypass admin user (seeded id=1 is_admin=true), assert the Admin nav entry is visible and /admin renders "Admin Settings" + the Members section. Add a non-admin assertion by route-mocking GET /api/me to `isAdmin:false` (per [[dev-data-user1-no-calendars]] route-mock idiom and the lists.spec page.route precedent) and asserting the Admin nav entry is absent and /admin redirects to /calendar. THEN run a playwright-cli browser check against the dev stack to confirm the guard + nav gating + sheet open behaviors interactively (use the playwright-cli skill — this replaces a human-verify checkpoint since it is desktop-Chromium-drivable per CLAUDE.md). All styling via `var(--token)`; 44px touch targets. + + + cd /home/luc/Projects/familysync && pnpm --filter @familysync/pwa exec tsc --noEmit 2>&1 | tail -6 && pnpm --filter @familysync/pwa test:e2e -- admin 2>&1 | tail -20 + + + - `apps/pwa/src/App.tsx` has a `/admin` Route gated on `meQuery.data?.user.isAdmin` with a `` fallback for non-admins, and mounts ``. + - `AppNav.tsx` and `BottomTabBar.tsx` render the Admin entry (ShieldCheck, `aria-label="Admin settings"`) ONLY when isAdmin is true (`grep -c "ShieldCheck" apps/pwa/src/components/AppNav.tsx apps/pwa/src/components/BottomTabBar.tsx` >= 2). + - `AdminPage.tsx` renders "Admin Settings", a MEMBERS list, and a SHARED CALENDAR exclusive radio group with a two-tap Save and an empty state. + - `apps/pwa/e2e/admin.spec.ts` asserts: admin sees the nav entry + reaches /admin; a non-admin (route-mocked isAdmin:false) does NOT see it and /admin redirects to /calendar. + - `pnpm --filter @familysync/pwa test:e2e -- admin` passes; the playwright-cli interactive check confirms the guard + nav gating (recorded in the SUMMARY). + - `pnpm --filter @familysync/pwa exec tsc --noEmit` exits 0. + + /admin route gated, AdminPage wired to the admin API, conditional nav entries + SetupBanner mounted, e2e + playwright-cli verification green. + + + + + +New symbols/files created by this plan (excluded from drift verification): +- `MeUser.isAdmin` + `MeUser.needsProviderSetup` fields in `apps/pwa/src/api/client.ts` +- fetchers `fetchAdminMembers`, `saveCredential`, `fetchAdminCalendars`, `setSharedCalendar`, `saveMyCredential` +- `apps/pwa/src/routes/AdminPage.tsx` +- `apps/pwa/src/components/CredentialSheet.tsx` +- `apps/pwa/src/components/SetupBanner.tsx` +- `/admin` Route + isAdmin gate + `` mount in `apps/pwa/src/App.tsx` +- conditional Admin nav entry (ShieldCheck) in `apps/pwa/src/components/AppNav.tsx` + `apps/pwa/src/components/BottomTabBar.tsx` +- `apps/pwa/e2e/admin.spec.ts` + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| browser isAdmin flag → UI gating | the client isAdmin flag controls nav/route visibility only; it is NOT the security boundary | +| credential sheet → API | the app password is entered in the browser and sent to /api/admin/credentials or /api/me/credential over the in-flight request only | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-10-14 | Elevation of Privilege | a non-admin bypassing the client route guard (e.g. typing /admin) | accept | The client redirect is UX (D-03); the real boundary is the server-side 403 on every /api/admin/* request (Plan 03). A non-admin who forces /admin sees no data — all admin fetches return 403. e2e asserts the redirect anyway | +| T-10-15 | Information Disclosure | app password persisted/echoed client-side | mitigate | Password field never pre-filled, never written to localStorage/state beyond the in-flight mutation; no console.log of the value (acceptance: never pre-filled) | +| T-10-16 | Information Disclosure | password autofilled with the current credential | mitigate | autoComplete="new-password" (never "current-password"); the existing credential is never fetched to the client | +| T-10-SC | Tampering | npm/pip/cargo installs | mitigate | No new packages this phase (lucide-react already at 1.17.0 per RESEARCH Package Legitimacy Audit); no install task | + + + +- `pnpm --filter @familysync/pwa exec tsc --noEmit` passes. +- `pnpm --filter @familysync/pwa test:e2e -- admin` passes (admin sees nav + /admin; non-admin redirected, no nav entry). +- playwright-cli interactive check confirms the guard + nav gating + sheet open (recorded in SUMMARY). +- Run the full CI fast-checks gate locally before declaring done (lint + typecheck + test + format:check + md:lint + PWA tests) per [[feedback-run-full-ci-gate-before-push]]. + + + +- Admin sees the Admin section, lists members + credential status, rotates a credential via the sheet, picks the shared calendar (Success Criteria 1, 2, 3). +- Non-admin never sees the entry and is redirected from /admin (Success Criterion 1; client-side UX over the server 403). +- needsProviderSetup member sees the SetupBanner and can self-serve their own credential (D-07). + + + +Create `.planning/phases/10-admin-role-settings/10-04-SUMMARY.md` when done. + diff --git a/.planning/phases/10-admin-role-settings/10-VALIDATION.md b/.planning/phases/10-admin-role-settings/10-VALIDATION.md index 2f66422..f2033c1 100644 --- a/.planning/phases/10-admin-role-settings/10-VALIDATION.md +++ b/.planning/phases/10-admin-role-settings/10-VALIDATION.md @@ -2,7 +2,7 @@ phase: 10 slug: admin-role-settings status: draft -nyquist_compliant: false +nyquist_compliant: true wave_0_complete: false created: 2026-06-13 --- @@ -17,20 +17,25 @@ created: 2026-06-13 | Property | Value | |----------|-------| -| **Framework** | {pytest 7.x / jest 29.x / vitest / go test / other} | -| **Config file** | {path or "none — Wave 0 installs"} | -| **Quick run command** | `{quick command}` | -| **Full suite command** | `{full command}` | -| **Estimated runtime** | ~{N} seconds | +| **Framework** | Vitest 4.x (apps/api unit + real-DB integration), Vitest 4.x + Playwright (apps/pwa unit + e2e) | +| **Config file** | apps/api/vitest.config.ts, apps/pwa/vitest.config.ts, apps/pwa/playwright.config.ts | +| **Quick run command** | `pnpm --filter @familysync/api test` | +| **Full suite command** | `pnpm --filter @familysync/api test && pnpm --filter @familysync/pwa test && pnpm --filter @familysync/pwa test:e2e` | +| **Estimated runtime** | ~60–120 seconds (e2e dominates) | + +Notes: +- API real-DB integration tests need the dev MariaDB bound on 3306 + `DB_HOST=127.0.0.1` + `.env` creds; API tests live in `apps/api/tests/` (never `src/`) — see [[api-integration-test-db]]. +- `tsc --noEmit` MUST be run separately in both apps — vitest stays green on type errors ([[vitest-passes-tsc-fails]]). +- e2e relies on the dev-bypass admin user (id=1 is_admin=true) seeded by Plan 01 Task 3 in `apps/pwa/e2e/global-setup.ts`. --- ## Sampling Rate -- **After every task commit:** Run `{quick run command}` -- **After every plan wave:** Run `{full suite command}` -- **Before `/gsd-verify-work`:** Full suite must be green -- **Max feedback latency:** {N} seconds +- **After every task commit:** Run `pnpm --filter @familysync/api test` (API tasks) or `pnpm --filter @familysync/pwa exec tsc --noEmit` (PWA tasks). +- **After every plan wave:** Run `pnpm --filter @familysync/api test && pnpm --filter @familysync/api exec tsc --noEmit && pnpm --filter @familysync/pwa exec tsc --noEmit`. +- **Before `/gsd-verify-work`:** Full CI fast-checks gate green (lint + typecheck + test + format:check + md:lint + PWA tests + e2e) — [[feedback-run-full-ci-gate-before-push]]. +- **Max feedback latency:** ~15s for the API quick run; ~120s for the full suite. --- @@ -38,19 +43,34 @@ created: 2026-06-13 | Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | |---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| -| {N}-01-01 | 01 | 1 | REQ-{XX} | T-{N}-01 / — | {expected secure behavior or "N/A"} | unit | `{command}` | ✅ / ❌ W0 | ⬜ pending | +| 10-01-01 | 01 | 1 | ADMIN-01/02/03 | T-10-01 | schema additive only; no destructive DDL | typecheck | `pnpm --filter @familysync/api exec tsc --noEmit` | ✅ existing | ⬜ pending | +| 10-01-02 | 01 | 1 | ADMIN-01/02/03 | T-10-01 | generate+migrate (never push); live DB columns verified by query | integration (DB) | mysql2 `SHOW COLUMNS`/`SHOW TABLES` assert (see plan verify) | ✅ existing | ⬜ pending | +| 10-01-03 | 01 | 1 | ADMIN-03 | T-10-03 | dev-bypass admin row scoped to guarded dev/e2e DB only | source | `grep -c is_admin apps/pwa/e2e/global-setup.ts` | ✅ existing | ⬜ pending | +| 10-02-01 | 02 | 2 | ADMIN-03 | T-10-04/05/06/07 | requireAdmin 403s non-admin; role from DB not client flag | unit | `pnpm --filter @familysync/api test -- requireAdmin` | ❌ W0 → `apps/api/tests/lib/requireAdmin.test.ts` | ⬜ pending | +| 10-02-02 | 02 | 2 | ADMIN-03 | T-10-04 | first-login-wins is_admin; member-count-agnostic | integration (DB) | `pnpm --filter @familysync/api test -- user` | ✅ `apps/api/tests/auth/user.test.ts` (extend) | ⬜ pending | +| 10-02-03 | 02 | 2 | ADMIN-03 | T-10-06/07 | /api/me exposes isAdmin + needsProviderSetup (UX-only flag) | integration | `pnpm --filter @familysync/api test -- me` | ✅ `apps/api/tests/routes/me.test.ts` (extend) | ⬜ pending | +| 10-03-01 | 03 | 3 | ADMIN-01 | T-10-SC | broker helpers exported; bodies unchanged | unit | `pnpm --filter @familysync/api test -- outbox` | ✅ `apps/api/tests/broker/` | ⬜ pending | +| 10-03-02 | 03 | 3 | ADMIN-01/02/03 | T-10-08/09/10/11/13 | guard-first 403; no password echo/log; encrypted at rest; exclusive is_shared | integration | `pnpm --filter @familysync/api test -- admin` | ❌ W0 → `apps/api/tests/routes/admin.test.ts` | ⬜ pending | +| 10-03-03 | 03 | 3 | ADMIN-01 | T-10-09/10/12 | self-service member-scoped; no cross-member write; no echo | integration | `pnpm --filter @familysync/api test -- credential` | ❌ W0 → in `apps/api/tests/routes/admin.test.ts` | ⬜ pending | +| 10-04-01 | 04 | 4 | ADMIN-01/02/03 | T-10-14 | MeUser flags UX-only; self-service payload has no userId | typecheck | `pnpm --filter @familysync/pwa exec tsc --noEmit` | ✅ existing | ⬜ pending | +| 10-04-02 | 04 | 4 | ADMIN-01 | T-10-15/16 | password never pre-filled; autocomplete=new-password | typecheck + source | `pnpm --filter @familysync/pwa exec tsc --noEmit` + grep new-password | ✅ existing | ⬜ pending | +| 10-04-03 | 04 | 4 | ADMIN-03 | T-10-14 | /admin redirect for non-admin; nav entry hidden for non-admin | e2e (playwright) | `pnpm --filter @familysync/pwa test:e2e -- admin` | ❌ W0 → `apps/pwa/e2e/admin.spec.ts` | ⬜ pending | *Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* +Sampling continuity: every task has an `` verify; no 3 consecutive tasks lack automated coverage. + --- ## Wave 0 Requirements -- [ ] `{tests/test_file.py}` — stubs for REQ-{XX} -- [ ] `{tests/conftest.py}` — shared fixtures -- [ ] `{framework install}` — if no framework detected +- [ ] `apps/api/tests/lib/requireAdmin.test.ts` — NEW unit tests for the guard (403 non-admin / next() admin / no-user / DB-not-client-flag). Created in Plan 02 Task 1 (RED first). +- [ ] `apps/api/tests/routes/admin.test.ts` — NEW integration tests covering ADMIN-01 (credential validate/encrypt/no-echo), ADMIN-02 (exclusive is_shared), ADMIN-03 (403 for non-admin on every admin route — import `app`, never adminRouter directly), and the member self-service credential cases. Created in Plan 03 Tasks 2 & 3 (RED first). +- [ ] `apps/pwa/e2e/admin.spec.ts` — NEW e2e: admin sees nav entry + reaches /admin; non-admin (route-mocked isAdmin:false) sees no entry and is redirected. Created in Plan 04 Task 3 (RED first). +- [ ] Extend `apps/api/tests/auth/user.test.ts` — first-login-wins cases (Plan 02 Task 2). +- [ ] Extend `apps/api/tests/routes/me.test.ts` — isAdmin + needsProviderSetup cases (Plan 02 Task 3). -*If none: "Existing infrastructure covers all phase requirements."* +Existing infrastructure (Vitest + Playwright + real-DB harness + global-setup seed) covers all framework needs — no framework install required. --- @@ -58,19 +78,19 @@ created: 2026-06-13 | Behavior | Requirement | Why Manual | Test Instructions | |----------|-------------|------------|-------------------| -| {behavior} | REQ-{XX} | {reason} | {steps} | +| Live CalDAV PROPFIND against the real Fastmail account on a real credential save | ADMIN-01 | CI/e2e mock CalDAV (dev-bypass user 1 has no Fastmail credential — [[dev-data-user1-no-calendars]]); a true end-to-end save against Fastmail needs a real app password | At go-live, an admin enters a real Fastmail app password in the credential sheet; confirm 200 + the member's calendar appears after the initial sync. Optional operator spot-check, not a phase gate. | -*If none: "All phase behaviors have automated verification."* +All other phase behaviors (route guard, no-echo, encryption-at-rest, exclusive is_shared, nav gating, /admin redirect) have automated coverage (Vitest + Playwright/playwright-cli — the route guard and nav gating are desktop-Chromium-drivable per CLAUDE.md, so no human checkpoint). --- ## Validation Sign-Off -- [ ] All tasks have `` verify or Wave 0 dependencies -- [ ] Sampling continuity: no 3 consecutive tasks without automated verify -- [ ] Wave 0 covers all MISSING references -- [ ] No watch-mode flags -- [ ] Feedback latency < {N}s -- [ ] `nyquist_compliant: true` set in frontmatter +- [x] All tasks have `` verify or Wave 0 dependencies +- [x] Sampling continuity: no 3 consecutive tasks without automated verify +- [x] Wave 0 covers all MISSING references (requireAdmin.test.ts, admin.test.ts, admin.spec.ts + the two extends) +- [x] No watch-mode flags +- [x] Feedback latency < 120s (full) / < 15s (quick) +- [x] `nyquist_compliant: true` set in frontmatter -**Approval:** {pending / approved YYYY-MM-DD} +**Approval:** approved 2026-06-13