From a0a82ac9b65f6078aa8bfd5a30bdaecdb022268c Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 18 Jun 2026 17:18:14 -0400 Subject: [PATCH 1/3] test(20-01): add failing tests for member-profile update + last-admin guard + isAdmin read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Test A: PATCH displayName happy path → 200, GET reflects change - Test B: PATCH isAdmin promote → 200, GET shows isAdmin true - Test C: last-admin guard → 409 when only admin demotes self - Test D: self-demotion → 200 when second admin exists - Test E: non-admin PATCH → 403 (requireAdmin boundary) - Test F: wrong-type body → 400 { error: "Invalid request" }; malformed :id → 400 - Test G: non-existent member id → 404 - Test H: GET /members includes boolean isAdmin per member All 7 new tests fail RED for the right reasons (route 404 / isAdmin missing) --- apps/api/tests/routes/admin.test.ts | 179 ++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/apps/api/tests/routes/admin.test.ts b/apps/api/tests/routes/admin.test.ts index 9c4bfd9..9bd1062 100644 --- a/apps/api/tests/routes/admin.test.ts +++ b/apps/api/tests/routes/admin.test.ts @@ -1071,3 +1071,182 @@ describe('POST /api/admin/members', () => { expect(parsed.error).toBe('Invalid request'); }); }); + +// =========================================================================== +// PATCH /api/admin/members/:id — member-profile update + last-admin guard (Plan 20-01) +// =========================================================================== + +describe('PATCH /api/admin/members/:id', () => { + // Test A: happy path — update displayName only + it('Test A (happy path displayName): PATCH with { displayName } as admin returns 200; GET reflects new name', async () => { + const adminId = await seedUser('admin-patch-name', true); + const memberId = await seedUser('member-patch-target', false); + currentDevUserId = adminId; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', `/api/admin/members/${memberId}`, { displayName: 'New Name' }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { ok: boolean }; + expect(body.ok).toBe(true); + + // GET /members should reflect the updated displayName + const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members')); + expect(getRes.status).toBe(200); + const getBody = (await getRes.json()) as { members: Array<{ id: number; displayName: string }> }; + const updated = getBody.members.find((m) => m.id === memberId); + expect(updated).toBeDefined(); + expect(updated!.displayName).toBe('New Name'); + }); + + // Test B: happy path — promote non-admin to admin + it('Test B (happy path isAdmin promote): PATCH with { isAdmin: true } returns 200; GET shows isAdmin true', async () => { + const adminId = await seedUser('admin-patch-promote', true); + const memberId = await seedUser('member-patch-promote', false); + currentDevUserId = adminId; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', `/api/admin/members/${memberId}`, { isAdmin: true }), + ); + expect(res.status).toBe(200); + + const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members')); + expect(getRes.status).toBe(200); + const getBody = (await getRes.json()) as { + members: Array<{ id: number; isAdmin: boolean }>; + }; + const promoted = getBody.members.find((m) => m.id === memberId); + expect(promoted).toBeDefined(); + expect(promoted!.isAdmin).toBe(true); + }); + + // Test C: last-admin guard — only admin cannot demote themselves + it('Test C (last-admin guard): with exactly one admin, PATCH { isAdmin: false } returns 409; member stays admin', async () => { + const adminId = await seedUser('admin-last-admin', true); + currentDevUserId = adminId; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', `/api/admin/members/${adminId}`, { isAdmin: false }), + ); + expect(res.status).toBe(409); + const body = (await res.json()) as { error: string }; + expect(typeof body.error).toBe('string'); + expect(body.error.length).toBeGreaterThan(0); + + // The admin flag must still be true after the rejected demotion + const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members')); + expect(getRes.status).toBe(200); + const getBody = (await getRes.json()) as { + members: Array<{ id: number; isAdmin: boolean }>; + }; + const adminRow = getBody.members.find((m) => m.id === adminId); + expect(adminRow).toBeDefined(); + expect(adminRow!.isAdmin).toBe(true); + }); + + // Test D: self-demotion allowed when another admin exists + it('Test D (self-demotion allowed): with two admins, PATCH { isAdmin: false } returns 200; one admin remains', async () => { + const adminId1 = await seedUser('admin-demote-1', true); + const adminId2 = await seedUser('admin-demote-2', true); + currentDevUserId = adminId1; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', `/api/admin/members/${adminId1}`, { isAdmin: false }), + ); + expect(res.status).toBe(200); + + // Only adminId2 should remain as admin + const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members')); + expect(getRes.status).toBe(200); + const getBody = (await getRes.json()) as { + members: Array<{ id: number; isAdmin: boolean }>; + }; + const row1 = getBody.members.find((m) => m.id === adminId1); + const row2 = getBody.members.find((m) => m.id === adminId2); + expect(row1!.isAdmin).toBe(false); + expect(row2!.isAdmin).toBe(true); + }); + + // Test E: auth boundary — non-admin gets 403 + it('Test E (auth boundary): non-admin PATCH returns 403', async () => { + const adminId = await seedUser('admin-patch-auth', true); + const nonAdminId = await seedUser('non-admin-patch', false); + currentDevUserId = nonAdminId; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', `/api/admin/members/${adminId}`, { displayName: 'Hacked' }), + ); + expect(res.status).toBe(403); + }); + + // Test F: validation — wrong type and malformed id + it('Test F (validation): PATCH with { isAdmin: "yes" } returns 400 { error: "Invalid request" }', async () => { + const adminId = await seedUser('admin-patch-validation', true); + const memberId = await seedUser('member-patch-validation', false); + currentDevUserId = adminId; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', `/api/admin/members/${memberId}`, { isAdmin: 'yes' }), + ); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe('Invalid request'); + }); + + it('Test F (malformed id): PATCH with malformed :id (e.g. "1abc") returns 400', async () => { + const adminId = await seedUser('admin-patch-badid', true); + currentDevUserId = adminId; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', '/api/admin/members/1abc', { displayName: 'Test' }), + ); + expect(res.status).toBe(400); + }); + + // Test G: not found — non-existent member id + it('Test G (not found): PATCH non-existent member id returns 404', async () => { + const adminId = await seedUser('admin-patch-notfound', true); + currentDevUserId = adminId; + const app = await getApp(); + + const res = await app.fetch( + jsonRequest('PATCH', '/api/admin/members/99999999', { displayName: 'Ghost' }), + ); + expect(res.status).toBe(404); + }); +}); + +// =========================================================================== +// GET /api/admin/members — isAdmin field (Plan 20-01) +// =========================================================================== + +describe('GET /api/admin/members — isAdmin field', () => { + it('Test H (GET isAdmin field): each member object includes a boolean isAdmin field', async () => { + const adminId = await seedUser('admin-isadmin-field', true); + const memberId = await seedUser('member-isadmin-field', false); + currentDevUserId = adminId; + const app = await getApp(); + + const res = await app.fetch(jsonRequest('GET', '/api/admin/members')); + expect(res.status).toBe(200); + const body = (await res.json()) as { + members: Array<{ id: number; isAdmin: boolean }>; + }; + // Both seeded users should have a boolean isAdmin field + const adminRow = body.members.find((m) => m.id === adminId); + const memberRow = body.members.find((m) => m.id === memberId); + expect(adminRow).toBeDefined(); + expect(typeof adminRow!.isAdmin).toBe('boolean'); + expect(adminRow!.isAdmin).toBe(true); + expect(memberRow).toBeDefined(); + expect(typeof memberRow!.isAdmin).toBe('boolean'); + expect(memberRow!.isAdmin).toBe(false); + }); +}); From bc4863275687a5747c4816118e379a9c6dc1caee Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 18 Jun 2026 17:19:43 -0400 Subject: [PATCH 2/3] feat(20-01): add PATCH /members/:id member-profile update with last-admin guard - Add isAdmin field to GET /members select and mapped member object (D-02) - Add updateMemberSchema (displayName optional string, isAdmin optional boolean) - Register adminRouter.patch('/members/:id') with noEchoHook and requireAdmin (inherited) - Handler: parsePositiveIntParam id validation (400), existence check (404), D-03 last-admin guard via COUNT(*) query (409), partial set() update (200) - Fix Test D: switch to adminId2 for GET after self-demotion (adminId1 no longer admin) - All 44 tests green including 7 new PATCH/isAdmin tests --- apps/api/src/routes/admin.ts | 83 ++++++++++++++++++++++++++--- apps/api/tests/routes/admin.test.ts | 5 +- 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/apps/api/src/routes/admin.ts b/apps/api/src/routes/admin.ts index 3f62d77..1ffd207 100644 --- a/apps/api/src/routes/admin.ts +++ b/apps/api/src/routes/admin.ts @@ -11,12 +11,13 @@ * - No console.log of request bodies or passwords in any handler (T-10-10). * * Routes: - * GET /api/admin/members → list members + credential status (UI-SPEC Surface 2) - * POST /api/admin/members → create local member: users row + local_credentials (AUTH-LOCAL-07) - * POST /api/admin/members/:id/password → admin reset local member password (AUTH-LOCAL-08) - * POST /api/admin/credentials → validate+encrypt+store for any member (ADMIN-01) - * GET /api/admin/calendars → list synced calendars (UI-SPEC Surface 5) - * PUT /api/admin/calendars/:id/shared → exclusive is_shared designation (ADMIN-02) + * GET /api/admin/members → list members + credential status + isAdmin (UI-SPEC Surface 2) + * POST /api/admin/members → create local member: users row + local_credentials (AUTH-LOCAL-07) + * PATCH /api/admin/members/:id → update member profile: displayName and/or isAdmin (Plan 20-01) + * POST /api/admin/members/:id/password → admin reset local member password (AUTH-LOCAL-08) + * POST /api/admin/credentials → validate+encrypt+store for any member (ADMIN-01) + * GET /api/admin/calendars → list synced calendars (UI-SPEC Surface 5) + * PUT /api/admin/calendars/:id/shared → exclusive is_shared designation (ADMIN-02) * * Mounted in index.ts: app.route('/api/admin', adminRouter) */ @@ -105,6 +106,7 @@ adminRouter.get('/members', async (c) => { id: users.id, displayName: users.displayName, color: users.color, + isAdmin: users.isAdmin, // Plan 20-01: feeds editor admin toggle initial state (D-02) credentialId: memberCredentials.id, localCredId: localCredentials.id, // LEFT JOIN — null when no local_credentials row }) @@ -116,6 +118,7 @@ adminRouter.get('/members', async (c) => { id: row.id, displayName: row.displayName, color: row.color, + isAdmin: row.isAdmin, // Plan 20-01 hasCredential: row.credentialId !== null, hasLocalCredential: row.localCredId !== null, // AUTH-LOCAL-17 })); @@ -208,6 +211,74 @@ adminRouter.post('/members', zValidator('json', createMemberSchema, noEchoHook), } }); +// --------------------------------------------------------------------------- +// PATCH /api/admin/members/:id +// +// Updates a member's displayName and/or isAdmin flag (Plan 20-01, D-02, D-03). +// Security: +// - requireAdmin: inherited from adminRouter.use('*', requireAdmin) (D-02 — no second guard) +// - noEchoHook: applied for consistency with other admin write routes (T-20-04) +// - D-03 last-admin guard: rejects isAdmin=false when target is the sole remaining admin (T-20-02) +// - parsePositiveIntParam: rejects malformed ids (T-20-03) +// --------------------------------------------------------------------------- + +const updateMemberSchema = z.object({ + displayName: z.string().min(1).max(256).optional(), + isAdmin: z.boolean().optional(), +}); + +adminRouter.patch( + '/members/:id', + zValidator('json', updateMemberSchema, noEchoHook), + async (c) => { + const targetId = parsePositiveIntParam(c.req.param('id')); + if (targetId === null) { + return c.json({ error: 'Invalid member id' }, 400); + } + + const { displayName, isAdmin } = c.req.valid('json'); + // T-20-04: NEVER log request body + + // Verify the target user exists (404 if not) + const [target] = await db + .select({ id: users.id, isAdmin: users.isAdmin }) + .from(users) + .where(eq(users.id, targetId)) + .limit(1); + + if (!target) { + return c.json({ error: 'Member not found' }, 404); + } + + // D-03 last-admin guard: reject demotion of the only remaining admin (T-20-02) + if (isAdmin === false && target.isAdmin) { + const [{ count }] = await db + .select({ count: sql`COUNT(*)` }) + .from(users) + .where(eq(users.isAdmin, true)); + if (Number(count) <= 1) { + return c.json({ error: 'Cannot remove the last admin' }, 409); + } + } + + // Build a partial set() from whichever fields are present + const updates: { displayName?: string; isAdmin?: boolean } = {}; + if (displayName !== undefined) updates.displayName = displayName; + if (isAdmin !== undefined) updates.isAdmin = isAdmin; + + try { + await db.update(users).set(updates).where(eq(users.id, targetId)); + return c.json({ ok: true }, 200); + } catch (err) { + console.error( + '[admin/PATCH /members/:id] Unexpected error:', + err instanceof Error ? err.message : String(err), + ); + return c.json({ error: 'Service unavailable' }, 503); + } + }, +); + // --------------------------------------------------------------------------- // POST /api/admin/members/:id/password // diff --git a/apps/api/tests/routes/admin.test.ts b/apps/api/tests/routes/admin.test.ts index 9bd1062..54012de 100644 --- a/apps/api/tests/routes/admin.test.ts +++ b/apps/api/tests/routes/admin.test.ts @@ -1151,6 +1151,7 @@ describe('PATCH /api/admin/members/:id', () => { it('Test D (self-demotion allowed): with two admins, PATCH { isAdmin: false } returns 200; one admin remains', async () => { const adminId1 = await seedUser('admin-demote-1', true); const adminId2 = await seedUser('admin-demote-2', true); + // Log in as adminId1 to perform the self-demotion currentDevUserId = adminId1; const app = await getApp(); @@ -1159,7 +1160,9 @@ describe('PATCH /api/admin/members/:id', () => { ); expect(res.status).toBe(200); - // Only adminId2 should remain as admin + // Switch to adminId2 to verify the outcome — adminId1 is now non-admin + // and can no longer call GET /members (would 403). + currentDevUserId = adminId2; const getRes = await app.fetch(jsonRequest('GET', '/api/admin/members')); expect(getRes.status).toBe(200); const getBody = (await getRes.json()) as { From 4a179a943d1e3b2bcd552511b1bb50a72dc57db4 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Thu, 18 Jun 2026 17:21:30 -0400 Subject: [PATCH 3/3] =?UTF-8?q?docs(20-01):=20complete=20member-profile=20?= =?UTF-8?q?update=20plan=20=E2=80=94=20SUMMARY=20+=20all=20CI=20gates=20gr?= =?UTF-8?q?een?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../20-01-SUMMARY.md | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 .planning/phases/20-admin-member-editor-form-declutter/20-01-SUMMARY.md diff --git a/.planning/phases/20-admin-member-editor-form-declutter/20-01-SUMMARY.md b/.planning/phases/20-admin-member-editor-form-declutter/20-01-SUMMARY.md new file mode 100644 index 0000000..3e36069 --- /dev/null +++ b/.planning/phases/20-admin-member-editor-form-declutter/20-01-SUMMARY.md @@ -0,0 +1,120 @@ +--- +phase: 20-admin-member-editor-form-declutter +plan: "01" +subsystem: api/admin +status: complete +tags: [tdd, backend, admin, member-profile, last-admin-guard] +dependency_graph: + requires: [] + provides: + - "PATCH /api/admin/members/:id (member-profile update: displayName and/or isAdmin)" + - "isAdmin field on GET /api/admin/members response" + affects: + - apps/api/src/routes/admin.ts + - apps/api/tests/routes/admin.test.ts +tech_stack: + added: [] + patterns: + - "Last-admin guard via COUNT(*) query before demoting the only admin (D-03)" + - "Partial update via whichever fields are present in updateMemberSchema" + - "noEchoHook + parsePositiveIntParam reuse for new PATCH route" +key_files: + created: [] + modified: + - apps/api/src/routes/admin.ts + - apps/api/tests/routes/admin.test.ts +decisions: + - "Use PATCH verb for the member-profile update route (idiomatic REST for partial update)" + - "D-03 guard uses COUNT(*) on users.isAdmin — adapted from auth/user.ts:151-156 pattern" + - "noEchoHook applied to PATCH route for consistency even though body has no sensitive data" + - "Test D: switch currentDevUserId to adminId2 for GET verification after self-demotion (adminId1 is no longer admin post-PATCH)" +metrics: + duration: "4m" + completed: "2026-06-18" + tasks_completed: 3 + files_changed: 2 +--- + +# Phase 20 Plan 01: Member-profile update route + isAdmin read Summary + +PATCH /api/admin/members/:id with displayName/isAdmin partial update, D-03 last-admin guard (409), and isAdmin added to GET /members — implemented test-first. + +## Tasks Completed + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 | RED — failing tests for member-profile route + isAdmin read | a0a82ac | apps/api/tests/routes/admin.test.ts | +| 2 | GREEN — implement PATCH /members/:id + isAdmin in GET /members | bc48632 | apps/api/src/routes/admin.ts, apps/api/tests/routes/admin.test.ts | +| 3 | REFACTOR — tidy + pass CI gates | (no changes needed) | — | + +## What Was Built + +- **`PATCH /api/admin/members/:id`** route in `apps/api/src/routes/admin.ts`: + - Accepts `{ displayName?: string; isAdmin?: boolean }` via `updateMemberSchema` + - Protected by router-wide `requireAdmin` (no second guard — D-02) + - `parsePositiveIntParam` rejects malformed ids → 400 + - Existence check → 404 for unknown member ids + - D-03 last-admin guard: when demoting the only admin → 409 `{ error: 'Cannot remove the last admin' }` + - Self-demotion with a second admin present → 200 + - Partial `set()` from whichever fields are present; try/catch 503 fallback + - `noEchoHook` applied per T-20-04 consistency posture +- **`isAdmin` field** added to `GET /api/admin/members` select and mapped response object + +## Test Coverage (8 scenarios, all passing) + +| Test | Scenario | Status | +|------|----------|--------| +| A | displayName update → 200; GET reflects change | GREEN | +| B | isAdmin promote → 200; GET shows isAdmin true | GREEN | +| C | Last-admin demotion → 409; member stays admin | GREEN | +| D | Self-demotion with second admin → 200; one admin remains | GREEN | +| E | Non-admin PATCH → 403 (requireAdmin boundary) | GREEN | +| F | Wrong-type body → 400 Invalid request; malformed :id → 400 | GREEN | +| G | Non-existent member id → 404 | GREEN | +| H | GET /members includes boolean isAdmin per member | GREEN | + +Full suite: **44 tests passed, 0 failed**. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Test D GET called with demoted user** +- **Found during:** Task 2 (GREEN run) +- **Issue:** Test D called `GET /members` while `currentDevUserId` was still `adminId1`, who had just been demoted — resulting in 403 instead of 200 for the verification GET +- **Fix:** Switched `currentDevUserId = adminId2` before the GET call so the verification uses the remaining admin's session +- **Files modified:** apps/api/tests/routes/admin.test.ts +- **Commit:** bc48632 + +## CI Gates + +All gates pass for modified files: +- `tsc --noEmit`: pass +- `eslint src/routes/admin.ts`: pass +- `prettier --check`: pass (both files) + +## TDD Gate Compliance + +- RED gate commit: `a0a82ac` (`test(20-01): ...`) — 7 tests failing for right reasons +- GREEN gate commit: `bc48632` (`feat(20-01): ...`) — all 44 tests passing +- REFACTOR: no code changes needed — code was already clean from GREEN + +## Known Stubs + +None. + +## Threat Flags + +None — no new network surfaces beyond the planned PATCH route. All T-20-xx mitigations applied as specified. + +## Self-Check: PASSED + +| Check | Result | +|-------|--------| +| apps/api/src/routes/admin.ts | FOUND | +| apps/api/tests/routes/admin.test.ts | FOUND | +| 20-01-SUMMARY.md | FOUND | +| Commit a0a82ac (RED) | FOUND | +| Commit bc48632 (GREEN) | FOUND | +| PATCH route registered | FOUND (line 230) | +| isAdmin in GET /members select | FOUND (line 109) |