harden(api): adminGuard rejects banned admins; fix mixed-case create + real last-admin guard coverage
This commit is contained in:
@@ -4,23 +4,26 @@ import { auth } from '../auth';
|
|||||||
export interface SessionUser {
|
export interface SessionUser {
|
||||||
id: string;
|
id: string;
|
||||||
role: string;
|
role: string;
|
||||||
|
banned: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function getSessionUser(c: Context): Promise<SessionUser | null> {
|
export async function getSessionUser(c: Context): Promise<SessionUser | null> {
|
||||||
const session = await auth.api.getSession({ headers: c.req.raw.headers });
|
const session = await auth.api.getSession({ headers: c.req.raw.headers });
|
||||||
if (!session) return null;
|
if (!session) return null;
|
||||||
const role = (session.user as { role?: string | null }).role ?? 'worker';
|
const u = session.user as { role?: string | null; banned?: boolean | null };
|
||||||
return { id: session.user.id, role };
|
return { id: session.user.id, role: u.role ?? 'worker', banned: Boolean(u.banned) };
|
||||||
}
|
}
|
||||||
|
|
||||||
export function isAdmin(u: SessionUser | null): boolean {
|
export function isAdmin(u: SessionUser | null): boolean {
|
||||||
return u?.role === 'admin';
|
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) => {
|
export const adminGuard: MiddlewareHandler = async (c, next) => {
|
||||||
const u = await getSessionUser(c);
|
const u = await getSessionUser(c);
|
||||||
if (!u) return c.json({ error: 'Unauthorized' }, 401);
|
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();
|
await next();
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -57,7 +57,9 @@ adminUsersRoutes.post('/api/admin/users', async (c) => {
|
|||||||
return c.json({ error: 'E-mailadres bestaat al.' }, 409);
|
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));
|
return c.json(toListItem(row));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { authToken, bearer } from './helpers';
|
|||||||
import { db } from '../src/db/client';
|
import { db } from '../src/db/client';
|
||||||
import { user } from '../src/db/schema';
|
import { user } from '../src/db/schema';
|
||||||
import { eq } from 'drizzle-orm';
|
import { eq } from 'drizzle-orm';
|
||||||
|
import { activeAdminsExcluding } from '../src/routes/admin-users';
|
||||||
|
|
||||||
const PASSWORD = 'sterk-wachtwoord-123';
|
const PASSWORD = 'sterk-wachtwoord-123';
|
||||||
|
|
||||||
@@ -116,6 +117,23 @@ describe('POST /api/admin/users', () => {
|
|||||||
});
|
});
|
||||||
expect(res.status).toBe(403);
|
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<string> {
|
async function userIdByEmail(email: string): Promise<string> {
|
||||||
@@ -161,36 +179,53 @@ describe('POST /api/admin/users/:id/role', () => {
|
|||||||
expect((await res.json()).error).toContain('degraderen');
|
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 app = createApp();
|
||||||
const adminTok = await authToken(app, 'role-last-admin@example.com', 'admin');
|
const adminTok = await authToken(app, 'role-peer-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.
|
|
||||||
await authToken(app, 'role-peer@example.com', 'admin');
|
await authToken(app, 'role-peer@example.com', 'admin');
|
||||||
const peerId = await userIdByEmail('role-peer@example.com');
|
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`, {
|
const res = await app.request(`/api/admin/users/${peerId}/role`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: bearer(adminTok),
|
headers: bearer(adminTok),
|
||||||
body: JSON.stringify({ role: 'worker' }),
|
body: JSON.stringify({ role: 'worker' }),
|
||||||
});
|
});
|
||||||
// caller is still admin, so demoting the peer is allowed (>=1 admin remains).
|
|
||||||
expect(res.status).toBe(200);
|
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');
|
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 app = createApp();
|
||||||
const adminTok = await authToken(app, 'deact-last-admin@example.com', 'admin');
|
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');
|
await authToken(app, 'deact-peer@example.com', 'admin');
|
||||||
const peerId = await userIdByEmail('deact-peer@example.com');
|
const peerId = await userIdByEmail('deact-peer@example.com');
|
||||||
const res = await app.request(`/api/admin/users/${peerId}/deactivate`, {
|
const res = await app.request(`/api/admin/users/${peerId}/deactivate`, {
|
||||||
@@ -261,13 +297,14 @@ describe('deactivate / reactivate', () => {
|
|||||||
headers: bearer(adminTok),
|
headers: bearer(adminTok),
|
||||||
});
|
});
|
||||||
expect(res.status).toBe(200); // caller still admin → allowed
|
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 callerId = await userIdByEmail('deact-last-admin@example.com');
|
||||||
const selfRes = await app.request(`/api/admin/users/${callerId}/deactivate`, {
|
const selfRes = await app.request(`/api/admin/users/${callerId}/deactivate`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: bearer(adminTok),
|
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 () => {
|
it('404s on an unknown id and 403s for a worker', async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user