diff --git a/apps/sim/app/api/table/[tableId]/route.ts b/apps/sim/app/api/table/[tableId]/route.ts index df31b1175..7715cceff 100644 --- a/apps/sim/app/api/table/[tableId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/route.ts @@ -1,65 +1,19 @@ import { db } from '@sim/db' -import { permissions, userTableDefinitions, userTableRows, workspace } from '@sim/db/schema' +import { userTableDefinitions, userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' import { checkHybridAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' +import { checkTableAccess, checkTableWriteAccess } from '../utils' const logger = createLogger('TableDetailAPI') const GetTableSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility }) -/** - * Check if user has write access to workspace - */ -async function checkWorkspaceAccess(workspaceId: string, userId: string) { - const [workspaceData] = await db - .select({ - id: workspace.id, - ownerId: workspace.ownerId, - }) - .from(workspace) - .where(eq(workspace.id, workspaceId)) - .limit(1) - - if (!workspaceData) { - return { hasAccess: false, canWrite: false } - } - - if (workspaceData.ownerId === userId) { - return { hasAccess: true, canWrite: true } - } - - const [permission] = await db - .select({ - permissionType: permissions.permissionType, - }) - .from(permissions) - .where( - and( - eq(permissions.userId, userId), - eq(permissions.entityType, 'workspace'), - eq(permissions.entityId, workspaceId) - ) - ) - .limit(1) - - if (!permission) { - return { hasAccess: false, canWrite: false } - } - - const canWrite = permission.permissionType === 'admin' || permission.permissionType === 'write' - - return { - hasAccess: true, - canWrite, - } -} - /** * GET /api/table/[tableId]?workspaceId=xxx * Get table details @@ -74,39 +28,36 @@ export async function GET( try { const authResult = await checkHybridAuth(request) if (!authResult.success || !authResult.userId) { + logger.warn(`[${requestId}] Unauthorized table access attempt`) return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) } - const { searchParams } = new URL(request.url) - const validated = GetTableSchema.parse({ - workspaceId: searchParams.get('workspaceId'), - }) + // Check table access (similar to knowledge base access control) + const accessCheck = await checkTableAccess(tableId, authResult.userId) - // Check workspace access - const { hasAccess } = await checkWorkspaceAccess(validated.workspaceId, authResult.userId) - - if (!hasAccess) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to access unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } - // Get table + // Get table (workspaceId validation is now handled by access check) const [table] = await db .select() .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) - ) - ) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) .limit(1) if (!table) { return NextResponse.json({ error: 'Table not found' }, { status: 404 }) } - logger.info(`[${requestId}] Retrieved table ${tableId}`) + logger.info(`[${requestId}] Retrieved table ${tableId} for user ${authResult.userId}`) return NextResponse.json({ table: { @@ -138,30 +89,30 @@ export async function GET( * Delete a table (hard delete) */ export async function DELETE( - request: NextRequest, + _request: NextRequest, { params }: { params: Promise<{ tableId: string }> } ) { const requestId = generateRequestId() const { tableId } = await params try { - const authResult = await checkHybridAuth(request) + const authResult = await checkHybridAuth(_request) if (!authResult.success || !authResult.userId) { + logger.warn(`[${requestId}] Unauthorized table delete attempt`) return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) } - const { searchParams } = new URL(request.url) - const validated = GetTableSchema.parse({ - workspaceId: searchParams.get('workspaceId'), - }) + // Check table write access (similar to knowledge base write access control) + const accessCheck = await checkTableWriteAccess(tableId, authResult.userId) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess( - validated.workspaceId, - authResult.userId - ) - - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to delete unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } @@ -171,19 +122,14 @@ export async function DELETE( // Hard delete table const [deletedTable] = await db .delete(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId) - ) - ) + .where(eq(userTableDefinitions.id, tableId)) .returning() if (!deletedTable) { return NextResponse.json({ error: 'Table not found' }, { status: 404 }) } - logger.info(`[${requestId}] Deleted table ${tableId}`) + logger.info(`[${requestId}] Deleted table ${tableId} for user ${authResult.userId}`) return NextResponse.json({ message: 'Table deleted successfully', diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts index 61a291f2f..5cd27d0cd 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { permissions, userTableDefinitions, userTableRows, workspace } from '@sim/db/schema' +import { userTableDefinitions, userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' @@ -13,69 +13,23 @@ import { validateRowSize, validateUniqueConstraints, } from '@/lib/table' +import { checkTableAccess, checkTableWriteAccess, verifyTableWorkspace } from '../../utils' const logger = createLogger('TableRowAPI') const GetRowSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access }) const UpdateRowSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access data: z.record(z.any()), }) const DeleteRowSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access }) -/** - * Check if user has write access to workspace - */ -async function checkWorkspaceAccess(workspaceId: string, userId: string) { - const [workspaceData] = await db - .select({ - id: workspace.id, - ownerId: workspace.ownerId, - }) - .from(workspace) - .where(eq(workspace.id, workspaceId)) - .limit(1) - - if (!workspaceData) { - return { hasAccess: false, canWrite: false } - } - - if (workspaceData.ownerId === userId) { - return { hasAccess: true, canWrite: true } - } - - const [permission] = await db - .select({ - permissionType: permissions.permissionType, - }) - .from(permissions) - .where( - and( - eq(permissions.userId, userId), - eq(permissions.entityType, 'workspace'), - eq(permissions.entityId, workspaceId) - ) - ) - .limit(1) - - if (!permission) { - return { hasAccess: false, canWrite: false } - } - - const canWrite = permission.permissionType === 'admin' || permission.permissionType === 'write' - - return { - hasAccess: true, - canWrite, - } -} - /** * GET /api/table/[tableId]/rows/[rowId]?workspaceId=xxx * Get a single row by ID @@ -98,13 +52,32 @@ export async function GET( workspaceId: searchParams.get('workspaceId'), }) - // Check workspace access - const { hasAccess } = await checkWorkspaceAccess(validated.workspaceId, authResult.userId) + // Check table access (centralized access control) + const accessCheck = await checkTableAccess(tableId, authResult.userId) - if (!hasAccess) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to access row from unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } + // Security check: If workspaceId is provided, verify it matches the table's workspace + const actualWorkspaceId = validated.workspaceId || accessCheck.table.workspaceId + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` + ) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } + } + // Get row const [row] = await db .select({ @@ -118,7 +91,7 @@ export async function GET( and( eq(userTableRows.id, rowId), eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId) + eq(userTableRows.workspaceId, actualWorkspaceId) ) ) .limit(1) @@ -170,27 +143,37 @@ export async function PATCH( const body = await request.json() const validated = UpdateRowSchema.parse(body) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess( - validated.workspaceId, - authResult.userId - ) + // Check table write access (centralized access control) + const accessCheck = await checkTableWriteAccess(tableId, authResult.userId) - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to update row in unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } + // Security check: If workspaceId is provided, verify it matches the table's workspace + const actualWorkspaceId = validated.workspaceId || accessCheck.table.workspaceId + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` + ) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } + } + // Get table definition const [table] = await db .select() .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) - ) - ) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) .limit(1) if (!table) { @@ -255,7 +238,7 @@ export async function PATCH( and( eq(userTableRows.id, rowId), eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId) + eq(userTableRows.workspaceId, actualWorkspaceId) ) ) .returning() @@ -308,16 +291,32 @@ export async function DELETE( const body = await request.json() const validated = DeleteRowSchema.parse(body) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess( - validated.workspaceId, - authResult.userId - ) + // Check table write access (centralized access control) + const accessCheck = await checkTableWriteAccess(tableId, authResult.userId) - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to delete row from unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } + // Security check: If workspaceId is provided, verify it matches the table's workspace + const actualWorkspaceId = validated.workspaceId || accessCheck.table.workspaceId + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` + ) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } + } + // Delete row const [deletedRow] = await db .delete(userTableRows) @@ -325,7 +324,7 @@ export async function DELETE( and( eq(userTableRows.id, rowId), eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId) + eq(userTableRows.workspaceId, actualWorkspaceId) ) ) .returning() diff --git a/apps/sim/app/api/table/[tableId]/rows/route.ts b/apps/sim/app/api/table/[tableId]/rows/route.ts index af1a58fc0..c0cc42a0a 100644 --- a/apps/sim/app/api/table/[tableId]/rows/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/route.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { permissions, userTableDefinitions, userTableRows, workspace } from '@sim/db/schema' +import { userTableDefinitions, userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' @@ -15,21 +15,22 @@ import { validateUniqueConstraints, } from '@/lib/table' import { buildFilterClause, buildSortClause } from '@/lib/table/query-builder' +import { checkTableAccess, checkTableWriteAccess, verifyTableWorkspace } from '../../utils' const logger = createLogger('TableRowsAPI') const InsertRowSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access data: z.record(z.any()), }) const BatchInsertRowsSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access rows: z.array(z.record(z.any())).min(1).max(1000), // Max 1000 rows per batch }) const QueryRowsSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access filter: z.record(z.any()).optional(), sort: z.record(z.enum(['asc', 'desc'])).optional(), limit: z.coerce.number().int().min(1).max(TABLE_LIMITS.MAX_QUERY_LIMIT).optional().default(100), @@ -37,95 +38,63 @@ const QueryRowsSchema = z.object({ }) const UpdateRowsByFilterSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access filter: z.record(z.any()), // Required - must specify what to update data: z.record(z.any()), // New data to set limit: z.coerce.number().int().min(1).max(1000).optional(), // Safety limit for bulk updates }) const DeleteRowsByFilterSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access filter: z.record(z.any()), // Required - must specify what to delete limit: z.coerce.number().int().min(1).max(1000).optional(), // Safety limit for bulk deletes }) -/** - * Check if user has write access to workspace - */ -async function checkWorkspaceAccess(workspaceId: string, userId: string) { - const [workspaceData] = await db - .select({ - id: workspace.id, - ownerId: workspace.ownerId, - }) - .from(workspace) - .where(eq(workspace.id, workspaceId)) - .limit(1) - - if (!workspaceData) { - return { hasAccess: false, canWrite: false } - } - - if (workspaceData.ownerId === userId) { - return { hasAccess: true, canWrite: true } - } - - const [permission] = await db - .select({ - permissionType: permissions.permissionType, - }) - .from(permissions) - .where( - and( - eq(permissions.userId, userId), - eq(permissions.entityType, 'workspace'), - eq(permissions.entityId, workspaceId) - ) - ) - .limit(1) - - if (!permission) { - return { hasAccess: false, canWrite: false } - } - - const canWrite = permission.permissionType === 'admin' || permission.permissionType === 'write' - - return { - hasAccess: true, - canWrite, - } -} - /** * Handle batch insert of multiple rows */ async function handleBatchInsert(requestId: string, tableId: string, body: any, userId: string) { const validated = BatchInsertRowsSchema.parse(body) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess(validated.workspaceId, userId) + // Check table write access (centralized access control) + const accessCheck = await checkTableWriteAccess(tableId, userId) - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${userId} attempted to insert rows into unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } + // Security check: If workspaceId is provided, verify it matches the table's workspace + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` + ) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } + } + // Get table definition const [table] = await db .select() .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) - ) - ) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) .limit(1) if (!table) { return NextResponse.json({ error: 'Table not found' }, { status: 404 }) } + // Use the workspaceId from the access check (more secure) + const workspaceId = validated.workspaceId || accessCheck.table.workspaceId + // Check row count limit const remainingCapacity = table.maxRows - table.rowCount if (remainingCapacity < validated.rows.length) { @@ -190,7 +159,7 @@ async function handleBatchInsert(requestId: string, tableId: string, body: any, })) const uniqueValidation = validateUniqueConstraints(rowData, table.schema as TableSchema, [ - ...existingRows, + ...existingRows.map((r) => ({ id: r.id, data: r.data as Record })), ...batchRows, ]) @@ -215,7 +184,7 @@ async function handleBatchInsert(requestId: string, tableId: string, body: any, const rowsToInsert = validated.rows.map((data) => ({ id: `row_${crypto.randomUUID().replace(/-/g, '')}`, tableId, - workspaceId: validated.workspaceId, + workspaceId, data, createdAt: now, updatedAt: now, @@ -275,33 +244,45 @@ export async function POST( // Single row insert const validated = InsertRowSchema.parse(body) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess( - validated.workspaceId, - authResult.userId - ) + // Check table write access (centralized access control) + const accessCheck = await checkTableWriteAccess(tableId, authResult.userId) - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to insert row into unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } + // Security check: If workspaceId is provided, verify it matches the table's workspace + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` + ) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } + } + // Get table definition const [table] = await db .select() .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) - ) - ) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) .limit(1) if (!table) { return NextResponse.json({ error: 'Table not found' }, { status: 404 }) } + // Use the workspaceId from the access check (more secure) + const workspaceId = validated.workspaceId || accessCheck.table.workspaceId + // Validate row size const sizeValidation = validateRowSize(validated.data) if (!sizeValidation.valid) { @@ -335,7 +316,7 @@ export async function POST( const uniqueValidation = validateUniqueConstraints( validated.data, table.schema as TableSchema, - existingRows + existingRows.map((r) => ({ id: r.id, data: r.data as Record })) ) if (!uniqueValidation.valid) { @@ -363,7 +344,7 @@ export async function POST( .values({ id: rowId, tableId, - workspaceId: validated.workspaceId, + workspaceId, data: validated.data, createdAt: now, updatedAt: now, @@ -450,34 +431,38 @@ export async function GET( offset, }) - // Check workspace access - const { hasAccess } = await checkWorkspaceAccess(validated.workspaceId, authResult.userId) + // Check table access (centralized access control) + const accessCheck = await checkTableAccess(tableId, authResult.userId) - if (!hasAccess) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to query rows from unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } - // Verify table exists - const [table] = await db - .select({ id: userTableDefinitions.id }) - .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) + // Security check: If workspaceId is provided, verify it matches the table's workspace + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` ) - ) - .limit(1) - - if (!table) { - return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } } + // Use the workspaceId from the access check (more secure) + const actualWorkspaceId = validated.workspaceId || accessCheck.table.workspaceId + // Build base where conditions const baseConditions = [ eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId), + eq(userTableRows.workspaceId, actualWorkspaceId), ] // Add filter conditions if provided @@ -570,33 +555,45 @@ export async function PUT( const body = await request.json() const validated = UpdateRowsByFilterSchema.parse(body) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess( - validated.workspaceId, - authResult.userId - ) + // Check table write access (centralized access control) + const accessCheck = await checkTableWriteAccess(tableId, authResult.userId) - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to update rows in unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } + // Security check: If workspaceId is provided, verify it matches the table's workspace + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` + ) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } + } + // Get table definition const [table] = await db .select() .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) - ) - ) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) .limit(1) if (!table) { return NextResponse.json({ error: 'Table not found' }, { status: 404 }) } + // Use the workspaceId from the access check (more secure) + const actualWorkspaceId = validated.workspaceId || accessCheck.table.workspaceId + // Validate new data size const sizeValidation = validateRowSize(validated.data) if (!sizeValidation.valid) { @@ -609,7 +606,7 @@ export async function PUT( // Build base where conditions const baseConditions = [ eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId), + eq(userTableRows.workspaceId, actualWorkspaceId), ] // Add filter conditions @@ -650,7 +647,7 @@ export async function PUT( // Validate that merged data matches schema for each row for (const row of matchingRows) { - const mergedData = { ...row.data, ...validated.data } + const mergedData = { ...(row.data as Record), ...validated.data } const rowValidation = validateRowAgainstSchema(mergedData, table.schema as TableSchema) if (!rowValidation.valid) { return NextResponse.json( @@ -678,11 +675,11 @@ export async function PUT( // Validate each updated row for unique constraints for (const row of matchingRows) { - const mergedData = { ...row.data, ...validated.data } + const mergedData = { ...(row.data as Record), ...validated.data } const uniqueValidation = validateUniqueConstraints( mergedData, table.schema as TableSchema, - allRows, + allRows.map((r) => ({ id: r.id, data: r.data as Record })), row.id // Exclude the current row being updated ) @@ -710,7 +707,7 @@ export async function PUT( db .update(userTableRows) .set({ - data: { ...row.data, ...validated.data }, + data: { ...(row.data as Record), ...validated.data }, updatedAt: now, }) .where(eq(userTableRows.id, row.id)) @@ -767,37 +764,38 @@ export async function DELETE( const body = await request.json() const validated = DeleteRowsByFilterSchema.parse(body) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess( - validated.workspaceId, - authResult.userId - ) + // Check table write access (centralized access control) + const accessCheck = await checkTableWriteAccess(tableId, authResult.userId) - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to delete rows from unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } - // Verify table exists - const [table] = await db - .select({ id: userTableDefinitions.id }) - .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) + // Security check: If workspaceId is provided, verify it matches the table's workspace + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` ) - ) - .limit(1) - - if (!table) { - return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } } + // Use the workspaceId from the access check (more secure) + const actualWorkspaceId = validated.workspaceId || accessCheck.table.workspaceId + // Build base where conditions const baseConditions = [ eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId), + eq(userTableRows.workspaceId, actualWorkspaceId), ] // Add filter conditions @@ -843,7 +841,7 @@ export async function DELETE( await db.delete(userTableRows).where( and( eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId), + eq(userTableRows.workspaceId, actualWorkspaceId), sql`${userTableRows.id} = ANY(ARRAY[${sql.join( batch.map((id) => sql`${id}`), sql`, ` diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts index 3393b9fb7..3d15143ab 100644 --- a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts @@ -8,63 +8,15 @@ import { checkHybridAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import type { TableSchema } from '@/lib/table' import { getUniqueColumns, validateRowAgainstSchema, validateRowSize } from '@/lib/table' +import { checkTableWriteAccess, verifyTableWorkspace } from '../../utils' const logger = createLogger('TableUpsertAPI') const UpsertRowSchema = z.object({ - workspaceId: z.string().min(1), + workspaceId: z.string().min(1).optional(), // Optional for backward compatibility, validated via table access data: z.record(z.any()), }) -/** - * Check if user has write access to workspace - */ -async function checkWorkspaceAccess(workspaceId: string, userId: string) { - const { workspace, permissions } = await import('@sim/db/schema') - - const [workspaceData] = await db - .select({ - id: workspace.id, - ownerId: workspace.ownerId, - }) - .from(workspace) - .where(eq(workspace.id, workspaceId)) - .limit(1) - - if (!workspaceData) { - return { hasAccess: false, canWrite: false } - } - - if (workspaceData.ownerId === userId) { - return { hasAccess: true, canWrite: true } - } - - const [permission] = await db - .select({ - permissionType: permissions.permissionType, - }) - .from(permissions) - .where( - and( - eq(permissions.userId, userId), - eq(permissions.entityType, 'workspace'), - eq(permissions.entityId, workspaceId) - ) - ) - .limit(1) - - if (!permission) { - return { hasAccess: false, canWrite: false } - } - - const canWrite = permission.permissionType === 'admin' || permission.permissionType === 'write' - - return { - hasAccess: true, - canWrite, - } -} - /** * POST /api/table/[tableId]/rows/upsert * Insert or update a row based on unique column constraints @@ -86,27 +38,37 @@ export async function POST( const body = await request.json() const validated = UpsertRowSchema.parse(body) - // Check workspace access - const { hasAccess, canWrite } = await checkWorkspaceAccess( - validated.workspaceId, - authResult.userId - ) + // Check table write access (centralized access control) + const accessCheck = await checkTableWriteAccess(tableId, authResult.userId) - if (!hasAccess || !canWrite) { + if (!accessCheck.hasAccess) { + if ('notFound' in accessCheck && accessCheck.notFound) { + logger.warn(`[${requestId}] Table not found: ${tableId}`) + return NextResponse.json({ error: 'Table not found' }, { status: 404 }) + } + logger.warn( + `[${requestId}] User ${authResult.userId} attempted to upsert row in unauthorized table ${tableId}` + ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } + // Security check: If workspaceId is provided, verify it matches the table's workspace + const actualWorkspaceId = validated.workspaceId || accessCheck.table.workspaceId + if (validated.workspaceId) { + const isValidWorkspace = await verifyTableWorkspace(tableId, validated.workspaceId) + if (!isValidWorkspace) { + logger.warn( + `[${requestId}] Workspace ID mismatch for table ${tableId}. Provided: ${validated.workspaceId}, Actual: ${accessCheck.table.workspaceId}` + ) + return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) + } + } + // Get table definition const [table] = await db .select() .from(userTableDefinitions) - .where( - and( - eq(userTableDefinitions.id, tableId), - eq(userTableDefinitions.workspaceId, validated.workspaceId), - isNull(userTableDefinitions.deletedAt) - ) - ) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) .limit(1) if (!table) { @@ -174,7 +136,7 @@ export async function POST( .where( and( eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId), + eq(userTableRows.workspaceId, actualWorkspaceId), ...validUniqueFilters ) ) @@ -211,10 +173,11 @@ export async function POST( .insert(userTableRows) .values({ tableId, - workspaceId: validated.workspaceId, + workspaceId: actualWorkspaceId, data: validated.data, createdAt: now, updatedAt: now, + createdBy: authResult.userId, }) .returning() diff --git a/apps/sim/app/api/table/utils.ts b/apps/sim/app/api/table/utils.ts new file mode 100644 index 000000000..0d2cb9bd6 --- /dev/null +++ b/apps/sim/app/api/table/utils.ts @@ -0,0 +1,127 @@ +import { db } from '@sim/db' +import { userTableDefinitions } from '@sim/db/schema' +import { and, eq, isNull } from 'drizzle-orm' +import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' + +export interface TableData { + id: string + workspaceId: string + createdBy: string + name: string + description?: string | null + schema: unknown + maxRows: number + rowCount: number + deletedAt?: Date | null + createdAt: Date + updatedAt: Date +} + +export interface TableAccessResult { + hasAccess: true + table: Pick +} + +export interface TableAccessDenied { + hasAccess: false + notFound?: boolean + reason?: string +} + +export type TableAccessCheck = TableAccessResult | TableAccessDenied + +/** + * Check if a user has access to a table + * Access is granted if: + * 1. User created the table directly, OR + * 2. User has any permission (read/write/admin) on the table's workspace + */ +export async function checkTableAccess(tableId: string, userId: string): Promise { + const table = await db + .select({ + id: userTableDefinitions.id, + createdBy: userTableDefinitions.createdBy, + workspaceId: userTableDefinitions.workspaceId, + }) + .from(userTableDefinitions) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) + .limit(1) + + if (table.length === 0) { + return { hasAccess: false, notFound: true } + } + + const tableData = table[0] + + // Case 1: User created the table directly + if (tableData.createdBy === userId) { + return { hasAccess: true, table: tableData } + } + + // Case 2: Table belongs to a workspace the user has permissions for + const userPermission = await getUserEntityPermissions(userId, 'workspace', tableData.workspaceId) + if (userPermission !== null) { + return { hasAccess: true, table: tableData } + } + + return { hasAccess: false } +} + +/** + * Check if a user has write access to a table + * Write access is granted if: + * 1. User created the table directly, OR + * 2. User has write or admin permissions on the table's workspace + */ +export async function checkTableWriteAccess( + tableId: string, + userId: string +): Promise { + const table = await db + .select({ + id: userTableDefinitions.id, + createdBy: userTableDefinitions.createdBy, + workspaceId: userTableDefinitions.workspaceId, + }) + .from(userTableDefinitions) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) + .limit(1) + + if (table.length === 0) { + return { hasAccess: false, notFound: true } + } + + const tableData = table[0] + + // Case 1: User created the table directly + if (tableData.createdBy === userId) { + return { hasAccess: true, table: tableData } + } + + // Case 2: Table belongs to a workspace and user has write/admin permissions + const userPermission = await getUserEntityPermissions(userId, 'workspace', tableData.workspaceId) + if (userPermission === 'write' || userPermission === 'admin') { + return { hasAccess: true, table: tableData } + } + + return { hasAccess: false } +} + +/** + * Verify that a table belongs to a specific workspace + * This is a security check to prevent workspace ID spoofing + * Use this when workspaceId is provided as a parameter to ensure it matches the table's actual workspace + */ +export async function verifyTableWorkspace(tableId: string, workspaceId: string): Promise { + const table = await db + .select({ workspaceId: userTableDefinitions.workspaceId }) + .from(userTableDefinitions) + .where(and(eq(userTableDefinitions.id, tableId), isNull(userTableDefinitions.deletedAt))) + .limit(1) + + if (table.length === 0) { + return false + } + + return table[0].workspaceId === workspaceId +}