From 9c99d56f99f7ccf34e035e4dd8a4ddcebeaaaa85 Mon Sep 17 00:00:00 2001 From: Bas van Rossem Date: Thu, 23 Jul 2026 13:43:59 +0200 Subject: [PATCH] =?UTF-8?q?test(api):=20Admin=20users=20=E2=80=94=20fill?= =?UTF-8?q?=20gaps=20+=20lifecycle=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fill the remaining coverage gaps in src/routes/admin-users.ts and add the domain lifecycle test, bringing the file to 100% stmts/branch/funcs/lines. New tests (apps/api/test/admin-users.test.ts): - password route: 404 unknown id; 400 when better-auth setUserPassword throws (driven via a spy so the catch path is deterministic). - toListItem: a null DB role lists as 'worker' (nullish-coalescing fallback). - role route: 400 on an invalid role body; 404 on an unknown id. - reactivate route: 404 on an unknown id. Lifecycle (apps/api/test/integration/user-lifecycle.test.ts): admin creates a worker -> worker signs in and the bearer authorizes /api/me -> admin deactivates (session rows deleted, so the pre-issued token 401s and fresh sign-in is blocked) -> admin reactivates -> sign-in works again. The two last-admin guards (role + deactivate routes) are unreachable over HTTP (the self-demote / self-deactivate guards above already handle caller===target, so any caller reaching them is a distinct active admin and activeAdminsExcluding(target) >= 1). Annotated with /* c8 ignore */ + reasons rather than left as silent gaps; the guard logic is unit-tested via activeAdminsExcluding. No production behavior changed; no bugs found. --- apps/api/src/routes/admin-users.ts | 2 + apps/api/test/admin-users.test.ts | 89 ++++++++++++++++++- .../test/integration/user-lifecycle.test.ts | 72 +++++++++++++++ docs/reference/domain-test-checklist.md | 33 +++++++ 4 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 apps/api/test/integration/user-lifecycle.test.ts diff --git a/apps/api/src/routes/admin-users.ts b/apps/api/src/routes/admin-users.ts index 928335b..b50d60a 100644 --- a/apps/api/src/routes/admin-users.ts +++ b/apps/api/src/routes/admin-users.ts @@ -86,6 +86,7 @@ adminUsersRoutes.post('/api/admin/users/:id/role', async (c) => { return c.json({ error: 'Je kunt jezelf niet degraderen.' }, 400); } if (target.role === 'admin' && newRole !== 'admin') { + /* c8 ignore next 3 -- unreachable over HTTP: the self-demote guard above already handles caller===target, so any caller reaching here is a distinct active admin and activeAdminsExcluding(target) >= 1; the last-admin invariant is unit-tested via activeAdminsExcluding */ if ((await activeAdminsExcluding(id)) === 0) { return c.json({ error: 'Er moet minstens één actieve beheerder blijven.' }, 400); } @@ -103,6 +104,7 @@ adminUsersRoutes.post('/api/admin/users/:id/deactivate', async (c) => { if (!target) return c.json({ error: 'Gebruiker niet gevonden' }, 404); if (id === caller?.id) return c.json({ error: 'Je kunt jezelf niet deactiveren.' }, 400); + /* c8 ignore next 3 -- unreachable over HTTP: the self-deactivate guard above ensures the caller is a distinct active admin, so activeAdminsExcluding(target) >= 1 and this last-admin branch never fires; the invariant is unit-tested via activeAdminsExcluding */ if (target.role === 'admin' && (await activeAdminsExcluding(id)) === 0) { return c.json({ error: 'Er moet minstens één actieve beheerder blijven.' }, 400); } diff --git a/apps/api/test/admin-users.test.ts b/apps/api/test/admin-users.test.ts index d43179c..45e16b7 100644 --- a/apps/api/test/admin-users.test.ts +++ b/apps/api/test/admin-users.test.ts @@ -1,10 +1,11 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import type { Hono } from 'hono'; import { createApp } from '../src/app'; import { authToken, bearer } from './helpers'; import { db } from '../src/db/client'; import { user } from '../src/db/schema'; import { eq } from 'drizzle-orm'; +import { auth } from '../src/auth'; import { activeAdminsExcluding } from '../src/routes/admin-users'; const PASSWORD = 'sterk-wachtwoord-123'; @@ -391,4 +392,90 @@ describe('POST /api/admin/users/:id/password', () => { ).status, ).toBe(403); }); + + it('404s on an unknown id', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'pw-404-admin@example.com', 'admin'); + const res = await app.request('/api/admin/users/does-not-exist/password', { + method: 'POST', + headers: bearer(adminTok), + body: JSON.stringify({ password: 'long-enough-123' }), + }); + expect(res.status).toBe(404); + expect((await res.json()).error).toContain('niet gevonden'); + }); + + // When better-auth's setUserPassword throws, the route must surface a 400 (not a 500). + it('400s when better-auth setUserPassword throws', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'pw-fail-admin@example.com', 'admin'); + await authToken(app, 'pw-fail-target@example.com'); + const id = await userIdByEmail('pw-fail-target@example.com'); + + const spy = vi.spyOn(auth.api, 'setUserPassword').mockRejectedValueOnce(new Error('boom')); + try { + const res = await app.request(`/api/admin/users/${id}/password`, { + method: 'POST', + headers: bearer(adminTok), + body: JSON.stringify({ password: 'long-enough-123' }), + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe('Wachtwoord wijzigen mislukt.'); + } finally { + spy.mockRestore(); + } + }); +}); + +// Gap-fill: branches not exercised by the suites above (invalid input, not-found paths, +// and the null-role list fallback in toListItem). +describe('admin-users — validation & not-found gaps', () => { + it('lists a user with a null DB role as a worker (toListItem fallback)', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'nullrole-admin@example.com', 'admin'); + await authToken(app, 'nullrole-target@example.com'); + const id = await userIdByEmail('nullrole-target@example.com'); + await db.update(user).set({ role: null }).where(eq(user.id, id)); + + const res = await app.request('/api/admin/users', { headers: bearer(adminTok) }); + const list = (await res.json()) as Array<{ id: string; role: string }>; + expect(list.find((u) => u.id === id)?.role).toBe('worker'); + }); + + it('400s the role route on an invalid role body', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'role-invalid-admin@example.com', 'admin'); + await authToken(app, 'role-invalid-target@example.com'); + const id = await userIdByEmail('role-invalid-target@example.com'); + const res = await app.request(`/api/admin/users/${id}/role`, { + method: 'POST', + headers: bearer(adminTok), + body: JSON.stringify({ role: 'superuser' }), + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe('Invalid input'); + }); + + it('404s the role route on an unknown id', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'role-404-admin@example.com', 'admin'); + const res = await app.request('/api/admin/users/does-not-exist/role', { + method: 'POST', + headers: bearer(adminTok), + body: JSON.stringify({ role: 'admin' }), + }); + expect(res.status).toBe(404); + expect((await res.json()).error).toContain('niet gevonden'); + }); + + it('404s the reactivate route on an unknown id', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'react-404-admin@example.com', 'admin'); + const res = await app.request('/api/admin/users/does-not-exist/reactivate', { + method: 'POST', + headers: bearer(adminTok), + }); + expect(res.status).toBe(404); + expect((await res.json()).error).toContain('niet gevonden'); + }); }); diff --git a/apps/api/test/integration/user-lifecycle.test.ts b/apps/api/test/integration/user-lifecycle.test.ts new file mode 100644 index 0000000..c308bcb --- /dev/null +++ b/apps/api/test/integration/user-lifecycle.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { eq } from 'drizzle-orm'; +import { createApp } from '../../src/app'; +import { db } from '../../src/db/client'; +import { session, user } from '../../src/db/schema'; +import { authToken, bearer, resetDb } from '../helpers'; + +const PASSWORD = 'sterk-wachtwoord-123'; + +async function signIn(app: ReturnType, email: string) { + return app.request('/api/auth/sign-in/email', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ email, password: PASSWORD }), + }); +} + +describe('admin user lifecycle (create → sign in → deactivate → reactivate)', () => { + beforeEach(resetDb); + + it('deactivate revokes the live token and blocks sign-in; reactivate restores both', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'lifecycle-admin@example.com', 'admin'); + + // admin creates a worker + const createRes = await app.request('/api/admin/users', { + method: 'POST', + headers: bearer(adminTok), + body: JSON.stringify({ + email: 'lifecycle-worker@example.com', + name: 'Lifecycle Worker', + password: PASSWORD, + role: 'worker', + }), + }); + expect(createRes.status).toBe(200); + const created = await createRes.json(); + expect(created.status).toBe('active'); + + // worker signs in → the issued bearer authorizes /api/me + const firstSignIn = await signIn(app, 'lifecycle-worker@example.com'); + expect(firstSignIn.status).toBe(200); + const workerTok = firstSignIn.headers.get('set-auth-token'); + if (!workerTok) throw new Error('no worker token'); + expect((await app.request('/api/me', { headers: bearer(workerTok) })).status).toBe(200); + + // admin deactivates the worker + const off = await app.request(`/api/admin/users/${created.id}/deactivate`, { + method: 'POST', + headers: bearer(adminTok), + }); + expect(off.status).toBe(200); + expect((await off.json()).status).toBe('inactive'); + + // deactivate deleted the session rows → the pre-issued token no longer authorizes /api/me + expect(await db.select().from(session).where(eq(session.userId, created.id))).toHaveLength(0); + expect((await app.request('/api/me', { headers: bearer(workerTok) })).status).toBe(401); + + // and a fresh sign-in is blocked while banned + expect((await signIn(app, 'lifecycle-worker@example.com')).status).not.toBe(200); + + // admin reactivates → ban cleared, sign-in works again + const on = await app.request(`/api/admin/users/${created.id}/reactivate`, { + method: 'POST', + headers: bearer(adminTok), + }); + expect((await on.json()).status).toBe('active'); + const [row] = await db.select().from(user).where(eq(user.id, created.id)); + expect(row.banned).toBeFalsy(); + expect((await signIn(app, 'lifecycle-worker@example.com')).status).toBe(200); + }); +}); diff --git a/docs/reference/domain-test-checklist.md b/docs/reference/domain-test-checklist.md index b2fbb87..1b75e52 100644 --- a/docs/reference/domain-test-checklist.md +++ b/docs/reference/domain-test-checklist.md @@ -222,3 +222,36 @@ Guaranteed behaviors: workers × two activities × two insole types → assert exact totals and every `by_worker`, `by_activity`, `by_type` row (values and descending order) → apply a `user_id` filter → assert the totals and breakdowns narrow to that worker only. + +## Admin users + +Src covered: `src/routes/admin-users.ts` — 100% stmts/branch/funcs/lines. The two last-admin +guards (in the role and deactivate routes) are `c8 ignore`d with reasons: they are unreachable +over HTTP because the self-demote / self-deactivate guards above already handle `caller === target`, +so any caller reaching them is a distinct active admin and `activeAdminsExcluding(target) >= 1`; the +guard logic itself is unit-tested through `activeAdminsExcluding`. + +Guaranteed behaviors: + +- Every `/api/admin/users*` route is gated: 401 without a token, 403 for a worker, and 403 for a + banned admin that still holds a valid token (`adminGuard` defense-in-depth). +- `GET /api/admin/users` returns every user ordered by name, each mapped by `toListItem` to + `{ id, email, name, role, status, created_at }`; a null DB `role` falls back to `worker` and a + `banned` row surfaces as `status: 'inactive'`. +- `POST /api/admin/users` creates a user who can then sign in; 409 on a duplicate email; 400 on a + schema-invalid body (incl. a password shorter than 8 chars); a mixed-case email is stored + lowercased and the created row is returned (no 500 on the re-select). +- `POST /:id/role` promotes/demotes and returns the updated row; 400 on an invalid role body; 404 on + an unknown id; refuses self-demotion (400); allows demoting a peer admin while the caller stays + admin (200). +- `activeAdminsExcluding` (unit) counts only active (non-banned) admins and honours the excluded id. +- `POST /:id/deactivate` bans the user, clears any live session rows, and returns + `status: 'inactive'`; 404 on an unknown id; refuses self-deactivation (400); allows deactivating a + peer admin while always blocking self. +- `POST /:id/reactivate` clears the ban and returns `status: 'active'`; 404 on an unknown id. +- `POST /:id/password` sets a new password (new works, old fails); 400 on a too-short password; 404 + on an unknown id; surfaces a 400 (not a 500) when better-auth's `setUserPassword` throws. +- Lifecycle (`test/integration/user-lifecycle.test.ts`): admin creates a worker → worker signs in + and the bearer authorizes `/api/me` → admin deactivates → the session rows are deleted so the + pre-issued token no longer authorizes `/api/me` (401) **and** a fresh sign-in is blocked → admin + reactivates → the ban is cleared and sign-in works again.