audit: 33-agent comprehensive audit + critical fixes

Full team audit run, all reports verbatim in docs/AUDIT-2026-05-12.md
(5900+ lines, 30+ critical findings). Already-fixed this commit:
- permission-overrides PUT: self-target block + RolePermissions allow-list + cross-tenant guard
- /api/auth/resolve-identifier: rate-limit + synthetic miss-email kill enumeration
- admin email-change: rotates account.accountId + revokes sessions
- middleware: token-gated email confirm/cancel routes whitelisted
- NAV_CATALOG: 10 dead-link sweeps to existing /admin/<x> targets

Feature work landing same commit: optional username sign-in
(migration 0054), per-user permission overrides (0055) with three-state
matrix tabbed inside UserForm, user disable button, role + outcome +
stage label normalisation across the platform, admin email-change
with auto-notification template.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-12 16:52:35 +02:00
parent 660553c074
commit 4b9743a594
31 changed files with 7042 additions and 81 deletions

View File

@@ -0,0 +1,93 @@
/**
* Resolves an email-or-username sign-in identifier to a canonical email
* that Better Auth's email/password flow accepts.
*
* Public endpoint by design — the login form calls it BEFORE the user is
* authenticated, so it can't sit behind `withAuth`.
*
* **Anti-enumeration:** the response shape is identical for hit and
* miss. On a miss we return a synthetic `@auth.invalid` email derived
* from the input so Better Auth's `signIn.email` call fails uniformly
* with "invalid credentials" — an attacker can't tell whether the
* username exists from this endpoint's response. (Previously a miss
* returned the bare input string, which lacked an `@` and was visibly
* different from a hit's real email.)
*
* **Rate limiting:** shares the `auth` bucket (5/15min/ip), so an
* attacker can't iterate a wordlist faster than they could brute-force
* passwords directly.
*/
import { NextResponse, type NextRequest } from 'next/server';
import { sql } from 'drizzle-orm';
import { db } from '@/lib/db';
import { user, userProfiles } from '@/lib/db/schema/users';
import { eq } from 'drizzle-orm';
import { checkRateLimit, rateLimitHeaders, rateLimiters } from '@/lib/rate-limit';
const EMAIL_HINT = /@/;
/** Synthetic, definitively-invalid email used for the miss path. The
* `.invalid` TLD is reserved by RFC 2606 — no real domain can use it,
* so a downstream signIn call always fails as "invalid credentials"
* without ever leaking the lookup outcome. */
function syntheticEmail(raw: string): string {
const slug = raw.replace(/[^a-z0-9._-]/gi, '').slice(0, 30) || 'unknown';
return `${slug}@auth.invalid`;
}
function clientIp(req: NextRequest): string {
return (
req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ??
req.headers.get('x-real-ip') ??
'unknown'
);
}
export async function POST(req: NextRequest) {
try {
// Rate-limit on IP — same 5/15min bucket the actual sign-in uses.
// Without this an attacker can wordlist usernames at full HTTP
// bandwidth and only funnel the validated emails into the slower
// signIn flow.
const ip = clientIp(req);
const rl = await checkRateLimit(ip, rateLimiters.auth);
if (!rl.allowed) {
return NextResponse.json(
{ email: '' },
{ status: 429, headers: rateLimitHeaders(rl) },
);
}
const body = (await req.json().catch(() => ({}))) as { identifier?: string };
const raw = (body.identifier ?? '').trim();
if (!raw) return NextResponse.json({ email: syntheticEmail('empty') });
// Looks like an email → already canonical. Hand it straight back.
if (EMAIL_HINT.test(raw)) {
return NextResponse.json({ email: raw });
}
// Otherwise treat the input as a username and look up the linked
// Better Auth email. Case-insensitive match against the
// `LOWER(username)` unique index.
const normalized = raw.toLowerCase();
const rows = await db
.select({ email: user.email })
.from(userProfiles)
.innerJoin(user, eq(userProfiles.userId, user.id))
.where(sql`LOWER(${userProfiles.username}) = ${normalized}`)
.limit(1);
if (rows.length === 0) {
// Synthetic `.invalid` email — indistinguishable from a hit in
// shape (has `@`, has a tld), guaranteed to fail downstream auth.
return NextResponse.json({ email: syntheticEmail(normalized) });
}
return NextResponse.json({ email: rows[0]!.email });
} catch {
// Defensive — never expose internals from a public endpoint.
return NextResponse.json({ email: syntheticEmail('error') }, { status: 200 });
}
}

View File

@@ -0,0 +1,260 @@
/**
* GET / PUT per-user permission overrides for the current port.
*
* GET returns the effective baseline (role + port-role-overrides + residential
* toggle) AND the current user-specific override map so the UI can render
* three states per leaf: inherit, force-grant, force-deny.
*
* PUT accepts a Partial<RolePermissions> map (use null at a leaf to clear an
* override) and upserts it onto user_permission_overrides for (userId, portId).
* Permission `admin.manage_users` is required — same gate as the user-edit
* drawer that hosts the matrix.
*/
import { and, eq } from 'drizzle-orm';
import { NextResponse } from 'next/server';
import { withAuth, withPermission } from '@/lib/api/helpers';
import { parseBody } from '@/lib/api/route-helpers';
import { db } from '@/lib/db';
import {
portRoleOverrides,
roles,
userPermissionOverrides,
userPortRoles,
userProfiles,
type RolePermissions,
} from '@/lib/db/schema';
import { createAuditLog } from '@/lib/audit';
import { errorResponse, ForbiddenError, NotFoundError, ValidationError } from '@/lib/errors';
import { z } from 'zod';
/**
* Mirrors `RolePermissions` in src/lib/db/schema/users.ts. Used as the
* allow-list for the PUT body so a client can't write arbitrary keys
* that the resolver would happily merge into the effective permission
* map. Keep this in sync when RolePermissions gains a leaf.
*/
const ALLOWED_RESOURCE_ACTIONS: Record<string, Set<string>> = {
clients: new Set(['view', 'create', 'edit', 'delete', 'merge', 'export']),
interests: new Set([
'view',
'create',
'edit',
'delete',
'change_stage',
'override_stage',
'generate_eoi',
'export',
]),
berths: new Set(['view', 'edit', 'import', 'manage_waiting_list']),
documents: new Set([
'view',
'create',
'edit',
'send_for_signing',
'upload_signed',
'delete',
'manage_folders',
]),
expenses: new Set(['view', 'create', 'edit', 'delete', 'export', 'scan_receipt']),
invoices: new Set(['view', 'create', 'edit', 'delete', 'send', 'record_payment', 'export']),
files: new Set(['view', 'upload', 'edit', 'delete', 'manage_folders']),
email: new Set(['view', 'send', 'configure_account']),
reminders: new Set(['view_own', 'view_all', 'create', 'edit_own', 'edit_all', 'assign_others']),
calendar: new Set(['connect', 'view_events']),
reports: new Set(['view_dashboard', 'view_analytics', 'export']),
document_templates: new Set(['view', 'generate', 'manage']),
yachts: new Set(['view', 'create', 'edit', 'delete', 'transfer']),
companies: new Set(['view', 'create', 'edit', 'delete']),
memberships: new Set(['view', 'manage']),
reservations: new Set(['view', 'create', 'activate', 'cancel']),
admin: new Set([
'manage_users',
'view_audit_log',
'manage_settings',
'manage_webhooks',
'manage_reports',
'manage_custom_fields',
'manage_forms',
'manage_tags',
'system_backup',
'permanently_delete_clients',
]),
residential_clients: new Set(['view', 'create', 'edit', 'delete']),
residential_interests: new Set(['view', 'create', 'edit', 'delete', 'change_stage']),
};
const updateOverridesSchema = z.object({
/** Partial<RolePermissions> — passthrough JSON. Validated structurally
* by limiting depth + leaf type below. */
overrides: z.record(z.string(), z.record(z.string(), z.boolean())).default({}),
});
export const GET = withAuth(
withPermission('admin', 'manage_users', async (_req, ctx, params) => {
try {
const targetUserId = params.id!;
const portId = ctx.portId;
const profile = await db.query.userProfiles.findFirst({
where: eq(userProfiles.userId, targetUserId),
});
if (!profile) throw new NotFoundError('User');
// Resolve the role's baseline + port-role override (super-admin
// edge-case: no role row, so the matrix is empty / not editable).
let baseline: RolePermissions | null = null;
if (!profile.isSuperAdmin) {
const portRole = await db.query.userPortRoles.findFirst({
where: and(
eq(userPortRoles.userId, targetUserId),
eq(userPortRoles.portId, portId),
),
});
if (portRole) {
const role = await db.query.roles.findFirst({
where: eq(roles.id, portRole.roleId),
});
baseline = (role?.permissions as RolePermissions | null) ?? null;
const portOverride = await db.query.portRoleOverrides.findFirst({
where: and(
eq(portRoleOverrides.portId, portId),
eq(portRoleOverrides.roleId, portRole.roleId),
),
});
if (baseline && portOverride?.permissionOverrides) {
// Cheap structural merge — same shape as helpers.ts's deepMerge.
baseline = mergePerms(baseline, portOverride.permissionOverrides);
}
}
}
const userOverride = await db.query.userPermissionOverrides.findFirst({
where: and(
eq(userPermissionOverrides.userId, targetUserId),
eq(userPermissionOverrides.portId, portId),
),
});
return NextResponse.json({
data: {
baseline,
overrides: userOverride?.permissionOverrides ?? {},
isSuperAdmin: profile.isSuperAdmin,
},
});
} catch (error) {
return errorResponse(error);
}
}),
);
export const PUT = withAuth(
withPermission('admin', 'manage_users', async (req, ctx, params) => {
try {
const targetUserId = params.id!;
const portId = ctx.portId;
// CRITICAL: refuse self-target. Without this an admin with
// admin.manage_users can grant themselves every permission leaf
// (incl. permanently_delete_clients, system_backup, etc.) and the
// override layer in withAuth resolves it on the very next request.
if (targetUserId === ctx.userId) {
throw new ForbiddenError('You cannot edit your own permission overrides.');
}
// Reject overrides for users that aren't actually assigned to this
// port — prevents cross-tenant pollution where an admin in port A
// writes a row keyed on (userIdFromPortB, portA). The withAuth
// resolver scopes lookups to the caller's port so the row would
// never apply, but it still consumes a unique slot and confuses
// future audits.
const targetPortRole = await db.query.userPortRoles.findFirst({
where: and(
eq(userPortRoles.userId, targetUserId),
eq(userPortRoles.portId, portId),
),
});
if (!targetPortRole) {
throw new NotFoundError('User not assigned to this port');
}
const { overrides } = await parseBody(req, updateOverridesSchema);
// Strip anything outside the canonical RolePermissions allow-list.
// The Zod schema only enforces shape (string → string → boolean);
// here we drop unknown resources/actions so a malicious client
// can't seed garbage keys that a future resolver might accidentally
// honour.
const sanitized: Record<string, Record<string, boolean>> = {};
for (const [resource, actions] of Object.entries(overrides)) {
const allowed = ALLOWED_RESOURCE_ACTIONS[resource];
if (!allowed) continue;
const cleaned: Record<string, boolean> = {};
for (const [action, value] of Object.entries(actions)) {
if (!allowed.has(action)) continue;
if (typeof value !== 'boolean') {
throw new ValidationError(
`permission override for ${resource}.${action} must be true or false`,
);
}
cleaned[action] = value;
}
if (Object.keys(cleaned).length > 0) sanitized[resource] = cleaned;
}
const existing = await db.query.userPermissionOverrides.findFirst({
where: and(
eq(userPermissionOverrides.userId, targetUserId),
eq(userPermissionOverrides.portId, portId),
),
});
if (existing) {
await db
.update(userPermissionOverrides)
.set({ permissionOverrides: sanitized, updatedAt: new Date() })
.where(eq(userPermissionOverrides.id, existing.id));
} else {
await db.insert(userPermissionOverrides).values({
userId: targetUserId,
portId,
permissionOverrides: sanitized,
});
}
void createAuditLog({
userId: ctx.userId,
portId,
action: 'update',
entityType: 'user',
entityId: targetUserId,
oldValue: { permissionOverrides: existing?.permissionOverrides ?? {} },
newValue: { permissionOverrides: sanitized },
ipAddress: ctx.ipAddress,
userAgent: ctx.userAgent,
});
return NextResponse.json({ data: { overrides: sanitized } });
} catch (error) {
return errorResponse(error);
}
}),
);
/** Local shallow-merge by resource (matches the RolePermissions shape:
* one-level-deep map of resource → action map). Same semantics the
* withAuth resolver uses; copied here to avoid pulling that file into
* a route module. */
function mergePerms(
base: RolePermissions,
patch: Partial<RolePermissions> | Record<string, Record<string, boolean>>,
): RolePermissions {
const out = { ...(base as unknown as Record<string, Record<string, boolean>>) };
for (const [resource, actions] of Object.entries(patch)) {
if (!actions) continue;
out[resource] = { ...(out[resource] ?? {}), ...(actions as Record<string, boolean>) };
}
return out as unknown as RolePermissions;
}

View File

@@ -5,13 +5,28 @@ import { withAuth, type AuthContext } from '@/lib/api/helpers';
import { parseBody } from '@/lib/api/route-helpers';
import { db } from '@/lib/db';
import { userProfiles } from '@/lib/db/schema';
import { errorResponse, NotFoundError, ValidationError } from '@/lib/errors';
import { ConflictError, errorResponse, NotFoundError, ValidationError } from '@/lib/errors';
import { isReservedUsername, USERNAME_REGEX } from '@/lib/validators/username';
import { sql } from 'drizzle-orm';
import { z } from 'zod';
const updateProfileSchema = z.object({
firstName: z.string().min(1).max(120).nullable().optional(),
lastName: z.string().min(1).max(120).nullable().optional(),
displayName: z.string().min(1).max(200).optional(),
/**
* Optional sign-in alias. `null` clears the existing value; a string
* must match the 230 lowercase shape pinned by USERNAME_REGEX (also
* enforced by `chk_user_profiles_username_shape` in migration 0054).
* Uniqueness is checked below before the UPDATE — collisions surface
* as a 409 with a friendly message.
*/
username: z
.union([
z.string().transform((s) => s.trim().toLowerCase()),
z.null(),
])
.optional(),
phone: z.string().nullable().optional(),
// Refuse `javascript:` / `data:` schemes — z.string().url() lets them
// through and `<a href={avatarUrl}>` would otherwise be a stored-XSS
@@ -64,6 +79,7 @@ export const GET = withAuth(async (_req, ctx: AuthContext) => {
firstName: true,
lastName: true,
displayName: true,
username: true,
},
});
@@ -82,6 +98,7 @@ export const GET = withAuth(async (_req, ctx: AuthContext) => {
firstName: profile?.firstName ?? null,
lastName: profile?.lastName ?? null,
displayName: profile?.displayName ?? null,
username: profile?.username ?? null,
},
},
});
@@ -102,6 +119,32 @@ export const PATCH = withAuth(async (req, ctx: AuthContext) => {
if (body.displayName !== undefined) updates.displayName = body.displayName;
if (body.phone !== undefined) updates.phone = body.phone;
if (body.avatarUrl !== undefined) updates.avatarUrl = body.avatarUrl;
if (body.username !== undefined) {
// null clears; non-null must match the shape + not be reserved +
// be unique (case-insensitive) across all users in the install.
if (body.username === null || body.username === '') {
updates.username = null;
} else {
const candidate = body.username;
if (!USERNAME_REGEX.test(candidate)) {
throw new ValidationError(
'Username must be 230 lowercase letters, digits, dot, underscore, or hyphen.',
);
}
if (isReservedUsername(candidate)) {
throw new ValidationError('That username is reserved. Please pick another.');
}
const taken = await db
.select({ userId: userProfiles.userId })
.from(userProfiles)
.where(sql`LOWER(${userProfiles.username}) = ${candidate}`)
.limit(1);
if (taken.length > 0 && taken[0]!.userId !== ctx.userId) {
throw new ConflictError('That username is already taken.');
}
updates.username = candidate;
}
}
if (body.preferences !== undefined) {
// Allow-list — only retain keys defined in the strict schema. Pre-
// strict rows may carry extra keys from when the schema was
@@ -136,6 +179,7 @@ export const PATCH = withAuth(async (req, ctx: AuthContext) => {
firstName: updated!.firstName,
lastName: updated!.lastName,
displayName: updated!.displayName,
username: updated!.username,
phone: updated!.phone,
avatarUrl: updated!.avatarUrl,
preferences: updated!.preferences,