fix(audit-wave-11): authz hardening — caller-superset on role assign
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<string, Record<string, boolean>> | null,
|
||||
ctx.isSuperAdmin,
|
||||
);
|
||||
return NextResponse.json({ data });
|
||||
} catch (error) {
|
||||
return errorResponse(error);
|
||||
|
||||
@@ -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<string, Record<string, boolean>> | 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<string, Record<string, boolean>>;
|
||||
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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user