From 8902752c172824a6b3c649962b6a3e9d1eeaab68 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 10 Feb 2026 10:55:53 -0800 Subject: [PATCH 1/8] improvement(timeouts): files/base64 should use max timeouts --- apps/sim/app/api/jobs/[jobId]/route.ts | 2 +- apps/sim/lib/knowledge/documents/document-processor.ts | 2 +- apps/sim/lib/uploads/utils/file-utils.server.ts | 6 +++++- apps/sim/lib/uploads/utils/user-file-base64.server.ts | 3 ++- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/sim/app/api/jobs/[jobId]/route.ts b/apps/sim/app/api/jobs/[jobId]/route.ts index 34c2e42171..14be54facb 100644 --- a/apps/sim/app/api/jobs/[jobId]/route.ts +++ b/apps/sim/app/api/jobs/[jobId]/route.ts @@ -76,7 +76,7 @@ export async function GET( } if (job.status === JOB_STATUS.PROCESSING || job.status === JOB_STATUS.PENDING) { - response.estimatedDuration = 180000 + response.estimatedDuration = 300000 } return NextResponse.json(response) diff --git a/apps/sim/lib/knowledge/documents/document-processor.ts b/apps/sim/lib/knowledge/documents/document-processor.ts index 37896d9a3f..80789e81be 100644 --- a/apps/sim/lib/knowledge/documents/document-processor.ts +++ b/apps/sim/lib/knowledge/documents/document-processor.ts @@ -14,7 +14,7 @@ import { mistralParserTool } from '@/tools/mistral/parser' const logger = createLogger('DocumentProcessor') const TIMEOUTS = { - FILE_DOWNLOAD: 180000, + FILE_DOWNLOAD: 600000, MISTRAL_OCR_API: 120000, } as const diff --git a/apps/sim/lib/uploads/utils/file-utils.server.ts b/apps/sim/lib/uploads/utils/file-utils.server.ts index b759918d01..69077c663d 100644 --- a/apps/sim/lib/uploads/utils/file-utils.server.ts +++ b/apps/sim/lib/uploads/utils/file-utils.server.ts @@ -1,6 +1,7 @@ 'use server' import type { Logger } from '@sim/logger' +import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' import { secureFetchWithPinnedIP, validateUrlWithDNS, @@ -135,7 +136,10 @@ export async function resolveFileInputToUrl( * For internal URLs, uses direct storage access (server-side only) * For external URLs, validates DNS/SSRF and uses secure fetch with IP pinning */ -export async function downloadFileFromUrl(fileUrl: string, timeoutMs = 180000): Promise { +export async function downloadFileFromUrl( + fileUrl: string, + timeoutMs = getMaxExecutionTimeout() +): Promise { const { parseInternalFileUrl } = await import('./file-utils') if (isInternalFileUrl(fileUrl)) { diff --git a/apps/sim/lib/uploads/utils/user-file-base64.server.ts b/apps/sim/lib/uploads/utils/user-file-base64.server.ts index 33f7e62591..f3abdf5acd 100644 --- a/apps/sim/lib/uploads/utils/user-file-base64.server.ts +++ b/apps/sim/lib/uploads/utils/user-file-base64.server.ts @@ -1,13 +1,14 @@ import type { Logger } from '@sim/logger' import { createLogger } from '@sim/logger' import { getRedisClient } from '@/lib/core/config/redis' +import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' import { isUserFileWithMetadata } from '@/lib/core/utils/user-file' import { bufferToBase64 } from '@/lib/uploads/utils/file-utils' import { downloadFileFromStorage, downloadFileFromUrl } from '@/lib/uploads/utils/file-utils.server' import type { UserFile } from '@/executor/types' const DEFAULT_MAX_BASE64_BYTES = 10 * 1024 * 1024 -const DEFAULT_TIMEOUT_MS = 180000 +const DEFAULT_TIMEOUT_MS = getMaxExecutionTimeout() const DEFAULT_CACHE_TTL_SECONDS = 300 const REDIS_KEY_PREFIX = 'user-file:base64:' From 0ff7e57f99ead61c515b14eafc334a5804ce0303 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 10 Feb 2026 11:41:45 -0800 Subject: [PATCH 2/8] fix(billing): attribute cost to caller when info available" --- .../[executionId]/[contextId]/route.ts | 1 - apps/sim/lib/execution/preprocessing.ts | 166 ++++-------------- apps/sim/lib/logs/execution/logger.ts | 69 +++++--- .../executor/human-in-the-loop-manager.ts | 1 - 4 files changed, 78 insertions(+), 159 deletions(-) diff --git a/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts b/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts index 6c0f44ff54..0f3252d859 100644 --- a/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts +++ b/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts @@ -59,7 +59,6 @@ export async function POST( checkDeployment: false, // Resuming existing execution, deployment already checked skipUsageLimits: true, // Resume is continuation of authorized execution - don't recheck limits workspaceId: workflow.workspaceId || undefined, - isResumeContext: true, // Enable billing fallback for paused workflow resumes }) if (!preprocessResult.success) { diff --git a/apps/sim/lib/execution/preprocessing.ts b/apps/sim/lib/execution/preprocessing.ts index 3eb14813e3..5b0a65262e 100644 --- a/apps/sim/lib/execution/preprocessing.ts +++ b/apps/sim/lib/execution/preprocessing.ts @@ -19,94 +19,6 @@ const BILLING_ERROR_MESSAGES = { BILLING_ERROR_GENERIC: 'Error resolving billing account', } as const -/** - * Attempts to resolve billing actor with fallback for resume contexts. - * Returns the resolved actor user ID or null if resolution fails and should block execution. - * - * For resume contexts, this function allows fallback to the workflow owner if workspace - * billing cannot be resolved, ensuring users can complete their paused workflows even - * if billing configuration changes mid-execution. - * - * @returns Object containing actorUserId (null if should block) and shouldBlock flag - */ -async function resolveBillingActorWithFallback(params: { - requestId: string - workflowId: string - workspaceId: string - executionId: string - triggerType: string - workflowRecord: WorkflowRecord - userId: string - isResumeContext: boolean - baseActorUserId: string | null - failureReason: 'null' | 'error' - error?: unknown - loggingSession?: LoggingSession -}): Promise< - { actorUserId: string; shouldBlock: false } | { actorUserId: null; shouldBlock: true } -> { - const { - requestId, - workflowId, - workspaceId, - executionId, - triggerType, - workflowRecord, - userId, - isResumeContext, - baseActorUserId, - failureReason, - error, - loggingSession, - } = params - - if (baseActorUserId) { - return { actorUserId: baseActorUserId, shouldBlock: false } - } - - const workflowOwner = workflowRecord.userId?.trim() - if (isResumeContext && workflowOwner) { - const logMessage = - failureReason === 'null' - ? '[BILLING_FALLBACK] Workspace billing account is null. Using workflow owner for billing.' - : '[BILLING_FALLBACK] Exception during workspace billing resolution. Using workflow owner for billing.' - - logger.warn(`[${requestId}] ${logMessage}`, { - workflowId, - workspaceId, - fallbackUserId: workflowOwner, - ...(error ? { error } : {}), - }) - - return { actorUserId: workflowOwner, shouldBlock: false } - } - - const fallbackUserId = workflowRecord.userId || userId || 'unknown' - const errorMessage = - failureReason === 'null' - ? BILLING_ERROR_MESSAGES.BILLING_REQUIRED - : BILLING_ERROR_MESSAGES.BILLING_ERROR_GENERIC - - logger.warn(`[${requestId}] ${errorMessage}`, { - workflowId, - workspaceId, - ...(error ? { error } : {}), - }) - - await logPreprocessingError({ - workflowId, - executionId, - triggerType, - requestId, - userId: fallbackUserId, - workspaceId, - errorMessage, - loggingSession, - }) - - return { actorUserId: null, shouldBlock: true } -} - export interface PreprocessExecutionOptions { // Required fields workflowId: string @@ -123,7 +35,7 @@ export interface PreprocessExecutionOptions { // Context information workspaceId?: string // If known, used for billing resolution loggingSession?: LoggingSession // If provided, will be used for error logging - isResumeContext?: boolean // If true, allows fallback billing on resolution failure (for paused workflow resumes) + isResumeContext?: boolean // Deprecated: no billing fallback is allowed useAuthenticatedUserAsActor?: boolean // If true, use the authenticated userId as actorUserId (for client-side executions and personal API keys) /** @deprecated No longer used - background/async executions always use deployed state */ useDraftState?: boolean @@ -170,7 +82,7 @@ export async function preprocessExecution( skipUsageLimits = false, workspaceId: providedWorkspaceId, loggingSession: providedLoggingSession, - isResumeContext = false, + isResumeContext: _isResumeContext = false, useAuthenticatedUserAsActor = false, } = options @@ -274,68 +186,54 @@ export async function preprocessExecution( } if (!actorUserId) { - actorUserId = workflowRecord.userId || userId - logger.info(`[${requestId}] Using workflow owner as actor: ${actorUserId}`) - } - - if (!actorUserId) { - const result = await resolveBillingActorWithFallback({ - requestId, + const fallbackUserId = userId || workflowRecord.userId || 'unknown' + logger.warn(`[${requestId}] ${BILLING_ERROR_MESSAGES.BILLING_REQUIRED}`, { workflowId, workspaceId, + }) + + await logPreprocessingError({ + workflowId, executionId, triggerType, - workflowRecord, - userId, - isResumeContext, - baseActorUserId: actorUserId, - failureReason: 'null', + requestId, + userId: fallbackUserId, + workspaceId, + errorMessage: BILLING_ERROR_MESSAGES.BILLING_REQUIRED, loggingSession: providedLoggingSession, }) - if (result.shouldBlock) { - return { - success: false, - error: { - message: 'Unable to resolve billing account', - statusCode: 500, - logCreated: true, - }, - } + return { + success: false, + error: { + message: 'Unable to resolve billing account', + statusCode: 500, + logCreated: true, + }, } - - actorUserId = result.actorUserId } } catch (error) { logger.error(`[${requestId}] Error resolving billing actor`, { error, workflowId }) - - const result = await resolveBillingActorWithFallback({ - requestId, + const fallbackUserId = userId || workflowRecord.userId || 'unknown' + await logPreprocessingError({ workflowId, - workspaceId, executionId, triggerType, - workflowRecord, - userId, - isResumeContext, - baseActorUserId: null, - failureReason: 'error', - error, + requestId, + userId: fallbackUserId, + workspaceId, + errorMessage: BILLING_ERROR_MESSAGES.BILLING_ERROR_GENERIC, loggingSession: providedLoggingSession, }) - if (result.shouldBlock) { - return { - success: false, - error: { - message: 'Error resolving billing account', - statusCode: 500, - logCreated: true, - }, - } + return { + success: false, + error: { + message: 'Error resolving billing account', + statusCode: 500, + logCreated: true, + }, } - - actorUserId = result.actorUserId } // ========== STEP 4: Get User Subscription ========== diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts index 3c8fa42242..3cecd35852 100644 --- a/apps/sim/lib/logs/execution/logger.ts +++ b/apps/sim/lib/logs/execution/logger.ts @@ -33,7 +33,6 @@ import type { WorkflowExecutionSnapshot, WorkflowState, } from '@/lib/logs/types' -import { getWorkspaceBilledAccountUserId } from '@/lib/workspaces/utils' import type { SerializableExecutionState } from '@/executor/execution/types' export interface ToolCall { @@ -210,16 +209,15 @@ export class ExecutionLogger implements IExecutionLoggerService { logger.debug(`Completing workflow execution ${executionId}`, { isResume }) - // If this is a resume, fetch the existing log to merge data - let existingLog: any = null - if (isResume) { - const [existing] = await db - .select() - .from(workflowExecutionLogs) - .where(eq(workflowExecutionLogs.executionId, executionId)) - .limit(1) - existingLog = existing - } + const [existingLog] = await db + .select() + .from(workflowExecutionLogs) + .where(eq(workflowExecutionLogs.executionId, executionId)) + .limit(1) + const billingUserId = this.extractBillingUserId(existingLog?.executionData) + const existingExecutionData = existingLog?.executionData as + | { traceSpans?: TraceSpan[] } + | undefined // Determine if workflow failed by checking trace spans for errors // Use the override if provided (for cost-only fallback scenarios) @@ -244,7 +242,7 @@ export class ExecutionLogger implements IExecutionLoggerService { const mergedTraceSpans = isResume ? traceSpans && traceSpans.length > 0 ? traceSpans - : existingLog?.executionData?.traceSpans || [] + : existingExecutionData?.traceSpans || [] : traceSpans const filteredTraceSpans = filterForDisplay(mergedTraceSpans) @@ -329,7 +327,8 @@ export class ExecutionLogger implements IExecutionLoggerService { updatedLog.workflowId, costSummary, updatedLog.trigger as ExecutionTrigger['type'], - executionId + executionId, + billingUserId ) const limit = before.usageData.limit @@ -367,7 +366,8 @@ export class ExecutionLogger implements IExecutionLoggerService { updatedLog.workflowId, costSummary, updatedLog.trigger as ExecutionTrigger['type'], - executionId + executionId, + billingUserId ) const percentBefore = @@ -393,7 +393,8 @@ export class ExecutionLogger implements IExecutionLoggerService { updatedLog.workflowId, costSummary, updatedLog.trigger as ExecutionTrigger['type'], - executionId + executionId, + billingUserId ) } } else { @@ -401,7 +402,8 @@ export class ExecutionLogger implements IExecutionLoggerService { updatedLog.workflowId, costSummary, updatedLog.trigger as ExecutionTrigger['type'], - executionId + executionId, + billingUserId ) } } catch (e) { @@ -410,7 +412,8 @@ export class ExecutionLogger implements IExecutionLoggerService { updatedLog.workflowId, costSummary, updatedLog.trigger as ExecutionTrigger['type'], - executionId + executionId, + billingUserId ) } catch {} logger.warn('Usage threshold notification check failed (non-fatal)', { error: e }) @@ -472,6 +475,22 @@ export class ExecutionLogger implements IExecutionLoggerService { * Updates user stats with cost and token information * Maintains same logic as original execution logger for billing consistency */ + private extractBillingUserId(executionData: unknown): string | null { + if (!executionData || typeof executionData !== 'object') { + return null + } + + const environment = (executionData as { environment?: { userId?: unknown } }).environment + const userId = environment?.userId + + if (typeof userId !== 'string') { + return null + } + + const trimmedUserId = userId.trim() + return trimmedUserId.length > 0 ? trimmedUserId : null + } + private async updateUserStats( workflowId: string | null, costSummary: { @@ -494,7 +513,8 @@ export class ExecutionLogger implements IExecutionLoggerService { > }, trigger: ExecutionTrigger['type'], - executionId?: string + executionId?: string, + billingUserId?: string | null ): Promise { if (!isBillingEnabled) { logger.debug('Billing is disabled, skipping user stats cost update') @@ -512,7 +532,6 @@ export class ExecutionLogger implements IExecutionLoggerService { } try { - // Get the workflow record to get workspace and fallback userId const [workflowRecord] = await db .select() .from(workflow) @@ -524,12 +543,16 @@ export class ExecutionLogger implements IExecutionLoggerService { return } - let billingUserId: string | null = null - if (workflowRecord.workspaceId) { - billingUserId = await getWorkspaceBilledAccountUserId(workflowRecord.workspaceId) + const userId = billingUserId?.trim() || null + if (!userId) { + logger.error('Missing billing actor in execution context; skipping stats update', { + workflowId, + trigger, + executionId, + }) + return } - const userId = billingUserId || workflowRecord.userId const costToStore = costSummary.totalCost const existing = await db.select().from(userStats).where(eq(userStats.userId, userId)) diff --git a/apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts b/apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts index ee176ad9b6..5649cf40e6 100644 --- a/apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts +++ b/apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts @@ -739,7 +739,6 @@ export class PauseResumeManager { skipUsageLimits: true, // Resume is continuation of authorized execution - don't recheck limits workspaceId: baseSnapshot.metadata.workspaceId, loggingSession, - isResumeContext: true, // Enable billing fallback for paused workflow resumes }) if (!preprocessingResult.success) { From 908f7eac6b7e5fb7ac24ea9e7450816d90065a91 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 10 Feb 2026 11:58:08 -0800 Subject: [PATCH 3/8] remove owner reference in billing --- apps/sim/lib/execution/preprocessing.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/execution/preprocessing.ts b/apps/sim/lib/execution/preprocessing.ts index 5b0a65262e..3b6ee43fbe 100644 --- a/apps/sim/lib/execution/preprocessing.ts +++ b/apps/sim/lib/execution/preprocessing.ts @@ -186,7 +186,7 @@ export async function preprocessExecution( } if (!actorUserId) { - const fallbackUserId = userId || workflowRecord.userId || 'unknown' + const fallbackUserId = userId || 'unknown' logger.warn(`[${requestId}] ${BILLING_ERROR_MESSAGES.BILLING_REQUIRED}`, { workflowId, workspaceId, @@ -214,7 +214,7 @@ export async function preprocessExecution( } } catch (error) { logger.error(`[${requestId}] Error resolving billing actor`, { error, workflowId }) - const fallbackUserId = userId || workflowRecord.userId || 'unknown' + const fallbackUserId = userId || 'unknown' await logPreprocessingError({ workflowId, executionId, From b9171fa0e3bbec5be2a56b6a49e8b2a66c90b2b7 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 10 Feb 2026 14:56:56 -0800 Subject: [PATCH 4/8] fix auth --- .../sim/app/api/a2a/agents/[agentId]/route.ts | 7 +- apps/sim/app/api/a2a/agents/route.ts | 17 +- apps/sim/app/api/a2a/serve/[agentId]/route.ts | 119 ++++++-- apps/sim/app/api/a2a/serve/[agentId]/utils.ts | 3 +- .../app/api/auth/oauth/credentials/route.ts | 76 ++--- apps/sim/app/api/auth/oauth/utils.test.ts | 47 +-- apps/sim/app/api/auth/oauth/utils.ts | 38 +-- apps/sim/app/api/billing/portal/route.ts | 6 + apps/sim/app/api/chat/utils.ts | 43 ++- .../copilot/checkpoints/revert/route.test.ts | 13 + .../api/copilot/checkpoints/revert/route.ts | 8 +- .../app/api/cron/renew-subscriptions/route.ts | 27 +- apps/sim/app/api/files/upload/route.test.ts | 4 + apps/sim/app/api/files/upload/route.ts | 7 + apps/sim/app/api/form/utils.ts | 48 ++-- apps/sim/app/api/guardrails/validate/route.ts | 21 ++ .../documents/[documentId]/chunks/route.ts | 46 +-- .../[id]/documents/[documentId]/route.ts | 29 +- .../app/api/knowledge/[id]/documents/route.ts | 29 +- apps/sim/app/api/knowledge/[id]/route.ts | 35 +-- .../app/api/knowledge/search/route.test.ts | 36 ++- apps/sim/app/api/knowledge/search/route.ts | 25 +- apps/sim/app/api/mcp/copilot/route.ts | 16 +- .../api/mcp/serve/[serverId]/route.test.ts | 227 +++++++++++++++ .../sim/app/api/mcp/serve/[serverId]/route.ts | 66 ++++- .../[executionId]/[contextId]/route.ts | 23 +- apps/sim/app/api/schedules/[id]/route.test.ts | 47 ++- apps/sim/app/api/schedules/[id]/route.ts | 34 +-- apps/sim/app/api/schedules/route.test.ts | 50 ++-- apps/sim/app/api/schedules/route.ts | 32 +-- apps/sim/app/api/tools/custom/route.test.ts | 28 +- apps/sim/app/api/tools/custom/route.ts | 38 +-- apps/sim/app/api/usage/route.ts | 7 + apps/sim/app/api/wand/route.ts | 17 +- apps/sim/app/api/webhooks/[id]/route.ts | 101 +++---- apps/sim/app/api/webhooks/route.ts | 63 ++--- .../api/workflows/[id]/autolayout/route.ts | 30 +- .../workflows/[id]/chat/status/route.test.ts | 131 +++++++++ .../api/workflows/[id]/chat/status/route.ts | 22 +- .../app/api/workflows/[id]/duplicate/route.ts | 15 +- .../app/api/workflows/[id]/execute/route.ts | 17 +- .../executions/[executionId]/cancel/route.ts | 13 + .../workflows/[id]/form/status/route.test.ts | 119 ++++++++ .../api/workflows/[id]/form/status/route.ts | 19 +- apps/sim/app/api/workflows/[id]/log/route.ts | 9 +- apps/sim/app/api/workflows/[id]/route.test.ts | 141 +++++---- apps/sim/app/api/workflows/[id]/route.ts | 171 ++++------- .../sim/app/api/workflows/[id]/state/route.ts | 34 ++- .../workflows/[id]/variables/route.test.ts | 117 ++++---- .../app/api/workflows/[id]/variables/route.ts | 59 ++-- apps/sim/app/api/workflows/middleware.ts | 65 +++-- apps/sim/app/api/workflows/reorder/route.ts | 12 +- apps/sim/app/api/workflows/route.ts | 76 ++--- .../workspace-notification-delivery.ts | 11 +- apps/sim/lib/billing/core/usage-log.ts | 24 +- apps/sim/lib/copilot/auth/permissions.test.ts | 56 +--- apps/sim/lib/copilot/auth/permissions.ts | 54 ++-- .../orchestrator/tool-executor/access.ts | 51 ++-- .../server/workflow/edit-workflow/index.ts | 13 + .../server/workflow/get-workflow-console.ts | 15 +- apps/sim/lib/execution/preprocessing.ts | 13 + .../lib/guardrails/validate_hallucination.ts | 13 +- apps/sim/lib/knowledge/service.ts | 2 +- apps/sim/lib/logs/execution/logger.ts | 4 +- apps/sim/lib/webhooks/processor.ts | 12 +- .../lib/webhooks/provider-subscriptions.ts | 91 +++++- .../lib/workflows/executor/execution-core.ts | 7 +- .../lib/workflows/persistence/duplicate.ts | 41 ++- apps/sim/lib/workflows/utils.test.ts | 140 +-------- apps/sim/lib/workflows/utils.ts | 267 ++++++++++-------- apps/sim/socket/handlers/subblocks.ts | 39 +++ apps/sim/socket/handlers/variables.ts | 39 +++ apps/sim/socket/middleware/permissions.ts | 60 ++-- 73 files changed, 2084 insertions(+), 1351 deletions(-) create mode 100644 apps/sim/app/api/mcp/serve/[serverId]/route.test.ts create mode 100644 apps/sim/app/api/workflows/[id]/chat/status/route.test.ts create mode 100644 apps/sim/app/api/workflows/[id]/form/status/route.test.ts diff --git a/apps/sim/app/api/a2a/agents/[agentId]/route.ts b/apps/sim/app/api/a2a/agents/[agentId]/route.ts index 1c8eea273b..77f1fe149a 100644 --- a/apps/sim/app/api/a2a/agents/[agentId]/route.ts +++ b/apps/sim/app/api/a2a/agents/[agentId]/route.ts @@ -41,7 +41,12 @@ export async function GET(request: NextRequest, { params }: { params: Promise { if (!params?.message) { return NextResponse.json( @@ -318,6 +370,13 @@ async function handleMessageSend( ) } + if (existingTask.agentId !== agent.id) { + return NextResponse.json( + createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), + { status: 404 } + ) + } + if (isTerminalState(existingTask.status as TaskState)) { return NextResponse.json( createError(id, A2A_ERROR_CODES.TASK_ALREADY_COMPLETE, 'Task already in terminal state'), @@ -363,6 +422,7 @@ async function handleMessageSend( } = await buildExecuteRequest({ workflowId: agent.workflowId, apiKey, + userId: executionUserId, }) logger.info(`Executing workflow ${agent.workflowId} for A2A task ${taskId}`) @@ -475,7 +535,8 @@ async function handleMessageStream( workspaceId: string }, params: MessageSendParams, - apiKey?: string | null + apiKey?: string | null, + executionUserId?: string ): Promise { if (!params?.message) { return NextResponse.json( @@ -522,6 +583,13 @@ async function handleMessageStream( }) } + if (existingTask.agentId !== agent.id) { + await releaseLock(lockKey, lockValue) + return NextResponse.json(createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), { + status: 404, + }) + } + if (isTerminalState(existingTask.status as TaskState)) { await releaseLock(lockKey, lockValue) return NextResponse.json( @@ -595,6 +663,7 @@ async function handleMessageStream( } = await buildExecuteRequest({ workflowId: agent.workflowId, apiKey, + userId: executionUserId, stream: true, }) @@ -788,7 +857,11 @@ async function handleMessageStream( /** * Handle tasks/get - Query task status */ -async function handleTaskGet(id: string | number, params: TaskIdParams): Promise { +async function handleTaskGet( + id: string | number, + agentId: string, + params: TaskIdParams +): Promise { if (!params?.id) { return NextResponse.json( createError(id, A2A_ERROR_CODES.INVALID_PARAMS, 'Task ID is required'), @@ -801,7 +874,7 @@ async function handleTaskGet(id: string | number, params: TaskIdParams): Promise ? params.historyLength : undefined - const [task] = await db.select().from(a2aTask).where(eq(a2aTask.id, params.id)).limit(1) + const task = await getTaskForAgent(params.id, agentId) if (!task) { return NextResponse.json(createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), { @@ -825,7 +898,11 @@ async function handleTaskGet(id: string | number, params: TaskIdParams): Promise /** * Handle tasks/cancel - Cancel a running task */ -async function handleTaskCancel(id: string | number, params: TaskIdParams): Promise { +async function handleTaskCancel( + id: string | number, + agentId: string, + params: TaskIdParams +): Promise { if (!params?.id) { return NextResponse.json( createError(id, A2A_ERROR_CODES.INVALID_PARAMS, 'Task ID is required'), @@ -833,7 +910,7 @@ async function handleTaskCancel(id: string | number, params: TaskIdParams): Prom ) } - const [task] = await db.select().from(a2aTask).where(eq(a2aTask.id, params.id)).limit(1) + const task = await getTaskForAgent(params.id, agentId) if (!task) { return NextResponse.json(createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), { @@ -897,6 +974,7 @@ async function handleTaskCancel(id: string | number, params: TaskIdParams): Prom async function handleTaskResubscribe( request: NextRequest, id: string | number, + agentId: string, params: TaskIdParams ): Promise { if (!params?.id) { @@ -906,7 +984,7 @@ async function handleTaskResubscribe( ) } - const [task] = await db.select().from(a2aTask).where(eq(a2aTask.id, params.id)).limit(1) + const task = await getTaskForAgent(params.id, agentId) if (!task) { return NextResponse.json(createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), { @@ -1103,6 +1181,7 @@ async function handleTaskResubscribe( */ async function handlePushNotificationSet( id: string | number, + agentId: string, params: PushNotificationSetParams ): Promise { if (!params?.id) { @@ -1130,7 +1209,7 @@ async function handlePushNotificationSet( ) } - const [task] = await db.select().from(a2aTask).where(eq(a2aTask.id, params.id)).limit(1) + const task = await getTaskForAgent(params.id, agentId) if (!task) { return NextResponse.json(createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), { @@ -1181,6 +1260,7 @@ async function handlePushNotificationSet( */ async function handlePushNotificationGet( id: string | number, + agentId: string, params: TaskIdParams ): Promise { if (!params?.id) { @@ -1190,7 +1270,7 @@ async function handlePushNotificationGet( ) } - const [task] = await db.select().from(a2aTask).where(eq(a2aTask.id, params.id)).limit(1) + const task = await getTaskForAgent(params.id, agentId) if (!task) { return NextResponse.json(createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), { @@ -1224,6 +1304,7 @@ async function handlePushNotificationGet( */ async function handlePushNotificationDelete( id: string | number, + agentId: string, params: TaskIdParams ): Promise { if (!params?.id) { @@ -1233,7 +1314,7 @@ async function handlePushNotificationDelete( ) } - const [task] = await db.select().from(a2aTask).where(eq(a2aTask.id, params.id)).limit(1) + const task = await getTaskForAgent(params.id, agentId) if (!task) { return NextResponse.json(createError(id, A2A_ERROR_CODES.TASK_NOT_FOUND, 'Task not found'), { diff --git a/apps/sim/app/api/a2a/serve/[agentId]/utils.ts b/apps/sim/app/api/a2a/serve/[agentId]/utils.ts index f157d1efb3..1e8f855887 100644 --- a/apps/sim/app/api/a2a/serve/[agentId]/utils.ts +++ b/apps/sim/app/api/a2a/serve/[agentId]/utils.ts @@ -105,6 +105,7 @@ export function formatTaskResponse(task: Task, historyLength?: number): Task { export interface ExecuteRequestConfig { workflowId: string apiKey?: string | null + userId?: string stream?: boolean } @@ -124,7 +125,7 @@ export async function buildExecuteRequest( if (config.apiKey) { headers['X-API-Key'] = config.apiKey } else { - const internalToken = await generateInternalToken() + const internalToken = await generateInternalToken(config.userId) headers.Authorization = `Bearer ${internalToken}` useInternalAuth = true } diff --git a/apps/sim/app/api/auth/oauth/credentials/route.ts b/apps/sim/app/api/auth/oauth/credentials/route.ts index 24ca149d21..f0b69a8378 100644 --- a/apps/sim/app/api/auth/oauth/credentials/route.ts +++ b/apps/sim/app/api/auth/oauth/credentials/route.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { account, user, workflow } from '@sim/db/schema' +import { account, user } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import { jwtDecode } from 'jwt-decode' @@ -8,7 +8,7 @@ import { z } from 'zod' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { evaluateScopeCoverage, type OAuthProvider, parseProvider } from '@/lib/oauth' -import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' export const dynamic = 'force-dynamic' @@ -80,7 +80,7 @@ export async function GET(request: NextRequest) { const { provider: providerParam, workflowId, credentialId } = parseResult.data - // Authenticate requester (supports session, API key, internal JWT) + // Authenticate requester (supports session and internal JWT) const authResult = await checkSessionOrInternalAuth(request) if (!authResult.success || !authResult.userId) { logger.warn(`[${requestId}] Unauthenticated credentials request rejected`) @@ -88,47 +88,24 @@ export async function GET(request: NextRequest) { } const requesterUserId = authResult.userId - // Resolve effective user id: workflow owner if workflowId provided (with access check); else requester - let effectiveUserId: string + const effectiveUserId = requesterUserId if (workflowId) { - // Load workflow owner and workspace for access control - const rows = await db - .select({ userId: workflow.userId, workspaceId: workflow.workspaceId }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1) - - if (!rows.length) { - logger.warn(`[${requestId}] Workflow not found for credentials request`, { workflowId }) - return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) - } - - const wf = rows[0] - - if (requesterUserId !== wf.userId) { - if (!wf.workspaceId) { - logger.warn( - `[${requestId}] Forbidden - workflow has no workspace and requester is not owner`, - { - requesterUserId, - } - ) - return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) - } - - const perm = await getUserEntityPermissions(requesterUserId, 'workspace', wf.workspaceId) - if (perm === null) { - logger.warn(`[${requestId}] Forbidden credentials request - no workspace access`, { - requesterUserId, - workspaceId: wf.workspaceId, - }) - return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) - } + const workflowAuthorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId: requesterUserId, + action: 'read', + }) + if (!workflowAuthorization.allowed) { + logger.warn(`[${requestId}] Forbidden credentials request for workflow`, { + requesterUserId, + workflowId, + status: workflowAuthorization.status, + }) + return NextResponse.json( + { error: workflowAuthorization.message || 'Forbidden' }, + { status: workflowAuthorization.status } + ) } - - effectiveUserId = wf.userId - } else { - effectiveUserId = requesterUserId } // Parse the provider to get base provider and feature type (if provider is present) @@ -137,17 +114,10 @@ export async function GET(request: NextRequest) { let accountsData if (credentialId) { - // Foreign-aware lookup for a specific credential by id - // If workflowId is provided and requester has access (checked above), allow fetching by id only - if (workflowId) { - accountsData = await db.select().from(account).where(eq(account.id, credentialId)) - } else { - // Fallback: constrain to requester's own credentials when not in a workflow context - accountsData = await db - .select() - .from(account) - .where(and(eq(account.userId, effectiveUserId), eq(account.id, credentialId))) - } + accountsData = await db + .select() + .from(account) + .where(and(eq(account.userId, effectiveUserId), eq(account.id, credentialId))) } else { // Fetch all credentials for provider and effective user accountsData = await db diff --git a/apps/sim/app/api/auth/oauth/utils.test.ts b/apps/sim/app/api/auth/oauth/utils.test.ts index e144221a80..ca1d2c8ebd 100644 --- a/apps/sim/app/api/auth/oauth/utils.test.ts +++ b/apps/sim/app/api/auth/oauth/utils.test.ts @@ -4,16 +4,9 @@ * @vitest-environment node */ -import { createSession, loggerMock } from '@sim/testing' +import { loggerMock } from '@sim/testing' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const mockSession = createSession({ userId: 'test-user-id' }) -const mockGetSession = vi.fn() - -vi.mock('@/lib/auth', () => ({ - getSession: () => mockGetSession(), -})) - vi.mock('@sim/db', () => ({ db: { select: vi.fn().mockReturnThis(), @@ -37,7 +30,6 @@ import { db } from '@sim/db' import { refreshOAuthToken } from '@/lib/oauth' import { getCredential, - getUserId, refreshAccessTokenIfNeeded, refreshTokenIfNeeded, } from '@/app/api/auth/oauth/utils' @@ -48,7 +40,6 @@ const mockRefreshOAuthToken = refreshOAuthToken as any describe('OAuth Utils', () => { beforeEach(() => { vi.clearAllMocks() - mockGetSession.mockResolvedValue(mockSession) mockDbTyped.limit.mockReturnValue([]) }) @@ -56,42 +47,6 @@ describe('OAuth Utils', () => { vi.clearAllMocks() }) - describe('getUserId', () => { - it('should get user ID from session when no workflowId is provided', async () => { - const userId = await getUserId('request-id') - - expect(userId).toBe('test-user-id') - }) - - it('should get user ID from workflow when workflowId is provided', async () => { - mockDbTyped.limit.mockReturnValueOnce([{ userId: 'workflow-owner-id' }]) - - const userId = await getUserId('request-id', 'workflow-id') - - expect(mockDbTyped.select).toHaveBeenCalled() - expect(mockDbTyped.from).toHaveBeenCalled() - expect(mockDbTyped.where).toHaveBeenCalled() - expect(mockDbTyped.limit).toHaveBeenCalledWith(1) - expect(userId).toBe('workflow-owner-id') - }) - - it('should return undefined if no session is found', async () => { - mockGetSession.mockResolvedValueOnce(null) - - const userId = await getUserId('request-id') - - expect(userId).toBeUndefined() - }) - - it('should return undefined if workflow is not found', async () => { - mockDbTyped.limit.mockReturnValueOnce([]) - - const userId = await getUserId('request-id', 'nonexistent-workflow-id') - - expect(userId).toBeUndefined() - }) - }) - describe('getCredential', () => { it('should return credential when found', async () => { const mockCredential = { id: 'credential-id', userId: 'test-user-id' } diff --git a/apps/sim/app/api/auth/oauth/utils.ts b/apps/sim/app/api/auth/oauth/utils.ts index 9fe7d8510e..891e4ca4d1 100644 --- a/apps/sim/app/api/auth/oauth/utils.ts +++ b/apps/sim/app/api/auth/oauth/utils.ts @@ -1,8 +1,7 @@ import { db } from '@sim/db' -import { account, credentialSetMember, workflow } from '@sim/db/schema' +import { account, credentialSetMember } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, desc, eq, inArray } from 'drizzle-orm' -import { getSession } from '@/lib/auth' import { refreshOAuthToken } from '@/lib/oauth' import { getMicrosoftRefreshTokenExpiry, @@ -49,41 +48,6 @@ export async function safeAccountInsert( } } -/** - * Get the user ID based on either a session or a workflow ID - */ -export async function getUserId( - requestId: string, - workflowId?: string -): Promise { - // If workflowId is provided, this is a server-side request - if (workflowId) { - // Get the workflow to verify the user ID - const workflows = await db - .select({ userId: workflow.userId }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1) - - if (!workflows.length) { - logger.warn(`[${requestId}] Workflow not found`) - return undefined - } - - return workflows[0].userId - } - // This is a client-side request, use the session - const session = await getSession() - - // Check if the user is authenticated - if (!session?.user?.id) { - logger.warn(`[${requestId}] Unauthenticated request rejected`) - return undefined - } - - return session.user.id -} - /** * Get a credential by ID and verify it belongs to the user */ diff --git a/apps/sim/app/api/billing/portal/route.ts b/apps/sim/app/api/billing/portal/route.ts index eb1a860550..34807f0c6d 100644 --- a/apps/sim/app/api/billing/portal/route.ts +++ b/apps/sim/app/api/billing/portal/route.ts @@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger' import { and, eq, or } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { getSession } from '@/lib/auth' +import { isOrganizationOwnerOrAdmin } from '@/lib/billing/core/organization' import { requireStripeClient } from '@/lib/billing/stripe-client' import { getBaseUrl } from '@/lib/core/utils/urls' @@ -32,6 +33,11 @@ export async function POST(request: NextRequest) { return NextResponse.json({ error: 'organizationId is required' }, { status: 400 }) } + const hasPermission = await isOrganizationOwnerOrAdmin(session.user.id, organizationId) + if (!hasPermission) { + return NextResponse.json({ error: 'Permission denied' }, { status: 403 }) + } + const rows = await db .select({ customer: subscriptionTable.stripeCustomerId }) .from(subscriptionTable) diff --git a/apps/sim/app/api/chat/utils.ts b/apps/sim/app/api/chat/utils.ts index 654c36c8ba..e6c5a55211 100644 --- a/apps/sim/app/api/chat/utils.ts +++ b/apps/sim/app/api/chat/utils.ts @@ -9,7 +9,7 @@ import { validateAuthToken, } from '@/lib/core/security/deployment' import { decryptSecret } from '@/lib/core/security/encryption' -import { hasAdminPermission } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' const logger = createLogger('ChatAuthUtils') @@ -24,29 +24,23 @@ export function setChatAuthCookie( /** * Check if user has permission to create a chat for a specific workflow - * Either the user owns the workflow directly OR has admin permission for the workflow's workspace */ export async function checkWorkflowAccessForChatCreation( workflowId: string, userId: string ): Promise<{ hasAccess: boolean; workflow?: any }> { - const workflowData = await db.select().from(workflow).where(eq(workflow.id, workflowId)).limit(1) + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'admin', + }) - if (workflowData.length === 0) { + if (!authorization.workflow) { return { hasAccess: false } } - const workflowRecord = workflowData[0] - - if (workflowRecord.userId === userId) { - return { hasAccess: true, workflow: workflowRecord } - } - - if (workflowRecord.workspaceId) { - const hasAdmin = await hasAdminPermission(userId, workflowRecord.workspaceId) - if (hasAdmin) { - return { hasAccess: true, workflow: workflowRecord } - } + if (authorization.allowed) { + return { hasAccess: true, workflow: authorization.workflow } } return { hasAccess: false } @@ -54,7 +48,6 @@ export async function checkWorkflowAccessForChatCreation( /** * Check if user has access to view/edit/delete a specific chat - * Either the user owns the chat directly OR has admin permission for the workflow's workspace */ export async function checkChatAccess( chatId: string, @@ -75,19 +68,17 @@ export async function checkChatAccess( } const { chat: chatRecord, workflowWorkspaceId } = chatData[0] - - if (chatRecord.userId === userId) { - return { hasAccess: true, chat: chatRecord } + if (!workflowWorkspaceId) { + return { hasAccess: false } } - if (workflowWorkspaceId) { - const hasAdmin = await hasAdminPermission(userId, workflowWorkspaceId) - if (hasAdmin) { - return { hasAccess: true, chat: chatRecord } - } - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: chatRecord.workflowId, + userId, + action: 'admin', + }) - return { hasAccess: false } + return authorization.allowed ? { hasAccess: true, chat: chatRecord } : { hasAccess: false } } export async function validateChatAuth( diff --git a/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts b/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts index cd5c46d9e1..7193af66d3 100644 --- a/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts +++ b/apps/sim/app/api/copilot/checkpoints/revert/route.test.ts @@ -25,6 +25,13 @@ describe('Copilot Checkpoints Revert API Route', () => { getEmailDomain: vi.fn(() => 'localhost:3000'), })) + vi.doMock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: vi.fn().mockResolvedValue({ + allowed: true, + status: 200, + }), + })) + mockSelect.mockReturnValue({ from: mockFrom }) mockFrom.mockReturnValue({ where: mockWhere }) mockWhere.mockReturnValue({ then: mockThen }) @@ -212,6 +219,12 @@ describe('Copilot Checkpoints Revert API Route', () => { .mockResolvedValueOnce(mockCheckpoint) // Checkpoint found .mockResolvedValueOnce(mockWorkflow) // Workflow found but different user + const { authorizeWorkflowByWorkspacePermission } = await import('@/lib/workflows/utils') + vi.mocked(authorizeWorkflowByWorkspacePermission).mockResolvedValueOnce({ + allowed: false, + status: 403, + }) + const req = createMockRequest('POST', { checkpointId: 'checkpoint-123', }) diff --git a/apps/sim/app/api/copilot/checkpoints/revert/route.ts b/apps/sim/app/api/copilot/checkpoints/revert/route.ts index 7f65e0317e..72c79a262e 100644 --- a/apps/sim/app/api/copilot/checkpoints/revert/route.ts +++ b/apps/sim/app/api/copilot/checkpoints/revert/route.ts @@ -12,6 +12,7 @@ import { createUnauthorizedResponse, } from '@/lib/copilot/request-helpers' import { getBaseUrl } from '@/lib/core/utils/urls' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { isUuidV4 } from '@/executor/constants' const logger = createLogger('CheckpointRevertAPI') @@ -58,7 +59,12 @@ export async function POST(request: NextRequest) { return createNotFoundResponse('Workflow not found') } - if (workflowData.userId !== userId) { + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: checkpoint.workflowId, + userId, + action: 'write', + }) + if (!authorization.allowed) { return createUnauthorizedResponse() } diff --git a/apps/sim/app/api/cron/renew-subscriptions/route.ts b/apps/sim/app/api/cron/renew-subscriptions/route.ts index b60afc84cc..57def36986 100644 --- a/apps/sim/app/api/cron/renew-subscriptions/route.ts +++ b/apps/sim/app/api/cron/renew-subscriptions/route.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { webhook as webhookTable, workflow as workflowTable } from '@sim/db/schema' +import { account, webhook as webhookTable } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, or } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' @@ -8,6 +8,16 @@ import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils' const logger = createLogger('TeamsSubscriptionRenewal') +async function getCredentialOwnerUserId(credentialId: string): Promise { + const [credentialRecord] = await db + .select({ userId: account.userId }) + .from(account) + .where(eq(account.id, credentialId)) + .limit(1) + + return credentialRecord?.userId ?? null +} + /** * Cron endpoint to renew Microsoft Teams chat subscriptions before they expire * @@ -27,14 +37,12 @@ export async function GET(request: NextRequest) { let totalFailed = 0 let totalChecked = 0 - // Get all active Microsoft Teams webhooks with their workflows + // Get all active Microsoft Teams webhooks const webhooksWithWorkflows = await db .select({ webhook: webhookTable, - workflow: workflowTable, }) .from(webhookTable) - .innerJoin(workflowTable, eq(webhookTable.workflowId, workflowTable.id)) .where( and( eq(webhookTable.isActive, true), @@ -52,7 +60,7 @@ export async function GET(request: NextRequest) { // Renewal threshold: 48 hours before expiration const renewalThreshold = new Date(Date.now() + 48 * 60 * 60 * 1000) - for (const { webhook, workflow } of webhooksWithWorkflows) { + for (const { webhook } of webhooksWithWorkflows) { const config = (webhook.providerConfig as Record) || {} // Check if this is a Teams chat subscription that needs renewal @@ -80,10 +88,17 @@ export async function GET(request: NextRequest) { continue } + const credentialOwnerUserId = await getCredentialOwnerUserId(credentialId) + if (!credentialOwnerUserId) { + logger.error(`Credential owner not found for credential ${credentialId}`) + totalFailed++ + continue + } + // Get fresh access token const accessToken = await refreshAccessTokenIfNeeded( credentialId, - workflow.userId, + credentialOwnerUserId, `renewal-${webhook.id}` ) diff --git a/apps/sim/app/api/files/upload/route.test.ts b/apps/sim/app/api/files/upload/route.test.ts index a5ecc030b8..25c8f68adf 100644 --- a/apps/sim/app/api/files/upload/route.test.ts +++ b/apps/sim/app/api/files/upload/route.test.ts @@ -42,6 +42,10 @@ function setupFileApiMocks( verifyCopilotFileAccess: vi.fn().mockResolvedValue(true), })) + vi.doMock('@/lib/workspaces/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue('admin'), + })) + vi.doMock('@/lib/uploads/contexts/workspace', () => ({ uploadWorkspaceFile: vi.fn().mockResolvedValue({ id: 'test-file-id', diff --git a/apps/sim/app/api/files/upload/route.ts b/apps/sim/app/api/files/upload/route.ts index eca3667926..f227dd7420 100644 --- a/apps/sim/app/api/files/upload/route.ts +++ b/apps/sim/app/api/files/upload/route.ts @@ -206,6 +206,13 @@ export async function POST(request: NextRequest) { if (!workspaceId) { throw new InvalidRequestError('Workspace context requires workspaceId parameter') } + const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) + if (permission !== 'admin' && permission !== 'write') { + return NextResponse.json( + { error: 'Write or Admin access required for workspace uploads' }, + { status: 403 } + ) + } try { const { uploadWorkspaceFile } = await import('@/lib/uploads/contexts/workspace') diff --git a/apps/sim/app/api/form/utils.ts b/apps/sim/app/api/form/utils.ts index 34255df600..a5e89b5a01 100644 --- a/apps/sim/app/api/form/utils.ts +++ b/apps/sim/app/api/form/utils.ts @@ -9,7 +9,7 @@ import { validateAuthToken, } from '@/lib/core/security/deployment' import { decryptSecret } from '@/lib/core/security/encryption' -import { hasAdminPermission } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' const logger = createLogger('FormAuthUtils') @@ -24,29 +24,23 @@ export function setFormAuthCookie( /** * Check if user has permission to create a form for a specific workflow - * Either the user owns the workflow directly OR has admin permission for the workflow's workspace */ export async function checkWorkflowAccessForFormCreation( workflowId: string, userId: string ): Promise<{ hasAccess: boolean; workflow?: any }> { - const workflowData = await db.select().from(workflow).where(eq(workflow.id, workflowId)).limit(1) + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'admin', + }) - if (workflowData.length === 0) { + if (!authorization.workflow) { return { hasAccess: false } } - const workflowRecord = workflowData[0] - - if (workflowRecord.userId === userId) { - return { hasAccess: true, workflow: workflowRecord } - } - - if (workflowRecord.workspaceId) { - const hasAdmin = await hasAdminPermission(userId, workflowRecord.workspaceId) - if (hasAdmin) { - return { hasAccess: true, workflow: workflowRecord } - } + if (authorization.allowed) { + return { hasAccess: true, workflow: authorization.workflow } } return { hasAccess: false } @@ -54,17 +48,13 @@ export async function checkWorkflowAccessForFormCreation( /** * Check if user has access to view/edit/delete a specific form - * Either the user owns the form directly OR has admin permission for the workflow's workspace */ export async function checkFormAccess( formId: string, userId: string ): Promise<{ hasAccess: boolean; form?: any }> { const formData = await db - .select({ - form: form, - workflowWorkspaceId: workflow.workspaceId, - }) + .select({ form: form, workflowWorkspaceId: workflow.workspaceId }) .from(form) .innerJoin(workflow, eq(form.workflowId, workflow.id)) .where(eq(form.id, formId)) @@ -75,19 +65,17 @@ export async function checkFormAccess( } const { form: formRecord, workflowWorkspaceId } = formData[0] - - if (formRecord.userId === userId) { - return { hasAccess: true, form: formRecord } + if (!workflowWorkspaceId) { + return { hasAccess: false } } - if (workflowWorkspaceId) { - const hasAdmin = await hasAdminPermission(userId, workflowWorkspaceId) - if (hasAdmin) { - return { hasAccess: true, form: formRecord } - } - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: formRecord.workflowId, + userId, + action: 'admin', + }) - return { hasAccess: false } + return authorization.allowed ? { hasAccess: true, form: formRecord } : { hasAccess: false } } export async function validateFormAuth( diff --git a/apps/sim/app/api/guardrails/validate/route.ts b/apps/sim/app/api/guardrails/validate/route.ts index 6e1b65750c..9be7e33f0a 100644 --- a/apps/sim/app/api/guardrails/validate/route.ts +++ b/apps/sim/app/api/guardrails/validate/route.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { validateHallucination } from '@/lib/guardrails/validate_hallucination' import { validateJson } from '@/lib/guardrails/validate_json' @@ -13,6 +14,19 @@ export async function POST(request: NextRequest) { logger.info(`[${requestId}] Guardrails validation request received`) try { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { + return NextResponse.json({ + success: true, + output: { + passed: false, + validationType: 'unknown', + input: '', + error: 'Unauthorized', + }, + }) + } + const body = await request.json() const { validationType, @@ -109,6 +123,10 @@ export async function POST(request: NextRequest) { validationType, inputType: typeof input, }) + const authHeaders = { + cookie: request.headers.get('cookie') || undefined, + authorization: request.headers.get('authorization') || undefined, + } const validationResult = await executeValidation( validationType, @@ -134,6 +152,7 @@ export async function POST(request: NextRequest) { piiEntityTypes, piiMode, piiLanguage, + authHeaders, requestId ) @@ -213,6 +232,7 @@ async function executeValidation( piiEntityTypes: string[] | undefined, piiMode: string | undefined, piiLanguage: string | undefined, + authHeaders: { cookie?: string; authorization?: string } | undefined, requestId: string ): Promise<{ passed: boolean @@ -253,6 +273,7 @@ async function executeValidation( providerCredentials, workflowId, workspaceId, + authHeaders, requestId, }) } diff --git a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts index c5d8590097..c7979d41b0 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/chunks/route.ts @@ -1,10 +1,10 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { batchChunkOperation, createChunk, queryChunks } from '@/lib/knowledge/chunks/service' -import { getUserId } from '@/app/api/auth/oauth/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { checkDocumentAccess, checkDocumentWriteAccess } from '@/app/api/knowledge/utils' import { calculateCost } from '@/providers/utils' @@ -38,13 +38,14 @@ export async function GET( const { id: knowledgeBaseId, documentId } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized chunks access attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkDocumentAccess(knowledgeBaseId, documentId, session.user.id) + const accessCheck = await checkDocumentAccess(knowledgeBaseId, documentId, userId) if (!accessCheck.hasAccess) { if (accessCheck.notFound) { @@ -54,7 +55,7 @@ export async function GET( return NextResponse.json({ error: accessCheck.reason }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted unauthorized chunks access: ${accessCheck.reason}` + `[${requestId}] User ${userId} attempted unauthorized chunks access: ${accessCheck.reason}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } @@ -113,13 +114,25 @@ export async function POST( const body = await req.json() const { workflowId, ...searchParams } = body - const userId = await getUserId(requestId, workflowId) + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { + logger.warn(`[${requestId}] Authentication failed: ${auth.error || 'Unauthorized'}`) + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } + const userId = auth.userId - if (!userId) { - const errorMessage = workflowId ? 'Workflow not found' : 'Unauthorized' - const statusCode = workflowId ? 404 : 401 - logger.warn(`[${requestId}] Authentication failed: ${errorMessage}`) - return NextResponse.json({ error: errorMessage }, { status: statusCode }) + if (workflowId) { + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'write', + }) + if (!authorization.allowed) { + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status } + ) + } } const accessCheck = await checkDocumentWriteAccess(knowledgeBaseId, documentId, userId) @@ -248,13 +261,14 @@ export async function PATCH( const { id: knowledgeBaseId, documentId } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized batch chunk operation attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkDocumentAccess(knowledgeBaseId, documentId, session.user.id) + const accessCheck = await checkDocumentAccess(knowledgeBaseId, documentId, userId) if (!accessCheck.hasAccess) { if (accessCheck.notFound) { @@ -264,7 +278,7 @@ export async function PATCH( return NextResponse.json({ error: accessCheck.reason }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted unauthorized batch chunk operation: ${accessCheck.reason}` + `[${requestId}] User ${userId} attempted unauthorized batch chunk operation: ${accessCheck.reason}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } diff --git a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/route.ts b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/route.ts index 9d3ad15219..fb7add9e3e 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/[documentId]/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/[documentId]/route.ts @@ -1,7 +1,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { deleteDocument, @@ -54,13 +54,14 @@ export async function GET( const { id: knowledgeBaseId, documentId } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized document access attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkDocumentAccess(knowledgeBaseId, documentId, session.user.id) + const accessCheck = await checkDocumentAccess(knowledgeBaseId, documentId, userId) if (!accessCheck.hasAccess) { if (accessCheck.notFound) { @@ -70,7 +71,7 @@ export async function GET( return NextResponse.json({ error: accessCheck.reason }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted unauthorized document access: ${accessCheck.reason}` + `[${requestId}] User ${userId} attempted unauthorized document access: ${accessCheck.reason}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } @@ -97,13 +98,14 @@ export async function PUT( const { id: knowledgeBaseId, documentId } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized document update attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkDocumentWriteAccess(knowledgeBaseId, documentId, session.user.id) + const accessCheck = await checkDocumentWriteAccess(knowledgeBaseId, documentId, userId) if (!accessCheck.hasAccess) { if (accessCheck.notFound) { @@ -113,7 +115,7 @@ export async function PUT( return NextResponse.json({ error: accessCheck.reason }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted unauthorized document update: ${accessCheck.reason}` + `[${requestId}] User ${userId} attempted unauthorized document update: ${accessCheck.reason}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } @@ -227,13 +229,14 @@ export async function DELETE( const { id: knowledgeBaseId, documentId } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized document delete attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkDocumentWriteAccess(knowledgeBaseId, documentId, session.user.id) + const accessCheck = await checkDocumentWriteAccess(knowledgeBaseId, documentId, userId) if (!accessCheck.hasAccess) { if (accessCheck.notFound) { @@ -243,7 +246,7 @@ export async function DELETE( return NextResponse.json({ error: accessCheck.reason }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted unauthorized document deletion: ${accessCheck.reason}` + `[${requestId}] User ${userId} attempted unauthorized document deletion: ${accessCheck.reason}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } diff --git a/apps/sim/app/api/knowledge/[id]/documents/route.ts b/apps/sim/app/api/knowledge/[id]/documents/route.ts index ae9c135449..30c1beafa1 100644 --- a/apps/sim/app/api/knowledge/[id]/documents/route.ts +++ b/apps/sim/app/api/knowledge/[id]/documents/route.ts @@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { bulkDocumentOperation, bulkDocumentOperationByFilter, @@ -13,7 +14,7 @@ import { processDocumentsWithQueue, } from '@/lib/knowledge/documents/service' import type { DocumentSortField, SortOrder } from '@/lib/knowledge/documents/types' -import { getUserId } from '@/app/api/auth/oauth/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { checkKnowledgeBaseAccess, checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils' const logger = createLogger('DocumentsAPI') @@ -170,16 +171,28 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: bodyKeys: Object.keys(body), }) - const userId = await getUserId(requestId, workflowId) - - if (!userId) { - const errorMessage = workflowId ? 'Workflow not found' : 'Unauthorized' - const statusCode = workflowId ? 404 : 401 - logger.warn(`[${requestId}] Authentication failed: ${errorMessage}`, { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { + logger.warn(`[${requestId}] Authentication failed: ${auth.error || 'Unauthorized'}`, { workflowId, hasWorkflowId: !!workflowId, }) - return NextResponse.json({ error: errorMessage }, { status: statusCode }) + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } + const userId = auth.userId + + if (workflowId) { + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'write', + }) + if (!authorization.allowed) { + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status } + ) + } } const accessCheck = await checkKnowledgeBaseWriteAccess(knowledgeBaseId, userId) diff --git a/apps/sim/app/api/knowledge/[id]/route.ts b/apps/sim/app/api/knowledge/[id]/route.ts index 778dd75905..70da17626f 100644 --- a/apps/sim/app/api/knowledge/[id]/route.ts +++ b/apps/sim/app/api/knowledge/[id]/route.ts @@ -1,7 +1,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { @@ -54,13 +54,14 @@ export async function GET(_request: NextRequest, { params }: { params: Promise<{ const { id } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(_request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized knowledge base access attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkKnowledgeBaseAccess(id, session.user.id) + const accessCheck = await checkKnowledgeBaseAccess(id, userId) if (!accessCheck.hasAccess) { if ('notFound' in accessCheck && accessCheck.notFound) { @@ -68,7 +69,7 @@ export async function GET(_request: NextRequest, { params }: { params: Promise<{ return NextResponse.json({ error: 'Knowledge base not found' }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted to access unauthorized knowledge base ${id}` + `[${requestId}] User ${userId} attempted to access unauthorized knowledge base ${id}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } @@ -79,7 +80,7 @@ export async function GET(_request: NextRequest, { params }: { params: Promise<{ return NextResponse.json({ error: 'Knowledge base not found' }, { status: 404 }) } - logger.info(`[${requestId}] Retrieved knowledge base: ${id} for user ${session.user.id}`) + logger.info(`[${requestId}] Retrieved knowledge base: ${id} for user ${userId}`) return NextResponse.json({ success: true, @@ -96,13 +97,14 @@ export async function PUT(req: NextRequest, { params }: { params: Promise<{ id: const { id } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized knowledge base update attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkKnowledgeBaseWriteAccess(id, session.user.id) + const accessCheck = await checkKnowledgeBaseWriteAccess(id, userId) if (!accessCheck.hasAccess) { if ('notFound' in accessCheck && accessCheck.notFound) { @@ -110,7 +112,7 @@ export async function PUT(req: NextRequest, { params }: { params: Promise<{ id: return NextResponse.json({ error: 'Knowledge base not found' }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted to update unauthorized knowledge base ${id}` + `[${requestId}] User ${userId} attempted to update unauthorized knowledge base ${id}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } @@ -131,7 +133,7 @@ export async function PUT(req: NextRequest, { params }: { params: Promise<{ id: requestId ) - logger.info(`[${requestId}] Knowledge base updated: ${id} for user ${session.user.id}`) + logger.info(`[${requestId}] Knowledge base updated: ${id} for user ${userId}`) return NextResponse.json({ success: true, @@ -163,13 +165,14 @@ export async function DELETE( const { id } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(_request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized knowledge base delete attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - const accessCheck = await checkKnowledgeBaseWriteAccess(id, session.user.id) + const accessCheck = await checkKnowledgeBaseWriteAccess(id, userId) if (!accessCheck.hasAccess) { if ('notFound' in accessCheck && accessCheck.notFound) { @@ -177,7 +180,7 @@ export async function DELETE( return NextResponse.json({ error: 'Knowledge base not found' }, { status: 404 }) } logger.warn( - `[${requestId}] User ${session.user.id} attempted to delete unauthorized knowledge base ${id}` + `[${requestId}] User ${userId} attempted to delete unauthorized knowledge base ${id}` ) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } @@ -192,7 +195,7 @@ export async function DELETE( // Telemetry should not fail the operation } - logger.info(`[${requestId}] Knowledge base deleted: ${id} for user ${session.user.id}`) + logger.info(`[${requestId}] Knowledge base deleted: ${id} for user ${userId}`) return NextResponse.json({ success: true, diff --git a/apps/sim/app/api/knowledge/search/route.test.ts b/apps/sim/app/api/knowledge/search/route.test.ts index d5748b1063..b08ab969b4 100644 --- a/apps/sim/app/api/knowledge/search/route.test.ts +++ b/apps/sim/app/api/knowledge/search/route.test.ts @@ -104,6 +104,8 @@ describe('Knowledge Search API Route', () => { const mockGetUserId = vi.fn() const mockFetch = vi.fn() + const mockCheckSessionOrInternalAuth = vi.fn() + const mockAuthorizeWorkflowByWorkspacePermission = vi.fn() const mockEmbedding = [0.1, 0.2, 0.3, 0.4, 0.5] const mockSearchResults = [ @@ -132,8 +134,12 @@ describe('Knowledge Search API Route', () => { db: mockDbChain, })) - vi.doMock('@/app/api/auth/oauth/utils', () => ({ - getUserId: mockGetUserId, + vi.doMock('@/lib/auth/hybrid', () => ({ + checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth, + })) + + vi.doMock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: mockAuthorizeWorkflowByWorkspacePermission, })) Object.values(mockDbChain).forEach((fn) => { @@ -157,6 +163,15 @@ describe('Knowledge Search API Route', () => { doc2: 'Document 2', }) mockGetDocumentTagDefinitions.mockClear() + mockCheckSessionOrInternalAuth.mockClear().mockResolvedValue({ + success: true, + userId: 'user-123', + authType: 'session', + }) + mockAuthorizeWorkflowByWorkspacePermission.mockClear().mockResolvedValue({ + allowed: true, + status: 200, + }) vi.stubGlobal('crypto', { randomUUID: vi.fn().mockReturnValue('mock-uuid-1234-5678'), @@ -311,11 +326,18 @@ describe('Knowledge Search API Route', () => { expect(response.status).toBe(200) expect(data.success).toBe(true) - expect(mockGetUserId).toHaveBeenCalledWith(expect.any(String), 'workflow-123') + expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: 'workflow-123', + userId: 'user-123', + action: 'read', + }) }) it.concurrent('should return unauthorized for unauthenticated request', async () => { - mockGetUserId.mockResolvedValue(null) + mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ + success: false, + error: 'Unauthorized', + }) const req = createMockRequest('POST', validSearchData) const { POST } = await import('@/app/api/knowledge/search/route') @@ -332,7 +354,11 @@ describe('Knowledge Search API Route', () => { workflowId: 'nonexistent-workflow', } - mockGetUserId.mockResolvedValue(null) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: false, + status: 404, + message: 'Workflow not found', + }) const req = createMockRequest('POST', workflowData) const { POST } = await import('@/app/api/knowledge/search/route') diff --git a/apps/sim/app/api/knowledge/search/route.ts b/apps/sim/app/api/knowledge/search/route.ts index fdfce836b5..c3e6993140 100644 --- a/apps/sim/app/api/knowledge/search/route.ts +++ b/apps/sim/app/api/knowledge/search/route.ts @@ -1,6 +1,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { ALL_TAG_SLOTS } from '@/lib/knowledge/constants' @@ -8,7 +9,7 @@ import { getDocumentTagDefinitions } from '@/lib/knowledge/tags/service' import { buildUndefinedTagsError, validateTagValue } from '@/lib/knowledge/tags/utils' import type { StructuredFilter } from '@/lib/knowledge/types' import { estimateTokenCount } from '@/lib/tokenization/estimators' -import { getUserId } from '@/app/api/auth/oauth/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { generateSearchEmbedding, getDocumentNamesByIds, @@ -76,12 +77,24 @@ export async function POST(request: NextRequest) { const body = await request.json() const { workflowId, ...searchParams } = body - const userId = await getUserId(requestId, workflowId) + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } + const userId = auth.userId - if (!userId) { - const errorMessage = workflowId ? 'Workflow not found' : 'Unauthorized' - const statusCode = workflowId ? 404 : 401 - return NextResponse.json({ error: errorMessage }, { status: statusCode }) + if (workflowId) { + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'read', + }) + if (!authorization.allowed) { + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status } + ) + } } try { diff --git a/apps/sim/app/api/mcp/copilot/route.ts b/apps/sim/app/api/mcp/copilot/route.ts index 4d02ab122f..b9af602841 100644 --- a/apps/sim/app/api/mcp/copilot/route.ts +++ b/apps/sim/app/api/mcp/copilot/route.ts @@ -32,7 +32,10 @@ import { import { DIRECT_TOOL_DEFS, SUBAGENT_TOOL_DEFS } from '@/lib/copilot/tools/mcp/definitions' import { env } from '@/lib/core/config/env' import { RateLimiter } from '@/lib/core/rate-limiter' -import { resolveWorkflowIdForUser } from '@/lib/workflows/utils' +import { + authorizeWorkflowByWorkspacePermission, + resolveWorkflowIdForUser, +} from '@/lib/workflows/utils' const logger = createLogger('CopilotMcpAPI') const mcpRateLimiter = new RateLimiter() @@ -627,7 +630,16 @@ async function handleBuildToolCall( const { model } = getCopilotModel('chat') const workflowId = args.workflowId as string | undefined - const resolved = workflowId ? { workflowId } : await resolveWorkflowIdForUser(userId) + const resolved = workflowId + ? await (async () => { + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'read', + }) + return authorization.allowed ? { workflowId } : null + })() + : await resolveWorkflowIdForUser(userId) if (!resolved?.workflowId) { return { diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts new file mode 100644 index 0000000000..976678313e --- /dev/null +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.test.ts @@ -0,0 +1,227 @@ +/** + * Tests for MCP serve route auth propagation. + * + * @vitest-environment node + */ +import { NextRequest } from 'next/server' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mockCheckHybridAuth = vi.fn() +const mockGetUserEntityPermissions = vi.fn() +const mockGenerateInternalToken = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbWhere = vi.fn() +const mockDbLimit = vi.fn() +const fetchMock = vi.fn() + +describe('MCP Serve Route', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit }) + + vi.doMock('@sim/logger', () => ({ + createLogger: vi.fn(() => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + })), + })) + vi.doMock('drizzle-orm', () => ({ + and: vi.fn(), + eq: vi.fn(), + })) + vi.doMock('@sim/db', () => ({ + db: { + select: mockDbSelect, + }, + })) + vi.doMock('@sim/db/schema', () => ({ + workflowMcpServer: { + id: 'id', + name: 'name', + workspaceId: 'workspaceId', + isPublic: 'isPublic', + createdBy: 'createdBy', + }, + workflowMcpTool: { + serverId: 'serverId', + toolName: 'toolName', + toolDescription: 'toolDescription', + parameterSchema: 'parameterSchema', + workflowId: 'workflowId', + }, + workflow: { + id: 'id', + isDeployed: 'isDeployed', + }, + })) + vi.doMock('@/lib/auth/hybrid', () => ({ + checkHybridAuth: mockCheckHybridAuth, + })) + vi.doMock('@/lib/workspaces/permissions/utils', () => ({ + getUserEntityPermissions: mockGetUserEntityPermissions, + })) + vi.doMock('@/lib/auth/internal', () => ({ + generateInternalToken: mockGenerateInternalToken, + })) + vi.doMock('@/lib/core/utils/urls', () => ({ + getBaseUrl: () => 'http://localhost:3000', + })) + vi.doMock('@/lib/core/execution-limits', () => ({ + getMaxExecutionTimeout: () => 10_000, + })) + + vi.stubGlobal('fetch', fetchMock) + }) + + afterEach(() => { + vi.unstubAllGlobals() + vi.clearAllMocks() + }) + + it('returns 401 for private server when auth fails', async () => { + mockDbLimit.mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Private Server', + workspaceId: 'ws-1', + isPublic: false, + createdBy: 'owner-1', + }, + ]) + mockCheckHybridAuth.mockResolvedValueOnce({ success: false, error: 'Unauthorized' }) + + const { POST } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'ping' }), + }) + const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) }) + + expect(response.status).toBe(401) + }) + + it('returns 401 on GET for private server when auth fails', async () => { + mockDbLimit.mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Private Server', + workspaceId: 'ws-1', + isPublic: false, + createdBy: 'owner-1', + }, + ]) + mockCheckHybridAuth.mockResolvedValueOnce({ success: false, error: 'Unauthorized' }) + + const { GET } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1') + const response = await GET(req, { params: Promise.resolve({ serverId: 'server-1' }) }) + + expect(response.status).toBe(401) + }) + + it('forwards X-API-Key for private server api_key auth', async () => { + mockDbLimit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Private Server', + workspaceId: 'ws-1', + isPublic: false, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ isDeployed: true }]) + + mockCheckHybridAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'api_key', + apiKeyType: 'personal', + }) + mockGetUserEntityPermissions.mockResolvedValueOnce('write') + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ output: { ok: true } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ) + + const { POST } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + headers: { 'X-API-Key': 'pk_test_123' }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'tool_a', arguments: { q: 'test' } }, + }), + }) + const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) }) + + expect(response.status).toBe(200) + expect(fetchMock).toHaveBeenCalledTimes(1) + const fetchOptions = fetchMock.mock.calls[0][1] as RequestInit + const headers = fetchOptions.headers as Record + expect(headers['X-API-Key']).toBe('pk_test_123') + expect(headers.Authorization).toBeUndefined() + expect(mockGenerateInternalToken).not.toHaveBeenCalled() + }) + + it('forwards internal token for private server session auth', async () => { + mockDbLimit + .mockResolvedValueOnce([ + { + id: 'server-1', + name: 'Private Server', + workspaceId: 'ws-1', + isPublic: false, + createdBy: 'owner-1', + }, + ]) + .mockResolvedValueOnce([{ toolName: 'tool_a', workflowId: 'wf-1' }]) + .mockResolvedValueOnce([{ isDeployed: true }]) + + mockCheckHybridAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'session', + }) + mockGetUserEntityPermissions.mockResolvedValueOnce('read') + mockGenerateInternalToken.mockResolvedValueOnce('internal-token-user-1') + fetchMock.mockResolvedValueOnce( + new Response(JSON.stringify({ output: { ok: true } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ) + + const { POST } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/mcp/serve/server-1', { + method: 'POST', + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'tool_a' }, + }), + }) + const response = await POST(req, { params: Promise.resolve({ serverId: 'server-1' }) }) + + expect(response.status).toBe(200) + expect(fetchMock).toHaveBeenCalledTimes(1) + const fetchOptions = fetchMock.mock.calls[0][1] as RequestInit + const headers = fetchOptions.headers as Record + expect(headers.Authorization).toBe('Bearer internal-token-user-1') + expect(headers['X-API-Key']).toBeUndefined() + expect(mockGenerateInternalToken).toHaveBeenCalledWith('user-1') + }) +}) diff --git a/apps/sim/app/api/mcp/serve/[serverId]/route.ts b/apps/sim/app/api/mcp/serve/[serverId]/route.ts index 9d9f917bd6..38649860ae 100644 --- a/apps/sim/app/api/mcp/serve/[serverId]/route.ts +++ b/apps/sim/app/api/mcp/serve/[serverId]/route.ts @@ -23,6 +23,7 @@ import { checkHybridAuth } from '@/lib/auth/hybrid' import { generateInternalToken } from '@/lib/auth/internal' import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' import { getBaseUrl } from '@/lib/core/utils/urls' +import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' const logger = createLogger('WorkflowMcpServeAPI') @@ -32,6 +33,12 @@ interface RouteParams { serverId: string } +interface ExecuteAuthContext { + authType?: 'session' | 'api_key' | 'internal_jwt' + userId: string + apiKey?: string | null +} + function createResponse(id: RequestId, result: unknown): JSONRPCResponse { return { jsonrpc: '2.0', @@ -73,6 +80,22 @@ export async function GET(request: NextRequest, { params }: { params: Promise }, - apiKey, + executeAuthContext, server.isPublic ? server.createdBy : undefined ) @@ -207,7 +243,7 @@ async function handleToolsCall( id: RequestId, serverId: string, params: { name: string; arguments?: Record } | undefined, - apiKey?: string | null, + executeAuthContext?: ExecuteAuthContext | null, publicServerOwnerId?: string ): Promise { try { @@ -255,8 +291,13 @@ async function handleToolsCall( if (publicServerOwnerId) { const internalToken = await generateInternalToken(publicServerOwnerId) headers.Authorization = `Bearer ${internalToken}` - } else if (apiKey) { - headers['X-API-Key'] = apiKey + } else if (executeAuthContext) { + if (executeAuthContext.authType === 'api_key' && executeAuthContext.apiKey) { + headers['X-API-Key'] = executeAuthContext.apiKey + } else { + const internalToken = await generateInternalToken(executeAuthContext.userId) + headers.Authorization = `Bearer ${internalToken}` + } } logger.info(`Executing workflow ${tool.workflowId} via MCP tool ${params.name}`) @@ -311,6 +352,17 @@ export async function DELETE(request: NextRequest, { params }: { params: Promise return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + if (!server.isPublic) { + const workspacePermission = await getUserEntityPermissions( + auth.userId, + 'workspace', + server.workspaceId + ) + if (workspacePermission === null) { + return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) + } + } + logger.info(`MCP session terminated for server ${serverId}`) return new NextResponse(null, { status: 204 }) } catch (error) { diff --git a/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts b/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts index 0f3252d859..bb4488a04d 100644 --- a/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts +++ b/apps/sim/app/api/resume/[workflowId]/[executionId]/[contextId]/route.ts @@ -4,6 +4,7 @@ import { type NextRequest, NextResponse } from 'next/server' import { generateRequestId } from '@/lib/core/utils/request' import { preprocessExecution } from '@/lib/execution/preprocessing' import { PauseResumeManager } from '@/lib/workflows/executor/human-in-the-loop-manager' +import { getWorkspaceBilledAccountUserId } from '@/lib/workspaces/utils' import { validateWorkflowAccess } from '@/app/api/workflows/middleware' const logger = createLogger('WorkflowResumeAPI') @@ -37,7 +38,26 @@ export async function POST( } const resumeInput = payload?.input ?? payload ?? {} - const userId = workflow.userId ?? '' + const isPersonalApiKeyCaller = + access.auth?.authType === 'api_key' && access.auth?.apiKeyType === 'personal' + + let userId: string + if (isPersonalApiKeyCaller && access.auth?.userId) { + userId = access.auth.userId + } else { + const billedAccountUserId = await getWorkspaceBilledAccountUserId(workflow.workspaceId) + if (!billedAccountUserId) { + logger.error('Unable to resolve workspace billed account for resume execution', { + workflowId, + workspaceId: workflow.workspaceId, + }) + return NextResponse.json( + { error: 'Unable to resolve billing account for this workspace' }, + { status: 500 } + ) + } + userId = billedAccountUserId + } const resumeExecutionId = randomUUID() const requestId = generateRequestId() @@ -58,6 +78,7 @@ export async function POST( checkRateLimit: false, // Manual triggers bypass rate limits checkDeployment: false, // Resuming existing execution, deployment already checked skipUsageLimits: true, // Resume is continuation of authorized execution - don't recheck limits + useAuthenticatedUserAsActor: isPersonalApiKeyCaller, workspaceId: workflow.workspaceId || undefined, }) diff --git a/apps/sim/app/api/schedules/[id]/route.test.ts b/apps/sim/app/api/schedules/[id]/route.test.ts index 4488606805..012f327d11 100644 --- a/apps/sim/app/api/schedules/[id]/route.test.ts +++ b/apps/sim/app/api/schedules/[id]/route.test.ts @@ -7,21 +7,20 @@ import { loggerMock } from '@sim/testing' import { NextRequest } from 'next/server' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGetSession, mockGetUserEntityPermissions, mockDbSelect, mockDbUpdate } = vi.hoisted( - () => ({ +const { mockGetSession, mockAuthorizeWorkflowByWorkspacePermission, mockDbSelect, mockDbUpdate } = + vi.hoisted(() => ({ mockGetSession: vi.fn(), - mockGetUserEntityPermissions: vi.fn(), + mockAuthorizeWorkflowByWorkspacePermission: vi.fn(), mockDbSelect: vi.fn(), mockDbUpdate: vi.fn(), - }) -) + })) vi.mock('@/lib/auth', () => ({ getSession: mockGetSession, })) -vi.mock('@/lib/workspaces/permissions/utils', () => ({ - getUserEntityPermissions: mockGetUserEntityPermissions, +vi.mock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: mockAuthorizeWorkflowByWorkspacePermission, })) vi.mock('@sim/db', () => ({ @@ -81,7 +80,12 @@ describe('Schedule PUT API (Reactivate)', () => { beforeEach(() => { vi.clearAllMocks() mockGetSession.mockResolvedValue({ user: { id: 'user-1' } }) - mockGetUserEntityPermissions.mockResolvedValue('write') + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: 'write', + }) }) afterEach(() => { @@ -140,6 +144,13 @@ describe('Schedule PUT API (Reactivate)', () => { }) it('returns 404 when workflow does not exist for schedule', async () => { + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 404, + workflow: null, + workspacePermission: null, + message: 'Workflow not found', + }) mockDbChain([[{ id: 'sched-1', workflowId: 'wf-1', status: 'disabled' }], []]) const res = await PUT(createRequest({ action: 'reactivate' }), createParams('sched-1')) @@ -152,6 +163,14 @@ describe('Schedule PUT API (Reactivate)', () => { describe('Authorization', () => { it('returns 403 when user is not workflow owner', async () => { + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + workflow: { id: 'wf-1', workspaceId: null }, + workspacePermission: null, + message: + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', + }) mockDbChain([ [{ id: 'sched-1', workflowId: 'wf-1', status: 'disabled' }], [{ userId: 'other-user', workspaceId: null }], @@ -161,11 +180,17 @@ describe('Schedule PUT API (Reactivate)', () => { expect(res.status).toBe(403) const data = await res.json() - expect(data.error).toBe('Not authorized to modify this schedule') + expect(data.error).toContain('Personal workflows are deprecated') }) it('returns 403 for workspace member with only read permission', async () => { - mockGetUserEntityPermissions.mockResolvedValue('read') + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: 'read', + message: 'Unauthorized: Access denied to write this workflow', + }) mockDbChain([ [{ id: 'sched-1', workflowId: 'wf-1', status: 'disabled' }], [{ userId: 'other-user', workspaceId: 'ws-1' }], @@ -198,7 +223,6 @@ describe('Schedule PUT API (Reactivate)', () => { }) it('allows workspace member with write permission to reactivate', async () => { - mockGetUserEntityPermissions.mockResolvedValue('write') mockDbChain([ [ { @@ -218,7 +242,6 @@ describe('Schedule PUT API (Reactivate)', () => { }) it('allows workspace admin to reactivate', async () => { - mockGetUserEntityPermissions.mockResolvedValue('admin') mockDbChain([ [ { diff --git a/apps/sim/app/api/schedules/[id]/route.ts b/apps/sim/app/api/schedules/[id]/route.ts index 031358ba25..3f0dff72e5 100644 --- a/apps/sim/app/api/schedules/[id]/route.ts +++ b/apps/sim/app/api/schedules/[id]/route.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { workflow, workflowSchedule } from '@sim/db/schema' +import { workflowSchedule } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' @@ -7,7 +7,7 @@ import { z } from 'zod' import { getSession } from '@/lib/auth' import { generateRequestId } from '@/lib/core/utils/request' import { validateCronExpression } from '@/lib/workflows/schedules/utils' -import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' const logger = createLogger('ScheduleAPI') @@ -57,31 +57,23 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ return NextResponse.json({ error: 'Schedule not found' }, { status: 404 }) } - const [workflowRecord] = await db - .select({ userId: workflow.userId, workspaceId: workflow.workspaceId }) - .from(workflow) - .where(eq(workflow.id, schedule.workflowId)) - .limit(1) + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: schedule.workflowId, + userId: session.user.id, + action: 'write', + }) - if (!workflowRecord) { + if (!authorization.workflow) { logger.warn(`[${requestId}] Workflow not found for schedule: ${scheduleId}`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - let isAuthorized = workflowRecord.userId === session.user.id - - if (!isAuthorized && workflowRecord.workspaceId) { - const userPermission = await getUserEntityPermissions( - session.user.id, - 'workspace', - workflowRecord.workspaceId - ) - isAuthorized = userPermission === 'write' || userPermission === 'admin' - } - - if (!isAuthorized) { + if (!authorization.allowed) { logger.warn(`[${requestId}] User not authorized to modify this schedule: ${scheduleId}`) - return NextResponse.json({ error: 'Not authorized to modify this schedule' }, { status: 403 }) + return NextResponse.json( + { error: authorization.message || 'Not authorized to modify this schedule' }, + { status: authorization.status } + ) } if (schedule.status === 'active') { diff --git a/apps/sim/app/api/schedules/route.test.ts b/apps/sim/app/api/schedules/route.test.ts index a7df3c9529..e6320b2b6c 100644 --- a/apps/sim/app/api/schedules/route.test.ts +++ b/apps/sim/app/api/schedules/route.test.ts @@ -7,18 +7,20 @@ import { loggerMock } from '@sim/testing' import { NextRequest } from 'next/server' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGetSession, mockGetUserEntityPermissions, mockDbSelect } = vi.hoisted(() => ({ - mockGetSession: vi.fn(), - mockGetUserEntityPermissions: vi.fn(), - mockDbSelect: vi.fn(), -})) +const { mockGetSession, mockAuthorizeWorkflowByWorkspacePermission, mockDbSelect } = vi.hoisted( + () => ({ + mockGetSession: vi.fn(), + mockAuthorizeWorkflowByWorkspacePermission: vi.fn(), + mockDbSelect: vi.fn(), + }) +) vi.mock('@/lib/auth', () => ({ getSession: mockGetSession, })) -vi.mock('@/lib/workspaces/permissions/utils', () => ({ - getUserEntityPermissions: mockGetUserEntityPermissions, +vi.mock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: mockAuthorizeWorkflowByWorkspacePermission, })) vi.mock('@sim/db', () => ({ @@ -80,7 +82,12 @@ describe('Schedule GET API', () => { beforeEach(() => { vi.clearAllMocks() mockGetSession.mockResolvedValue({ user: { id: 'user-1' } }) - mockGetUserEntityPermissions.mockResolvedValue('read') + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: 'read', + }) }) afterEach(() => { @@ -89,7 +96,6 @@ describe('Schedule GET API', () => { it('returns schedule data for authorized user', async () => { mockDbChain([ - [{ userId: 'user-1', workspaceId: null }], [ { schedule: { @@ -111,7 +117,7 @@ describe('Schedule GET API', () => { }) it('returns null when no schedule exists', async () => { - mockDbChain([[{ userId: 'user-1', workspaceId: null }], []]) + mockDbChain([[]]) const res = await GET(createRequest('http://test/api/schedules?workflowId=wf-1')) const data = await res.json() @@ -135,6 +141,13 @@ describe('Schedule GET API', () => { }) it('returns 404 for non-existent workflow', async () => { + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 404, + message: 'Workflow not found', + workflow: null, + workspacePermission: null, + }) mockDbChain([[]]) const res = await GET(createRequest('http://test/api/schedules?workflowId=wf-1')) @@ -143,6 +156,13 @@ describe('Schedule GET API', () => { }) it('denies access for unauthorized user', async () => { + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to read this workflow', + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: null, + }) mockDbChain([[{ userId: 'other-user', workspaceId: null }]]) const res = await GET(createRequest('http://test/api/schedules?workflowId=wf-1')) @@ -151,10 +171,7 @@ describe('Schedule GET API', () => { }) it('allows workspace members to view', async () => { - mockDbChain([ - [{ userId: 'other-user', workspaceId: 'ws-1' }], - [{ schedule: { id: 'sched-1', status: 'active', failedCount: 0 } }], - ]) + mockDbChain([[{ schedule: { id: 'sched-1', status: 'active', failedCount: 0 } }]]) const res = await GET(createRequest('http://test/api/schedules?workflowId=wf-1')) @@ -162,10 +179,7 @@ describe('Schedule GET API', () => { }) it('indicates disabled schedule with failures', async () => { - mockDbChain([ - [{ userId: 'user-1', workspaceId: null }], - [{ schedule: { id: 'sched-1', status: 'disabled', failedCount: 100 } }], - ]) + mockDbChain([[{ schedule: { id: 'sched-1', status: 'disabled', failedCount: 100 } }]]) const res = await GET(createRequest('http://test/api/schedules?workflowId=wf-1')) const data = await res.json() diff --git a/apps/sim/app/api/schedules/route.ts b/apps/sim/app/api/schedules/route.ts index 50cf346065..575acd45f9 100644 --- a/apps/sim/app/api/schedules/route.ts +++ b/apps/sim/app/api/schedules/route.ts @@ -1,11 +1,11 @@ import { db } from '@sim/db' -import { workflow, workflowDeploymentVersion, workflowSchedule } from '@sim/db/schema' +import { workflowDeploymentVersion, workflowSchedule } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull, or } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { getSession } from '@/lib/auth' import { generateRequestId } from '@/lib/core/utils/request' -import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' const logger = createLogger('ScheduledAPI') @@ -29,29 +29,21 @@ export async function GET(req: NextRequest) { return NextResponse.json({ error: 'Missing workflowId parameter' }, { status: 400 }) } - const [workflowRecord] = await db - .select({ userId: workflow.userId, workspaceId: workflow.workspaceId }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1) + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId: session.user.id, + action: 'read', + }) - if (!workflowRecord) { + if (!authorization.workflow) { return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - let isAuthorized = workflowRecord.userId === session.user.id - - if (!isAuthorized && workflowRecord.workspaceId) { - const userPermission = await getUserEntityPermissions( - session.user.id, - 'workspace', - workflowRecord.workspaceId + if (!authorization.allowed) { + return NextResponse.json( + { error: authorization.message || 'Not authorized to view this workflow' }, + { status: authorization.status } ) - isAuthorized = userPermission !== null - } - - if (!isAuthorized) { - return NextResponse.json({ error: 'Not authorized to view this workflow' }, { status: 403 }) } logger.info(`[${requestId}] Getting schedule for workflow ${workflowId}`) diff --git a/apps/sim/app/api/tools/custom/route.test.ts b/apps/sim/app/api/tools/custom/route.test.ts index f1e8899137..a5db0632a3 100644 --- a/apps/sim/app/api/tools/custom/route.test.ts +++ b/apps/sim/app/api/tools/custom/route.test.ts @@ -214,6 +214,14 @@ describe('Custom Tools API Routes', () => { vi.doMock('@/lib/workflows/custom-tools/operations', () => ({ upsertCustomTools: vi.fn().mockResolvedValue(sampleTools), })) + + vi.doMock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: vi.fn().mockResolvedValue({ + allowed: true, + status: 200, + workflow: { workspaceId: 'workspace-123' }, + }), + })) }) afterEach(() => { @@ -272,20 +280,6 @@ describe('Custom Tools API Routes', () => { it('should handle workflowId parameter', async () => { const req = new NextRequest('http://localhost:3000/api/tools/custom?workflowId=workflow-123') - mockLimit.mockResolvedValueOnce([{ workspaceId: 'workspace-123' }]) - - mockWhere.mockImplementationOnce((condition) => { - const queryBuilder = { - limit: mockLimit, - then: (resolve: (value: typeof sampleTools) => void) => { - resolve(sampleTools) - return queryBuilder - }, - catch: (reject: (error: Error) => void) => queryBuilder, - } - return queryBuilder - }) - const { GET } = await import('@/app/api/tools/custom/route') const response = await GET(req) @@ -375,7 +369,8 @@ describe('Custom Tools API Routes', () => { }) it('should handle tool not found', async () => { - mockLimit.mockResolvedValueOnce([]) + const mockLimitNotFound = vi.fn().mockResolvedValue([]) + mockWhere.mockReturnValueOnce({ limit: mockLimitNotFound }) const req = new NextRequest('http://localhost:3000/api/tools/custom?id=non-existent') @@ -398,7 +393,8 @@ describe('Custom Tools API Routes', () => { })) const userScopedTool = { ...sampleTools[0], workspaceId: null, userId: 'user-123' } - mockLimit.mockResolvedValueOnce([userScopedTool]) + const mockLimitUserScoped = vi.fn().mockResolvedValue([userScopedTool]) + mockWhere.mockReturnValueOnce({ limit: mockLimitUserScoped }) const req = new NextRequest('http://localhost:3000/api/tools/custom?id=tool-1') diff --git a/apps/sim/app/api/tools/custom/route.ts b/apps/sim/app/api/tools/custom/route.ts index abd9e41020..9979a378f3 100644 --- a/apps/sim/app/api/tools/custom/route.ts +++ b/apps/sim/app/api/tools/custom/route.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { customTools, workflow } from '@sim/db/schema' +import { customTools } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, desc, eq, isNull, or } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' @@ -7,6 +7,7 @@ import { z } from 'zod' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { upsertCustomTools } from '@/lib/workflows/custom-tools/operations' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' const logger = createLogger('CustomToolsAPI') @@ -52,27 +53,32 @@ export async function GET(request: NextRequest) { const userId = authResult.userId let resolvedWorkspaceId: string | null = workspaceId + let resolvedFromWorkflowAuthorization = false if (!resolvedWorkspaceId && workflowId) { - const [workflowData] = await db - .select({ workspaceId: workflow.workspaceId }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1) - - if (!workflowData) { - logger.warn(`[${requestId}] Workflow not found: ${workflowId}`) - return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) + const workflowAuthorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'read', + }) + if (!workflowAuthorization.allowed) { + logger.warn(`[${requestId}] Workflow authorization failed for custom tools`, { + workflowId, + userId, + status: workflowAuthorization.status, + }) + return NextResponse.json( + { error: workflowAuthorization.message || 'Access denied' }, + { status: workflowAuthorization.status } + ) } - resolvedWorkspaceId = workflowData.workspaceId + resolvedWorkspaceId = workflowAuthorization.workflow?.workspaceId ?? null + resolvedFromWorkflowAuthorization = true } - // Check workspace permissions - // For internal JWT with workflowId: checkSessionOrInternalAuth already resolved userId from workflow owner - // For session: verify user has access to the workspace - // For legacy (no workspaceId): skip workspace check, rely on userId match - if (resolvedWorkspaceId && !(authResult.authType === 'internal_jwt' && workflowId)) { + // Check workspace permissions for all auth types + if (resolvedWorkspaceId && !resolvedFromWorkflowAuthorization) { const userPermission = await getUserEntityPermissions( userId, 'workspace', diff --git a/apps/sim/app/api/usage/route.ts b/apps/sim/app/api/usage/route.ts index f55f57c496..12d98d57cc 100644 --- a/apps/sim/app/api/usage/route.ts +++ b/apps/sim/app/api/usage/route.ts @@ -7,6 +7,7 @@ import { getOrganizationBillingData, isOrganizationOwnerOrAdmin, } from '@/lib/billing/core/organization' +import { isUserMemberOfOrganization } from '@/lib/billing/organizations/membership' const logger = createLogger('UnifiedUsageAPI') @@ -61,6 +62,12 @@ export async function GET(request: NextRequest) { { status: 400 } ) } + + const membership = await isUserMemberOfOrganization(session.user.id, organizationId) + if (!membership.isMember) { + return NextResponse.json({ error: 'Permission denied' }, { status: 403 }) + } + const org = await getOrganizationBillingData(organizationId) return NextResponse.json({ success: true, diff --git a/apps/sim/app/api/wand/route.ts b/apps/sim/app/api/wand/route.ts index bba2c2c2df..aba3b14eff 100644 --- a/apps/sim/app/api/wand/route.ts +++ b/apps/sim/app/api/wand/route.ts @@ -160,7 +160,7 @@ export async function POST(req: NextRequest) { let workspaceId: string | null = null if (workflowId) { const [workflowRecord] = await db - .select({ workspaceId: workflow.workspaceId, userId: workflow.userId }) + .select({ workspaceId: workflow.workspaceId }) .from(workflow) .where(eq(workflow.id, workflowId)) .limit(1) @@ -183,9 +183,18 @@ export async function POST(req: NextRequest) { ) return NextResponse.json({ success: false, error: 'Unauthorized' }, { status: 403 }) } - } else if (workflowRecord.userId !== session.user.id) { - logger.warn(`[${requestId}] User ${session.user.id} does not own workflow ${workflowId}`) - return NextResponse.json({ success: false, error: 'Unauthorized' }, { status: 403 }) + } else { + logger.warn( + `[${requestId}] Workflow ${workflowId} has no workspaceId; wand request blocked` + ) + return NextResponse.json( + { + success: false, + error: + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', + }, + { status: 403 } + ) } } diff --git a/apps/sim/app/api/webhooks/[id]/route.ts b/apps/sim/app/api/webhooks/[id]/route.ts index e4c381cad4..b3e7bf3de6 100644 --- a/apps/sim/app/api/webhooks/[id]/route.ts +++ b/apps/sim/app/api/webhooks/[id]/route.ts @@ -3,12 +3,12 @@ import { webhook, workflow } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { validateInteger } from '@/lib/core/security/input-validation' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { cleanupExternalWebhook } from '@/lib/webhooks/provider-subscriptions' -import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' const logger = createLogger('WebhookAPI') @@ -22,11 +22,12 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ const { id } = await params logger.debug(`[${requestId}] Fetching webhook with ID: ${id}`) - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized webhook access attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId const webhooks = await db .select({ @@ -50,28 +51,15 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ const webhookData = webhooks[0] - // Check if user has permission to access this webhook - let hasAccess = false - - // Case 1: User owns the workflow - if (webhookData.workflow.userId === session.user.id) { - hasAccess = true - } - - // Case 2: Workflow belongs to a workspace and user has any permission - if (!hasAccess && webhookData.workflow.workspaceId) { - const userPermission = await getUserEntityPermissions( - session.user.id, - 'workspace', - webhookData.workflow.workspaceId - ) - if (userPermission !== null) { - hasAccess = true - } - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: webhookData.workflow.id, + userId, + action: 'read', + }) + const hasAccess = authorization.allowed if (!hasAccess) { - logger.warn(`[${requestId}] User ${session.user.id} denied access to webhook: ${id}`) + logger.warn(`[${requestId}] User ${userId} denied access to webhook: ${id}`) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } @@ -90,11 +78,12 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { id } = await params logger.debug(`[${requestId}] Updating webhook with ID: ${id}`) - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized webhook update attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId const body = await request.json() const { isActive, failedCount } = body @@ -127,27 +116,15 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< } const webhookData = webhooks[0] - let canModify = false - - if (webhookData.workflow.userId === session.user.id) { - canModify = true - } - - if (!canModify && webhookData.workflow.workspaceId) { - const userPermission = await getUserEntityPermissions( - session.user.id, - 'workspace', - webhookData.workflow.workspaceId - ) - if (userPermission === 'write' || userPermission === 'admin') { - canModify = true - } - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: webhookData.workflow.id, + userId, + action: 'write', + }) + const canModify = authorization.allowed if (!canModify) { - logger.warn( - `[${requestId}] User ${session.user.id} denied permission to modify webhook: ${id}` - ) + logger.warn(`[${requestId}] User ${userId} denied permission to modify webhook: ${id}`) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } @@ -185,11 +162,12 @@ export async function DELETE( const { id } = await params logger.debug(`[${requestId}] Deleting webhook with ID: ${id}`) - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized webhook deletion attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId // Find the webhook and check permissions const webhooks = await db @@ -213,30 +191,15 @@ export async function DELETE( const webhookData = webhooks[0] - // Check if user has permission to delete this webhook - let canDelete = false - - // Case 1: User owns the workflow - if (webhookData.workflow.userId === session.user.id) { - canDelete = true - } - - // Case 2: Workflow belongs to a workspace and user has write or admin permission - if (!canDelete && webhookData.workflow.workspaceId) { - const userPermission = await getUserEntityPermissions( - session.user.id, - 'workspace', - webhookData.workflow.workspaceId - ) - if (userPermission === 'write' || userPermission === 'admin') { - canDelete = true - } - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: webhookData.workflow.id, + userId, + action: 'write', + }) + const canDelete = authorization.allowed if (!canDelete) { - logger.warn( - `[${requestId}] User ${session.user.id} denied permission to delete webhook: ${id}` - ) + logger.warn(`[${requestId}] User ${userId} denied permission to delete webhook: ${id}`) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } diff --git a/apps/sim/app/api/webhooks/route.ts b/apps/sim/app/api/webhooks/route.ts index 6cc9876f7f..b5f978792a 100644 --- a/apps/sim/app/api/webhooks/route.ts +++ b/apps/sim/app/api/webhooks/route.ts @@ -1,7 +1,7 @@ import { db } from '@sim/db' -import { webhook, workflow, workflowDeploymentVersion } from '@sim/db/schema' +import { permissions, webhook, workflow, workflowDeploymentVersion } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, desc, eq, isNull, or } from 'drizzle-orm' +import { and, desc, eq, inArray, isNull, or } from 'drizzle-orm' import { nanoid } from 'nanoid' import { type NextRequest, NextResponse } from 'next/server' import { getSession } from '@/lib/auth' @@ -20,7 +20,7 @@ import { configureRssPolling, syncWebhooksForCredentialSet, } from '@/lib/webhooks/utils.server' -import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { extractCredentialSetId, isCredentialSetValue } from '@/executor/constants' const logger = createLogger('WebhooksAPI') @@ -57,15 +57,12 @@ export async function GET(request: NextRequest) { } const wfRecord = wf[0] - let canRead = wfRecord.userId === session.user.id - if (!canRead && wfRecord.workspaceId) { - const permission = await getUserEntityPermissions( - session.user.id, - 'workspace', - wfRecord.workspaceId - ) - canRead = permission === 'read' || permission === 'write' || permission === 'admin' - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: wfRecord.id, + userId: session.user.id, + action: 'read', + }) + const canRead = authorization.allowed if (!canRead) { logger.warn( @@ -114,8 +111,17 @@ export async function GET(request: NextRequest) { return NextResponse.json({ webhooks: [] }, { status: 200 }) } - // Default: list webhooks owned by the session user - logger.debug(`[${requestId}] Fetching user-owned webhooks for ${session.user.id}`) + logger.debug(`[${requestId}] Fetching workspace-accessible webhooks for ${session.user.id}`) + const workspacePermissionRows = await db + .select({ workspaceId: permissions.entityId }) + .from(permissions) + .where(and(eq(permissions.userId, session.user.id), eq(permissions.entityType, 'workspace'))) + + const workspaceIds = workspacePermissionRows.map((row) => row.workspaceId) + if (workspaceIds.length === 0) { + return NextResponse.json({ webhooks: [] }, { status: 200 }) + } + const webhooks = await db .select({ webhook: webhook, @@ -126,9 +132,9 @@ export async function GET(request: NextRequest) { }) .from(webhook) .innerJoin(workflow, eq(webhook.workflowId, workflow.id)) - .where(eq(workflow.userId, session.user.id)) + .where(inArray(workflow.workspaceId, workspaceIds)) - logger.info(`[${requestId}] Retrieved ${webhooks.length} user-owned webhooks`) + logger.info(`[${requestId}] Retrieved ${webhooks.length} workspace-accessible webhooks`) return NextResponse.json({ webhooks }, { status: 200 }) } catch (error) { logger.error(`[${requestId}] Error fetching webhooks`, error) @@ -237,25 +243,12 @@ export async function POST(request: NextRequest) { const workflowRecord = workflowData[0] - // Check if user has permission to modify this workflow - let canModify = false - - // Case 1: User owns the workflow - if (workflowRecord.userId === userId) { - canModify = true - } - - // Case 2: Workflow belongs to a workspace and user has write or admin permission - if (!canModify && workflowRecord.workspaceId) { - const userPermission = await getUserEntityPermissions( - userId, - 'workspace', - workflowRecord.workspaceId - ) - if (userPermission === 'write' || userPermission === 'admin') { - canModify = true - } - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'write', + }) + const canModify = authorization.allowed if (!canModify) { logger.warn( diff --git a/apps/sim/app/api/workflows/[id]/autolayout/route.ts b/apps/sim/app/api/workflows/[id]/autolayout/route.ts index a55c23da1e..907e1ea049 100644 --- a/apps/sim/app/api/workflows/[id]/autolayout/route.ts +++ b/apps/sim/app/api/workflows/[id]/autolayout/route.ts @@ -1,7 +1,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { applyAutoLayout } from '@/lib/workflows/autolayout' import { @@ -13,7 +13,7 @@ import { loadWorkflowFromNormalizedTables, type NormalizedWorkflowData, } from '@/lib/workflows/persistence/utils' -import { getWorkflowAccessContext } from '@/lib/workflows/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' export const dynamic = 'force-dynamic' @@ -52,13 +52,13 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ const { id: workflowId } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized autolayout attempt for workflow ${workflowId}`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - const userId = session.user.id + const userId = auth.userId const body = await request.json() const layoutOptions = AutoLayoutRequestSchema.parse(body) @@ -67,26 +67,28 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ userId, }) - const accessContext = await getWorkflowAccessContext(workflowId, userId) - const workflowData = accessContext?.workflow + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'write', + }) + const workflowData = authorization.workflow if (!workflowData) { logger.warn(`[${requestId}] Workflow ${workflowId} not found for autolayout`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - const canUpdate = - accessContext?.isOwner || - (workflowData.workspaceId - ? accessContext?.workspacePermission === 'write' || - accessContext?.workspacePermission === 'admin' - : false) + const canUpdate = authorization.allowed if (!canUpdate) { logger.warn( `[${requestId}] User ${userId} denied permission to autolayout workflow ${workflowId}` ) - return NextResponse.json({ error: 'Access denied' }, { status: 403 }) + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status || 403 } + ) } let currentWorkflowData: NormalizedWorkflowData | null diff --git a/apps/sim/app/api/workflows/[id]/chat/status/route.test.ts b/apps/sim/app/api/workflows/[id]/chat/status/route.test.ts new file mode 100644 index 0000000000..8e09e10b0f --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/chat/status/route.test.ts @@ -0,0 +1,131 @@ +/** + * Tests for workflow chat status route auth and access. + * + * @vitest-environment node + */ +import { loggerMock } from '@sim/testing' +import { NextRequest } from 'next/server' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mockCheckSessionOrInternalAuth = vi.fn() +const mockAuthorizeWorkflowByWorkspacePermission = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbWhere = vi.fn() +const mockDbLimit = vi.fn() + +describe('Workflow Chat Status Route', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit.mockResolvedValue([]) + + vi.doMock('@sim/logger', () => loggerMock) + vi.doMock('drizzle-orm', () => ({ + eq: vi.fn(), + })) + vi.doMock('@sim/db', () => ({ + db: { + select: mockDbSelect, + }, + })) + vi.doMock('@sim/db/schema', () => ({ + chat: { + id: 'id', + identifier: 'identifier', + title: 'title', + description: 'description', + customizations: 'customizations', + authType: 'authType', + allowedEmails: 'allowedEmails', + outputConfigs: 'outputConfigs', + password: 'password', + isActive: 'isActive', + workflowId: 'workflowId', + }, + })) + vi.doMock('@/lib/auth/hybrid', () => ({ + checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth, + })) + vi.doMock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: mockAuthorizeWorkflowByWorkspacePermission, + })) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + it('returns 401 when unauthenticated', async () => { + mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ success: false }) + + const { GET } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/chat/status') + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(401) + }) + + it('returns 403 when user lacks workspace access', async () => { + mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'session', + }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: false, + status: 403, + message: 'Access denied', + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: null, + }) + + const { GET } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/chat/status') + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(403) + }) + + it('returns deployment details when authorized', async () => { + mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'session', + }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: true, + status: 200, + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: 'read', + }) + mockDbLimit.mockResolvedValueOnce([ + { + id: 'chat-1', + identifier: 'assistant', + title: 'Support Bot', + description: 'desc', + customizations: { theme: 'dark' }, + authType: 'public', + allowedEmails: [], + outputConfigs: {}, + password: 'secret', + isActive: true, + }, + ]) + + const { GET } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/chat/status') + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.isDeployed).toBe(true) + expect(data.deployment.id).toBe('chat-1') + expect(data.deployment.hasPassword).toBe(true) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/chat/status/route.ts b/apps/sim/app/api/workflows/[id]/chat/status/route.ts index 1bd930b7ef..4dbaf214eb 100644 --- a/apps/sim/app/api/workflows/[id]/chat/status/route.ts +++ b/apps/sim/app/api/workflows/[id]/chat/status/route.ts @@ -2,7 +2,10 @@ import { db } from '@sim/db' import { chat } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' +import type { NextRequest } from 'next/server' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('ChatStatusAPI') @@ -10,11 +13,28 @@ const logger = createLogger('ChatStatusAPI') /** * GET endpoint to check if a workflow has an active chat deployment */ -export async function GET(_request: Request, { params }: { params: Promise<{ id: string }> }) { +export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { const { id } = await params const requestId = generateRequestId() try { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { + return createErrorResponse('Unauthorized', 401) + } + + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: id, + userId: auth.userId, + action: 'read', + }) + if (!authorization.allowed) { + return createErrorResponse( + authorization.message || 'Access denied', + authorization.status || 403 + ) + } + logger.debug(`[${requestId}] Checking chat deployment status for workflow: ${id}`) // Find any active chat deployments for this workflow diff --git a/apps/sim/app/api/workflows/[id]/duplicate/route.ts b/apps/sim/app/api/workflows/[id]/duplicate/route.ts index 935dc4ed0f..5a35fe86e6 100644 --- a/apps/sim/app/api/workflows/[id]/duplicate/route.ts +++ b/apps/sim/app/api/workflows/[id]/duplicate/route.ts @@ -1,7 +1,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { duplicateWorkflow } from '@/lib/workflows/persistence/duplicate' @@ -22,23 +22,22 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: const requestId = generateRequestId() const startTime = Date.now() - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized workflow duplication attempt for ${sourceWorkflowId}`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId try { const body = await req.json() const { name, description, color, workspaceId, folderId } = DuplicateRequestSchema.parse(body) - logger.info( - `[${requestId}] Duplicating workflow ${sourceWorkflowId} for user ${session.user.id}` - ) + logger.info(`[${requestId}] Duplicating workflow ${sourceWorkflowId} for user ${userId}`) const result = await duplicateWorkflow({ sourceWorkflowId, - userId: session.user.id, + userId, name, description, color, @@ -72,7 +71,7 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: if (error.message === 'Source workflow not found or access denied') { logger.warn( - `[${requestId}] User ${session.user.id} denied access to source workflow ${sourceWorkflowId}` + `[${requestId}] User ${userId} denied access to source workflow ${sourceWorkflowId}` ) return NextResponse.json({ error: 'Access denied' }, { status: 403 }) } diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index a343fb3e9a..b047f23e87 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -29,7 +29,11 @@ import { loadWorkflowFromNormalizedTables, } from '@/lib/workflows/persistence/utils' import { createStreamingResponse } from '@/lib/workflows/streaming/streaming' -import { createHttpResponseFromBlock, workflowHasResponseBlock } from '@/lib/workflows/utils' +import { + authorizeWorkflowByWorkspacePermission, + createHttpResponseFromBlock, + workflowHasResponseBlock, +} from '@/lib/workflows/utils' import { executeWorkflowJob, type WorkflowExecutionPayload } from '@/background/workflow-execution' import { normalizeName } from '@/executor/constants' import { ExecutionSnapshot } from '@/executor/execution/snapshot' @@ -340,6 +344,17 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: : validatedInput const shouldUseDraftState = useDraftState ?? auth.authType === 'session' + const workflowAuthorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: shouldUseDraftState ? 'write' : 'read', + }) + if (!workflowAuthorization.allowed) { + return NextResponse.json( + { error: workflowAuthorization.message || 'Access denied' }, + { status: workflowAuthorization.status } + ) + } const streamHeader = req.headers.get('X-Stream-Response') === 'true' const enableSSE = streamHeader || streamParam === true diff --git a/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts b/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts index 2544bb342c..49c99e1ede 100644 --- a/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts +++ b/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { checkHybridAuth } from '@/lib/auth/hybrid' import { markExecutionCancelled } from '@/lib/execution/cancellation' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' const logger = createLogger('CancelExecutionAPI') @@ -20,6 +21,18 @@ export async function POST( return NextResponse.json({ error: auth.error || 'Unauthorized' }, { status: 401 }) } + const workflowAuthorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId: auth.userId, + action: 'write', + }) + if (!workflowAuthorization.allowed) { + return NextResponse.json( + { error: workflowAuthorization.message || 'Access denied' }, + { status: workflowAuthorization.status } + ) + } + logger.info('Cancel execution requested', { workflowId, executionId, userId: auth.userId }) const marked = await markExecutionCancelled(executionId) diff --git a/apps/sim/app/api/workflows/[id]/form/status/route.test.ts b/apps/sim/app/api/workflows/[id]/form/status/route.test.ts new file mode 100644 index 0000000000..d4274a6faf --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/form/status/route.test.ts @@ -0,0 +1,119 @@ +/** + * Tests for workflow form status route auth and access. + * + * @vitest-environment node + */ +import { loggerMock } from '@sim/testing' +import { NextRequest } from 'next/server' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mockCheckSessionOrInternalAuth = vi.fn() +const mockAuthorizeWorkflowByWorkspacePermission = vi.fn() +const mockDbSelect = vi.fn() +const mockDbFrom = vi.fn() +const mockDbWhere = vi.fn() +const mockDbLimit = vi.fn() + +describe('Workflow Form Status Route', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit.mockResolvedValue([]) + + vi.doMock('@sim/logger', () => loggerMock) + vi.doMock('drizzle-orm', () => ({ + and: vi.fn(), + eq: vi.fn(), + })) + vi.doMock('@sim/db', () => ({ + db: { + select: mockDbSelect, + }, + })) + vi.doMock('@sim/db/schema', () => ({ + form: { + id: 'id', + identifier: 'identifier', + title: 'title', + workflowId: 'workflowId', + isActive: 'isActive', + }, + })) + vi.doMock('@/lib/auth/hybrid', () => ({ + checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth, + })) + vi.doMock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: mockAuthorizeWorkflowByWorkspacePermission, + })) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + it('returns 401 when unauthenticated', async () => { + mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ success: false }) + + const { GET } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/form/status') + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(401) + }) + + it('returns 403 when user lacks workspace access', async () => { + mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'session', + }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: false, + status: 403, + message: 'Access denied', + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: null, + }) + + const { GET } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/form/status') + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(403) + }) + + it('returns deployed form when authorized', async () => { + mockCheckSessionOrInternalAuth.mockResolvedValueOnce({ + success: true, + userId: 'user-1', + authType: 'session', + }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: true, + status: 200, + workflow: { id: 'wf-1', workspaceId: 'ws-1' }, + workspacePermission: 'read', + }) + mockDbLimit.mockResolvedValueOnce([ + { + id: 'form-1', + identifier: 'feedback-form', + title: 'Feedback', + isActive: true, + }, + ]) + + const { GET } = await import('./route') + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/form/status') + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.isDeployed).toBe(true) + expect(data.form.id).toBe('form-1') + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/form/status/route.ts b/apps/sim/app/api/workflows/[id]/form/status/route.ts index a14abe736f..6f22afafd6 100644 --- a/apps/sim/app/api/workflows/[id]/form/status/route.ts +++ b/apps/sim/app/api/workflows/[id]/form/status/route.ts @@ -3,20 +3,31 @@ import { form } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('FormStatusAPI') export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { try { - const session = await getSession() - - if (!session) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { return createErrorResponse('Unauthorized', 401) } const { id: workflowId } = await params + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId: auth.userId, + action: 'read', + }) + if (!authorization.allowed) { + return createErrorResponse( + authorization.message || 'Access denied', + authorization.status || 403 + ) + } const formResult = await db .select({ diff --git a/apps/sim/app/api/workflows/[id]/log/route.ts b/apps/sim/app/api/workflows/[id]/log/route.ts index 744b4b545b..dc50fa6bd4 100644 --- a/apps/sim/app/api/workflows/[id]/log/route.ts +++ b/apps/sim/app/api/workflows/[id]/log/route.ts @@ -4,6 +4,7 @@ import { z } from 'zod' import { generateRequestId } from '@/lib/core/utils/request' import { LoggingSession } from '@/lib/logs/execution/logging-session' import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' +import { getWorkspaceBilledAccountUserId } from '@/lib/workspaces/utils' import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' import type { ExecutionResult } from '@/executor/types' @@ -69,15 +70,19 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ const triggerType = isChatExecution ? 'chat' : 'manual' const loggingSession = new LoggingSession(id, executionId, triggerType, requestId) - const userId = accessValidation.workflow.userId const workspaceId = accessValidation.workflow.workspaceId if (!workspaceId) { logger.error(`[${requestId}] Workflow ${id} has no workspaceId`) return createErrorResponse('Workflow has no associated workspace', 500) } + const billedAccountUserId = await getWorkspaceBilledAccountUserId(workspaceId) + if (!billedAccountUserId) { + logger.error(`[${requestId}] Unable to resolve billed account for workspace ${workspaceId}`) + return createErrorResponse('Unable to resolve billing account for this workspace', 500) + } await loggingSession.safeStart({ - userId, + userId: billedAccountUserId, workspaceId, variables: {}, }) diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index 35f3d3473c..7012453b11 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -12,7 +12,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const mockGetSession = vi.fn() const mockLoadWorkflowFromNormalizedTables = vi.fn() const mockGetWorkflowById = vi.fn() -const mockGetWorkflowAccessContext = vi.fn() +const mockAuthorizeWorkflowByWorkspacePermission = vi.fn() const mockDbDelete = vi.fn() const mockDbUpdate = vi.fn() const mockDbSelect = vi.fn() @@ -35,8 +35,11 @@ vi.mock('@/lib/workflows/utils', async () => { return { ...actual, getWorkflowById: (workflowId: string) => mockGetWorkflowById(workflowId), - getWorkflowAccessContext: (workflowId: string, userId?: string) => - mockGetWorkflowAccessContext(workflowId, userId), + authorizeWorkflowByWorkspacePermission: (params: { + workflowId: string + userId: string + action?: 'read' | 'write' | 'admin' + }) => mockAuthorizeWorkflowByWorkspacePermission(params), } }) @@ -86,13 +89,6 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(null) - mockGetWorkflowAccessContext.mockResolvedValue({ - workflow: null, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: false, - isWorkspaceOwner: false, - }) const req = new NextRequest('http://localhost:3000/api/workflows/nonexistent') const params = Promise.resolve({ id: 'nonexistent' }) @@ -104,12 +100,12 @@ describe('Workflow By ID API Route', () => { expect(data.error).toBe('Workflow not found') }) - it.concurrent('should allow access when user owns the workflow', async () => { + it.concurrent('should allow access when user has admin workspace permission', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', name: 'Test Workflow', - workspaceId: null, + workspaceId: 'workspace-456', } const mockNormalizedData = { @@ -125,12 +121,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'admin', }) mockLoadWorkflowFromNormalizedTables.mockResolvedValue(mockNormalizedData) @@ -166,12 +161,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: 'workspace-456', workspacePermission: 'read', - isOwner: false, - isWorkspaceOwner: false, }) mockLoadWorkflowFromNormalizedTables.mockResolvedValue(mockNormalizedData) @@ -199,12 +193,12 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to read this workflow', workflow: mockWorkflow, - workspaceOwnerId: 'workspace-456', workspacePermission: null, - isOwner: false, - isWorkspaceOwner: false, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123') @@ -214,7 +208,7 @@ describe('Workflow By ID API Route', () => { expect(response.status).toBe(403) const data = await response.json() - expect(data.error).toBe('Access denied') + expect(data.error).toBe('Unauthorized: Access denied to read this workflow') }) it.concurrent('should use normalized tables when available', async () => { @@ -222,7 +216,7 @@ describe('Workflow By ID API Route', () => { id: 'workflow-123', userId: 'user-123', name: 'Test Workflow', - workspaceId: null, + workspaceId: 'workspace-456', } const mockNormalizedData = { @@ -238,12 +232,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'admin', }) mockLoadWorkflowFromNormalizedTables.mockResolvedValue(mockNormalizedData) @@ -261,12 +254,12 @@ describe('Workflow By ID API Route', () => { }) describe('DELETE /api/workflows/[id]', () => { - it('should allow owner to delete workflow', async () => { + it('should allow admin to delete workflow', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', name: 'Test Workflow', - workspaceId: null, + workspaceId: 'workspace-456', } mockGetSession.mockResolvedValue({ @@ -274,12 +267,17 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'admin', + }) + + mockDbSelect.mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }, { id: 'workflow-456' }]), + }), }) mockDbDelete.mockReturnValue({ @@ -315,12 +313,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: 'workspace-456', workspacePermission: 'admin', - isOwner: false, - isWorkspaceOwner: false, }) // Mock db.select() to return multiple workflows so deletion is allowed @@ -363,12 +360,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: 'workspace-456', workspacePermission: 'admin', - isOwner: true, - isWorkspaceOwner: false, }) // Mock db.select() to return only 1 workflow (the one being deleted) @@ -403,12 +399,12 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to admin this workflow', workflow: mockWorkflow, - workspaceOwnerId: 'workspace-456', workspacePermission: null, - isOwner: false, - isWorkspaceOwner: false, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { @@ -420,17 +416,17 @@ describe('Workflow By ID API Route', () => { expect(response.status).toBe(403) const data = await response.json() - expect(data.error).toBe('Access denied') + expect(data.error).toBe('Unauthorized: Access denied to admin this workflow') }) }) describe('PUT /api/workflows/[id]', () => { - it('should allow owner to update workflow', async () => { + it('should allow user with write permission to update workflow', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', name: 'Test Workflow', - workspaceId: null, + workspaceId: 'workspace-456', } const updateData = { name: 'Updated Workflow' } @@ -441,12 +437,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'write', }) mockDbUpdate.mockReturnValue({ @@ -486,12 +481,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: 'workspace-456', workspacePermission: 'write', - isOwner: false, - isWorkspaceOwner: false, }) mockDbUpdate.mockReturnValue({ @@ -530,12 +524,12 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to write this workflow', workflow: mockWorkflow, - workspaceOwnerId: 'workspace-456', workspacePermission: 'read', - isOwner: false, - isWorkspaceOwner: false, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { @@ -548,7 +542,7 @@ describe('Workflow By ID API Route', () => { expect(response.status).toBe(403) const data = await response.json() - expect(data.error).toBe('Access denied') + expect(data.error).toBe('Unauthorized: Access denied to write this workflow') }) it.concurrent('should validate request data', async () => { @@ -556,7 +550,7 @@ describe('Workflow By ID API Route', () => { id: 'workflow-123', userId: 'user-123', name: 'Test Workflow', - workspaceId: null, + workspaceId: 'workspace-456', } mockGetSession.mockResolvedValue({ @@ -564,12 +558,11 @@ describe('Workflow By ID API Route', () => { }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockGetWorkflowAccessContext.mockResolvedValue({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'write', }) const invalidData = { name: '' } diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 2ceee222d7..b0964b91f6 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -4,14 +4,12 @@ import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { authenticateApiKeyFromHeader, updateApiKeyLastUsed } from '@/lib/api-key/service' -import { getSession } from '@/lib/auth' -import { verifyInternalToken } from '@/lib/auth/internal' +import { checkHybridAuth, checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { env } from '@/lib/core/config/env' import { PlatformEvents } from '@/lib/core/telemetry' import { generateRequestId } from '@/lib/core/utils/request' import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils' -import { getWorkflowAccessContext, getWorkflowById } from '@/lib/workflows/utils' +import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils' const logger = createLogger('WorkflowByIdAPI') @@ -34,50 +32,14 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ const { id: workflowId } = await params try { - const authHeader = request.headers.get('authorization') - let isInternalCall = false - - if (authHeader?.startsWith('Bearer ')) { - const token = authHeader.split(' ')[1] - const verification = await verifyInternalToken(token) - isInternalCall = verification.valid + const auth = await checkHybridAuth(request, { requireWorkflowId: false }) + if (!auth.success) { + logger.warn(`[${requestId}] Unauthorized access attempt for workflow ${workflowId}`) + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - let userId: string | null = null - - if (isInternalCall) { - logger.info(`[${requestId}] Internal API call for workflow ${workflowId}`) - } else { - const session = await getSession() - let authenticatedUserId: string | null = session?.user?.id || null - - if (!authenticatedUserId) { - const apiKeyHeader = request.headers.get('x-api-key') - if (apiKeyHeader) { - const authResult = await authenticateApiKeyFromHeader(apiKeyHeader) - if (authResult.success && authResult.userId) { - authenticatedUserId = authResult.userId - if (authResult.keyId) { - await updateApiKeyLastUsed(authResult.keyId).catch((error) => { - logger.warn(`[${requestId}] Failed to update API key last used timestamp:`, { - keyId: authResult.keyId, - error, - }) - }) - } - } - } - } - - if (!authenticatedUserId) { - logger.warn(`[${requestId}] Unauthorized access attempt for workflow ${workflowId}`) - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) - } + const userId = auth.userId || null - userId = authenticatedUserId - } - - let accessContext = null let workflowData = await getWorkflowById(workflowId) if (!workflowData) { @@ -86,36 +48,28 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ } // Check if user has access to this workflow - let hasAccess = false - - if (isInternalCall) { - // Internal calls have full access - hasAccess = true - } else { - // Case 1: User owns the workflow - if (workflowData) { - accessContext = await getWorkflowAccessContext(workflowId, userId ?? undefined) - - if (!accessContext) { - logger.warn(`[${requestId}] Workflow ${workflowId} not found`) - return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) - } - - workflowData = accessContext.workflow - - if (accessContext.isOwner) { - hasAccess = true - } + if (!userId) { + logger.warn(`[${requestId}] Unauthorized access attempt for workflow ${workflowId}`) + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } - if (!hasAccess && workflowData.workspaceId && accessContext.workspacePermission) { - hasAccess = true - } - } + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'read', + }) + if (!authorization.workflow) { + logger.warn(`[${requestId}] Workflow ${workflowId} not found`) + return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) + } - if (!hasAccess) { - logger.warn(`[${requestId}] User ${userId} denied access to workflow ${workflowId}`) - return NextResponse.json({ error: 'Access denied' }, { status: 403 }) - } + workflowData = authorization.workflow + if (!authorization.allowed) { + logger.warn(`[${requestId}] User ${userId} denied access to workflow ${workflowId}`) + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status } + ) } logger.debug(`[${requestId}] Attempting to load workflow ${workflowId} from normalized tables`) @@ -196,43 +150,36 @@ export async function DELETE( const { id: workflowId } = await params try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized deletion attempt for workflow ${workflowId}`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - const userId = session.user.id + const userId = auth.userId - const accessContext = await getWorkflowAccessContext(workflowId, userId) - const workflowData = accessContext?.workflow || (await getWorkflowById(workflowId)) + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'admin', + }) + const workflowData = authorization.workflow || (await getWorkflowById(workflowId)) if (!workflowData) { logger.warn(`[${requestId}] Workflow ${workflowId} not found for deletion`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - // Check if user has permission to delete this workflow - let canDelete = false - - // Case 1: User owns the workflow - if (workflowData.userId === userId) { - canDelete = true - } - - // Case 2: Workflow belongs to a workspace and user has admin permission - if (!canDelete && workflowData.workspaceId) { - const context = accessContext || (await getWorkflowAccessContext(workflowId, userId)) - if (context?.workspacePermission === 'admin') { - canDelete = true - } - } + const canDelete = authorization.allowed if (!canDelete) { logger.warn( `[${requestId}] User ${userId} denied permission to delete workflow ${workflowId}` ) - return NextResponse.json({ error: 'Access denied' }, { status: 403 }) + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status || 403 } + ) } // Check if this is the last workflow in the workspace @@ -403,48 +350,40 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ const { id: workflowId } = await params try { - // Get the session - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized update attempt for workflow ${workflowId}`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - const userId = session.user.id + const userId = auth.userId const body = await request.json() const updates = UpdateWorkflowSchema.parse(body) // Fetch the workflow to check ownership/access - const accessContext = await getWorkflowAccessContext(workflowId, userId) - const workflowData = accessContext?.workflow || (await getWorkflowById(workflowId)) + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'write', + }) + const workflowData = authorization.workflow || (await getWorkflowById(workflowId)) if (!workflowData) { logger.warn(`[${requestId}] Workflow ${workflowId} not found for update`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - // Check if user has permission to update this workflow - let canUpdate = false - - // Case 1: User owns the workflow - if (workflowData.userId === userId) { - canUpdate = true - } - - // Case 2: Workflow belongs to a workspace and user has write or admin permission - if (!canUpdate && workflowData.workspaceId) { - const context = accessContext || (await getWorkflowAccessContext(workflowId, userId)) - if (context?.workspacePermission === 'write' || context?.workspacePermission === 'admin') { - canUpdate = true - } - } + const canUpdate = authorization.allowed if (!canUpdate) { logger.warn( `[${requestId}] User ${userId} denied permission to update workflow ${workflowId}` ) - return NextResponse.json({ error: 'Access denied' }, { status: 403 }) + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status || 403 } + ) } const updateData: Record = { updatedAt: new Date() } diff --git a/apps/sim/app/api/workflows/[id]/state/route.ts b/apps/sim/app/api/workflows/[id]/state/route.ts index f99f47c275..1e7beaefcd 100644 --- a/apps/sim/app/api/workflows/[id]/state/route.ts +++ b/apps/sim/app/api/workflows/[id]/state/route.ts @@ -4,13 +4,13 @@ import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { env } from '@/lib/core/config/env' import { generateRequestId } from '@/lib/core/utils/request' import { extractAndPersistCustomTools } from '@/lib/workflows/persistence/custom-tools-persistence' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' import { sanitizeAgentToolsInBlocks } from '@/lib/workflows/sanitization/validation' -import { getWorkflowAccessContext } from '@/lib/workflows/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import type { BlockState } from '@/stores/workflows/workflow/types' import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils' @@ -118,40 +118,38 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ const { id: workflowId } = await params try { - // Get the session - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized state update attempt for workflow ${workflowId}`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - - const userId = session.user.id + const userId = auth.userId const body = await request.json() const state = WorkflowStateSchema.parse(body) - // Fetch the workflow to check ownership/access - const accessContext = await getWorkflowAccessContext(workflowId, userId) - const workflowData = accessContext?.workflow + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'write', + }) + const workflowData = authorization.workflow if (!workflowData) { logger.warn(`[${requestId}] Workflow ${workflowId} not found for state update`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - // Check if user has permission to update this workflow - const canUpdate = - accessContext?.isOwner || - (workflowData.workspaceId - ? accessContext?.workspacePermission === 'write' || - accessContext?.workspacePermission === 'admin' - : false) + const canUpdate = authorization.allowed if (!canUpdate) { logger.warn( `[${requestId}] User ${userId} denied permission to update workflow state ${workflowId}` ) - return NextResponse.json({ error: 'Access denied' }, { status: 403 }) + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status || 403 } + ) } // Sanitize custom tools in agent blocks before saving diff --git a/apps/sim/app/api/workflows/[id]/variables/route.test.ts b/apps/sim/app/api/workflows/[id]/variables/route.test.ts index 949b52ebc4..1e67caf3bf 100644 --- a/apps/sim/app/api/workflows/[id]/variables/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/variables/route.test.ts @@ -16,19 +16,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' describe('Workflow Variables API Route', () => { let authMocks: ReturnType - const mockGetWorkflowAccessContext = vi.fn() + const mockAuthorizeWorkflowByWorkspacePermission = vi.fn() beforeEach(() => { vi.resetModules() setupCommonApiMocks() mockCryptoUuid('mock-request-id-12345678') authMocks = mockAuth(defaultMockUser) - mockGetWorkflowAccessContext.mockReset() + mockAuthorizeWorkflowByWorkspacePermission.mockReset() vi.doMock('@sim/db', () => databaseMock) vi.doMock('@/lib/workflows/utils', () => ({ - getWorkflowAccessContext: mockGetWorkflowAccessContext, + authorizeWorkflowByWorkspacePermission: mockAuthorizeWorkflowByWorkspacePermission, })) }) @@ -43,7 +43,7 @@ describe('Workflow Variables API Route', () => { const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') const params = Promise.resolve({ id: 'workflow-123' }) - const { GET } = await import('@/app/api/workflows/[id]/variables/route') + const { GET } = await import('./route') const response = await GET(req, { params }) expect(response.status).toBe(401) @@ -53,12 +53,18 @@ describe('Workflow Variables API Route', () => { it('should return 404 when workflow does not exist', async () => { authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce(null) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: false, + status: 404, + message: 'Workflow not found', + workflow: null, + workspacePermission: null, + }) const req = new NextRequest('http://localhost:3000/api/workflows/nonexistent/variables') const params = Promise.resolve({ id: 'nonexistent' }) - const { GET } = await import('@/app/api/workflows/[id]/variables/route') + const { GET } = await import('./route') const response = await GET(req, { params }) expect(response.status).toBe(404) @@ -66,29 +72,28 @@ describe('Workflow Variables API Route', () => { expect(data.error).toBe('Workflow not found') }) - it('should allow access when user owns the workflow', async () => { + it('should allow access when user has workspace permission', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', - workspaceId: null, + workspaceId: 'workspace-456', variables: { 'var-1': { id: 'var-1', name: 'test', type: 'string', value: 'hello' }, }, } authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'admin', }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') const params = Promise.resolve({ id: 'workflow-123' }) - const { GET } = await import('@/app/api/workflows/[id]/variables/route') + const { GET } = await import('./route') const response = await GET(req, { params }) expect(response.status).toBe(200) @@ -107,18 +112,17 @@ describe('Workflow Variables API Route', () => { } authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: 'workspace-owner', workspacePermission: 'read', - isOwner: false, - isWorkspaceOwner: false, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') const params = Promise.resolve({ id: 'workflow-123' }) - const { GET } = await import('@/app/api/workflows/[id]/variables/route') + const { GET } = await import('./route') const response = await GET(req, { params }) expect(response.status).toBe(200) @@ -135,48 +139,47 @@ describe('Workflow Variables API Route', () => { } authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to read this workflow', workflow: mockWorkflow, - workspaceOwnerId: 'workspace-owner', workspacePermission: null, - isOwner: false, - isWorkspaceOwner: false, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') const params = Promise.resolve({ id: 'workflow-123' }) - const { GET } = await import('@/app/api/workflows/[id]/variables/route') + const { GET } = await import('./route') const response = await GET(req, { params }) - expect(response.status).toBe(401) + expect(response.status).toBe(403) const data = await response.json() - expect(data.error).toBe('Unauthorized') + expect(data.error).toBe('Unauthorized: Access denied to read this workflow') }) it.concurrent('should include proper cache headers', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', - workspaceId: null, + workspaceId: 'workspace-456', variables: { 'var-1': { id: 'var-1', name: 'test', type: 'string', value: 'hello' }, }, } authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'admin', }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') const params = Promise.resolve({ id: 'workflow-123' }) - const { GET } = await import('@/app/api/workflows/[id]/variables/route') + const { GET } = await import('./route') const response = await GET(req, { params }) expect(response.status).toBe(200) @@ -186,21 +189,20 @@ describe('Workflow Variables API Route', () => { }) describe('POST /api/workflows/[id]/variables', () => { - it('should allow owner to update variables', async () => { + it('should allow user with write permission to update variables', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', - workspaceId: null, + workspaceId: 'workspace-456', variables: {}, } authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'write', }) const variables = { @@ -219,7 +221,7 @@ describe('Workflow Variables API Route', () => { }) const params = Promise.resolve({ id: 'workflow-123' }) - const { POST } = await import('@/app/api/workflows/[id]/variables/route') + const { POST } = await import('./route') const response = await POST(req, { params }) expect(response.status).toBe(200) @@ -236,12 +238,12 @@ describe('Workflow Variables API Route', () => { } authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to write this workflow', workflow: mockWorkflow, - workspaceOwnerId: 'workspace-owner', workspacePermission: null, - isOwner: false, - isWorkspaceOwner: false, }) const variables = { @@ -260,29 +262,28 @@ describe('Workflow Variables API Route', () => { }) const params = Promise.resolve({ id: 'workflow-123' }) - const { POST } = await import('@/app/api/workflows/[id]/variables/route') + const { POST } = await import('./route') const response = await POST(req, { params }) - expect(response.status).toBe(401) + expect(response.status).toBe(403) const data = await response.json() - expect(data.error).toBe('Unauthorized') + expect(data.error).toBe('Unauthorized: Access denied to write this workflow') }) it.concurrent('should validate request data schema', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', - workspaceId: null, + workspaceId: 'workspace-456', variables: {}, } authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockResolvedValueOnce({ + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({ + allowed: true, + status: 200, workflow: mockWorkflow, - workspaceOwnerId: null, - workspacePermission: null, - isOwner: true, - isWorkspaceOwner: false, + workspacePermission: 'write', }) const invalidData = { variables: [{ name: 'test' }] } @@ -293,7 +294,7 @@ describe('Workflow Variables API Route', () => { }) const params = Promise.resolve({ id: 'workflow-123' }) - const { POST } = await import('@/app/api/workflows/[id]/variables/route') + const { POST } = await import('./route') const response = await POST(req, { params }) expect(response.status).toBe(400) @@ -305,12 +306,14 @@ describe('Workflow Variables API Route', () => { describe('Error handling', () => { it.concurrent('should handle database errors gracefully', async () => { authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) - mockGetWorkflowAccessContext.mockRejectedValueOnce(new Error('Database connection failed')) + mockAuthorizeWorkflowByWorkspacePermission.mockRejectedValueOnce( + new Error('Database connection failed') + ) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') const params = Promise.resolve({ id: 'workflow-123' }) - const { GET } = await import('@/app/api/workflows/[id]/variables/route') + const { GET } = await import('./route') const response = await GET(req, { params }) expect(response.status).toBe(500) diff --git a/apps/sim/app/api/workflows/[id]/variables/route.ts b/apps/sim/app/api/workflows/[id]/variables/route.ts index f107f31748..786103756d 100644 --- a/apps/sim/app/api/workflows/[id]/variables/route.ts +++ b/apps/sim/app/api/workflows/[id]/variables/route.ts @@ -4,9 +4,9 @@ import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' -import { getWorkflowAccessContext } from '@/lib/workflows/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import type { Variable } from '@/stores/panel/variables/types' const logger = createLogger('WorkflowVariablesAPI') @@ -34,31 +34,34 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: const workflowId = (await params).id try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized workflow variables update attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - // Get the workflow record - const accessContext = await getWorkflowAccessContext(workflowId, session.user.id) - const workflowData = accessContext?.workflow + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'write', + }) + const workflowData = authorization.workflow if (!workflowData) { logger.warn(`[${requestId}] Workflow not found: ${workflowId}`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - const workspaceId = workflowData.workspaceId - - // Check authorization - either the user owns the workflow or has workspace permissions - const isAuthorized = - accessContext?.isOwner || (workspaceId ? accessContext?.workspacePermission !== null : false) + const isAuthorized = authorization.allowed if (!isAuthorized) { logger.warn( - `[${requestId}] User ${session.user.id} attempted to update variables for workflow ${workflowId} without permission` + `[${requestId}] User ${userId} attempted to update variables for workflow ${workflowId} without permission` + ) + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status || 403 } ) - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } const body = await req.json() @@ -100,32 +103,34 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: const workflowId = (await params).id try { - // Get the session directly in the API route - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized workflow variables access attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId - // Get the workflow record - const accessContext = await getWorkflowAccessContext(workflowId, session.user.id) - const workflowData = accessContext?.workflow + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'read', + }) + const workflowData = authorization.workflow if (!workflowData) { logger.warn(`[${requestId}] Workflow not found: ${workflowId}`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - const workspaceId = workflowData.workspaceId - - // Check authorization - either the user owns the workflow or has workspace permissions - const isAuthorized = - accessContext?.isOwner || (workspaceId ? accessContext?.workspacePermission !== null : false) + const isAuthorized = authorization.allowed if (!isAuthorized) { logger.warn( - `[${requestId}] User ${session.user.id} attempted to access variables for workflow ${workflowId} without permission` + `[${requestId}] User ${userId} attempted to access variables for workflow ${workflowId} without permission` + ) + return NextResponse.json( + { error: authorization.message || 'Access denied' }, + { status: authorization.status || 403 } ) - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } // Return variables if they exist diff --git a/apps/sim/app/api/workflows/middleware.ts b/apps/sim/app/api/workflows/middleware.ts index d3cbfa3b6d..d6260002fe 100644 --- a/apps/sim/app/api/workflows/middleware.ts +++ b/apps/sim/app/api/workflows/middleware.ts @@ -5,14 +5,16 @@ import { authenticateApiKeyFromHeader, updateApiKeyLastUsed, } from '@/lib/api-key/service' +import { type AuthResult, checkHybridAuth } from '@/lib/auth/hybrid' import { env } from '@/lib/core/config/env' -import { getWorkflowById } from '@/lib/workflows/utils' +import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils' const logger = createLogger('WorkflowMiddleware') export interface ValidationResult { error?: { message: string; status: number } workflow?: any + auth?: AuthResult } export async function validateWorkflowAccess( @@ -31,6 +33,44 @@ export async function validateWorkflowAccess( } } + if (!workflow.workspaceId) { + return { + error: { + message: + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', + status: 403, + }, + } + } + + if (!requireDeployment) { + const auth = await checkHybridAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { + return { + error: { + message: auth.error || 'Unauthorized', + status: 401, + }, + } + } + + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId: auth.userId, + action: 'read', + }) + if (!authorization.allowed) { + return { + error: { + message: authorization.message || 'Access denied', + status: authorization.status, + }, + } + } + + return { workflow, auth } + } + if (requireDeployment) { if (!workflow.isDeployed) { return { @@ -65,24 +105,13 @@ export async function validateWorkflowAccess( let validResult: ApiKeyAuthResult | null = null - if (workflow.workspaceId) { - const workspaceResult = await authenticateApiKeyFromHeader(apiKeyHeader, { - workspaceId: workflow.workspaceId as string, - keyTypes: ['workspace', 'personal'], - }) + const workspaceResult = await authenticateApiKeyFromHeader(apiKeyHeader, { + workspaceId: workflow.workspaceId as string, + keyTypes: ['workspace', 'personal'], + }) - if (workspaceResult.success) { - validResult = workspaceResult - } - } else { - const personalResult = await authenticateApiKeyFromHeader(apiKeyHeader, { - userId: workflow.userId as string, - keyTypes: ['personal'], - }) - - if (personalResult.success) { - validResult = personalResult - } + if (workspaceResult.success) { + validResult = workspaceResult } if (!validResult) { diff --git a/apps/sim/app/api/workflows/reorder/route.ts b/apps/sim/app/api/workflows/reorder/route.ts index 3989be4e99..d574ea1e66 100644 --- a/apps/sim/app/api/workflows/reorder/route.ts +++ b/apps/sim/app/api/workflows/reorder/route.ts @@ -4,7 +4,7 @@ import { createLogger } from '@sim/logger' import { eq, inArray } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' @@ -23,21 +23,21 @@ const ReorderSchema = z.object({ export async function PUT(req: NextRequest) { const requestId = generateRequestId() - const session = await getSession() - - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized reorder attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId try { const body = await req.json() const { workspaceId, updates } = ReorderSchema.parse(body) - const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) + const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) if (!permission || permission === 'read') { logger.warn( - `[${requestId}] User ${session.user.id} lacks write permission for workspace ${workspaceId}` + `[${requestId}] User ${userId} lacks write permission for workspace ${workspaceId}` ) return NextResponse.json({ error: 'Write access required' }, { status: 403 }) } diff --git a/apps/sim/app/api/workflows/route.ts b/apps/sim/app/api/workflows/route.ts index a35bb07526..b21435ed20 100644 --- a/apps/sim/app/api/workflows/route.ts +++ b/apps/sim/app/api/workflows/route.ts @@ -1,10 +1,10 @@ import { db } from '@sim/db' -import { workflow } from '@sim/db/schema' +import { permissions, workflow } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, asc, eq, isNull, min } from 'drizzle-orm' +import { and, asc, eq, inArray, isNull, min } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' -import { getSession } from '@/lib/auth' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { getUserEntityPermissions, workspaceExists } from '@/lib/workspaces/permissions/utils' import { verifyWorkspaceMembership } from '@/app/api/workflows/utils' @@ -21,20 +21,19 @@ const CreateWorkflowSchema = z.object({ }) // GET /api/workflows - Get workflows for user (optionally filtered by workspaceId) -export async function GET(request: Request) { +export async function GET(request: NextRequest) { const requestId = generateRequestId() const startTime = Date.now() const url = new URL(request.url) const workspaceId = url.searchParams.get('workspaceId') try { - const session = await getSession() - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized workflow access attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - - const userId = session.user.id + const userId = auth.userId if (workspaceId) { const wsExists = await workspaceExists(workspaceId) @@ -73,10 +72,18 @@ export async function GET(request: Request) { .where(eq(workflow.workspaceId, workspaceId)) .orderBy(...orderByClause) } else { + const workspacePermissionRows = await db + .select({ workspaceId: permissions.entityId }) + .from(permissions) + .where(and(eq(permissions.userId, userId), eq(permissions.entityType, 'workspace'))) + const workspaceIds = workspacePermissionRows.map((row) => row.workspaceId) + if (workspaceIds.length === 0) { + return NextResponse.json({ data: [] }, { status: 200 }) + } workflows = await db .select() .from(workflow) - .where(eq(workflow.userId, userId)) + .where(inArray(workflow.workspaceId, workspaceIds)) .orderBy(...orderByClause) } @@ -91,12 +98,12 @@ export async function GET(request: Request) { // POST /api/workflows - Create a new workflow export async function POST(req: NextRequest) { const requestId = generateRequestId() - const session = await getSession() - - if (!session?.user?.id) { + const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) + if (!auth.success || !auth.userId) { logger.warn(`[${requestId}] Unauthorized workflow creation attempt`) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } + const userId = auth.userId try { const body = await req.json() @@ -109,28 +116,33 @@ export async function POST(req: NextRequest) { sortOrder: providedSortOrder, } = CreateWorkflowSchema.parse(body) - if (workspaceId) { - const workspacePermission = await getUserEntityPermissions( - session.user.id, - 'workspace', - workspaceId + if (!workspaceId) { + logger.warn(`[${requestId}] Workflow creation blocked: missing workspaceId`) + return NextResponse.json( + { + error: + 'workspaceId is required. Personal workflows are deprecated and cannot be created.', + }, + { status: 400 } ) + } - if (!workspacePermission || workspacePermission === 'read') { - logger.warn( - `[${requestId}] User ${session.user.id} attempted to create workflow in workspace ${workspaceId} without write permissions` - ) - return NextResponse.json( - { error: 'Write or Admin access required to create workflows in this workspace' }, - { status: 403 } - ) - } + const workspacePermission = await getUserEntityPermissions(userId, 'workspace', workspaceId) + + if (!workspacePermission || workspacePermission === 'read') { + logger.warn( + `[${requestId}] User ${userId} attempted to create workflow in workspace ${workspaceId} without write permissions` + ) + return NextResponse.json( + { error: 'Write or Admin access required to create workflows in this workspace' }, + { status: 403 } + ) } const workflowId = crypto.randomUUID() const now = new Date() - logger.info(`[${requestId}] Creating workflow ${workflowId} for user ${session.user.id}`) + logger.info(`[${requestId}] Creating workflow ${workflowId} for user ${userId}`) import('@/lib/core/telemetry') .then(({ PlatformEvents }) => { @@ -153,18 +165,14 @@ export async function POST(req: NextRequest) { const [minResult] = await db .select({ minOrder: min(workflow.sortOrder) }) .from(workflow) - .where( - workspaceId - ? and(eq(workflow.workspaceId, workspaceId), folderCondition) - : and(eq(workflow.userId, session.user.id), folderCondition) - ) + .where(and(eq(workflow.workspaceId, workspaceId), folderCondition)) sortOrder = (minResult?.minOrder ?? 1) - 1 } await db.insert(workflow).values({ id: workflowId, - userId: session.user.id, - workspaceId: workspaceId || null, + userId, + workspaceId, folderId: folderId || null, sortOrder, name, diff --git a/apps/sim/background/workspace-notification-delivery.ts b/apps/sim/background/workspace-notification-delivery.ts index 0d9a8254b8..3dd22af7d2 100644 --- a/apps/sim/background/workspace-notification-delivery.ts +++ b/apps/sim/background/workspace-notification-delivery.ts @@ -24,6 +24,7 @@ import { getBaseUrl } from '@/lib/core/utils/urls' import type { TraceSpan, WorkflowExecutionLog } from '@/lib/logs/types' import { sendEmail } from '@/lib/messaging/email/mailer' import type { AlertConfig } from '@/lib/notifications/alert-rules' +import { getWorkspaceBilledAccountUserId } from '@/lib/workspaces/utils' const logger = createLogger('WorkspaceNotificationDelivery') @@ -72,14 +73,20 @@ async function buildPayload( if (!log.workflowId) return null const workflowData = await db - .select({ name: workflowTable.name, userId: workflowTable.userId }) + .select({ + name: workflowTable.name, + workspaceId: workflowTable.workspaceId, + }) .from(workflowTable) .where(eq(workflowTable.id, log.workflowId)) .limit(1) const timestamp = Date.now() const executionData = (log.executionData || {}) as Record - const userId = workflowData[0]?.userId + const workflowRecord = workflowData[0] + const userId = workflowRecord?.workspaceId + ? await getWorkspaceBilledAccountUserId(workflowRecord.workspaceId) + : null const payload: NotificationPayload = { id: `evt_${uuidv4()}`, diff --git a/apps/sim/lib/billing/core/usage-log.ts b/apps/sim/lib/billing/core/usage-log.ts index 9c4e6851c6..b21fb552f7 100644 --- a/apps/sim/lib/billing/core/usage-log.ts +++ b/apps/sim/lib/billing/core/usage-log.ts @@ -1,5 +1,5 @@ import { db } from '@sim/db' -import { usageLog, workflow } from '@sim/db/schema' +import { usageLog } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, desc, eq, gte, lte, sql } from 'drizzle-orm' import { isBillingEnabled } from '@/lib/core/config/feature-flags' @@ -397,25 +397,3 @@ export async function getUserUsageLogs( throw error } } - -/** - * Get the user ID associated with a workflow - * Helper function for cases where we only have a workflow ID - */ -export async function getUserIdFromWorkflow(workflowId: string): Promise { - try { - const [workflowRecord] = await db - .select({ userId: workflow.userId }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1) - - return workflowRecord?.userId ?? null - } catch (error) { - logger.error('Failed to get user ID from workflow', { - error: error instanceof Error ? error.message : String(error), - workflowId, - }) - return null - } -} diff --git a/apps/sim/lib/copilot/auth/permissions.test.ts b/apps/sim/lib/copilot/auth/permissions.test.ts index ff1b121d78..7821e4cbb4 100644 --- a/apps/sim/lib/copilot/auth/permissions.test.ts +++ b/apps/sim/lib/copilot/auth/permissions.test.ts @@ -29,7 +29,6 @@ describe('Copilot Auth Permissions', () => { vi.doMock('@sim/db/schema', () => ({ workflow: { id: 'id', - userId: 'userId', workspaceId: 'workspaceId', }, })) @@ -58,49 +57,11 @@ describe('Copilot Auth Permissions', () => { expect(result).toEqual({ hasAccess: false, userPermission: null, - isOwner: false, }) }) - it('should return admin access for workflow owner', async () => { + it('should check workspace permissions for workflow with workspace', async () => { const workflowData = { - userId: 'user-123', - workspaceId: 'workspace-456', - } - mockLimit.mockResolvedValueOnce([workflowData]) - - const { verifyWorkflowAccess } = await import('@/lib/copilot/auth/permissions') - const result = await verifyWorkflowAccess('user-123', 'workflow-789') - - expect(result).toEqual({ - hasAccess: true, - userPermission: 'admin', - workspaceId: 'workspace-456', - isOwner: true, - }) - }) - - it('should return admin access for workflow owner without workspace', async () => { - const workflowData = { - userId: 'user-123', - workspaceId: null, - } - mockLimit.mockResolvedValueOnce([workflowData]) - - const { verifyWorkflowAccess } = await import('@/lib/copilot/auth/permissions') - const result = await verifyWorkflowAccess('user-123', 'workflow-789') - - expect(result).toEqual({ - hasAccess: true, - userPermission: 'admin', - workspaceId: undefined, - isOwner: true, - }) - }) - - it('should check workspace permissions for non-owner with workspace', async () => { - const workflowData = { - userId: 'other-user', workspaceId: 'workspace-456', } mockLimit.mockResolvedValueOnce([workflowData]) @@ -115,7 +76,6 @@ describe('Copilot Auth Permissions', () => { hasAccess: true, userPermission: 'write', workspaceId: 'workspace-456', - isOwner: false, }) expect(getUserEntityPermissions).toHaveBeenCalledWith( @@ -127,7 +87,6 @@ describe('Copilot Auth Permissions', () => { it('should return read permission through workspace', async () => { const workflowData = { - userId: 'other-user', workspaceId: 'workspace-456', } mockLimit.mockResolvedValueOnce([workflowData]) @@ -142,13 +101,11 @@ describe('Copilot Auth Permissions', () => { hasAccess: true, userPermission: 'read', workspaceId: 'workspace-456', - isOwner: false, }) }) it('should return admin permission through workspace', async () => { const workflowData = { - userId: 'other-user', workspaceId: 'workspace-456', } mockLimit.mockResolvedValueOnce([workflowData]) @@ -163,13 +120,11 @@ describe('Copilot Auth Permissions', () => { hasAccess: true, userPermission: 'admin', workspaceId: 'workspace-456', - isOwner: false, }) }) - it('should return no access for non-owner without workspace permissions', async () => { + it('should return no access without workspace permissions', async () => { const workflowData = { - userId: 'other-user', workspaceId: 'workspace-456', } mockLimit.mockResolvedValueOnce([workflowData]) @@ -184,13 +139,11 @@ describe('Copilot Auth Permissions', () => { hasAccess: false, userPermission: null, workspaceId: 'workspace-456', - isOwner: false, }) }) - it('should return no access for non-owner of workflow without workspace', async () => { + it('should return no access for workflow without workspace', async () => { const workflowData = { - userId: 'other-user', workspaceId: null, } mockLimit.mockResolvedValueOnce([workflowData]) @@ -202,7 +155,6 @@ describe('Copilot Auth Permissions', () => { hasAccess: false, userPermission: null, workspaceId: undefined, - isOwner: false, }) }) @@ -215,7 +167,6 @@ describe('Copilot Auth Permissions', () => { expect(result).toEqual({ hasAccess: false, userPermission: null, - isOwner: false, }) }) @@ -237,7 +188,6 @@ describe('Copilot Auth Permissions', () => { expect(result).toEqual({ hasAccess: false, userPermission: null, - isOwner: false, }) }) }) diff --git a/apps/sim/lib/copilot/auth/permissions.ts b/apps/sim/lib/copilot/auth/permissions.ts index 037b7b0fc3..e45d61a551 100644 --- a/apps/sim/lib/copilot/auth/permissions.ts +++ b/apps/sim/lib/copilot/auth/permissions.ts @@ -11,7 +11,7 @@ const logger = createLogger('CopilotPermissions') * * @param userId - The authenticated user ID * @param workflowId - The workflow ID to check access for - * @returns Promise<{ hasAccess: boolean; userPermission: PermissionType | null; workspaceId?: string; isOwner: boolean }> + * @returns Promise<{ hasAccess: boolean; userPermission: PermissionType | null; workspaceId?: string }> */ export async function verifyWorkflowAccess( userId: string, @@ -20,12 +20,10 @@ export async function verifyWorkflowAccess( hasAccess: boolean userPermission: PermissionType | null workspaceId?: string - isOwner: boolean }> { try { const workflowData = await db .select({ - userId: workflow.userId, workspaceId: workflow.workspaceId, }) .from(workflow) @@ -37,37 +35,35 @@ export async function verifyWorkflowAccess( workflowId, userId, }) - return { hasAccess: false, userPermission: null, isOwner: false } + return { hasAccess: false, userPermission: null } } - const { userId: workflowOwnerId, workspaceId } = workflowData[0] - - if (workflowOwnerId === userId) { - logger.debug('User has direct ownership of workflow', { workflowId, userId }) + const { workspaceId } = workflowData[0] + if (!workspaceId) { + logger.warn('Workflow is not attached to a workspace; access denied', { + workflowId, + userId, + }) return { - hasAccess: true, - userPermission: 'admin', - workspaceId: workspaceId || undefined, - isOwner: true, + hasAccess: false, + userPermission: null, + workspaceId: undefined, } } - if (workspaceId && userId) { - const userPermission = await getUserEntityPermissions(userId, 'workspace', workspaceId) + const userPermission = await getUserEntityPermissions(userId, 'workspace', workspaceId) - if (userPermission !== null) { - logger.debug('User has workspace permission for workflow', { - workflowId, - userId, - workspaceId, - userPermission, - }) - return { - hasAccess: true, - userPermission, - workspaceId: workspaceId || undefined, - isOwner: false, - } + if (userPermission !== null) { + logger.debug('User has workspace permission for workflow', { + workflowId, + userId, + workspaceId, + userPermission, + }) + return { + hasAccess: true, + userPermission, + workspaceId, } } @@ -75,17 +71,15 @@ export async function verifyWorkflowAccess( workflowId, userId, workspaceId, - workflowOwnerId, }) return { hasAccess: false, userPermission: null, workspaceId: workspaceId || undefined, - isOwner: false, } } catch (error) { logger.error('Error verifying workflow access', { error, workflowId, userId }) - return { hasAccess: false, userPermission: null, isOwner: false } + return { hasAccess: false, userPermission: null } } } diff --git a/apps/sim/lib/copilot/orchestrator/tool-executor/access.ts b/apps/sim/lib/copilot/orchestrator/tool-executor/access.ts index b19459afa0..801750cc2b 100644 --- a/apps/sim/lib/copilot/orchestrator/tool-executor/access.ts +++ b/apps/sim/lib/copilot/orchestrator/tool-executor/access.ts @@ -1,6 +1,6 @@ import { db } from '@sim/db' import { permissions, workflow, workspace } from '@sim/db/schema' -import { and, asc, desc, eq, inArray, or } from 'drizzle-orm' +import { and, asc, desc, eq, inArray } from 'drizzle-orm' type WorkflowRecord = typeof workflow.$inferSelect @@ -20,25 +20,25 @@ export async function ensureWorkflowAccess( throw new Error(`Workflow ${workflowId} not found`) } - if (workflowRecord.userId === userId) { - return { workflow: workflowRecord, workspaceId: workflowRecord.workspaceId } + if (!workflowRecord.workspaceId) { + throw new Error( + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.' + ) } - if (workflowRecord.workspaceId) { - const [permissionRow] = await db - .select({ permissionType: permissions.permissionType }) - .from(permissions) - .where( - and( - eq(permissions.entityType, 'workspace'), - eq(permissions.entityId, workflowRecord.workspaceId), - eq(permissions.userId, userId) - ) + const [permissionRow] = await db + .select({ permissionType: permissions.permissionType }) + .from(permissions) + .where( + and( + eq(permissions.entityType, 'workspace'), + eq(permissions.entityId, workflowRecord.workspaceId), + eq(permissions.userId, userId) ) - .limit(1) - if (permissionRow) { - return { workflow: workflowRecord, workspaceId: workflowRecord.workspaceId } - } + ) + .limit(1) + if (permissionRow) { + return { workflow: workflowRecord, workspaceId: workflowRecord.workspaceId } } throw new Error('Unauthorized workflow access') @@ -69,10 +69,8 @@ export async function ensureWorkspaceAccess( const [row] = await db .select({ permissionType: permissions.permissionType, - ownerId: workspace.ownerId, }) .from(permissions) - .innerJoin(workspace, eq(permissions.entityId, workspace.id)) .where( and( eq(permissions.entityType, 'workspace'), @@ -86,9 +84,8 @@ export async function ensureWorkspaceAccess( throw new Error(`Workspace ${workspaceId} not found`) } - const isOwner = row.ownerId === userId const permissionType = row.permissionType - const canWrite = isOwner || permissionType === 'admin' || permissionType === 'write' + const canWrite = permissionType === 'admin' || permissionType === 'write' if (requireWrite && !canWrite) { throw new Error('Write or admin access required for this workspace') @@ -109,11 +106,15 @@ export async function getAccessibleWorkflowsForUser( .where(and(eq(permissions.userId, userId), eq(permissions.entityType, 'workspace'))) const workspaceIdList = workspaceIds.map((row) => row.entityId) + if (workspaceIdList.length === 0) { + return [] + } - const workflowConditions = [eq(workflow.userId, userId)] - if (workspaceIdList.length > 0) { - workflowConditions.push(inArray(workflow.workspaceId, workspaceIdList)) + if (options?.workspaceId && !workspaceIdList.includes(options.workspaceId)) { + return [] } + + const workflowConditions = [inArray(workflow.workspaceId, workspaceIdList)] if (options?.workspaceId) { workflowConditions.push(eq(workflow.workspaceId, options.workspaceId)) } @@ -124,6 +125,6 @@ export async function getAccessibleWorkflowsForUser( return db .select() .from(workflow) - .where(or(...workflowConditions)) + .where(and(...workflowConditions)) .orderBy(asc(workflow.sortOrder), asc(workflow.createdAt), asc(workflow.id)) } diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts index 5532c404a2..6b8592f79e 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts @@ -10,6 +10,7 @@ import { saveWorkflowToNormalizedTables, } from '@/lib/workflows/persistence/utils' import { validateWorkflowState } from '@/lib/workflows/sanitization/validation' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { getUserPermissionConfig } from '@/ee/access-control/utils/permission-check' import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils' import { applyOperationsToWorkflowState } from './engine' @@ -76,6 +77,18 @@ export const editWorkflowServerTool: BaseServerTool throw new Error('operations are required and must be an array') } if (!workflowId) throw new Error('workflowId is required') + if (!context?.userId) { + throw new Error('Unauthorized workflow access') + } + + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId: context.userId, + action: 'write', + }) + if (!authorization.allowed) { + throw new Error(authorization.message || 'Unauthorized workflow access') + } logger.info('Executing edit_workflow', { operationCount: operations.length, diff --git a/apps/sim/lib/copilot/tools/server/workflow/get-workflow-console.ts b/apps/sim/lib/copilot/tools/server/workflow/get-workflow-console.ts index 080e339692..1da90c5e68 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/get-workflow-console.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/get-workflow-console.ts @@ -3,6 +3,7 @@ import { workflowExecutionLogs } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { desc, eq } from 'drizzle-orm' import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' const logger = createLogger('GetWorkflowConsoleServerTool') @@ -223,7 +224,7 @@ function deriveExecutionErrorSummary(params: { export const getWorkflowConsoleServerTool: BaseServerTool = { name: 'get_workflow_console', - async execute(rawArgs: GetWorkflowConsoleArgs): Promise { + async execute(rawArgs: GetWorkflowConsoleArgs, context?: { userId: string }): Promise { const { workflowId, limit = 2, @@ -233,6 +234,18 @@ export const getWorkflowConsoleServerTool: BaseServerTool { try { logger.info(`[${requestId}] Querying knowledge base`, { @@ -62,6 +67,8 @@ async function queryKnowledgeBase( method: 'POST', headers: { 'Content-Type': 'application/json', + ...(authHeaders?.cookie ? { Cookie: authHeaders.cookie } : {}), + ...(authHeaders?.authorization ? { Authorization: authHeaders.authorization } : {}), }, body: JSON.stringify({ knowledgeBaseIds: [knowledgeBaseId], @@ -236,6 +243,7 @@ export async function validateHallucination( providerCredentials, workflowId, workspaceId, + authHeaders, requestId, } = input @@ -260,7 +268,8 @@ export async function validateHallucination( userInput, topK, requestId, - workflowId + workflowId, + authHeaders ) if (ragContext.length === 0) { diff --git a/apps/sim/lib/knowledge/service.ts b/apps/sim/lib/knowledge/service.ts index d065ed5699..863a39b5cc 100644 --- a/apps/sim/lib/knowledge/service.ts +++ b/apps/sim/lib/knowledge/service.ts @@ -87,7 +87,7 @@ export async function createKnowledgeBase( const now = new Date() const hasPermission = await getUserEntityPermissions(data.userId, 'workspace', data.workspaceId) - if (hasPermission === null) { + if (hasPermission !== 'admin' && hasPermission !== 'write') { throw new Error('User does not have permission to create knowledge bases in this workspace') } diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts index 3cecd35852..cde4df3c69 100644 --- a/apps/sim/lib/logs/execution/logger.ts +++ b/apps/sim/lib/logs/execution/logger.ts @@ -304,11 +304,11 @@ export class ExecutionLogger implements IExecutionLoggerService { const wf = updatedLog.workflowId ? (await db.select().from(workflow).where(eq(workflow.id, updatedLog.workflowId)))[0] : undefined - if (wf) { + if (wf && billingUserId) { const [usr] = await db .select({ id: userTable.id, email: userTable.email, name: userTable.name }) .from(userTable) - .where(eq(userTable.id, wf.userId)) + .where(eq(userTable.id, billingUserId)) .limit(1) if (usr?.email) { diff --git a/apps/sim/lib/webhooks/processor.ts b/apps/sim/lib/webhooks/processor.ts index 15013ab2f1..5b65764f70 100644 --- a/apps/sim/lib/webhooks/processor.ts +++ b/apps/sim/lib/webhooks/processor.ts @@ -1,5 +1,5 @@ import { db, webhook, workflow, workflowDeploymentVersion } from '@sim/db' -import { credentialSet, subscription } from '@sim/db/schema' +import { account, credentialSet, subscription } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull, or } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' @@ -990,6 +990,15 @@ export async function queueWebhookExecution( // Note: Each webhook now has its own credentialId (credential sets are fanned out at save time) const providerConfig = (foundWebhook.providerConfig as Record) || {} const credentialId = providerConfig.credentialId as string | undefined + let credentialAccountUserId: string | undefined + if (credentialId) { + const [credentialRecord] = await db + .select({ userId: account.userId }) + .from(account) + .where(eq(account.id, credentialId)) + .limit(1) + credentialAccountUserId = credentialRecord?.userId + } // credentialSetId is a direct field on webhook table, not in providerConfig const credentialSetId = foundWebhook.credentialSetId as string | undefined @@ -1027,6 +1036,7 @@ export async function queueWebhookExecution( path: options.path || foundWebhook.path, blockId: foundWebhook.blockId, ...(credentialId ? { credentialId } : {}), + ...(credentialAccountUserId ? { credentialAccountUserId } : {}), } const jobQueue = await getJobQueue() diff --git a/apps/sim/lib/webhooks/provider-subscriptions.ts b/apps/sim/lib/webhooks/provider-subscriptions.ts index a3e049f767..9d64b4ce51 100644 --- a/apps/sim/lib/webhooks/provider-subscriptions.ts +++ b/apps/sim/lib/webhooks/provider-subscriptions.ts @@ -1,4 +1,7 @@ +import { db } from '@sim/db' +import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { validateAirtableId, validateAlphanumericId } from '@/lib/core/security/input-validation' import { getBaseUrl } from '@/lib/core/utils/urls' @@ -12,6 +15,7 @@ const calendlyLogger = createLogger('CalendlyWebhook') const grainLogger = createLogger('GrainWebhook') const lemlistLogger = createLogger('LemlistWebhook') const webflowLogger = createLogger('WebflowWebhook') +const providerSubscriptionsLogger = createLogger('WebhookProviderSubscriptions') function getProviderConfig(webhook: any): Record { return (webhook.providerConfig as Record) || {} @@ -21,6 +25,26 @@ function getNotificationUrl(webhook: any): string { return `${getBaseUrl()}/api/webhooks/trigger/${webhook.path}` } +async function getCredentialOwnerUserId( + credentialId: string, + requestId: string +): Promise { + const [credentialRecord] = await db + .select({ userId: account.userId }) + .from(account) + .where(eq(account.id, credentialId)) + .limit(1) + + if (!credentialRecord?.userId) { + providerSubscriptionsLogger.warn( + `[${requestId}] Credential owner not found for credentialId ${credentialId}` + ) + return null + } + + return credentialRecord.userId +} + /** * Create a Microsoft Teams chat subscription * Throws errors with friendly messages if subscription creation fails @@ -56,7 +80,10 @@ export async function createTeamsSubscription( ) } - const accessToken = await refreshAccessTokenIfNeeded(credentialId, workflow.userId, requestId) + const credentialOwnerUserId = await getCredentialOwnerUserId(credentialId, requestId) + const accessToken = credentialOwnerUserId + ? await refreshAccessTokenIfNeeded(credentialId, credentialOwnerUserId, requestId) + : null if (!accessToken) { teamsLogger.error( `[${requestId}] Failed to get access token for Teams subscription ${webhook.id}` @@ -189,7 +216,10 @@ export async function deleteTeamsSubscription( return } - const accessToken = await refreshAccessTokenIfNeeded(credentialId, workflow.userId, requestId) + const credentialOwnerUserId = await getCredentialOwnerUserId(credentialId, requestId) + const accessToken = credentialOwnerUserId + ? await refreshAccessTokenIfNeeded(credentialId, credentialOwnerUserId, requestId) + : null if (!accessToken) { teamsLogger.warn( `[${requestId}] Could not get access token to delete Teams subscription for webhook ${webhook.id}` @@ -343,7 +373,7 @@ export async function deleteTelegramWebhook(webhook: any, requestId: string): Pr */ export async function deleteAirtableWebhook( webhook: any, - workflow: any, + _workflow: any, requestId: string ): Promise { try { @@ -369,11 +399,21 @@ export async function deleteAirtableWebhook( return } - const userIdForToken = workflow.userId - const accessToken = await getOAuthToken(userIdForToken, 'airtable') + const credentialId = config.credentialId as string | undefined + if (!credentialId) { + airtableLogger.warn( + `[${requestId}] Missing credentialId for Airtable webhook deletion ${webhook.id}` + ) + return + } + + const credentialOwnerUserId = await getCredentialOwnerUserId(credentialId, requestId) + const accessToken = credentialOwnerUserId + ? await refreshAccessTokenIfNeeded(credentialId, credentialOwnerUserId, requestId) + : null if (!accessToken) { airtableLogger.warn( - `[${requestId}] Could not retrieve Airtable access token for user ${userIdForToken}. Cannot delete webhook in Airtable.`, + `[${requestId}] Could not retrieve Airtable access token. Cannot delete webhook in Airtable.`, { webhookId: webhook.id } ) return @@ -829,7 +869,7 @@ export async function deleteLemlistWebhook(webhook: any, requestId: string): Pro export async function deleteWebflowWebhook( webhook: any, - workflow: any, + _workflow: any, requestId: string ): Promise { try { @@ -869,10 +909,21 @@ export async function deleteWebflowWebhook( return } - const accessToken = await getOAuthToken(workflow.userId, 'webflow') + const credentialId = config.credentialId as string | undefined + if (!credentialId) { + webflowLogger.warn( + `[${requestId}] Missing credentialId for Webflow webhook deletion ${webhook.id}` + ) + return + } + + const credentialOwnerUserId = await getCredentialOwnerUserId(credentialId, requestId) + const accessToken = credentialOwnerUserId + ? await refreshAccessTokenIfNeeded(credentialId, credentialOwnerUserId, requestId) + : null if (!accessToken) { webflowLogger.warn( - `[${requestId}] Could not retrieve Webflow access token for user ${workflow.userId}. Cannot delete webhook.`, + `[${requestId}] Could not retrieve Webflow access token. Cannot delete webhook.`, { webhookId: webhook.id } ) return @@ -1154,7 +1205,7 @@ export async function createAirtableWebhookSubscription( ): Promise { try { const { path, providerConfig } = webhookData - const { baseId, tableId, includeCellValuesInFieldIds } = providerConfig || {} + const { baseId, tableId, includeCellValuesInFieldIds, credentialId } = providerConfig || {} if (!baseId || !tableId) { airtableLogger.warn( @@ -1178,7 +1229,14 @@ export async function createAirtableWebhookSubscription( throw new Error(tableIdValidation.error) } - const accessToken = await getOAuthToken(userId, 'airtable') + const credentialOwnerUserId = credentialId + ? await getCredentialOwnerUserId(credentialId, requestId) + : null + const accessToken = credentialId + ? credentialOwnerUserId + ? await refreshAccessTokenIfNeeded(credentialId, credentialOwnerUserId, requestId) + : null + : await getOAuthToken(userId, 'airtable') if (!accessToken) { airtableLogger.warn( `[${requestId}] Could not retrieve Airtable access token for user ${userId}. Cannot create webhook in Airtable.` @@ -1401,7 +1459,7 @@ export async function createWebflowWebhookSubscription( ): Promise { try { const { path, providerConfig } = webhookData - const { siteId, triggerId, collectionId, formName } = providerConfig || {} + const { siteId, triggerId, collectionId, formName, credentialId } = providerConfig || {} if (!siteId) { webflowLogger.warn(`[${requestId}] Missing siteId for Webflow webhook creation.`, { @@ -1422,7 +1480,14 @@ export async function createWebflowWebhookSubscription( throw new Error('Trigger type is required to create Webflow webhook') } - const accessToken = await getOAuthToken(userId, 'webflow') + const credentialOwnerUserId = credentialId + ? await getCredentialOwnerUserId(credentialId, requestId) + : null + const accessToken = credentialId + ? credentialOwnerUserId + ? await refreshAccessTokenIfNeeded(credentialId, credentialOwnerUserId, requestId) + : null + : await getOAuthToken(userId, 'webflow') if (!accessToken) { webflowLogger.warn( `[${requestId}] Could not retrieve Webflow access token for user ${userId}. Cannot create webhook in Webflow.` diff --git a/apps/sim/lib/workflows/executor/execution-core.ts b/apps/sim/lib/workflows/executor/execution-core.ts index 56926d6271..963256af2e 100644 --- a/apps/sim/lib/workflows/executor/execution-core.ts +++ b/apps/sim/lib/workflows/executor/execution-core.ts @@ -184,13 +184,10 @@ export async function executeWorkflowCore( const mergedStates = mergeSubblockStateWithValues(blocks) - const personalEnvUserId = - metadata.isClientSession && metadata.sessionUserId - ? metadata.sessionUserId - : metadata.workflowUserId + const personalEnvUserId = metadata.sessionUserId || metadata.userId if (!personalEnvUserId) { - throw new Error('Missing workflowUserId in execution metadata') + throw new Error('Missing execution actor for environment resolution') } const { personalEncrypted, workspaceEncrypted, personalDecrypted, workspaceDecrypted } = diff --git a/apps/sim/lib/workflows/persistence/duplicate.ts b/apps/sim/lib/workflows/persistence/duplicate.ts index 8e006e0769..817f58c9c8 100644 --- a/apps/sim/lib/workflows/persistence/duplicate.ts +++ b/apps/sim/lib/workflows/persistence/duplicate.ts @@ -2,6 +2,7 @@ import { db } from '@sim/db' import { workflow, workflowBlocks, workflowEdges, workflowSubflows } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq, isNull, min } from 'drizzle-orm' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' import type { Variable } from '@/stores/panel/variables/types' import type { LoopConfig, ParallelConfig } from '@/stores/workflows/workflow/types' @@ -68,28 +69,30 @@ export async function duplicateWorkflow( } const source = sourceWorkflowRow[0] - - // Check if user has permission to access the source workflow - let canAccessSource = false - - // Case 1: User owns the workflow - if (source.userId === userId) { - canAccessSource = true - } - - // Case 2: User has admin or write permission in the source workspace - if (!canAccessSource && source.workspaceId) { - const userPermission = await getUserEntityPermissions(userId, 'workspace', source.workspaceId) - if (userPermission === 'admin' || userPermission === 'write') { - canAccessSource = true - } + if (!source.workspaceId) { + throw new Error( + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be duplicated.' + ) } - if (!canAccessSource) { + const sourceAuthorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: sourceWorkflowId, + userId, + action: 'read', + }) + if (!sourceAuthorization.allowed) { throw new Error('Source workflow not found or access denied') } const targetWorkspaceId = workspaceId || source.workspaceId + const targetWorkspacePermission = await getUserEntityPermissions( + userId, + 'workspace', + targetWorkspaceId + ) + if (targetWorkspacePermission !== 'admin' && targetWorkspacePermission !== 'write') { + throw new Error('Write or admin access required for target workspace') + } const targetFolderId = folderId !== undefined ? folderId : source.folderId const folderCondition = targetFolderId ? eq(workflow.folderId, targetFolderId) @@ -98,11 +101,7 @@ export async function duplicateWorkflow( const [minResult] = await tx .select({ minOrder: min(workflow.sortOrder) }) .from(workflow) - .where( - targetWorkspaceId - ? and(eq(workflow.workspaceId, targetWorkspaceId), folderCondition) - : and(eq(workflow.userId, userId), folderCondition) - ) + .where(and(eq(workflow.workspaceId, targetWorkspaceId), folderCondition)) const sortOrder = (minResult?.minOrder ?? 1) - 1 // Create the new workflow first (required for foreign key constraints) diff --git a/apps/sim/lib/workflows/utils.test.ts b/apps/sim/lib/workflows/utils.test.ts index aa20db0f7e..e1787e2298 100644 --- a/apps/sim/lib/workflows/utils.test.ts +++ b/apps/sim/lib/workflows/utils.test.ts @@ -3,7 +3,6 @@ * * Tests cover: * - validateWorkflowPermissions for different user roles - * - getWorkflowAccessContext * - Owner vs workspace member access * - Read/write/admin action permissions */ @@ -28,7 +27,7 @@ vi.mock('@/lib/auth', () => ({ import { db } from '@sim/db' import { getSession } from '@/lib/auth' // Import after mocks are set up -import { getWorkflowAccessContext, validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowPermissions } from '@/lib/workflows/utils' describe('validateWorkflowPermissions', () => { const mockSession = createSession({ userId: 'user-1', email: 'user1@test.com' }) @@ -83,7 +82,7 @@ describe('validateWorkflowPermissions', () => { }) describe('owner access', () => { - it('should grant access to workflow owner for read action', async () => { + it('should deny access to workflow owner without workspace permissions for read action', async () => { const ownerSession = createSession({ userId: 'owner-1' }) vi.mocked(getSession).mockResolvedValue(ownerSession as any) @@ -95,10 +94,10 @@ describe('validateWorkflowPermissions', () => { const result = await validateWorkflowPermissions('wf-1', 'req-1', 'read') - expectWorkflowAccessGranted(result) + expectWorkflowAccessDenied(result, 403) }) - it('should grant access to workflow owner for write action', async () => { + it('should deny access to workflow owner without workspace permissions for write action', async () => { const ownerSession = createSession({ userId: 'owner-1' }) vi.mocked(getSession).mockResolvedValue(ownerSession as any) @@ -109,10 +108,10 @@ describe('validateWorkflowPermissions', () => { const result = await validateWorkflowPermissions('wf-1', 'req-1', 'write') - expectWorkflowAccessGranted(result) + expectWorkflowAccessDenied(result, 403) }) - it('should grant access to workflow owner for admin action', async () => { + it('should deny access to workflow owner without workspace permissions for admin action', async () => { const ownerSession = createSession({ userId: 'owner-1' }) vi.mocked(getSession).mockResolvedValue(ownerSession as any) @@ -123,7 +122,7 @@ describe('validateWorkflowPermissions', () => { const result = await validateWorkflowPermissions('wf-1', 'req-1', 'admin') - expectWorkflowAccessGranted(result) + expectWorkflowAccessDenied(result, 403) }) }) @@ -138,7 +137,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([{ permissionType: 'read' }]) }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -155,7 +153,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([{ permissionType: 'read' }]) }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -173,7 +170,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([{ permissionType: 'write' }]) }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -190,7 +186,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([{ permissionType: 'admin' }]) }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -207,7 +202,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([{ permissionType: 'write' }]) }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -225,7 +219,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([{ permissionType: 'admin' }]) }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -246,7 +239,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([]) // No permission record }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -279,7 +271,7 @@ describe('validateWorkflowPermissions', () => { expectWorkflowAccessDenied(result, 403) }) - it('should grant access to owner for workflow without workspace', async () => { + it('should deny access to owner for workflow without workspace', async () => { const workflowWithoutWorkspace = createWorkflowRecord({ id: 'wf-2', userId: 'user-1', @@ -295,7 +287,7 @@ describe('validateWorkflowPermissions', () => { const result = await validateWorkflowPermissions('wf-2', 'req-1', 'read') - expectWorkflowAccessGranted(result) + expectWorkflowAccessDenied(result, 403) }) }) @@ -307,7 +299,6 @@ describe('validateWorkflowPermissions', () => { const mockLimit = vi.fn().mockImplementation(() => { callCount++ if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) return Promise.resolve([{ permissionType: 'read' }]) }) const mockWhere = vi.fn(() => ({ limit: mockLimit })) @@ -320,116 +311,3 @@ describe('validateWorkflowPermissions', () => { }) }) }) - -describe('getWorkflowAccessContext', () => { - const mockWorkflow = createWorkflowRecord({ - id: 'wf-1', - userId: 'owner-1', - workspaceId: 'ws-1', - }) - - beforeEach(() => { - vi.clearAllMocks() - }) - - it('should return null for non-existent workflow', async () => { - const mockLimit = vi.fn().mockResolvedValue([]) - const mockWhere = vi.fn(() => ({ limit: mockLimit })) - const mockFrom = vi.fn(() => ({ where: mockWhere })) - vi.mocked(db.select).mockReturnValue({ from: mockFrom } as any) - - const result = await getWorkflowAccessContext('non-existent') - - expect(result).toBeNull() - }) - - it('should return context with isOwner true for workflow owner', async () => { - let callCount = 0 - const mockLimit = vi.fn().mockImplementation(() => { - callCount++ - if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) - return Promise.resolve([{ permissionType: 'read' }]) - }) - const mockWhere = vi.fn(() => ({ limit: mockLimit })) - const mockFrom = vi.fn(() => ({ where: mockWhere })) - vi.mocked(db.select).mockReturnValue({ from: mockFrom } as any) - - const result = await getWorkflowAccessContext('wf-1', 'owner-1') - - expect(result).not.toBeNull() - expect(result?.isOwner).toBe(true) - }) - - it('should return context with isOwner false for non-owner', async () => { - let callCount = 0 - const mockLimit = vi.fn().mockImplementation(() => { - callCount++ - if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) - return Promise.resolve([{ permissionType: 'read' }]) - }) - const mockWhere = vi.fn(() => ({ limit: mockLimit })) - const mockFrom = vi.fn(() => ({ where: mockWhere })) - vi.mocked(db.select).mockReturnValue({ from: mockFrom } as any) - - const result = await getWorkflowAccessContext('wf-1', 'other-user') - - expect(result).not.toBeNull() - expect(result?.isOwner).toBe(false) - }) - - it('should return context with workspace permission for workspace member', async () => { - let callCount = 0 - const mockLimit = vi.fn().mockImplementation(() => { - callCount++ - if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) - return Promise.resolve([{ permissionType: 'write' }]) - }) - const mockWhere = vi.fn(() => ({ limit: mockLimit })) - const mockFrom = vi.fn(() => ({ where: mockWhere })) - vi.mocked(db.select).mockReturnValue({ from: mockFrom } as any) - - const result = await getWorkflowAccessContext('wf-1', 'member-user') - - expect(result).not.toBeNull() - expect(result?.workspacePermission).toBe('write') - }) - - it('should return context without permission for non-member', async () => { - let callCount = 0 - const mockLimit = vi.fn().mockImplementation(() => { - callCount++ - if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) - return Promise.resolve([]) - }) - const mockWhere = vi.fn(() => ({ limit: mockLimit })) - const mockFrom = vi.fn(() => ({ where: mockWhere })) - vi.mocked(db.select).mockReturnValue({ from: mockFrom } as any) - - const result = await getWorkflowAccessContext('wf-1', 'stranger') - - expect(result).not.toBeNull() - expect(result?.workspacePermission).toBeNull() - }) - - it('should identify workspace owner correctly', async () => { - let callCount = 0 - const mockLimit = vi.fn().mockImplementation(() => { - callCount++ - if (callCount === 1) return Promise.resolve([mockWorkflow]) - if (callCount === 2) return Promise.resolve([{ ownerId: 'workspace-owner' }]) - return Promise.resolve([{ permissionType: 'admin' }]) - }) - const mockWhere = vi.fn(() => ({ limit: mockLimit })) - const mockFrom = vi.fn(() => ({ where: mockWhere })) - vi.mocked(db.select).mockReturnValue({ from: mockFrom } as any) - - const result = await getWorkflowAccessContext('wf-1', 'workspace-owner') - - expect(result).not.toBeNull() - expect(result?.isWorkspaceOwner).toBe(true) - }) -}) diff --git a/apps/sim/lib/workflows/utils.ts b/apps/sim/lib/workflows/utils.ts index 7f952510f1..7dc8bc05e4 100644 --- a/apps/sim/lib/workflows/utils.ts +++ b/apps/sim/lib/workflows/utils.ts @@ -1,10 +1,11 @@ import { db } from '@sim/db' import { permissions, userStats, workflow as workflowTable } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, asc, eq, inArray, or } from 'drizzle-orm' +import { and, asc, eq, inArray } from 'drizzle-orm' import { NextResponse } from 'next/server' import { getSession } from '@/lib/auth' -import { getWorkspaceWithOwner, type PermissionType } from '@/lib/workspaces/permissions/utils' +import type { PermissionType } from '@/lib/workspaces/permissions/utils' +import { getWorkspaceBilledAccountUserId } from '@/lib/workspaces/utils' import type { ExecutionResult } from '@/executor/types' const logger = createLogger('WorkflowUtils') @@ -21,6 +22,14 @@ export async function resolveWorkflowIdForUser( workflowName?: string ): Promise<{ workflowId: string; workflowName?: string } | null> { if (workflowId) { + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'read', + }) + if (!authorization.allowed) { + return null + } return { workflowId } } @@ -30,16 +39,14 @@ export async function resolveWorkflowIdForUser( .where(and(eq(permissions.userId, userId), eq(permissions.entityType, 'workspace'))) const workspaceIdList = workspaceIds.map((row) => row.entityId) - - const workflowConditions = [eq(workflowTable.userId, userId)] - if (workspaceIdList.length > 0) { - workflowConditions.push(inArray(workflowTable.workspaceId, workspaceIdList)) + if (workspaceIdList.length === 0) { + return null } const workflows = await db .select() .from(workflowTable) - .where(or(...workflowConditions)) + .where(inArray(workflowTable.workspaceId, workspaceIdList)) .orderBy(asc(workflowTable.sortOrder), asc(workflowTable.createdAt), asc(workflowTable.id)) if (workflows.length === 0) { @@ -66,61 +73,12 @@ type WorkflowRecord = ReturnType extends Promise : never -export interface WorkflowAccessContext { - workflow: WorkflowRecord - workspaceOwnerId: string | null +export interface WorkflowWorkspaceAuthorizationResult { + allowed: boolean + status: number + message?: string + workflow: WorkflowRecord | null workspacePermission: PermissionType | null - isOwner: boolean - isWorkspaceOwner: boolean -} - -export async function getWorkflowAccessContext( - workflowId: string, - userId?: string -): Promise { - const workflow = await getWorkflowById(workflowId) - - if (!workflow) { - return null - } - - let workspaceOwnerId: string | null = null - let workspacePermission: PermissionType | null = null - - if (workflow.workspaceId) { - const workspaceRow = await getWorkspaceWithOwner(workflow.workspaceId) - - workspaceOwnerId = workspaceRow?.ownerId ?? null - - if (userId) { - const [permissionRow] = await db - .select({ permissionType: permissions.permissionType }) - .from(permissions) - .where( - and( - eq(permissions.userId, userId), - eq(permissions.entityType, 'workspace'), - eq(permissions.entityId, workflow.workspaceId) - ) - ) - .limit(1) - - workspacePermission = permissionRow?.permissionType ?? null - } - } - - const resolvedUserId = userId ?? null - - const isOwner = resolvedUserId ? workflow.userId === resolvedUserId : false - const isWorkspaceOwner = resolvedUserId ? workspaceOwnerId === resolvedUserId : false - - return { - workflow, - workspaceOwnerId, - workspacePermission, - isOwner, - isWorkspaceOwner, - } } export async function updateWorkflowRunCounts(workflowId: string, runs = 1) { @@ -139,29 +97,51 @@ export async function updateWorkflowRunCounts(workflowId: string, runs = 1) { }) .where(eq(workflowTable.id, workflowId)) - try { - const existing = await db - .select() - .from(userStats) - .where(eq(userStats.userId, workflow.userId)) - .limit(1) - - if (existing.length === 0) { - logger.warn('User stats record not found - should be created during onboarding', { - userId: workflow.userId, + let activityUserId: string | null = null + if (workflow.workspaceId) { + try { + activityUserId = await getWorkspaceBilledAccountUserId(workflow.workspaceId) + } catch (error) { + logger.warn(`Error resolving billed account for workspace ${workflow.workspaceId}`, { workflowId, + error, }) - } else { - await db - .update(userStats) - .set({ - lastActive: new Date(), + } + } + + if (activityUserId) { + try { + const existing = await db + .select() + .from(userStats) + .where(eq(userStats.userId, activityUserId)) + .limit(1) + + if (existing.length === 0) { + logger.warn('User stats record not found - should be created during onboarding', { + userId: activityUserId, + workflowId, }) - .where(eq(userStats.userId, workflow.userId)) + } else { + await db + .update(userStats) + .set({ + lastActive: new Date(), + }) + .where(eq(userStats.userId, activityUserId)) + } + } catch (error) { + logger.error(`Error updating userStats lastActive for userId ${activityUserId}:`, error) + // Don't rethrow - we want to continue even if this fails } - } catch (error) { - logger.error(`Error updating userStats lastActive for userId ${workflow.userId}:`, error) - // Don't rethrow - we want to continue even if this fails + } else { + logger.warn( + 'Skipping userStats lastActive update: unable to resolve workspace billed account', + { + workflowId, + workspaceId: workflow.workspaceId, + } + ) } return { @@ -220,8 +200,13 @@ export async function validateWorkflowPermissions( } } - const accessContext = await getWorkflowAccessContext(workflowId, session.user.id) - if (!accessContext) { + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId: session.user.id, + action, + }) + + if (!authorization.workflow) { logger.warn(`[${requestId}] Workflow ${workflowId} not found`) return { error: { message: 'Workflow not found', status: 404 }, @@ -230,54 +215,104 @@ export async function validateWorkflowPermissions( } } - const { workflow, workspacePermission, isOwner } = accessContext + if (!authorization.allowed) { + const message = + authorization.message || `Unauthorized: Access denied to ${action} this workflow` + logger.warn( + `[${requestId}] User ${session.user.id} unauthorized to ${action} workflow ${workflowId}`, + { + action, + workflowId, + } + ) + return { + error: { message, status: authorization.status }, + session: null, + workflow: null, + } + } + + return { + error: null, + session, + workflow: authorization.workflow, + } +} - if (isOwner) { +export async function authorizeWorkflowByWorkspacePermission(params: { + workflowId: string + userId: string + action?: 'read' | 'write' | 'admin' +}): Promise { + const { workflowId, userId, action = 'read' } = params + + const workflow = await getWorkflowById(workflowId) + if (!workflow) { return { - error: null, - session, - workflow, + allowed: false, + status: 404, + message: 'Workflow not found', + workflow: null, + workspacePermission: null, } } - if (workflow.workspaceId) { - let hasPermission = false - - if (action === 'read') { - // Any workspace permission allows read - hasPermission = workspacePermission !== null - } else if (action === 'write') { - // Write or admin permission allows write - hasPermission = workspacePermission === 'write' || workspacePermission === 'admin' - } else if (action === 'admin') { - // Only admin permission allows admin actions - hasPermission = workspacePermission === 'admin' + if (!workflow.workspaceId) { + return { + allowed: false, + status: 403, + message: + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', + workflow, + workspacePermission: null, } + } - if (!hasPermission) { - logger.warn( - `[${requestId}] User ${session.user.id} unauthorized to ${action} workflow ${workflowId} in workspace ${workflow.workspaceId}` + const [permissionRow] = await db + .select({ permissionType: permissions.permissionType }) + .from(permissions) + .where( + and( + eq(permissions.userId, userId), + eq(permissions.entityType, 'workspace'), + eq(permissions.entityId, workflow.workspaceId) ) - return { - error: { message: `Unauthorized: Access denied to ${action} this workflow`, status: 403 }, - session: null, - workflow: null, - } - } - } else { - logger.warn( - `[${requestId}] User ${session.user.id} unauthorized to ${action} workflow ${workflowId} owned by ${workflow.userId}` ) + .limit(1) + + const workspacePermission = permissionRow?.permissionType ?? null + + if (workspacePermission === null) { return { - error: { message: `Unauthorized: Access denied to ${action} this workflow`, status: 403 }, - session: null, - workflow: null, + allowed: false, + status: 403, + message: `Unauthorized: Access denied to ${action} this workflow`, + workflow, + workspacePermission, + } + } + + const permissionSatisfied = + action === 'read' + ? true + : action === 'write' + ? workspacePermission === 'write' || workspacePermission === 'admin' + : workspacePermission === 'admin' + + if (!permissionSatisfied) { + return { + allowed: false, + status: 403, + message: `Unauthorized: Access denied to ${action} this workflow`, + workflow, + workspacePermission, } } return { - error: null, - session, + allowed: true, + status: 200, workflow, + workspacePermission, } } diff --git a/apps/sim/socket/handlers/subblocks.ts b/apps/sim/socket/handlers/subblocks.ts index 3ff5657c58..0c4e6e4490 100644 --- a/apps/sim/socket/handlers/subblocks.ts +++ b/apps/sim/socket/handlers/subblocks.ts @@ -2,7 +2,9 @@ import { db } from '@sim/db' import { workflow, workflowBlocks } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' +import { SUBBLOCK_OPERATIONS } from '@/socket/constants' import type { AuthenticatedSocket } from '@/socket/middleware/auth' +import { checkRolePermission } from '@/socket/middleware/permissions' import type { IRoomManager } from '@/socket/rooms' const logger = createLogger('SubblocksHandlers') @@ -112,6 +114,43 @@ export function setupSubblocksHandlers(socket: AuthenticatedSocket, roomManager: return } + const users = await roomManager.getWorkflowUsers(workflowId) + const userPresence = users.find((user) => user.socketId === socket.id) + if (!userPresence) { + socket.emit('operation-forbidden', { + type: 'SESSION_ERROR', + message: 'User session not found', + operation: SUBBLOCK_OPERATIONS.UPDATE, + target: 'subblock', + }) + if (operationId) { + socket.emit('operation-failed', { + operationId, + error: 'User session not found', + retryable: false, + }) + } + return + } + + const permissionCheck = checkRolePermission(userPresence.role, SUBBLOCK_OPERATIONS.UPDATE) + if (!permissionCheck.allowed) { + socket.emit('operation-forbidden', { + type: 'INSUFFICIENT_PERMISSIONS', + message: permissionCheck.reason || 'Insufficient permissions', + operation: SUBBLOCK_OPERATIONS.UPDATE, + target: 'subblock', + }) + if (operationId) { + socket.emit('operation-failed', { + operationId, + error: permissionCheck.reason || 'Insufficient permissions', + retryable: false, + }) + } + return + } + // Update user activity await roomManager.updateUserActivity(workflowId, socket.id, { lastActivity: Date.now() }) diff --git a/apps/sim/socket/handlers/variables.ts b/apps/sim/socket/handlers/variables.ts index 22e3aeed52..bf5114c839 100644 --- a/apps/sim/socket/handlers/variables.ts +++ b/apps/sim/socket/handlers/variables.ts @@ -2,7 +2,9 @@ import { db } from '@sim/db' import { workflow } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' +import { VARIABLE_OPERATIONS } from '@/socket/constants' import type { AuthenticatedSocket } from '@/socket/middleware/auth' +import { checkRolePermission } from '@/socket/middleware/permissions' import type { IRoomManager } from '@/socket/rooms' const logger = createLogger('VariablesHandlers') @@ -101,6 +103,43 @@ export function setupVariablesHandlers(socket: AuthenticatedSocket, roomManager: return } + const users = await roomManager.getWorkflowUsers(workflowId) + const userPresence = users.find((user) => user.socketId === socket.id) + if (!userPresence) { + socket.emit('operation-forbidden', { + type: 'SESSION_ERROR', + message: 'User session not found', + operation: VARIABLE_OPERATIONS.UPDATE, + target: 'variable', + }) + if (operationId) { + socket.emit('operation-failed', { + operationId, + error: 'User session not found', + retryable: false, + }) + } + return + } + + const permissionCheck = checkRolePermission(userPresence.role, VARIABLE_OPERATIONS.UPDATE) + if (!permissionCheck.allowed) { + socket.emit('operation-forbidden', { + type: 'INSUFFICIENT_PERMISSIONS', + message: permissionCheck.reason || 'Insufficient permissions', + operation: VARIABLE_OPERATIONS.UPDATE, + target: 'variable', + }) + if (operationId) { + socket.emit('operation-failed', { + operationId, + error: permissionCheck.reason || 'Insufficient permissions', + retryable: false, + }) + } + return + } + // Update user activity await roomManager.updateUserActivity(workflowId, socket.id, { lastActivity: Date.now() }) diff --git a/apps/sim/socket/middleware/permissions.ts b/apps/sim/socket/middleware/permissions.ts index b160a061be..9b63b5ba7e 100644 --- a/apps/sim/socket/middleware/permissions.ts +++ b/apps/sim/socket/middleware/permissions.ts @@ -2,13 +2,15 @@ import { db } from '@sim/db' import { workflow } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' -import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' import { BLOCK_OPERATIONS, BLOCKS_OPERATIONS, EDGE_OPERATIONS, EDGES_OPERATIONS, + SUBBLOCK_OPERATIONS, SUBFLOW_OPERATIONS, + VARIABLE_OPERATIONS, WORKFLOW_OPERATIONS, } from '@/socket/constants' @@ -42,6 +44,10 @@ const WRITE_OPERATIONS: string[] = [ EDGES_OPERATIONS.BATCH_REMOVE_EDGES, // Subflow operations SUBFLOW_OPERATIONS.UPDATE, + // Subblock operations + SUBBLOCK_OPERATIONS.UPDATE, + // Variable operations + VARIABLE_OPERATIONS.UPDATE, // Workflow operations WORKFLOW_OPERATIONS.REPLACE_STATE, ] @@ -76,19 +82,6 @@ export function checkRolePermission( return { allowed: true } } -async function verifyWorkspaceMembership( - userId: string, - workspaceId: string -): Promise { - try { - const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId) - return permission - } catch (error) { - logger.error(`Error verifying workspace permissions for ${userId} in ${workspaceId}:`, error) - return null - } -} - export async function verifyWorkflowAccess( userId: string, workflowId: string @@ -96,7 +89,6 @@ export async function verifyWorkflowAccess( try { const workflowData = await db .select({ - userId: workflow.userId, workspaceId: workflow.workspaceId, name: workflow.name, }) @@ -109,34 +101,28 @@ export async function verifyWorkflowAccess( return { hasAccess: false } } - const { userId: workflowUserId, workspaceId, name: workflowName } = workflowData[0] + const { workspaceId, name: workflowName } = workflowData[0] + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId, + userId, + action: 'read', + }) - // Check if user owns the workflow - treat as admin - if (workflowUserId === userId) { - logger.debug( - `User ${userId} has admin access to workflow ${workflowId} (${workflowName}) as owner` - ) - return { hasAccess: true, role: 'admin', workspaceId: workspaceId || undefined } - } - - // Check workspace membership if workflow belongs to a workspace - if (workspaceId) { - const userRole = await verifyWorkspaceMembership(userId, workspaceId) - if (userRole) { - logger.debug( - `User ${userId} has ${userRole} access to workflow ${workflowId} via workspace ${workspaceId}` - ) - return { hasAccess: true, role: userRole, workspaceId } - } + if (!authorization.allowed || !authorization.workspacePermission) { logger.warn( - `User ${userId} is not a member of workspace ${workspaceId} for workflow ${workflowId}` + `User ${userId} is not permitted to access workflow ${workflowId}: ${authorization.message}` ) return { hasAccess: false } } - // Workflow doesn't belong to a workspace and user doesn't own it - logger.warn(`User ${userId} has no access to workflow ${workflowId} (no workspace, not owner)`) - return { hasAccess: false } + logger.debug( + `User ${userId} has ${authorization.workspacePermission} access to workflow ${workflowId} (${workflowName}) via workspace ${workspaceId}` + ) + return { + hasAccess: true, + role: authorization.workspacePermission, + workspaceId: workspaceId || undefined, + } } catch (error) { logger.error( `Error verifying workflow access for user ${userId}, workflow ${workflowId}:`, From c0b7ec004370a4fc91e26878867a114a8e47a14c Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 10 Feb 2026 15:02:15 -0800 Subject: [PATCH 5/8] update tests --- apps/sim/app/api/chat/utils.test.ts | 4 ++++ apps/sim/app/api/form/utils.test.ts | 4 ++-- apps/sim/app/api/knowledge/route.test.ts | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/chat/utils.test.ts b/apps/sim/app/api/chat/utils.test.ts index f76ff1cd9f..84c3bb375a 100644 --- a/apps/sim/app/api/chat/utils.test.ts +++ b/apps/sim/app/api/chat/utils.test.ts @@ -47,6 +47,10 @@ vi.mock('@/lib/core/config/feature-flags', () => ({ isProd: false, })) +vi.mock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: vi.fn(), +})) + describe('Chat API Utils', () => { beforeEach(() => { vi.stubGlobal('process', { diff --git a/apps/sim/app/api/form/utils.test.ts b/apps/sim/app/api/form/utils.test.ts index 4c5a220eae..ad4d518fa2 100644 --- a/apps/sim/app/api/form/utils.test.ts +++ b/apps/sim/app/api/form/utils.test.ts @@ -22,8 +22,8 @@ vi.mock('@/lib/core/config/feature-flags', () => ({ isProd: false, })) -vi.mock('@/lib/workspaces/permissions/utils', () => ({ - hasAdminPermission: vi.fn(), +vi.mock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: vi.fn(), })) describe('Form API Utils', () => { diff --git a/apps/sim/app/api/knowledge/route.test.ts b/apps/sim/app/api/knowledge/route.test.ts index bd93fab3b3..c9b1272995 100644 --- a/apps/sim/app/api/knowledge/route.test.ts +++ b/apps/sim/app/api/knowledge/route.test.ts @@ -17,7 +17,7 @@ mockDrizzleOrm() mockConsoleLogger() vi.mock('@/lib/workspaces/permissions/utils', () => ({ - getUserEntityPermissions: vi.fn().mockResolvedValue({ role: 'owner' }), + getUserEntityPermissions: vi.fn().mockResolvedValue('admin'), })) describe('Knowledge Base API Route', () => { From b7aa96ce4b45697c1144a69ef6591fadf20bb455 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 10 Feb 2026 15:17:11 -0800 Subject: [PATCH 6/8] fix return code --- apps/sim/app/api/guardrails/validate/route.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/apps/sim/app/api/guardrails/validate/route.ts b/apps/sim/app/api/guardrails/validate/route.ts index 9be7e33f0a..82567422f2 100644 --- a/apps/sim/app/api/guardrails/validate/route.ts +++ b/apps/sim/app/api/guardrails/validate/route.ts @@ -16,15 +16,7 @@ export async function POST(request: NextRequest) { try { const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) if (!auth.success || !auth.userId) { - return NextResponse.json({ - success: true, - output: { - passed: false, - validationType: 'unknown', - input: '', - error: 'Unauthorized', - }, - }) + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } const body = await request.json() From c4d51efc09074f74a54379b501c91ae99e2f86ec Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 10 Feb 2026 15:25:06 -0800 Subject: [PATCH 7/8] use shared types --- apps/sim/app/api/a2a/serve/[agentId]/route.ts | 6 +++--- apps/sim/app/api/mcp/serve/[serverId]/route.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/sim/app/api/a2a/serve/[agentId]/route.ts b/apps/sim/app/api/a2a/serve/[agentId]/route.ts index e05689cfcf..d10ef0eea2 100644 --- a/apps/sim/app/api/a2a/serve/[agentId]/route.ts +++ b/apps/sim/app/api/a2a/serve/[agentId]/route.ts @@ -13,7 +13,7 @@ import { isTerminalState, parseWorkflowSSEChunk, } from '@/lib/a2a/utils' -import { checkHybridAuth } from '@/lib/auth/hybrid' +import { type AuthResult, checkHybridAuth } from '@/lib/auth/hybrid' import { acquireLock, getRedisClient, releaseLock } from '@/lib/core/config/redis' import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server' import { SSE_HEADERS } from '@/lib/core/utils/sse' @@ -194,8 +194,8 @@ export async function POST(request: NextRequest, { params }: { params: Promise Date: Tue, 10 Feb 2026 15:43:22 -0800 Subject: [PATCH 8/8] fix bugbot comment --- apps/sim/app/api/auth/oauth/credentials/route.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/api/auth/oauth/credentials/route.ts b/apps/sim/app/api/auth/oauth/credentials/route.ts index f0b69a8378..7809e5543c 100644 --- a/apps/sim/app/api/auth/oauth/credentials/route.ts +++ b/apps/sim/app/api/auth/oauth/credentials/route.ts @@ -113,7 +113,12 @@ export async function GET(request: NextRequest) { let accountsData - if (credentialId) { + if (credentialId && workflowId) { + // When both workflowId and credentialId are provided, fetch by ID only. + // Workspace authorization above already proves access; the credential + // may belong to another workspace member (e.g. for display name resolution). + accountsData = await db.select().from(account).where(eq(account.id, credentialId)) + } else if (credentialId) { accountsData = await db .select() .from(account)