From 72237a0191ecfcc0a16ebebc82d9995e705568ae Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 13 May 2026 12:54:29 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit-wave-11):=20authz=20hardening=20?= =?UTF-8?q?=E2=80=94=20caller-superset=20on=20role=20assign?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit authz-auditor C-1 second half: while the permission-overrides PUT route already enforces caller-superset (prior wave), the `updateUser` role-reassignment path didn't. A port admin holding only \`admin.manage_users\` could PATCH a peer's roleId to a sales-director- equivalent and have the colleague execute permissions the granter didn't hold. \`updateUser\` now takes optional `callerPermissions` + `callerIsSuperAdmin` parameters and, when both are supplied (every interactive admin route), walks the new role's effective permission tree and refuses any \`true\` leaf the caller doesn't already hold. Super admins bypass by definition. Wired \`ctx.permissions\` + \`ctx.isSuperAdmin\` through the single caller (`/api/v1/admin/users/[id]` PATCH). Legacy callers that omit the args (none currently) would silently skip the check; if any future system job calls \`updateUser\` it should pass `callerPermissions=ctx.permissions` explicitly. Other authz items confirmed resolved by earlier work or by-design: - C-1 (permission-overrides PUT): caller-superset already shipped in an earlier wave; verified by reading the route. - H-1 (alerts GET ungated): already gated on \`admin.view_audit_log\` per the auditor's tier-4 recommendation. Tests 1315/1315. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/api/v1/admin/users/[id]/route.ts | 19 +++++++++----- src/lib/services/users.service.ts | 32 +++++++++++++++++++++++- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/app/api/v1/admin/users/[id]/route.ts b/src/app/api/v1/admin/users/[id]/route.ts index 01f9bea0..6902e9ae 100644 --- a/src/app/api/v1/admin/users/[id]/route.ts +++ b/src/app/api/v1/admin/users/[id]/route.ts @@ -21,12 +21,19 @@ export const PATCH = withAuth( withPermission('admin', 'manage_users', async (req, ctx, params) => { try { const body = await parseBody(req, updateUserSchema); - const data = await updateUser(params.id!, ctx.portId, body, { - userId: ctx.userId, - portId: ctx.portId, - ipAddress: ctx.ipAddress, - userAgent: ctx.userAgent, - }); + const data = await updateUser( + params.id!, + ctx.portId, + body, + { + userId: ctx.userId, + portId: ctx.portId, + ipAddress: ctx.ipAddress, + userAgent: ctx.userAgent, + }, + ctx.permissions as Record> | null, + ctx.isSuperAdmin, + ); return NextResponse.json({ data }); } catch (error) { return errorResponse(error); diff --git a/src/lib/services/users.service.ts b/src/lib/services/users.service.ts index c440ae11..940c20b1 100644 --- a/src/lib/services/users.service.ts +++ b/src/lib/services/users.service.ts @@ -4,7 +4,7 @@ import { db } from '@/lib/db'; import { account, session, user, userProfiles, userPortRoles, roles, ports } from '@/lib/db/schema'; import { auth } from '@/lib/auth'; import { createAuditLog, type AuditMeta } from '@/lib/audit'; -import { ConflictError, NotFoundError, ValidationError } from '@/lib/errors'; +import { ConflictError, ForbiddenError, NotFoundError, ValidationError } from '@/lib/errors'; import { emitToRoom } from '@/lib/socket/server'; import { sendEmail } from '@/lib/email'; import { adminEmailChangeEmail } from '@/lib/email/templates/admin-email-change'; @@ -207,6 +207,15 @@ export async function updateUser( portId: string, data: UpdateUserInput, meta: AuditMeta, + /** + * Caller's effective permission map at call time. Used to enforce the + * caller-superset rule on role reassignment so a port admin holding + * only `admin.manage_users` can't promote a peer to a role that grants + * permissions the caller doesn't themselves hold (authz-auditor C-1 + * parallel path). Super admins bypass via `callerIsSuperAdmin`. + */ + callerPermissions?: Record> | null, + callerIsSuperAdmin?: boolean, ) { const profile = await db.query.userProfiles.findFirst({ where: eq(userProfiles.userId, userId), @@ -287,6 +296,27 @@ export async function updateUser( where: eq(roles.id, data.roleId), }); if (!newRole) throw new ValidationError('Invalid role ID'); + + // Caller-superset check (authz-auditor C-1 parallel path): refuse to + // assign a role whose effective permission set contains any leaf + // the caller doesn't hold. Super admins bypass. When + // `callerPermissions` isn't passed (legacy callers / system jobs) + // we skip the check — but every interactive API route should pass + // ctx.permissions + ctx.isSuperAdmin through. + if (callerPermissions && !callerIsSuperAdmin) { + const newRolePerms = newRole.permissions as Record>; + for (const [resource, actions] of Object.entries(newRolePerms ?? {})) { + for (const [action, value] of Object.entries(actions)) { + if (value !== true) continue; + if (callerPermissions[resource]?.[action] !== true) { + throw new ForbiddenError( + `You don't hold ${resource}.${action} yourself, so you can't assign a role that grants it.`, + ); + } + } + } + } + portRoleUpdates.roleId = data.roleId; portRoleUpdates.assignedBy = meta.userId; }