From 9a5479c2c735e1c7f32d1af08d8bc8802429b250 Mon Sep 17 00:00:00 2001 From: Matt Ciaccio Date: Wed, 29 Apr 2026 03:00:55 +0200 Subject: [PATCH] sec: lock down socket.io room subscription + crm-invite cross-tenant ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. HIGH — Socket.IO accepted client-supplied `auth.portId` in the handshake without verifying the user actually held a role in that port, then unconditionally joined the socket to `port:${portId}`. The `join:entity` handler also skipped authorization. This let any authenticated CRM user receive realtime events from any other tenant: invoice numbers + totals + client names, document signer emails, registration events with full client name + berth, file uploads, etc. Auth middleware now resolves the user's userPortRoles (or isSuperAdmin) before honouring portId, and join:entity verifies the entity's port matches a port the user has access to. Pre-existing pre-branch issue but fixed here given the explicit "all data is extremely sensitive" directive. 2. MEDIUM — listCrmInvites issued a global SELECT with no port scope. The crm_user_invites table has no portId column (invites mint global better-auth users, then port roles are assigned later). The previous gating on per-port admin.manage_users let any director enumerate every other tenant's pending invitee emails + isSuperAdmin flags — a phishing target list and a super-admin onboarding timing oracle. Restrict GET (list), DELETE (revoke), and POST resend to ctx.isSuperAdmin. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../v1/admin/invitations/[id]/resend/route.ts | 8 +- .../api/v1/admin/invitations/[id]/route.ts | 8 +- src/app/api/v1/admin/invitations/route.ts | 10 +- src/lib/socket/server.ts | 92 +++++++++++++++++-- 4 files changed, 109 insertions(+), 9 deletions(-) diff --git a/src/app/api/v1/admin/invitations/[id]/resend/route.ts b/src/app/api/v1/admin/invitations/[id]/resend/route.ts index cd22300..da39a76 100644 --- a/src/app/api/v1/admin/invitations/[id]/resend/route.ts +++ b/src/app/api/v1/admin/invitations/[id]/resend/route.ts @@ -1,12 +1,18 @@ import { NextResponse } from 'next/server'; import { withAuth, withPermission } from '@/lib/api/helpers'; -import { errorResponse } from '@/lib/errors'; +import { errorResponse, ForbiddenError } from '@/lib/errors'; import { resendCrmInvite } from '@/lib/services/crm-invite.service'; +// Resend mints a fresh token + new email on a global invite row; +// restrict to super-admins to match revoke/list and avoid cross-tenant +// re-issuance of foreign-port invitations. export const POST = withAuth( withPermission('admin', 'manage_users', async (_req, ctx, params) => { try { + if (!ctx.isSuperAdmin) { + throw new ForbiddenError('Resending CRM invites requires super-admin'); + } const id = params.id ?? ''; const result = await resendCrmInvite(id, { userId: ctx.userId, diff --git a/src/app/api/v1/admin/invitations/[id]/route.ts b/src/app/api/v1/admin/invitations/[id]/route.ts index e8aac2d..05f838e 100644 --- a/src/app/api/v1/admin/invitations/[id]/route.ts +++ b/src/app/api/v1/admin/invitations/[id]/route.ts @@ -1,12 +1,18 @@ import { NextResponse } from 'next/server'; import { withAuth, withPermission } from '@/lib/api/helpers'; -import { errorResponse } from '@/lib/errors'; +import { errorResponse, ForbiddenError } from '@/lib/errors'; import { revokeCrmInvite } from '@/lib/services/crm-invite.service'; +// Invites are a global resource (no portId column). Revoking a foreign +// tenant's pending invite by id would be cross-tenant tampering; +// restrict to super-admins to match the listing endpoint. export const DELETE = withAuth( withPermission('admin', 'manage_users', async (_req, ctx, params) => { try { + if (!ctx.isSuperAdmin) { + throw new ForbiddenError('Revoking CRM invites requires super-admin'); + } const id = params.id ?? ''; await revokeCrmInvite(id, { userId: ctx.userId, diff --git a/src/app/api/v1/admin/invitations/route.ts b/src/app/api/v1/admin/invitations/route.ts index 6cfbdde..acfbbfb 100644 --- a/src/app/api/v1/admin/invitations/route.ts +++ b/src/app/api/v1/admin/invitations/route.ts @@ -7,8 +7,16 @@ import { errorResponse, ForbiddenError } from '@/lib/errors'; import { createCrmInvite, listCrmInvites } from '@/lib/services/crm-invite.service'; export const GET = withAuth( - withPermission('admin', 'manage_users', async (_req, _ctx) => { + withPermission('admin', 'manage_users', async (_req, ctx) => { try { + // crm_user_invites is a global table (no per-port column) — invites + // mint better-auth users that may later be assigned roles in any + // port. Listing it cross-tenant would let a port-A director + // enumerate pending invitee emails, names, and isSuperAdmin flags + // for every other tenant. Restrict the listing to super-admins. + if (!ctx.isSuperAdmin) { + throw new ForbiddenError('Listing CRM invites requires super-admin'); + } const data = await listCrmInvites(); return NextResponse.json({ data }); } catch (error) { diff --git a/src/lib/socket/server.ts b/src/lib/socket/server.ts index 1fd115b..d4b8b92 100644 --- a/src/lib/socket/server.ts +++ b/src/lib/socket/server.ts @@ -1,15 +1,75 @@ import { Server } from 'socket.io'; import { createAdapter } from '@socket.io/redis-adapter'; import type { Server as HTTPServer } from 'node:http'; +import { and, eq } from 'drizzle-orm'; import { redis } from '@/lib/redis'; import { auth } from '@/lib/auth'; +import { db } from '@/lib/db'; +import { userProfiles, userPortRoles } from '@/lib/db/schema/users'; +import { berths } from '@/lib/db/schema/berths'; +import { clients } from '@/lib/db/schema/clients'; +import { interests } from '@/lib/db/schema/interests'; import { logger } from '@/lib/logger'; import type { ServerToClientEvents, ClientToServerEvents } from './events'; let io: Server | null = null; -export function initSocketServer(httpServer: HTTPServer): Server { +/** + * Returns true if the user is a super-admin OR holds a userPortRoles row + * for the given portId. The Socket.IO auth middleware uses this to decide + * whether to honour a client-supplied `auth.portId` — the prior code + * trusted whatever the client passed and thereby joined the socket to a + * foreign tenant's broadcast room. + */ +async function userCanAccessPort(userId: string, portId: string): Promise { + const profile = await db.query.userProfiles.findFirst({ + where: eq(userProfiles.userId, userId), + }); + if (profile?.isSuperAdmin) return true; + const role = await db.query.userPortRoles.findFirst({ + where: and(eq(userPortRoles.userId, userId), eq(userPortRoles.portId, portId)), + }); + return Boolean(role); +} + +/** + * Verify the user can join an entity-scoped room. Each entity type's own + * tenant column is checked — if the user can access the entity's port, + * they may subscribe to that entity's room. + */ +async function userCanJoinEntity( + userId: string, + type: 'berth' | 'client' | 'interest', + id: string, +): Promise { + const profile = await db.query.userProfiles.findFirst({ + where: eq(userProfiles.userId, userId), + }); + if (profile?.isSuperAdmin) return true; + + let entityPortId: string | null = null; + if (type === 'berth') { + const row = await db.query.berths.findFirst({ where: eq(berths.id, id) }); + entityPortId = row?.portId ?? null; + } else if (type === 'client') { + const row = await db.query.clients.findFirst({ where: eq(clients.id, id) }); + entityPortId = row?.portId ?? null; + } else if (type === 'interest') { + const row = await db.query.interests.findFirst({ where: eq(interests.id, id) }); + entityPortId = row?.portId ?? null; + } + if (!entityPortId) return false; + + const role = await db.query.userPortRoles.findFirst({ + where: and(eq(userPortRoles.userId, userId), eq(userPortRoles.portId, entityPortId)), + }); + return Boolean(role); +} + +export function initSocketServer( + httpServer: HTTPServer, +): Server { const pubClient = redis.duplicate(); const subClient = redis.duplicate(); @@ -24,7 +84,10 @@ export function initSocketServer(httpServer: HTTPServer): Server { try { const cookie = socket.handshake.headers.cookie; @@ -42,9 +105,17 @@ export function initSocketServer(httpServer: HTTPServer): Server { - socket.join(`${type}:${id}`); + // Entity-level room management — verify the user can access the + // entity's port before joining. Without this, any authenticated user + // could subscribe to a foreign-tenant entity's broadcast (note + // previews, signer emails, etc.) by guessing or harvesting an id. + socket.on('join:entity', async ({ type, id }) => { + try { + const ok = await userCanJoinEntity(userId, type, id); + if (ok) socket.join(`${type}:${id}`); + else logger.warn({ userId, type, id }, 'Socket denied join:entity'); + } catch (err) { + logger.warn({ err, userId, type, id }, 'join:entity check failed'); + } }); socket.on('leave:entity', ({ type, id }) => { socket.leave(`${type}:${id}`);