diff --git a/apps/api/src/lib/require-user.ts b/apps/api/src/lib/require-user.ts index 0c5dd12..3c90b90 100644 --- a/apps/api/src/lib/require-user.ts +++ b/apps/api/src/lib/require-user.ts @@ -4,23 +4,26 @@ import { auth } from '../auth'; export interface SessionUser { id: string; role: string; + banned: boolean; } export async function getSessionUser(c: Context): Promise { const session = await auth.api.getSession({ headers: c.req.raw.headers }); if (!session) return null; - const role = (session.user as { role?: string | null }).role ?? 'worker'; - return { id: session.user.id, role }; + const u = session.user as { role?: string | null; banned?: boolean | null }; + return { id: session.user.id, role: u.role ?? 'worker', banned: Boolean(u.banned) }; } export function isAdmin(u: SessionUser | null): boolean { return u?.role === 'admin'; } -// Reusable gate for the whole /api/admin/* surface: 401 if unauthenticated, 403 if not an admin. +// Reusable gate for the whole /api/admin/* surface: 401 if unauthenticated, 403 if not an +// active admin. A banned admin is rejected even with a still-valid session (defense-in-depth +// beyond deactivate revoking their sessions). export const adminGuard: MiddlewareHandler = async (c, next) => { const u = await getSessionUser(c); if (!u) return c.json({ error: 'Unauthorized' }, 401); - if (!isAdmin(u)) return c.json({ error: 'Forbidden' }, 403); + if (u.banned || !isAdmin(u)) return c.json({ error: 'Forbidden' }, 403); await next(); }; diff --git a/apps/api/src/routes/admin-users.ts b/apps/api/src/routes/admin-users.ts index e288203..928335b 100644 --- a/apps/api/src/routes/admin-users.ts +++ b/apps/api/src/routes/admin-users.ts @@ -57,7 +57,9 @@ adminUsersRoutes.post('/api/admin/users', async (c) => { return c.json({ error: 'E-mailadres bestaat al.' }, 409); } - const [row] = await db.select().from(user).where(eq(user.email, d.email)); + // better-auth lowercases the email before storing, so re-select on the normalized form + // (zod's .email() does not lowercase) — otherwise a mixed-case email would miss and 500. + const [row] = await db.select().from(user).where(eq(user.email, d.email.toLowerCase())); return c.json(toListItem(row)); }); diff --git a/apps/api/test/admin-users.test.ts b/apps/api/test/admin-users.test.ts index 8c50f75..d43179c 100644 --- a/apps/api/test/admin-users.test.ts +++ b/apps/api/test/admin-users.test.ts @@ -5,6 +5,7 @@ import { authToken, bearer } from './helpers'; import { db } from '../src/db/client'; import { user } from '../src/db/schema'; import { eq } from 'drizzle-orm'; +import { activeAdminsExcluding } from '../src/routes/admin-users'; const PASSWORD = 'sterk-wachtwoord-123'; @@ -116,6 +117,23 @@ describe('POST /api/admin/users', () => { }); expect(res.status).toBe(403); }); + + it('stores a mixed-case email lowercased and returns the row (no 500)', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'mixed-admin@example.com', 'admin'); + const res = await app.request('/api/admin/users', { + method: 'POST', + headers: bearer(adminTok), + body: JSON.stringify({ + email: 'Mixed.Case@Example.com', + name: 'Mixed', + password: PASSWORD, + role: 'worker', + }), + }); + expect(res.status).toBe(200); + expect((await res.json()).email).toBe('mixed.case@example.com'); + }); }); async function userIdByEmail(email: string): Promise { @@ -161,36 +179,53 @@ describe('POST /api/admin/users/:id/role', () => { expect((await res.json()).error).toContain('degraderen'); }); - it('refuses demoting the last active admin', async () => { + // Demoting *another* admin is allowed while the acting admin remains active — this is the + // expected 200 path. The genuine last-admin guard logic is covered as a unit test of + // activeAdminsExcluding below (the route branch is unreachable over HTTP because the caller + // is always an active admin and never the target). + it('allows demoting another admin while the caller stays admin', async () => { const app = createApp(); - const adminTok = await authToken(app, 'role-last-admin@example.com', 'admin'); - // Promote a worker, then that worker is the only *other* admin; demote them while the caller - // is also admin — but to hit the last-admin path, demote the caller's only peer after demoting caller is blocked. - // Simplest: single admin scenario — create a second admin, demote them (ok), then try to demote remaining peer. + const adminTok = await authToken(app, 'role-peer-admin@example.com', 'admin'); await authToken(app, 'role-peer@example.com', 'admin'); const peerId = await userIdByEmail('role-peer@example.com'); - // Demote the peer: now caller is the only admin. Allowed (caller remains). - await app.request(`/api/admin/users/${peerId}/role`, { - method: 'POST', - headers: bearer(adminTok), - body: JSON.stringify({ role: 'worker' }), - }); - // Re-promote peer, then demote caller-equivalent is blocked by self-guard; instead verify the - // invariant directly: ban the caller is self-blocked, so assert the helper via a constructed case. - // Here we assert: with only the caller as admin, demoting *any other* admin is impossible because none exist, - // so we validate the guard by promoting peer again and confirming a non-self demote still leaves >=1 admin (caller). - await app.request(`/api/admin/users/${peerId}/role`, { - method: 'POST', - headers: bearer(adminTok), - body: JSON.stringify({ role: 'admin' }), - }); const res = await app.request(`/api/admin/users/${peerId}/role`, { method: 'POST', headers: bearer(adminTok), body: JSON.stringify({ role: 'worker' }), }); - // caller is still admin, so demoting the peer is allowed (>=1 admin remains). expect(res.status).toBe(200); + expect((await res.json()).role).toBe('worker'); + }); +}); + +// Direct unit coverage of the last-admin invariant helper (the route branch that uses it is +// unreachable over HTTP, so this is where its !banned filter + exclusion logic is actually tested). +describe('activeAdminsExcluding', () => { + it('counts only active (non-banned) admins and honours the exclusion', async () => { + const app = createApp(); + const before = await activeAdminsExcluding('___none___'); + + await authToken(app, 'aae-fresh-admin@example.com', 'admin'); + expect(await activeAdminsExcluding('___none___')).toBe(before + 1); // a new active admin counts + + const freshId = await userIdByEmail('aae-fresh-admin@example.com'); + expect(await activeAdminsExcluding(freshId)).toBe(before); // excluding it drops the count back + + await db.update(user).set({ banned: true }).where(eq(user.id, freshId)); + expect(await activeAdminsExcluding('___none___')).toBe(before); // a banned admin is not counted + }); +}); + +describe('adminGuard rejects banned admins', () => { + it('403s a banned admin that still holds a valid token', async () => { + const app = createApp(); + const adminTok = await authToken(app, 'banned-admin@example.com', 'admin'); + expect((await app.request('/api/admin/users', { headers: bearer(adminTok) })).status).toBe(200); + + // Ban directly (not via /deactivate, which would also delete the session) so the token stays valid. + const id = await userIdByEmail('banned-admin@example.com'); + await db.update(user).set({ banned: true }).where(eq(user.id, id)); + expect((await app.request('/api/admin/users', { headers: bearer(adminTok) })).status).toBe(403); }); }); @@ -250,10 +285,11 @@ describe('deactivate / reactivate', () => { expect((await res.json()).error).toContain('deactiveren'); }); - it('refuses deactivating the last active admin', async () => { + // Deactivating another admin is allowed (the caller remains); the caller can never deactivate + // themselves even as the sole remaining active admin — together these preserve the invariant. + it('allows deactivating another admin but always blocks self-deactivate', async () => { const app = createApp(); const adminTok = await authToken(app, 'deact-last-admin@example.com', 'admin'); - // promote a peer, deactivate caller is self-blocked; deactivate the peer is allowed (caller remains). await authToken(app, 'deact-peer@example.com', 'admin'); const peerId = await userIdByEmail('deact-peer@example.com'); const res = await app.request(`/api/admin/users/${peerId}/deactivate`, { @@ -261,13 +297,14 @@ describe('deactivate / reactivate', () => { headers: bearer(adminTok), }); expect(res.status).toBe(200); // caller still admin → allowed - // Now the peer is inactive; the caller is the only active admin. Deactivating self is blocked: + + // The caller is now the only active admin; deactivating self is refused. const callerId = await userIdByEmail('deact-last-admin@example.com'); const selfRes = await app.request(`/api/admin/users/${callerId}/deactivate`, { method: 'POST', headers: bearer(adminTok), }); - expect(selfRes.status).toBe(400); // self-guard (which also preserves the last admin) + expect(selfRes.status).toBe(400); }); it('404s on an unknown id and 403s for a worker', async () => {