docs(10): revise phase plan per plan-checker feedback (3 blockers, 2 warnings)
This commit is contained in:
Lucas Berger
@@ -6,6 +6,7 @@ wave: 3
|
||||
depends_on: ["10-01", "10-02"]
|
||||
files_modified:
|
||||
- apps/api/src/broker/outboxWorker.ts
|
||||
- apps/api/src/broker/credentialSync.ts
|
||||
- apps/api/src/routes/admin.ts
|
||||
- apps/api/src/routes/me.ts
|
||||
- apps/api/src/index.ts
|
||||
@@ -18,11 +19,16 @@ must_haves:
|
||||
- "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"
|
||||
- "Both POST /api/admin/credentials and POST /api/me/credential call ONE shared validateEncryptAndStoreCredential helper (no duplicated validate/encrypt/store logic)"
|
||||
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/broker/credentialSync.ts"
|
||||
provides: "shared validateEncryptAndStoreCredential(userId, fastmailEmail, appPassword, providerType) helper used by BOTH admin + self-service paths"
|
||||
exports: ["validateEncryptAndStoreCredential"]
|
||||
contains: "validateEncryptAndStoreCredential"
|
||||
- path: "apps/api/src/routes/me.ts"
|
||||
provides: "POST /api/me/credential member-scoped self-service (currentUserId only)"
|
||||
contains: "credential"
|
||||
@@ -35,9 +41,13 @@ must_haves:
|
||||
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"
|
||||
to: "validateEncryptAndStoreCredential"
|
||||
via: "import from ../broker/credentialSync.js (shared validate→encrypt→sync path)"
|
||||
pattern: "validateEncryptAndStoreCredential"
|
||||
- from: "apps/api/src/routes/me.ts"
|
||||
to: "validateEncryptAndStoreCredential"
|
||||
via: "import from ../broker/credentialSync.js (same helper, currentUserId)"
|
||||
pattern: "validateEncryptAndStoreCredential"
|
||||
- from: "apps/api/src/index.ts"
|
||||
to: "adminRouter"
|
||||
via: "app.route('/api/admin', adminRouter)"
|
||||
@@ -45,10 +55,10 @@ must_haves:
|
||||
---
|
||||
|
||||
<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.
|
||||
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 `validateEncryptAndStoreCredential` validate→encrypt→initial-sync helper. Promote the broker's private resync helpers to exports so the shared helper can 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.
|
||||
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, and it MUST call the exact same credential helper to avoid divergence.
|
||||
Output: Exported broker helpers, a new shared `credentialSync.ts` helper, the 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>
|
||||
@@ -79,7 +89,7 @@ Output: Exported broker helpers, new `admin.ts` router, the `/api/me/credential`
|
||||
- .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.
|
||||
Add the `export` keyword to `loadClientForUser` and `triggerTargetedResync` in `apps/api/src/broker/outboxWorker.ts` so the shared `credentialSync.ts` helper (Task 2) 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 shared helper 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>
|
||||
@@ -93,8 +103,8 @@ Output: Exported broker helpers, new `admin.ts` router, the `/api/me/credential`
|
||||
</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>
|
||||
<name>Task 2: Shared credentialSync helper + adminRouter (guard + members + credentials + shared-calendar) (RED→GREEN→REFACTOR)</name>
|
||||
<files>apps/api/src/broker/credentialSync.ts, 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)
|
||||
@@ -107,34 +117,43 @@ Output: Exported broker helpers, new `admin.ts` router, the `/api/me/credential`
|
||||
</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 (Pitfall 7, validation→400 mapping): 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 (Pitfall 7, all failure modes map to one generic 400): a malformed/bad-email payload that makes `createFastmailClient` throw, AND a network/connection error before PROPFIND, BOTH return `{ error: 'Invalid request' }` with status 400 (same generic shape as a PROPFIND auth failure) and never echo the submitted password.
|
||||
- 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):
|
||||
First create the SHARED helper `apps/api/src/broker/credentialSync.ts` exporting ONE function `validateEncryptAndStoreCredential(userId: number, fastmailEmail: string, appPassword: string, providerType: string)`. This is the single source of the validate→encrypt→store→initial-sync path; both `/api/admin/credentials` (Task 2) and `/api/me/credential` (Task 3) MUST import and call it — do NOT inline this logic in admin.ts or me.ts. The helper:
|
||||
1. Wraps BOTH `createFastmailClient(fastmailEmail, appPassword)` AND `await client.fetchCalendars()` in ONE try/catch. ANY throw — bad email, malformed input, network/connection error, PROPFIND/auth failure — is treated identically as a credential-validation failure. Signal this to the caller as a single generic outcome (throw a typed `CredentialValidationError` or return a discriminated failure) that the routes map to `{ error: 'Invalid request' }` 400. NEVER include the submitted password (or any Zod/error detail) in the failure path.
|
||||
2. On success: `encryptPassword(appPassword)` → upsert `member_credentials` via `onDuplicateKeyUpdate` (uses the Plan-01 UNIQUE(user_id)) with the given `providerType`.
|
||||
3. Then fire-and-forget the initial full per-member sync (`loadClientForUser(userId)` → `fetchCalendars()` → `syncCalendar` per davCal — the helpers exported in Task 1).
|
||||
NEVER `console.log` the password, the request body, or `c.req.valid('json')` from anywhere in this path.
|
||||
|
||||
Then 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')`.
|
||||
- `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: call `validateEncryptAndStoreCredential(body.userId, body.fastmailEmail, body.appPassword, body.providerType)`; on the helper's validation-failure outcome return `c.json({ error: 'Invalid request' }, 400)` (no password in body); on success return 200. NEVER duplicate the createFastmailClient/fetchCalendars/encrypt logic here.
|
||||
- `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).
|
||||
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 the behaviors above (import `app`), confirm RED, implement to GREEN. Mock/stub CalDAV (createFastmailClient/fetchCalendars) for the validation outcomes — including the throw-on-createFastmailClient and network-error cases — 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/broker/credentialSync.ts` exports exactly one `validateEncryptAndStoreCredential` and is the only place the createFastmailClient + fetchCalendars + encryptPassword + upsert + initial-sync sequence appears (`grep -rl "createFastmailClient" apps/api/src/routes/` returns nothing — that logic lives only in credentialSync.ts).
|
||||
- createFastmailClient failures (bad email / malformed input / network error) AND fetchCalendars (PROPFIND/auth) failures BOTH return 400 with body `{ error: 'Invalid request' }`, and the submitted password string appears nowhere in the response or logs.
|
||||
- `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.
|
||||
- No `console.log`/`console.error` of request bodies in `apps/api/src/routes/admin.ts` or `apps/api/src/broker/credentialSync.ts` (`grep -ciE "console\.(log|error)\(.*(body|valid|password)" apps/api/src/routes/admin.ts apps/api/src/broker/credentialSync.ts` returns 0).
|
||||
- `pnpm --filter @familysync/api test -- admin` passes all behaviors.
|
||||
</acceptance_criteria>
|
||||
<done>adminRouter exists, guard-first, mounted; members/credentials/calendars/shared routes behave per contract; no-echo + 403 hard checks green.</done>
|
||||
<done>credentialSync.ts holds the single shared validate→encrypt→sync helper; adminRouter exists, guard-first, mounted; members/credentials/calendars/shared routes behave per contract; all credential-validation failures map to one generic 400; no-echo + 403 hard checks green.</done>
|
||||
</task>
|
||||
|
||||
<task type="tdd" tdd="true">
|
||||
@@ -142,31 +161,31 @@ Output: Exported broker helpers, new `admin.ts` router, the `/api/me/credential`
|
||||
<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)
|
||||
- apps/api/src/broker/credentialSync.ts (from Task 2 — the SHARED validateEncryptAndStoreCredential helper this route MUST call; do NOT re-implement validate/encrypt/store)
|
||||
- apps/api/src/routes/admin.ts (from Task 2 — reuse the SAME credentialSchema shape minus userId, the SAME noEchoHook, and the SAME 400-mapping convention)
|
||||
- .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 (Pitfall 7): POST /api/me/credential with a bad credential → 400 generic `{ error: 'Invalid request' }`, no password echoed (same helper, same 400 mapping as admin).
|
||||
- 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.
|
||||
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). It MUST call the SAME shared helper from Task 2: `validateEncryptAndStoreCredential(currentUserId, body.fastmailEmail, body.appPassword, body.providerType)` (import from `../broker/credentialSync.js`). Do NOT duplicate the validate/encrypt/store/sync sequence — admin passes the target member's userId from the route/body, self-service passes the authenticated currentUserId, but both call the identical helper with identical argument order (D-07 "identical path"). Map the helper's validation-failure outcome to `c.json({ error: 'Invalid request' }, 400)`; on success 200. Add the self-service test cases to `apps/api/tests/routes/admin.test.ts` (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`.
|
||||
- `validateEncryptAndStoreCredential` is defined once in `apps/api/src/broker/credentialSync.ts`; BOTH `apps/api/src/routes/admin.ts` (POST /credentials) and `apps/api/src/routes/me.ts` (POST /credential) import and call it with identical arguments — admin passes the target member's userId from the route/body, self-service passes the authenticated currentUserId, never a body userId. `grep -rc "validateEncryptAndStoreCredential" apps/api/src/routes/admin.ts apps/api/src/routes/me.ts` shows a call in each (and the function body exists only in credentialSync.ts).
|
||||
- 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 bad credential returns 400 generic `{ error: 'Invalid request' }` 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>
|
||||
<done>Member self-service credential endpoint exists, member-scoped (no cross-member write), calls the SAME shared validateEncryptAndStoreCredential helper as the admin path, tests green.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
@@ -174,11 +193,11 @@ Output: Exported broker helpers, new `admin.ts` router, the `/api/me/credential`
|
||||
<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/broker/credentialSync.ts` exporting the single shared `validateEncryptAndStoreCredential(userId, fastmailEmail, appPassword, providerType)` helper (validate→encrypt→store→initial-sync; the only place createFastmailClient + fetchCalendars + encryptPassword + upsert live)
|
||||
- `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)
|
||||
- `POST /api/me/credential` member-scoped self-service endpoint in `apps/api/src/routes/me.ts` (calls the shared helper)
|
||||
- `apps/api/tests/routes/admin.test.ts` (+ self-service credential test cases)
|
||||
</artifacts_this_phase_produces>
|
||||
|
||||
@@ -197,10 +216,10 @@ New symbols/files created by this plan (excluded from drift verification):
|
||||
| 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-09 | Information Disclosure | app password echoed in a Zod/validation error | mitigate | noEchoHook returns `{ error: 'Invalid request' }` with no result.error; ALL credential-validation failures (createFastmailClient throw, network error, PROPFIND/auth failure) map to one generic 400 in the shared helper; 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, me.ts, or credentialSync.ts (acceptance grep == 0) |
|
||||
| T-10-11 | Information Disclosure | plaintext credential at rest | mitigate | encryptPassword (AES-256-GCM via crypto.ts) applied in the shared helper 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 passes currentUserId from the session to the shared helper 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>
|
||||
@@ -209,17 +228,20 @@ New symbols/files created by this plan (excluded from drift 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.
|
||||
- `grep -ciE "console\.(log|error)\(.*(body|valid|password)" apps/api/src/routes/admin.ts apps/api/src/broker/credentialSync.ts` == 0.
|
||||
- `validateEncryptAndStoreCredential` is imported and called by both admin.ts and me.ts; its body exists only in credentialSync.ts (no duplicated createFastmailClient/encrypt block in the routes).
|
||||
</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).
|
||||
- D-07: member self-service credential, member-scoped, SAME shared helper path (no divergence).
|
||||
- Single shared surface — no /api/setup/* duplication (Phase 12 reuses these routes + the shared helper).
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/10-admin-role-settings/10-03-SUMMARY.md` when done.
|
||||
</output>
|
||||
</content>
|
||||
</invoke>
|
||||
|
||||
Reference in New Issue
Block a user