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
This commit is contained in:
Lucas Berger
@@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/gsd-core/workflows/execute-plan.md
|
||||
@$HOME/.claude/gsd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.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
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 1: Extend schema.ts with the v1.1 column/table bundle</name>
|
||||
<files>apps/api/src/db/schema.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && pnpm --filter @familysync/api exec tsc --noEmit 2>&1 | tail -5</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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()`).
|
||||
</acceptance_criteria>
|
||||
<done>schema.ts holds all four v1.1 additions, typechecks clean, no existing column altered.</done>
|
||||
</task>
|
||||
|
||||
<task type="execute" gate="blocking">
|
||||
<name>Task 2: [BLOCKING] Generate + migrate the v1.1 migration against the live dev DB</name>
|
||||
<files>apps/api/src/db/migrations/0001_v1_1_foundation.sql, apps/api/src/db/migrations/meta/_journal.json</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>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)})"</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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 '^--' <migration.sql> | 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).
|
||||
</acceptance_criteria>
|
||||
<done>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.</done>
|
||||
</task>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 3: Seed the dev-bypass user (id=1) as admin in the e2e global-setup</name>
|
||||
<files>apps/pwa/e2e/global-setup.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && grep -c "is_admin" apps/pwa/e2e/global-setup.ts</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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).
|
||||
</acceptance_criteria>
|
||||
<done>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.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
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`
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## 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 |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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).
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/10-admin-role-settings/10-01-SUMMARY.md` when done.
|
||||
</output>
|
||||
@@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/gsd-core/workflows/execute-plan.md
|
||||
@$HOME/.claude/gsd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.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
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 1: requireAdmin middleware (RED→GREEN→REFACTOR)</name>
|
||||
<files>apps/api/src/lib/requireAdmin.ts, apps/api/tests/lib/requireAdmin.test.ts</files>
|
||||
<read_first>
|
||||
- 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
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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).
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- requireAdmin 2>&1 | tail -15</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>requireAdmin guard exists, DB-backed, 403s non-admins, tests green.</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 2: First-login-wins is_admin bootstrap in upsertUser (RED→GREEN→REFACTOR)</name>
|
||||
<files>apps/api/src/auth/user.ts, apps/api/tests/auth/user.test.ts</files>
|
||||
<read_first>
|
||||
- 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")
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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).
|
||||
</behavior>
|
||||
<action>
|
||||
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<number>\`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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- user 2>&1 | tail -15</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>First-login-wins bootstrap writes is_admin on first insert, member-count-agnostic, Phase-12-safe, tests green.</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 3: Extend /api/me with isAdmin + needsProviderSetup (RED→GREEN→REFACTOR)</name>
|
||||
<files>apps/api/src/routes/me.ts, apps/api/tests/routes/me.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- me 2>&1 | tail -15</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>/api/me exposes isAdmin + needsProviderSetup on both paths, DB-backed, tests green.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
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`
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## 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 |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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`.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/10-admin-role-settings/10-02-SUMMARY.md` when done.
|
||||
</output>
|
||||
@@ -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'"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/gsd-core/workflows/execute-plan.md
|
||||
@$HOME/.claude/gsd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.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
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 1: Promote broker resync helpers to exports</name>
|
||||
<files>apps/api/src/broker/outboxWorker.ts</files>
|
||||
<read_first>
|
||||
- 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
|
||||
</read_first>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>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</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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).
|
||||
</acceptance_criteria>
|
||||
<done>Both broker resync helpers are exported, bodies unchanged, broker tests green.</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 2: adminRouter — guard + members + credentials + shared-calendar (RED→GREEN→REFACTOR)</name>
|
||||
<files>apps/api/src/routes/admin.ts, apps/api/src/index.ts, apps/api/tests/routes/admin.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- admin 2>&1 | tail -20</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>adminRouter exists, guard-first, mounted; members/credentials/calendars/shared routes behave per contract; no-echo + 403 hard checks green.</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
<name>Task 3: Member-scoped self-service credential endpoint POST /api/me/credential (RED→GREEN→REFACTOR)</name>
|
||||
<files>apps/api/src/routes/me.ts, apps/api/tests/routes/admin.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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).
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && pnpm --filter @familysync/api test -- credential 2>&1 | tail -15</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>Member self-service credential endpoint exists, member-scoped (no cross-member write), shares the admin validate→encrypt→sync path, tests green.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
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)
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## 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 |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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).
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/10-admin-role-settings/10-03-SUMMARY.md` when done.
|
||||
</output>
|
||||
@@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/gsd-core/workflows/execute-plan.md
|
||||
@$HOME/.claude/gsd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.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
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 1: Extend client.ts — MeUser fields + admin/self-service fetchers</name>
|
||||
<files>apps/pwa/src/api/client.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /home/luc/Projects/familysync && pnpm --filter @familysync/pwa exec tsc --noEmit 2>&1 | tail -8</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>client.ts exposes the new MeUser flags + five typed admin/self-service fetchers, typechecks clean.</done>
|
||||
</task>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 2: CredentialSheet + SetupBanner components</name>
|
||||
<files>apps/pwa/src/components/CredentialSheet.tsx, apps/pwa/src/components/SetupBanner.tsx</files>
|
||||
<read_first>
|
||||
- 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"
|
||||
</read_first>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>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</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>CredentialSheet (admin + self-service) and SetupBanner match the UI-SPEC contract, accessible, token-styled, typecheck clean.</done>
|
||||
</task>
|
||||
|
||||
<task type="execute">
|
||||
<name>Task 3: /admin route + AdminPage + conditional nav entries, with playwright-cli verification</name>
|
||||
<files>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</files>
|
||||
<read_first>
|
||||
- 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"
|
||||
</read_first>
|
||||
<action>
|
||||
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 ? <AdminPage/> : <Navigate to="/calendar" replace/>` — 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 `<SetupBanner/>` 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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>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</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `apps/pwa/src/App.tsx` has a `/admin` Route gated on `meQuery.data?.user.isAdmin` with a `<Navigate to="/calendar" replace/>` fallback for non-admins, and mounts `<SetupBanner/>`.
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>/admin route gated, AdminPage wired to the admin API, conditional nav entries + SetupBanner mounted, e2e + playwright-cli verification green.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<artifacts_this_phase_produces>
|
||||
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 + `<SetupBanner/>` 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`
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
<threat_model>
|
||||
## 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 |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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]].
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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).
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/10-admin-role-settings/10-04-SUMMARY.md` when done.
|
||||
</output>
|
||||
@@ -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 `<automated>` 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 `<automated>` 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 `<automated>` 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
|
||||
|
||||
Reference in New Issue
Block a user