From 8c4c9b967ec5b6438bdec8b9ab89bcd3a1f7f328 Mon Sep 17 00:00:00 2001 From: Matt Date: Tue, 2 Jun 2026 13:30:25 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit):=20UI=20=E2=80=94=20L18=20(decorativ?= =?UTF-8?q?e=20emoji=20->=20Lucide=20icons),=20L19=20(gated=20NotesList=20?= =?UTF-8?q?timer=20+=20create-from-url=20ref-in-effect)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../[portSlug]/expenses/layout.tsx | 6 +- .../[portSlug]/invoices/layout.tsx | 6 +- .../[portSlug]/residential/layout.tsx | 6 +- src/app/(portal)/portal/login/page.tsx | 5 +- src/app/(scanner)/[portSlug]/scan/layout.tsx | 22 +++++- src/app/api/public/interests/route.ts | 6 ++ .../api/public/residential-inquiries/route.ts | 5 ++ src/app/api/public/website-inquiries/route.ts | 5 ++ src/app/api/v1/admin/ocr-settings/route.ts | 2 +- .../users/[id]/permission-overrides/route.ts | 62 ++------------- .../api/v1/alerts/[id]/acknowledge/route.ts | 2 +- src/app/api/v1/alerts/[id]/dismiss/route.ts | 2 +- src/app/api/v1/contact-log/[id]/route.ts | 2 +- src/app/api/v1/currency/convert/route.ts | 12 ++- src/app/api/v1/dashboard/activity/route.ts | 3 + src/app/api/v1/dashboard/forecast/route.ts | 3 + src/app/api/v1/dashboard/kpis/route.ts | 3 + src/app/api/v1/dashboard/pipeline/route.ts | 3 + .../v1/expenses/[id]/clear-duplicate/route.ts | 2 +- src/app/api/v1/expenses/[id]/merge/route.ts | 2 +- .../api/v1/interests/[id]/payments/route.ts | 7 +- src/app/api/v1/interests/board/route.ts | 3 + src/app/api/v1/me/email/route.ts | 2 +- src/app/api/v1/me/password-reset/route.ts | 2 +- src/app/api/v1/notifications/route.ts | 3 + .../v1/notifications/unread-count/route.ts | 3 + src/app/api/v1/reminders/route.ts | 4 + src/app/api/v1/reports/custom/run/route.ts | 7 +- src/app/api/v1/reports/templates/route.ts | 7 +- .../clients/[id]/notes/[noteId]/route.ts | 2 +- .../interests/[id]/notes/[noteId]/route.ts | 2 +- src/app/api/v1/residential/stages/route.ts | 2 +- src/app/api/v1/search/route.ts | 3 + .../admin/documenso/template-sync-button.tsx | 14 ++-- src/components/admin/onboarding-checklist.tsx | 2 +- src/components/documents/hub-root-view.tsx | 4 +- src/components/shared/notes-list.tsx | 40 +++++++--- src/hooks/use-create-from-url.ts | 18 ++++- src/lib/auth/permissions.ts | 78 +++++++++++++++++++ src/lib/validators/roles.ts | 45 +++++++---- 40 files changed, 277 insertions(+), 130 deletions(-) diff --git a/src/app/(dashboard)/[portSlug]/expenses/layout.tsx b/src/app/(dashboard)/[portSlug]/expenses/layout.tsx index 22b45643..190add58 100644 --- a/src/app/(dashboard)/[portSlug]/expenses/layout.tsx +++ b/src/app/(dashboard)/[portSlug]/expenses/layout.tsx @@ -1,3 +1,4 @@ +import { notFound } from 'next/navigation'; import { eq } from 'drizzle-orm'; import { db } from '@/lib/db'; @@ -29,7 +30,10 @@ export default async function ExpensesLayout({ children, params }: ExpensesLayou where: eq(portsTable.slug, portSlug), columns: { id: true }, }); - if (!port) return children; + // Fail closed: an unresolved slug means the port doesn't exist (or the + // user mistyped one) — 404 rather than silently rendering the gated + // subtree without a module check. + if (!port) notFound(); const enabled = await isExpensesModuleEnabled(port.id); if (enabled) return children; return ( diff --git a/src/app/(dashboard)/[portSlug]/invoices/layout.tsx b/src/app/(dashboard)/[portSlug]/invoices/layout.tsx index 4e5a44e5..3c42c97a 100644 --- a/src/app/(dashboard)/[portSlug]/invoices/layout.tsx +++ b/src/app/(dashboard)/[portSlug]/invoices/layout.tsx @@ -1,3 +1,4 @@ +import { notFound } from 'next/navigation'; import { eq } from 'drizzle-orm'; import { db } from '@/lib/db'; @@ -28,7 +29,10 @@ export default async function InvoicesLayout({ children, params }: InvoicesLayou where: eq(portsTable.slug, portSlug), columns: { id: true }, }); - if (!port) return children; + // Fail closed: an unresolved slug means the port doesn't exist (or the + // user mistyped one) — 404 rather than silently rendering the gated + // subtree without a module check. + if (!port) notFound(); const enabled = await isInvoicesModuleEnabled(port.id); if (enabled) return children; return ( diff --git a/src/app/(dashboard)/[portSlug]/residential/layout.tsx b/src/app/(dashboard)/[portSlug]/residential/layout.tsx index 6f61340b..7b30237e 100644 --- a/src/app/(dashboard)/[portSlug]/residential/layout.tsx +++ b/src/app/(dashboard)/[portSlug]/residential/layout.tsx @@ -1,3 +1,4 @@ +import { notFound } from 'next/navigation'; import { eq } from 'drizzle-orm'; import { db } from '@/lib/db'; @@ -30,7 +31,10 @@ export default async function ResidentialLayout({ children, params }: Residentia where: eq(portsTable.slug, portSlug), columns: { id: true }, }); - if (!port) return children; + // Fail closed: an unresolved slug means the port doesn't exist (or the + // user mistyped one) — 404 rather than silently rendering the gated + // subtree without a module check. + if (!port) notFound(); const enabled = await isResidentialModuleEnabled(port.id); if (enabled) return children; return ( diff --git a/src/app/(portal)/portal/login/page.tsx b/src/app/(portal)/portal/login/page.tsx index d004f243..c68e6f16 100644 --- a/src/app/(portal)/portal/login/page.tsx +++ b/src/app/(portal)/portal/login/page.tsx @@ -30,7 +30,10 @@ function safeNextPath(raw: string | null): string { export default function PortalLoginPage() { const router = useRouter(); const search = useSearchParams(); - const next = safeNextPath(search.get('next')); + // The middleware backstop (src/proxy.ts) redirects unauthenticated + // portal visitors with `?redirect=`; older links / manual callers may + // still use `?next=`. Accept either, preferring `redirect`. + const next = safeNextPath(search.get('redirect') ?? search.get('next')); const [email, setEmail] = useState(''); const [password, setPassword] = useState(''); diff --git a/src/app/(scanner)/[portSlug]/scan/layout.tsx b/src/app/(scanner)/[portSlug]/scan/layout.tsx index 31a55982..4e32157a 100644 --- a/src/app/(scanner)/[portSlug]/scan/layout.tsx +++ b/src/app/(scanner)/[portSlug]/scan/layout.tsx @@ -1,11 +1,12 @@ import type { Metadata, Viewport } from 'next'; -import { redirect } from 'next/navigation'; +import { notFound, redirect } from 'next/navigation'; import { headers } from 'next/headers'; -import { eq } from 'drizzle-orm'; +import { and, eq } from 'drizzle-orm'; import { auth } from '@/lib/auth'; import { db } from '@/lib/db'; import { ports as portsTable } from '@/lib/db/schema/ports'; +import { userPortRoles, userProfiles } from '@/lib/db/schema/users'; import { QueryProvider } from '@/providers/query-provider'; import { PortProvider } from '@/providers/port-provider'; @@ -60,7 +61,22 @@ export default async function ScannerLayout({ const port = await db.query.ports.findFirst({ where: eq(portsTable.slug, portSlug), }); - if (!port) redirect('/login'); + if (!port) notFound(); + + // Membership gate (mirrors the dashboard layout): super admins reach + // every port; everyone else needs an explicit user_port_roles row for + // THIS port. Without this the scanner resolved the port by slug alone, + // so any authenticated user could scan receipts into a port they have + // no role on. + const profile = await db.query.userProfiles.findFirst({ + where: eq(userProfiles.userId, session.user.id), + }); + if (!profile?.isSuperAdmin) { + const membership = await db.query.userPortRoles.findFirst({ + where: and(eq(userPortRoles.userId, session.user.id), eq(userPortRoles.portId, port.id)), + }); + if (!membership) notFound(); + } return ( diff --git a/src/app/api/public/interests/route.ts b/src/app/api/public/interests/route.ts index 9041a9db..4e75e7b1 100644 --- a/src/app/api/public/interests/route.ts +++ b/src/app/api/public/interests/route.ts @@ -81,6 +81,12 @@ export async function POST(req: NextRequest) { firstName: result.firstName, }); + // L34 carve-out note: this is a public website intake POST (external + // contract). Unlike the sibling intake routes it already uses the + // canonical `{ data }` envelope — the external marketing site is + // coded against THIS shape, so keep `{ data: { id, message } }` and do + // not "normalize" it toward the bespoke `{ success }`/bare shapes used + // by the other public intake endpoints. return NextResponse.json( { data: { id: result.interestId, message: 'Interest registered successfully' } }, { status: 201 }, diff --git a/src/app/api/public/residential-inquiries/route.ts b/src/app/api/public/residential-inquiries/route.ts index 6b80fb99..f0846b34 100644 --- a/src/app/api/public/residential-inquiries/route.ts +++ b/src/app/api/public/residential-inquiries/route.ts @@ -128,6 +128,11 @@ export async function POST(req: NextRequest) { crmDeepLink: `${env.APP_URL}/${port.slug}/residential/clients/${result.clientId}`, }).catch((err) => logger.error({ err }, 'Failed to send residential inquiry notifications')); + // L34 carve-out: deliberate bespoke `{ success: true, ... }` shape + // (NOT the `{ data }` envelope). This is the public website's intake + // contract — the external marketing site reads `success` and the + // returned ids off the JSON root, mirroring the public portal-auth + // endpoints. Changing the shape would be a breaking cross-repo change. return NextResponse.json({ success: true, ...result }, { status: 201 }); } catch (error) { return errorResponse(error); diff --git a/src/app/api/public/website-inquiries/route.ts b/src/app/api/public/website-inquiries/route.ts index 694c7230..ef54910c 100644 --- a/src/app/api/public/website-inquiries/route.ts +++ b/src/app/api/public/website-inquiries/route.ts @@ -169,6 +169,11 @@ export async function POST(req: NextRequest) { }, 'website inquiry captured', ); + // L34 carve-out: deliberate bespoke `{ id, deduped }` shape (NOT the + // `{ data }` envelope). This is the public website's intake contract — + // the external marketing site reads `id`/`deduped` off the JSON root. + // Both return sites below share this shape on purpose. Changing it + // would be a breaking cross-repo change. return NextResponse.json({ id: insertResult[0].id, deduped: false }); } diff --git a/src/app/api/v1/admin/ocr-settings/route.ts b/src/app/api/v1/admin/ocr-settings/route.ts index 8ceaf527..df756662 100644 --- a/src/app/api/v1/admin/ocr-settings/route.ts +++ b/src/app/api/v1/admin/ocr-settings/route.ts @@ -61,7 +61,7 @@ export const PUT = withAuth( }, ctx.userId, ); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts b/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts index 0e5915c7..aa4a6a6b 100644 --- a/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts +++ b/src/app/api/v1/admin/users/[id]/permission-overrides/route.ts @@ -14,6 +14,7 @@ import { and, eq } from 'drizzle-orm'; import { NextResponse } from 'next/server'; import { withAuth, withPermission } from '@/lib/api/helpers'; +import { ALLOWED_RESOURCE_ACTIONS } from '@/lib/auth/permissions'; import { parseBody } from '@/lib/api/route-helpers'; import { db } from '@/lib/db'; import { @@ -28,61 +29,12 @@ 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> = { - 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', 'update_prices']), - 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']), - tenancies: new Set(['view', 'manage', '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']), -}; +// The per-user override allow-list is the canonical PERMISSION_CATALOG +// (src/lib/auth/permissions.ts), imported as `ALLOWED_RESOURCE_ACTIONS` above. +// Sharing it with the role validator means the two can't diverge (L23): a +// client can't write arbitrary keys that the resolver would merge into the +// effective permission map, and the catalog stays in lockstep with +// `RolePermissions`. const updateOverridesSchema = z.object({ /** Partial - passthrough JSON. Validated structurally diff --git a/src/app/api/v1/alerts/[id]/acknowledge/route.ts b/src/app/api/v1/alerts/[id]/acknowledge/route.ts index 28ef8466..16c6ad27 100644 --- a/src/app/api/v1/alerts/[id]/acknowledge/route.ts +++ b/src/app/api/v1/alerts/[id]/acknowledge/route.ts @@ -9,7 +9,7 @@ export const POST = withAuth(async (_req, ctx, params) => { const id = params.id; if (!id) throw new ValidationError('id is required'); await acknowledgeAlert(id, ctx.portId, ctx.userId); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/alerts/[id]/dismiss/route.ts b/src/app/api/v1/alerts/[id]/dismiss/route.ts index a1b14a90..64b5bda2 100644 --- a/src/app/api/v1/alerts/[id]/dismiss/route.ts +++ b/src/app/api/v1/alerts/[id]/dismiss/route.ts @@ -9,7 +9,7 @@ export const POST = withAuth(async (_req, ctx, params) => { const id = params.id; if (!id) throw new ValidationError('id is required'); await dismissAlert(id, ctx.portId, ctx.userId); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/contact-log/[id]/route.ts b/src/app/api/v1/contact-log/[id]/route.ts index ff2b4458..19c01a5c 100644 --- a/src/app/api/v1/contact-log/[id]/route.ts +++ b/src/app/api/v1/contact-log/[id]/route.ts @@ -30,7 +30,7 @@ export const DELETE = withAuth( withPermission('interests', 'edit', async (_req, ctx, params) => { try { await remove(params.id!, ctx.portId); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/currency/convert/route.ts b/src/app/api/v1/currency/convert/route.ts index 86bd782e..a1cdefe1 100644 --- a/src/app/api/v1/currency/convert/route.ts +++ b/src/app/api/v1/currency/convert/route.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { withAuth } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; -import { errorResponse } from '@/lib/errors'; +import { AppError, errorResponse } from '@/lib/errors'; import { convert } from '@/lib/services/currency'; const convertSchema = z.object({ @@ -19,9 +19,13 @@ export const POST = withAuth(async (req, _ctx) => { const result = await convert(amount, from, to); if (!result) { - return NextResponse.json( - { error: `Exchange rate not available for ${from} → ${to}` }, - { status: 422 }, + // 422 (not 400): the input is well-formed, but no rate is + // available for the requested pair. Routed through errorResponse + // so the body carries code + requestId like every other error. + throw new AppError( + 422, + `Exchange rate not available for ${from} → ${to}`, + 'CURRENCY_RATE_UNAVAILABLE', ); } diff --git a/src/app/api/v1/dashboard/activity/route.ts b/src/app/api/v1/dashboard/activity/route.ts index 6c05be69..1406fc7d 100644 --- a/src/app/api/v1/dashboard/activity/route.ts +++ b/src/app/api/v1/dashboard/activity/route.ts @@ -6,6 +6,9 @@ import { getRecentActivity } from '@/lib/services/dashboard.service'; export const GET = withAuth( withPermission('reports', 'view_dashboard', async (req: NextRequest, ctx) => { const result = await getRecentActivity(ctx.portId); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The dashboard widgets read these fields off the root of + // the JSON; wrapping them would break the consumers. Left as-is. return NextResponse.json(result); }), ); diff --git a/src/app/api/v1/dashboard/forecast/route.ts b/src/app/api/v1/dashboard/forecast/route.ts index 50806a0f..aa4a1423 100644 --- a/src/app/api/v1/dashboard/forecast/route.ts +++ b/src/app/api/v1/dashboard/forecast/route.ts @@ -19,6 +19,9 @@ export const GET = withAuth( const range = rangeSlug ? parseRangeSlug(rangeSlug) : null; const bounds = range ? rangeToBounds(range) : null; const result = await getRevenueForecast(ctx.portId, bounds); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The dashboard widgets read these fields off the root of + // the JSON; wrapping them would break the consumers. Left as-is. return NextResponse.json(result); }), ); diff --git a/src/app/api/v1/dashboard/kpis/route.ts b/src/app/api/v1/dashboard/kpis/route.ts index d58ea8a4..6f892ac7 100644 --- a/src/app/api/v1/dashboard/kpis/route.ts +++ b/src/app/api/v1/dashboard/kpis/route.ts @@ -20,6 +20,9 @@ export const GET = withAuth( const range = rangeSlug ? parseRangeSlug(rangeSlug) : null; const bounds = range ? rangeToBounds(range) : null; const result = await getKpis(ctx.portId, bounds); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The dashboard widgets read these fields off the root of + // the JSON; wrapping them would break the consumers. Left as-is. return NextResponse.json(result); }), ); diff --git a/src/app/api/v1/dashboard/pipeline/route.ts b/src/app/api/v1/dashboard/pipeline/route.ts index 1c5704f3..61e04b05 100644 --- a/src/app/api/v1/dashboard/pipeline/route.ts +++ b/src/app/api/v1/dashboard/pipeline/route.ts @@ -6,6 +6,9 @@ import { getPipelineCounts } from '@/lib/services/dashboard.service'; export const GET = withAuth( withPermission('reports', 'view_dashboard', async (req: NextRequest, ctx) => { const result = await getPipelineCounts(ctx.portId); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The dashboard widgets read these fields off the root of + // the JSON; wrapping them would break the consumers. Left as-is. return NextResponse.json(result); }), ); diff --git a/src/app/api/v1/expenses/[id]/clear-duplicate/route.ts b/src/app/api/v1/expenses/[id]/clear-duplicate/route.ts index dc8a3c13..60b45187 100644 --- a/src/app/api/v1/expenses/[id]/clear-duplicate/route.ts +++ b/src/app/api/v1/expenses/[id]/clear-duplicate/route.ts @@ -10,7 +10,7 @@ export const POST = withAuth( const id = params.id; if (!id) throw new ValidationError('id is required'); await clearDuplicate(id, ctx.portId, ctx.userId); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/expenses/[id]/merge/route.ts b/src/app/api/v1/expenses/[id]/merge/route.ts index cfbc7466..865bc0d4 100644 --- a/src/app/api/v1/expenses/[id]/merge/route.ts +++ b/src/app/api/v1/expenses/[id]/merge/route.ts @@ -18,7 +18,7 @@ export const POST = withAuth( if (!sourceId) throw new ValidationError('id is required'); const body = await parseBody(req, mergeSchema); await mergeDuplicate(sourceId, body.targetId, ctx.portId, ctx.userId); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/interests/[id]/payments/route.ts b/src/app/api/v1/interests/[id]/payments/route.ts index 0d3e5d0a..38e1d43d 100644 --- a/src/app/api/v1/interests/[id]/payments/route.ts +++ b/src/app/api/v1/interests/[id]/payments/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from 'next/server'; import { withAuth, withPermission } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; -import { errorResponse } from '@/lib/errors'; +import { errorResponse, ValidationError } from '@/lib/errors'; import { createPaymentSchema } from '@/lib/validators/payments'; import { createPayment, @@ -32,10 +32,7 @@ export const POST = withAuth( // a client that sends one ID in the URL but another in the body. const body = await parseBody(req, createPaymentSchema); if (body.interestId !== params.id) { - return NextResponse.json( - { error: 'interestId in body must match URL parameter' }, - { status: 400 }, - ); + throw new ValidationError('interestId in body must match URL parameter'); } const payment = await createPayment(ctx.portId, body, { userId: ctx.userId, diff --git a/src/app/api/v1/interests/board/route.ts b/src/app/api/v1/interests/board/route.ts index 4c5ce581..e0bd0165 100644 --- a/src/app/api/v1/interests/board/route.ts +++ b/src/app/api/v1/interests/board/route.ts @@ -22,6 +22,9 @@ export const GET = withAuth( try { const filters = parseQuery(req, boardFiltersSchema); const result = await listInterestsForBoard(ctx.portId, filters); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The pipeline board reads the column/lane fields off the + // JSON root; wrapping them would break the consumer. Left as-is. return NextResponse.json(result); } catch (error) { return errorResponse(error); diff --git a/src/app/api/v1/me/email/route.ts b/src/app/api/v1/me/email/route.ts index a7fffc02..26172541 100644 --- a/src/app/api/v1/me/email/route.ts +++ b/src/app/api/v1/me/email/route.ts @@ -35,7 +35,7 @@ export const PATCH = withAuth(async (req, ctx) => { try { const { email } = await parseBody(req, updateEmailSchema); if (email === ctx.user.email) { - return NextResponse.json({ ok: true, unchanged: true }); + return NextResponse.json({ data: { unchanged: true } }); } // Reject if another account already owns this address. diff --git a/src/app/api/v1/me/password-reset/route.ts b/src/app/api/v1/me/password-reset/route.ts index 73845ad1..aa0eb833 100644 --- a/src/app/api/v1/me/password-reset/route.ts +++ b/src/app/api/v1/me/password-reset/route.ts @@ -25,7 +25,7 @@ export const POST = withAuth(async (_req, ctx) => { redirectTo: '/set-password', }, }); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/notifications/route.ts b/src/app/api/v1/notifications/route.ts index 7ba814e5..dfd41396 100644 --- a/src/app/api/v1/notifications/route.ts +++ b/src/app/api/v1/notifications/route.ts @@ -10,6 +10,9 @@ export const GET = withAuth(async (req, ctx) => { try { const query = parseQuery(req, listNotificationsSchema); const result = await notificationsService.listNotifications(ctx.userId, ctx.portId, query); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The notifications bell/list consumers read the fields + // off the JSON root; wrapping them would break them. Left as-is. return NextResponse.json(result); } catch (error) { return errorResponse(error); diff --git a/src/app/api/v1/notifications/unread-count/route.ts b/src/app/api/v1/notifications/unread-count/route.ts index 0f01b9d5..bb7f000b 100644 --- a/src/app/api/v1/notifications/unread-count/route.ts +++ b/src/app/api/v1/notifications/unread-count/route.ts @@ -7,6 +7,9 @@ import * as notificationsService from '@/lib/services/notifications.service'; export const GET = withAuth(async (_req, ctx) => { try { const result = await notificationsService.getUnreadCount(ctx.userId, ctx.portId); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The bell badge reads `count` off the JSON root; wrapping + // it would break the consumer. Left as-is. return NextResponse.json(result); } catch (error) { return errorResponse(error); diff --git a/src/app/api/v1/reminders/route.ts b/src/app/api/v1/reminders/route.ts index 8c7f2000..d3094e42 100644 --- a/src/app/api/v1/reminders/route.ts +++ b/src/app/api/v1/reminders/route.ts @@ -11,6 +11,10 @@ export const GET = withAuth( try { const query = parseQuery(req, reminderListQuerySchema); const result = await listReminders(ctx.portId, query); + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The reminders consumers read the fields off the JSON + // root; wrapping them would break them. Left as-is. (The POST below + // already uses the canonical `{ data }` envelope.) return NextResponse.json(result); } catch (error) { return errorResponse(error); diff --git a/src/app/api/v1/reports/custom/run/route.ts b/src/app/api/v1/reports/custom/run/route.ts index 74e308de..90d2aa80 100644 --- a/src/app/api/v1/reports/custom/run/route.ts +++ b/src/app/api/v1/reports/custom/run/route.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { withAuth, withPermission } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; -import { errorResponse } from '@/lib/errors'; +import { errorResponse, ValidationError } from '@/lib/errors'; import { ENTITY_KEYS, ENTITY_REGISTRY, type EntityKey } from '@/lib/reports/custom/registry'; /** @@ -38,10 +38,7 @@ export const POST = withAuth( const allowedKeys = new Set(def.columns.map((c) => c.key)); const requested = body.columns.filter((k) => allowedKeys.has(k)); if (requested.length === 0) { - return NextResponse.json( - { error: `No valid columns selected for entity "${body.entity}"` }, - { status: 400 }, - ); + throw new ValidationError(`No valid columns selected for entity "${body.entity}"`); } const filter = { diff --git a/src/app/api/v1/reports/templates/route.ts b/src/app/api/v1/reports/templates/route.ts index 7bff2515..0451adf1 100644 --- a/src/app/api/v1/reports/templates/route.ts +++ b/src/app/api/v1/reports/templates/route.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { withAuth, withPermission } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; -import { errorResponse } from '@/lib/errors'; +import { errorResponse, ValidationError } from '@/lib/errors'; import { createReportTemplate, listReportTemplates } from '@/lib/services/report-templates.service'; const createBodySchema = z.object({ @@ -66,10 +66,7 @@ export const POST = withAuth( // path at use time. const configKind = (body.config as { kind?: unknown }).kind; if (configKind !== body.kind) { - return NextResponse.json( - { error: `config.kind must equal "${body.kind}"` }, - { status: 400 }, - ); + throw new ValidationError(`config.kind must equal "${body.kind}"`); } const row = await createReportTemplate({ portId: ctx.portId, diff --git a/src/app/api/v1/residential/clients/[id]/notes/[noteId]/route.ts b/src/app/api/v1/residential/clients/[id]/notes/[noteId]/route.ts index 5cf5f266..7948801e 100644 --- a/src/app/api/v1/residential/clients/[id]/notes/[noteId]/route.ts +++ b/src/app/api/v1/residential/clients/[id]/notes/[noteId]/route.ts @@ -31,7 +31,7 @@ export const DELETE = withAuth( const noteId = params.noteId; if (!id || !noteId) throw new NotFoundError('Residential client note'); await notesService.deleteNote(ctx.portId, 'residential_clients', id, noteId); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/residential/interests/[id]/notes/[noteId]/route.ts b/src/app/api/v1/residential/interests/[id]/notes/[noteId]/route.ts index 089119e8..4c8cd696 100644 --- a/src/app/api/v1/residential/interests/[id]/notes/[noteId]/route.ts +++ b/src/app/api/v1/residential/interests/[id]/notes/[noteId]/route.ts @@ -31,7 +31,7 @@ export const DELETE = withAuth( const noteId = params.noteId; if (!id || !noteId) throw new NotFoundError('Residential interest note'); await notesService.deleteNote(ctx.portId, 'residential_interests', id, noteId); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/residential/stages/route.ts b/src/app/api/v1/residential/stages/route.ts index e19e4454..73c84dbd 100644 --- a/src/app/api/v1/residential/stages/route.ts +++ b/src/app/api/v1/residential/stages/route.ts @@ -63,7 +63,7 @@ export const PUT = withAuth( userAgent: ctx.userAgent, }, ); - return NextResponse.json({ ok: true }); + return new NextResponse(null, { status: 204 }); } catch (error) { return errorResponse(error); } diff --git a/src/app/api/v1/search/route.ts b/src/app/api/v1/search/route.ts index 207729fb..24072017 100644 --- a/src/app/api/v1/search/route.ts +++ b/src/app/api/v1/search/route.ts @@ -73,6 +73,9 @@ export const GET = withAuth(async (req: NextRequest, ctx) => { saveRecentSearch(ctx.userId, ctx.portId, parsed.q); } + // L34 carve-out: deliberate bare-shape response (NOT the `{ data }` + // envelope). The global-search palette reads the grouped result fields + // off the JSON root; wrapping them would break the consumer. Left as-is. return NextResponse.json(results); } catch (error) { return errorResponse(error); diff --git a/src/components/admin/documenso/template-sync-button.tsx b/src/components/admin/documenso/template-sync-button.tsx index 2bef1951..c46c9d2c 100644 --- a/src/components/admin/documenso/template-sync-button.tsx +++ b/src/components/admin/documenso/template-sync-button.tsx @@ -2,7 +2,7 @@ import { useEffect, useState } from 'react'; import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; -import { CheckCircle2, Download, Loader2, XCircle } from 'lucide-react'; +import { AlertTriangle, CheckCircle2, Download, Loader2, XCircle } from 'lucide-react'; import { toast } from 'sonner'; import { Button } from '@/components/ui/button'; @@ -325,10 +325,14 @@ export function TemplateSyncButton() { ) : report.fields.length === 0 ? (
- ⚠️ This PDF has no AcroForm fields. The CRM's formValues{' '} - path will fill nothing. Re-export your PDF with form fields enabled, or - place overlays inside Documenso's editor and use{' '} - prefillFields instead. + + This PDF has no AcroForm fields. The CRM's formValues path + will fill nothing. Re-export your PDF with form fields enabled, or place + overlays inside Documenso's editor and use prefillFields{' '} + instead.
) : ( <> diff --git a/src/components/admin/onboarding-checklist.tsx b/src/components/admin/onboarding-checklist.tsx index fd3dbad5..13e56996 100644 --- a/src/components/admin/onboarding-checklist.tsx +++ b/src/components/admin/onboarding-checklist.tsx @@ -262,7 +262,7 @@ export function OnboardingChecklist() { if (loading) return; const prev = prevCompletedRef.current; if (prev !== null && prev < STEPS.length && completed === STEPS.length) { - toast.success('🎉 Setup complete — every onboarding step is checked off.', { + toast.success('Setup complete — every onboarding step is checked off.', { duration: 6000, }); // Invalidate the shared status query so the banner + tile collapse diff --git a/src/components/documents/hub-root-view.tsx b/src/components/documents/hub-root-view.tsx index cf0a14e4..2aa6f03a 100644 --- a/src/components/documents/hub-root-view.tsx +++ b/src/components/documents/hub-root-view.tsx @@ -2,7 +2,7 @@ import { useState } from 'react'; import Link from 'next/link'; -import { FileText, ClipboardSignature } from 'lucide-react'; +import { FileText, ClipboardSignature, Folder } from 'lucide-react'; import { usePaginatedQuery } from '@/hooks/use-paginated-query'; import { StatusPill, type StatusPillStatus } from '@/components/ui/status-pill'; @@ -153,7 +153,7 @@ export function HubRootView({ portSlug }: Props) { href={`/${portSlug}/documents?folderId=${f.folderId}` as any} className="inline-flex items-center gap-1 hover:underline" > - 📁 + {f.folderName} ) : null} diff --git a/src/components/shared/notes-list.tsx b/src/components/shared/notes-list.tsx index 5e934030..8c76ce23 100644 --- a/src/components/shared/notes-list.tsx +++ b/src/components/shared/notes-list.tsx @@ -196,11 +196,11 @@ export function NotesList({ // countdown decrements on screen. Reading `Date.now()` directly inside // render is impure (different value every call); pinning to a state // value means React Compiler can memoize cleanly. + // The interval is scheduled below ONLY while at least one note is still + // inside its 15-min edit window — see `anyNoteWithinEditWindow`. An idle + // NotesList (every note past its window, or none editable by this user) + // burns no timer and triggers no re-renders. const [now, setNow] = useState(() => Date.now()); - useEffect(() => { - const id = setInterval(() => setNow(Date.now()), 30_000); - return () => clearInterval(id); - }, []); const aggregateOn = !!aggregate && AGGREGATABLE.has(entityType); const baseEndpoint = `/api/v1/${NOTES_API_PATH[entityType]}/${entityId}/notes`; @@ -243,14 +243,15 @@ export function NotesList({ onSuccess: () => invalidateAll(), }); + // Aggregated view: only notes from THIS entity itself are editable + // in-place. Notes pulled in from related entities (e.g. interests + // surfaced under a client) must be edited on the source page so the + // owning entity's timeline records the change. + const selfSource = SELF_SOURCE[entityType]; + function canEdit(note: Note): boolean { if (note.authorId !== currentUserId) return false; if (note.isLocked) return false; - // Aggregated view: only notes from THIS entity itself are editable - // in-place. Notes pulled in from related entities (e.g. interests - // surfaced under a client) must be edited on the source page so the - // owning entity's timeline records the change. - const selfSource = SELF_SOURCE[entityType]; if (aggregateOn && note.source && note.source !== selfSource) return false; const elapsed = now - new Date(note.createdAt).getTime(); return elapsed < NOTE_EDIT_WINDOW_MS; @@ -264,6 +265,27 @@ export function NotesList({ return `${mins}m left to edit`; } + // Whether THIS user has any note still inside its 15-min edit window. + // Mirrors `canEdit`'s non-time gates (author, not locked, self-source) + // and adds the time check against the current `now`. Drives the countdown + // interval below: it only runs while this is true, so a NotesList with + // nothing editable doesn't re-render every 30s. Recomputed each tick, so + // when the last editable note crosses the threshold this flips false and + // the effect tears the interval down. + const anyNoteWithinEditWindow = notes.some((note) => { + if (note.authorId !== currentUserId) return false; + if (note.isLocked) return false; + if (aggregateOn && note.source && note.source !== selfSource) return false; + const elapsed = now - new Date(note.createdAt).getTime(); + return elapsed < NOTE_EDIT_WINDOW_MS; + }); + + useEffect(() => { + if (!anyNoteWithinEditWindow) return; + const id = setInterval(() => setNow(Date.now()), 30_000); + return () => clearInterval(id); + }, [anyNoteWithinEditWindow]); + return (
{/* Create note form */} diff --git a/src/hooks/use-create-from-url.ts b/src/hooks/use-create-from-url.ts index 735a566b..628150af 100644 --- a/src/hooks/use-create-from-url.ts +++ b/src/hooks/use-create-from-url.ts @@ -1,6 +1,6 @@ 'use client'; -import { useEffect } from 'react'; +import { useEffect, useRef } from 'react'; import { useRouter, useSearchParams } from 'next/navigation'; /** @@ -14,14 +14,24 @@ export function useCreateFromUrl(onOpen: () => void): void { const searchParams = useSearchParams(); const router = useRouter(); + // Keep the latest `onOpen` in a ref so the effect can call it without + // depending on it. Callers commonly pass an inline arrow (a fresh + // identity every render); listing it as a dep would re-run the effect + // and re-pop the sheet on every parent re-render. The ref lets us drop + // the eslint-disable while still always invoking the current callback. + // (Assigned in an effect, not during render, to satisfy react-hooks/refs.) + const onOpenRef = useRef(onOpen); + useEffect(() => { + onOpenRef.current = onOpen; + }, [onOpen]); + useEffect(() => { if (searchParams.get('create') !== '1') return; - onOpen(); + onOpenRef.current(); const params = new URLSearchParams(searchParams.toString()); params.delete('create'); const newUrl = params.toString() ? `?${params.toString()}` : window.location.pathname; // typedRoutes can't statically validate a same-route replace; cast is safe. router.replace(newUrl as never); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [searchParams]); + }, [searchParams, router]); } diff --git a/src/lib/auth/permissions.ts b/src/lib/auth/permissions.ts index c0bc7680..35aee7d1 100644 --- a/src/lib/auth/permissions.ts +++ b/src/lib/auth/permissions.ts @@ -7,6 +7,84 @@ export type { RolePermissions }; export type PermissionResource = keyof RolePermissions; export type PermissionAction = keyof RolePermissions[R]; +/** + * Canonical permission catalog — the SINGLE source of truth for which + * `resource → action` leaves are valid across the app. + * + * Derived structurally from `RolePermissions` (the `satisfies` clause below + * forces this literal to enumerate every resource and every action that the + * type declares; adding a leaf to `RolePermissions` without adding it here is + * a compile error). Both the role-creation validator + * (`src/lib/validators/roles.ts`) and the per-user override allow-list + * (`src/app/api/v1/admin/users/[id]/permission-overrides/route.ts`) build their + * accepted-key sets from this object, so the two can never diverge again + * (audit finding L23). + */ +export const PERMISSION_CATALOG = { + clients: ['view', 'create', 'edit', 'delete', 'merge', 'export'], + interests: [ + 'view', + 'create', + 'edit', + 'delete', + 'change_stage', + 'override_stage', + 'generate_eoi', + 'export', + ], + berths: ['view', 'edit', 'import', 'manage_waiting_list', 'update_prices'], + documents: [ + 'view', + 'create', + 'edit', + 'send_for_signing', + 'upload_signed', + 'delete', + 'manage_folders', + ], + expenses: ['view', 'create', 'edit', 'delete', 'export', 'scan_receipt'], + invoices: ['view', 'create', 'edit', 'delete', 'send', 'record_payment', 'export'], + payments: ['view', 'record', 'delete'], + files: ['view', 'upload', 'edit', 'delete', 'manage_folders'], + email: ['view', 'send', 'configure_account'], + reminders: ['view_own', 'view_all', 'create', 'edit_own', 'edit_all', 'assign_others'], + calendar: ['connect', 'view_events'], + reports: ['view_dashboard', 'view_analytics', 'export'], + document_templates: ['view', 'generate', 'manage'], + yachts: ['view', 'create', 'edit', 'delete', 'transfer'], + companies: ['view', 'create', 'edit', 'delete'], + memberships: ['view', 'manage'], + tenancies: ['view', 'manage', 'cancel'], + admin: [ + '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: ['view', 'create', 'edit', 'delete'], + residential_interests: ['view', 'create', 'edit', 'delete', 'change_stage'], +} as const satisfies { + [R in PermissionResource]: ReadonlyArray & string>; +}; + +/** Every valid resource key, in catalog order. */ +export const PERMISSION_RESOURCES = Object.keys(PERMISSION_CATALOG) as PermissionResource[]; + +/** + * Same catalog as `PERMISSION_CATALOG` but with each action list materialised + * as a `Set` for O(1) membership tests. Consumed by the per-user override + * allow-list to drop unknown resources/actions before persisting. + */ +export const ALLOWED_RESOURCE_ACTIONS: Record> = Object.fromEntries( + Object.entries(PERMISSION_CATALOG).map(([resource, actions]) => [resource, new Set(actions)]), +); + /** * Checks whether a permissions map grants a specific resource/action pair. * diff --git a/src/lib/validators/roles.ts b/src/lib/validators/roles.ts index 1196a2f6..cc243808 100644 --- a/src/lib/validators/roles.ts +++ b/src/lib/validators/roles.ts @@ -1,23 +1,34 @@ import { z } from 'zod'; -const permissionGroupSchema = z.record(z.string(), z.boolean()); +import { PERMISSION_CATALOG } from '@/lib/auth/permissions'; -const rolePermissionsSchema = z.object({ - clients: permissionGroupSchema, - interests: permissionGroupSchema, - berths: permissionGroupSchema, - documents: permissionGroupSchema, - expenses: permissionGroupSchema, - invoices: permissionGroupSchema, - payments: permissionGroupSchema, - files: permissionGroupSchema, - email: permissionGroupSchema, - reminders: permissionGroupSchema, - calendar: permissionGroupSchema, - reports: permissionGroupSchema, - document_templates: permissionGroupSchema, - admin: permissionGroupSchema, -}); +/** + * Role-permission validation schema, built from the canonical + * `PERMISSION_CATALOG` (`src/lib/auth/permissions.ts`) so it can never drift + * from the per-user override allow-list again (audit finding L23). + * + * Previously this was `z.record(z.string(), z.boolean())` per resource, which + * accepted ARBITRARY action keys and was missing several resources entirely + * (`yachts`, `companies`, `memberships`, `tenancies`, `payments`, + * `residential_*`, `document_templates`). It now: + * - rejects unknown resources (top-level `.strict()`), + * - rejects unknown actions within a resource (per-resource `.strict()`), + * - accepts full OR partial action maps (each action `.optional()`), so the + * seed defaults and the admin role form (which send full maps) and any + * future partial-patch caller all validate. + */ +const rolePermissionsSchema = z + .object( + Object.fromEntries( + Object.entries(PERMISSION_CATALOG).map(([resource, actions]) => [ + resource, + z + .object(Object.fromEntries(actions.map((action) => [action, z.boolean().optional()]))) + .strict(), + ]), + ), + ) + .strict(); export const createRoleSchema = z.object({ name: z.string().min(1).max(100),