diff --git a/apps/server/src/routes/roles.ts b/apps/server/src/routes/roles.ts index 66ef977..1a42bdd 100644 --- a/apps/server/src/routes/roles.ts +++ b/apps/server/src/routes/roles.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import type { FastifyInstance } from "fastify"; import { eq, rolePermissions, roles, users, type Db } from "@parking/db"; import { ADMIN_ROLE_ID, PERMISSIONS, type Permission } from "@parking/shared"; -import { bumpPermsCache, requirePermission } from "../auth.js"; +import { bumpPermsCache, permissionsFor, requirePermission } from "../auth.js"; // Role management (admin). Roles are DATA: an admin composes a role from the // code-defined PERMISSIONS grid (resource:action), and users are assigned one @@ -10,6 +10,13 @@ import { bumpPermsCache, requirePermission } from "../auth.js"; // edited or deleted and always resolves to every permission in code. Every write // here bumps the in-memory permission cache so changes take effect on the next // request. See @parking/shared PERMISSIONS and ../auth.ts. +// +// PRIVILEGE-ESCALATION GUARD: `role:update`/`role:create` must NOT let a caller +// grant a permission they don't themselves hold — otherwise a non-admin with +// `role:*` could edit their own role to add (say) `tariff:update`, or mint a role +// that grants admin-equivalent powers, and escalate. So a non-admin caller may +// only put permissions they ALREADY hold onto a role. An admin (full set) is +// unrestricted, which is the intended behaviour. interface RoleBody { name: string; @@ -77,6 +84,15 @@ export async function roleRoutes(app: FastifyInstance, db: Db): Promise { }; }); + /** Reject any permission in `perms` the caller does not themselves hold — so a + * non-admin can't grant privileges beyond their own. Returns the offending + * permission, or null if all are within the caller's set. (Admin holds the full + * set, so it never trips.) */ + function escalates(callerRoleId: string, perms: Permission[]): Permission | null { + const held = permissionsFor(callerRoleId); + return perms.find((p) => !held.has(p)) ?? null; + } + // Create a composable role from a name + a permission set. app.post<{ Body: RoleBody }>("/api/roles", { preHandler: createGuard }, async (req, reply) => { const name = (req.body?.name ?? "").trim(); @@ -86,6 +102,8 @@ export async function roleRoutes(app: FastifyInstance, db: Db): Promise { } const cleaned = cleanPermissions(req.body?.permissions ?? []); if (!cleaned.ok) return reply.code(400).send({ error: cleaned.bad }); + const over = escalates(req.user.roleId, cleaned.perms); + if (over) return reply.code(403).send({ error: `cannot grant a permission you do not hold: ${over}` }); const id = randomUUID(); db.insert(roles).values({ id, name, builtin: 0 }).run(); @@ -116,6 +134,8 @@ export async function roleRoutes(app: FastifyInstance, db: Db): Promise { if (req.body?.permissions != null) { const cleaned = cleanPermissions(req.body.permissions); if (!cleaned.ok) return reply.code(400).send({ error: cleaned.bad }); + const over = escalates(req.user.roleId, cleaned.perms); + if (over) return reply.code(403).send({ error: `cannot grant a permission you do not hold: ${over}` }); setPermissions(id, cleaned.perms); } bumpPermsCache(); diff --git a/apps/server/src/routes/users.ts b/apps/server/src/routes/users.ts index 12f3ee9..8967091 100644 --- a/apps/server/src/routes/users.ts +++ b/apps/server/src/routes/users.ts @@ -3,7 +3,7 @@ import bcrypt from "bcrypt"; import type { FastifyInstance } from "fastify"; import { eq, roles, users, type Db } from "@parking/db"; import { ADMIN_ROLE_ID } from "@parking/shared"; -import { requirePermission } from "../auth.js"; +import { permissionsFor, requirePermission } from "../auth.js"; // User management (admin). Users are created/edited at runtime here — the // install-time seed-admin.mjs only bootstraps the FIRST admin. Each user has one @@ -13,6 +13,13 @@ import { requirePermission } from "../auth.js"; // NO-LOCKOUT INVARIANT: the app refuses to delete, or move off the `admin` role, // the LAST user still holding `admin`. Administration can therefore never be // locked out of the appliance. See wiki/entities/local-jwt-auth.md. +// +// PRIVILEGE-ESCALATION GUARD: a non-admin caller with `user:*` must NOT be able to +// (a) ASSIGN a role whose permissions exceed their own (e.g. hand themselves or a +// peer the admin role, or any role broader than theirs), nor (b) MODIFY a user who +// already holds a role broader than the caller's (resetting an admin's password is +// account takeover; deleting an admin is sabotage). Both are blocked below by +// comparing permission SETS. An admin holds the full set, so it is unrestricted. interface CreateBody { username: string; @@ -51,6 +58,18 @@ export async function userRoutes(app: FastifyInstance, db: Db): Promise { return { id: u.id, username: u.username, roleId: u.roleId, language: u.language, createdAt: u.createdAt }; } + /** True if `targetRoleId` grants any permission the caller's role does NOT hold, + * i.e. assigning or touching it would let the caller act beyond their own + * privileges. (Admin holds the full set, so it never trips.) */ + function exceedsCaller(callerRoleId: string, targetRoleId: string): boolean { + if (callerRoleId === targetRoleId) return false; + const held = permissionsFor(callerRoleId); + for (const p of permissionsFor(targetRoleId)) { + if (!held.has(p)) return true; + } + return false; + } + // List all users (no password hashes) + their role names for display. app.get("/api/users", { preHandler: readGuard }, async () => { const rows = db.select().from(users).all(); @@ -75,6 +94,10 @@ export async function userRoutes(app: FastifyInstance, db: Db): Promise { if (!db.select().from(roles).where(eq(roles.id, roleId)).get()) { return reply.code(400).send({ error: "unknown roleId" }); } + // No-escalation: can't create a user with a role broader than your own. + if (exceedsCaller(req.user.roleId, roleId)) { + return reply.code(403).send({ error: "cannot assign a role with permissions beyond your own" }); + } if (db.select().from(users).where(eq(users.username, username)).get()) { return reply.code(409).send({ error: "username already exists" }); } @@ -93,6 +116,10 @@ export async function userRoutes(app: FastifyInstance, db: Db): Promise { const id = req.params.id; const existing = db.select().from(users).where(eq(users.id, id)).get(); if (!existing) return reply.code(404).send({ error: "user not found" }); + // No-escalation: can't modify a user who already outranks you. + if (exceedsCaller(req.user.roleId, existing.roleId)) { + return reply.code(403).send({ error: "cannot modify a user whose role exceeds your own" }); + } const next: { username?: string; roleId?: string } = {}; if (req.body?.username != null) { @@ -107,6 +134,10 @@ export async function userRoutes(app: FastifyInstance, db: Db): Promise { if (!db.select().from(roles).where(eq(roles.id, roleId)).get()) { return reply.code(400).send({ error: "unknown roleId" }); } + // No-escalation: can't promote a user into a role broader than your own. + if (exceedsCaller(req.user.roleId, roleId)) { + return reply.code(403).send({ error: "cannot assign a role with permissions beyond your own" }); + } // No-lockout: don't move the last admin off the admin role. if (roleId !== ADMIN_ROLE_ID && isLastAdmin(id)) { return reply.code(409).send({ error: "cannot change the role of the last admin" }); @@ -127,9 +158,15 @@ export async function userRoutes(app: FastifyInstance, db: Db): Promise { { preHandler: updateGuard }, async (req, reply) => { const id = req.params.id; - if (!db.select().from(users).where(eq(users.id, id)).get()) { + const target = db.select().from(users).where(eq(users.id, id)).get(); + if (!target) { return reply.code(404).send({ error: "user not found" }); } + // No-escalation: can't reset the password of a user who outranks you + // (that would be account takeover of a more-privileged account). + if (exceedsCaller(req.user.roleId, target.roleId)) { + return reply.code(403).send({ error: "cannot reset the password of a user whose role exceeds your own" }); + } const password = req.body?.password ?? ""; if (password.length < MIN_PASSWORD) { return reply.code(400).send({ error: `password must be at least ${MIN_PASSWORD} characters` }); @@ -146,9 +183,14 @@ export async function userRoutes(app: FastifyInstance, db: Db): Promise { { preHandler: deleteGuard }, async (req, reply) => { const id = req.params.id; - if (!db.select().from(users).where(eq(users.id, id)).get()) { + const target = db.select().from(users).where(eq(users.id, id)).get(); + if (!target) { return reply.code(404).send({ error: "user not found" }); } + // No-escalation: can't delete a user who outranks you. + if (exceedsCaller(req.user.roleId, target.roleId)) { + return reply.code(403).send({ error: "cannot delete a user whose role exceeds your own" }); + } if (isLastAdmin(id)) { return reply.code(409).send({ error: "cannot delete the last admin" }); } diff --git a/wiki/entities/local-jwt-auth.md b/wiki/entities/local-jwt-auth.md index 7e70f48..3866354 100644 --- a/wiki/entities/local-jwt-auth.md +++ b/wiki/entities/local-jwt-auth.md @@ -37,6 +37,12 @@ Authentication and authorization, kept **fully local** — a direct consequence **last user holding admin** — administration can never be locked out of the appliance. - `event:void` is a permission, NOT a ledger delete: the append-only signed chain is untouched; the permission only gates who may APPEND a void event (there is no void API route yet — forward seam). +- **No privilege escalation through the RBAC system itself.** `role:create`/`role:update` and + `user:create`/`user:update` are themselves grantable, so a non-admin could otherwise self-escalate. + Guards (`routes/roles.ts`, `routes/users.ts`): a caller may only put permissions on a role that + they *already hold*, and may only assign/modify users whose role is a SUBSET of the caller's own + (so no minting a privileged role, handing out the admin role, or resetting/deleting a more- + privileged account). An admin holds the full set, so it is unrestricted — the intended behaviour. ## Cookie session (browser auth)