diff --git a/scripts/dev-trigger-crm-invite.ts b/scripts/dev-trigger-crm-invite.ts index 341e9cc..d3c13e0 100644 --- a/scripts/dev-trigger-crm-invite.ts +++ b/scripts/dev-trigger-crm-invite.ts @@ -20,7 +20,15 @@ async function main() { const isSuperAdmin = args.includes('--super'); const name = args.find((a, i) => i > 0 && !a.startsWith('--')); - const { inviteId, link } = await createCrmInvite({ email, name, isSuperAdmin }); + // Dev script runs out-of-band (no HTTP request, no session). The service's + // super-admin gate requires `invitedBy.isSuperAdmin === true` for super + // invites; the script bypasses that with a synthetic caller identity. + const { inviteId, link } = await createCrmInvite({ + email, + name, + isSuperAdmin, + invitedBy: { userId: 'cli-script', isSuperAdmin: true }, + }); console.log(`✓ Invite created (id=${inviteId})`); console.log(` email: ${email}`); console.log(` super_admin: ${isSuperAdmin}`); diff --git a/src/app/api/v1/admin/invitations/route.ts b/src/app/api/v1/admin/invitations/route.ts index 0d2eab5..6cfbdde 100644 --- a/src/app/api/v1/admin/invitations/route.ts +++ b/src/app/api/v1/admin/invitations/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, ForbiddenError } from '@/lib/errors'; import { createCrmInvite, listCrmInvites } from '@/lib/services/crm-invite.service'; export const GET = withAuth( @@ -24,10 +24,17 @@ const createInviteSchema = z.object({ }); export const POST = withAuth( - withPermission('admin', 'manage_users', async (req, _ctx) => { + withPermission('admin', 'manage_users', async (req, ctx) => { try { const body = await parseBody(req, createInviteSchema); - const result = await createCrmInvite(body); + // Only existing super-admins can mint super-admin invitations. The + // manage_users permission is granted to port-scoped director roles, + // which must not be able to elevate themselves cross-tenant by + // inviting a fresh super_admin. + if (body.isSuperAdmin && !ctx.isSuperAdmin) { + throw new ForbiddenError('Only super admins can mint super-admin invitations'); + } + const result = await createCrmInvite({ ...body, invitedBy: ctx }); return NextResponse.json({ data: result }, { status: 201 }); } catch (error) { return errorResponse(error); diff --git a/src/app/api/v1/admin/ocr-settings/route.ts b/src/app/api/v1/admin/ocr-settings/route.ts index 99df9c7..01e40d1 100644 --- a/src/app/api/v1/admin/ocr-settings/route.ts +++ b/src/app/api/v1/admin/ocr-settings/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from 'next/server'; import { z } from 'zod'; -import { withAuth } from '@/lib/api/helpers'; +import { withAuth, withPermission } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; import { errorResponse } from '@/lib/errors'; import { getPublicOcrConfig, saveOcrConfig, OCR_MODELS } from '@/lib/services/ocr-config.service'; @@ -17,47 +17,56 @@ const saveSchema = z.object({ aiEnabled: z.boolean().optional(), }); -export const GET = withAuth(async (req, ctx) => { - try { - const url = new URL(req.url); - const scope = url.searchParams.get('scope') ?? 'port'; - if (scope === 'global' && !ctx.isSuperAdmin) { - return NextResponse.json({ error: 'Super admin only' }, { status: 403 }); +// Only role tiers that hold `admin.manage_settings` (director / super_admin) +// may read or write the OCR config: the apiKey is stored encrypted but is +// passed straight into the receipt-scan handler, so a swapped key would +// exfiltrate every subsequent receipt image to whatever endpoint that key +// authenticates with. +export const GET = withAuth( + withPermission('admin', 'manage_settings', async (req, ctx) => { + try { + const url = new URL(req.url); + const scope = url.searchParams.get('scope') ?? 'port'; + if (scope === 'global' && !ctx.isSuperAdmin) { + return NextResponse.json({ error: 'Super admin only' }, { status: 403 }); + } + const config = await getPublicOcrConfig(scope === 'global' ? null : ctx.portId); + return NextResponse.json({ data: config, models: OCR_MODELS }); + } catch (error) { + return errorResponse(error); } - const config = await getPublicOcrConfig(scope === 'global' ? null : ctx.portId); - return NextResponse.json({ data: config, models: OCR_MODELS }); - } catch (error) { - return errorResponse(error); - } -}); + }), +); -export const PUT = withAuth(async (req, ctx) => { - try { - const body = await parseBody(req, saveSchema); - if (body.scope === 'global' && !ctx.isSuperAdmin) { - return NextResponse.json({ error: 'Super admin only' }, { status: 403 }); - } - const validModels = OCR_MODELS[body.provider]; - if (!validModels.includes(body.model)) { - return NextResponse.json( - { error: `Invalid model for provider ${body.provider}` }, - { status: 400 }, +export const PUT = withAuth( + withPermission('admin', 'manage_settings', async (req, ctx) => { + try { + const body = await parseBody(req, saveSchema); + if (body.scope === 'global' && !ctx.isSuperAdmin) { + return NextResponse.json({ error: 'Super admin only' }, { status: 403 }); + } + const validModels = OCR_MODELS[body.provider]; + if (!validModels.includes(body.model)) { + return NextResponse.json( + { error: `Invalid model for provider ${body.provider}` }, + { status: 400 }, + ); + } + await saveOcrConfig( + body.scope === 'global' ? null : ctx.portId, + { + provider: body.provider, + model: body.model, + apiKey: body.apiKey, + clearApiKey: body.clearApiKey, + useGlobal: body.useGlobal, + aiEnabled: body.aiEnabled, + }, + ctx.userId, ); + return NextResponse.json({ ok: true }); + } catch (error) { + return errorResponse(error); } - await saveOcrConfig( - body.scope === 'global' ? null : ctx.portId, - { - provider: body.provider, - model: body.model, - apiKey: body.apiKey, - clearApiKey: body.clearApiKey, - useGlobal: body.useGlobal, - aiEnabled: body.aiEnabled, - }, - ctx.userId, - ); - return NextResponse.json({ ok: true }); - } catch (error) { - return errorResponse(error); - } -}); + }), +); diff --git a/src/app/api/v1/admin/ocr-settings/test/route.ts b/src/app/api/v1/admin/ocr-settings/test/route.ts index 2a733c4..9d4d520 100644 --- a/src/app/api/v1/admin/ocr-settings/test/route.ts +++ b/src/app/api/v1/admin/ocr-settings/test/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from 'next/server'; import { z } from 'zod'; -import { withAuth } from '@/lib/api/helpers'; +import { withAuth, withPermission } from '@/lib/api/helpers'; import { parseBody } from '@/lib/api/route-helpers'; import { errorResponse } from '@/lib/errors'; import { OCR_MODELS } from '@/lib/services/ocr-config.service'; @@ -13,15 +13,19 @@ const schema = z.object({ apiKey: z.string().min(1), }); -export const POST = withAuth(async (req) => { - try { - const body = await parseBody(req, schema); - if (!OCR_MODELS[body.provider].includes(body.model)) { - return NextResponse.json({ error: 'Invalid model' }, { status: 400 }); +// `manage_settings`-gated for parity with the parent OCR settings route — +// triggers outbound AI provider auth requests using a caller-supplied key. +export const POST = withAuth( + withPermission('admin', 'manage_settings', async (req) => { + try { + const body = await parseBody(req, schema); + if (!OCR_MODELS[body.provider].includes(body.model)) { + return NextResponse.json({ error: 'Invalid model' }, { status: 400 }); + } + const result = await testProvider(body.provider, body.apiKey, body.model); + return NextResponse.json(result); + } catch (error) { + return errorResponse(error); } - const result = await testProvider(body.provider, body.apiKey, body.model); - return NextResponse.json(result); - } catch (error) { - return errorResponse(error); - } -}); + }), +); diff --git a/src/app/api/v1/alerts/[id]/acknowledge/route.ts b/src/app/api/v1/alerts/[id]/acknowledge/route.ts index a54f1f5..5da53e6 100644 --- a/src/app/api/v1/alerts/[id]/acknowledge/route.ts +++ b/src/app/api/v1/alerts/[id]/acknowledge/route.ts @@ -6,6 +6,6 @@ import { acknowledgeAlert } from '@/lib/services/alerts.service'; export const POST = withAuth(async (_req, ctx, params) => { const id = params.id; if (!id) return NextResponse.json({ error: 'Missing id' }, { status: 400 }); - await acknowledgeAlert(id, ctx.userId); + await acknowledgeAlert(id, ctx.portId, ctx.userId); return NextResponse.json({ ok: true }); }); diff --git a/src/app/api/v1/alerts/[id]/dismiss/route.ts b/src/app/api/v1/alerts/[id]/dismiss/route.ts index 807f077..6319789 100644 --- a/src/app/api/v1/alerts/[id]/dismiss/route.ts +++ b/src/app/api/v1/alerts/[id]/dismiss/route.ts @@ -6,6 +6,6 @@ import { dismissAlert } from '@/lib/services/alerts.service'; export const POST = withAuth(async (_req, ctx, params) => { const id = params.id; if (!id) return NextResponse.json({ error: 'Missing id' }, { status: 400 }); - await dismissAlert(id, ctx.userId); + await dismissAlert(id, ctx.portId, ctx.userId); return NextResponse.json({ ok: true }); }); diff --git a/src/lib/services/alerts.service.ts b/src/lib/services/alerts.service.ts index 5de877a..acacb87 100644 --- a/src/lib/services/alerts.service.ts +++ b/src/lib/services/alerts.service.ts @@ -98,22 +98,28 @@ export async function reconcileAlertsForPort( } } -export async function dismissAlert(alertId: string, userId: string): Promise { +export async function dismissAlert(alertId: string, portId: string, userId: string): Promise { + // Tenant scope: the WHERE on portId means a foreign-tenant alert UUID + // returns zero rows rather than mutating someone else's alert. const [row] = await db .update(alerts) .set({ dismissedAt: sql`now()`, dismissedBy: userId }) - .where(eq(alerts.id, alertId)) + .where(and(eq(alerts.id, alertId), eq(alerts.portId, portId))) .returning({ id: alerts.id, portId: alerts.portId }); if (row) { emitToRoom(`port:${row.portId}`, 'alert:dismissed', { alertId: row.id, portId: row.portId }); } } -export async function acknowledgeAlert(alertId: string, userId: string): Promise { +export async function acknowledgeAlert( + alertId: string, + portId: string, + userId: string, +): Promise { await db .update(alerts) .set({ acknowledgedAt: sql`now()`, acknowledgedBy: userId }) - .where(eq(alerts.id, alertId)); + .where(and(eq(alerts.id, alertId), eq(alerts.portId, portId))); } export interface ListAlertsOptions { diff --git a/src/lib/services/crm-invite.service.ts b/src/lib/services/crm-invite.service.ts index c13bc20..201737c 100644 --- a/src/lib/services/crm-invite.service.ts +++ b/src/lib/services/crm-invite.service.ts @@ -19,10 +19,20 @@ export async function createCrmInvite(args: { email: string; name?: string; isSuperAdmin?: boolean; + /** + * Caller identity. Required when minting a super-admin invitation so the + * service can fail closed if the caller isn't already a super-admin — + * defense-in-depth for the route's authorization gate. + */ + invitedBy?: { userId: string; isSuperAdmin: boolean }; }): Promise<{ inviteId: string; link: string }> { const email = args.email.toLowerCase().trim(); const isSuperAdmin = args.isSuperAdmin ?? false; + if (isSuperAdmin && !args.invitedBy?.isSuperAdmin) { + throw new ValidationError('Only super admins can mint super-admin invitations'); + } + // Reject if there's already a better-auth user with this email — they // should reset their password instead. const sql = postgres(env.DATABASE_URL); diff --git a/tests/integration/alerts-tenant-isolation.test.ts b/tests/integration/alerts-tenant-isolation.test.ts new file mode 100644 index 0000000..dc07b27 --- /dev/null +++ b/tests/integration/alerts-tenant-isolation.test.ts @@ -0,0 +1,89 @@ +/** + * Security regression: dismissAlert / acknowledgeAlert must filter by + * portId so an alert UUID from port A can't be mutated by a session + * scoped to port B. + */ + +import { describe, it, expect, beforeAll } from 'vitest'; +import { eq } from 'drizzle-orm'; + +import { db } from '@/lib/db'; +import { alerts } from '@/lib/db/schema/insights'; +import { user } from '@/lib/db/schema/users'; +import { dismissAlert, acknowledgeAlert } from '@/lib/services/alerts.service'; +import { makePort } from '../helpers/factories'; + +let TEST_USER_ID = ''; + +beforeAll(async () => { + // dismissedBy / acknowledgedBy reference user.id; pull any seeded user. + const [u] = await db.select({ id: user.id }).from(user).limit(1); + if (!u) throw new Error('No user available; run pnpm db:seed first'); + TEST_USER_ID = u.id; +}); + +async function makeAlert(portId: string) { + const [row] = await db + .insert(alerts) + .values({ + portId, + ruleId: 'document_overdue', + severity: 'medium', + title: 'Test alert', + link: '/test', + fingerprint: `fp-${Math.random().toString(36).slice(2)}`, + metadata: {}, + }) + .returning({ id: alerts.id }); + if (!row) throw new Error('failed to insert alert'); + return row.id; +} + +describe('alerts service — tenant isolation', () => { + it('dismissAlert is a no-op when called with the wrong portId', async () => { + const portA = await makePort(); + const portB = await makePort(); + const alertId = await makeAlert(portA.id); + + // Attacker session scoped to port B tries to dismiss port A's alert. + await dismissAlert(alertId, portB.id, TEST_USER_ID); + + const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) }); + expect(after?.dismissedAt).toBeNull(); + expect(after?.dismissedBy).toBeNull(); + }); + + it('dismissAlert succeeds when portId matches', async () => { + const port = await makePort(); + const alertId = await makeAlert(port.id); + + await dismissAlert(alertId, port.id, TEST_USER_ID); + + const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) }); + expect(after?.dismissedAt).not.toBeNull(); + expect(after?.dismissedBy).toBe(TEST_USER_ID); + }); + + it('acknowledgeAlert is a no-op when called with the wrong portId', async () => { + const portA = await makePort(); + const portB = await makePort(); + const alertId = await makeAlert(portA.id); + + await acknowledgeAlert(alertId, portB.id, TEST_USER_ID); + + const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) }); + expect(after?.acknowledgedAt).toBeNull(); + expect(after?.acknowledgedBy).toBeNull(); + }); + + it('acknowledgeAlert succeeds when portId matches', async () => { + const port = await makePort(); + const alertId = await makeAlert(port.id); + + await acknowledgeAlert(alertId, port.id, TEST_USER_ID); + + const after = await db.query.alerts.findFirst({ where: eq(alerts.id, alertId) }); + expect(after?.acknowledgedAt).not.toBeNull(); + expect(after?.acknowledgedBy).toBe(TEST_USER_ID); + }); +}); diff --git a/tests/integration/crm-invite-super-admin-gate.test.ts b/tests/integration/crm-invite-super-admin-gate.test.ts new file mode 100644 index 0000000..7537e4b --- /dev/null +++ b/tests/integration/crm-invite-super-admin-gate.test.ts @@ -0,0 +1,32 @@ +/** + * Security regression: only an existing super-admin caller can mint a + * super-admin CRM invitation. A port `director` (or any caller without + * `invitedBy.isSuperAdmin === true`) must be rejected at the service layer + * even if the route handler somehow lets the body flag through. + */ + +import { describe, it, expect } from 'vitest'; + +import { createCrmInvite } from '@/lib/services/crm-invite.service'; +import { ValidationError } from '@/lib/errors'; + +describe('createCrmInvite — super-admin gate', () => { + it('rejects super-admin invites when caller is not a super-admin', async () => { + await expect( + createCrmInvite({ + email: `attacker-${Date.now()}@example.test`, + isSuperAdmin: true, + invitedBy: { userId: 'director-id', isSuperAdmin: false }, + }), + ).rejects.toThrow(ValidationError); + }); + + it('rejects super-admin invites when invitedBy is omitted entirely', async () => { + await expect( + createCrmInvite({ + email: `attacker-${Date.now()}-noctx@example.test`, + isSuperAdmin: true, + }), + ).rejects.toThrow(ValidationError); + }); +}); diff --git a/tests/integration/public-interest-trio.test.ts b/tests/integration/public-interest-trio.test.ts index dc6c613..da74476 100644 --- a/tests/integration/public-interest-trio.test.ts +++ b/tests/integration/public-interest-trio.test.ts @@ -18,12 +18,15 @@ vi.mock('@/lib/services/inquiry-notifications.service', () => ({ sendInquiryNotifications: vi.fn().mockResolvedValue(undefined), })); -// The rate-limiter is keyed by IP header and persists across requests inside a -// single process. Use a unique IP per test call to avoid 429s. +// The rate-limiter is keyed by IP header and is now redis-backed; entries +// pexpire after the publicForm window (1h). Randomize the high octets so a +// fresh test run doesn't collide with leftover redis state from a previous +// run sharing the same redis instance. +const IP_PREFIX = `10.${Math.floor(Math.random() * 200) + 10}`; let ipCounter = 1; function uniqueIp(): string { ipCounter += 1; - return `10.0.${Math.floor(ipCounter / 255) % 255}.${ipCounter % 255}`; + return `${IP_PREFIX}.${Math.floor(ipCounter / 255) % 255}.${ipCounter % 255}`; } describe('POST /api/public/interests — trio creation', () => { diff --git a/tests/integration/public-residential-inquiry.test.ts b/tests/integration/public-residential-inquiry.test.ts index 1f6836c..a5a419d 100644 --- a/tests/integration/public-residential-inquiry.test.ts +++ b/tests/integration/public-residential-inquiry.test.ts @@ -17,10 +17,13 @@ import { makeMockRequest } from '../helpers/route-tester'; vi.mock('@/lib/socket/server', () => ({ emitToRoom: vi.fn() })); vi.mock('@/lib/email', () => ({ sendEmail: vi.fn().mockResolvedValue(undefined) })); +// Randomize per-run prefix so leftover redis sliding-window entries from a +// previous run don't 429 the new run (publicForm limiter pexpires after 1h). +const IP_PREFIX = `10.${Math.floor(Math.random() * 200) + 10}`; let ipCounter = 1; function uniqueIp(): string { ipCounter += 1; - return `10.50.${Math.floor(ipCounter / 255) % 255}.${ipCounter % 255}`; + return `${IP_PREFIX}.${Math.floor(ipCounter / 255) % 255}.${ipCounter % 255}`; } describe('POST /api/public/residential-inquiries', () => {