diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts new file mode 100644 index 00000000000..e89f8f51207 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts @@ -0,0 +1,239 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockCleanupWebhooksForWorkflow, + mockRecordAudit, + mockDbLimit, + mockDbOrderBy, + mockDbFrom, + mockDbSelect, + mockDbSet, + mockDbUpdate, + mockDbWhere, + mockCreateSchedulesForDeploy, + mockDeployWorkflow, + mockLoadWorkflowFromNormalizedTables, + mockRemoveMcpToolsForWorkflow, + mockSaveTriggerWebhooksForDeploy, + mockSyncMcpToolsForWorkflow, + mockUndeployWorkflow, + mockValidatePublicApiAllowed, + mockValidateWorkflowAccess, + mockValidateWorkflowPermissions, +} = vi.hoisted(() => ({ + mockCleanupWebhooksForWorkflow: vi.fn(), + mockRecordAudit: vi.fn(), + mockDbLimit: vi.fn(), + mockDbOrderBy: vi.fn(), + mockDbFrom: vi.fn(), + mockDbSelect: vi.fn(), + mockDbSet: vi.fn(), + mockDbUpdate: vi.fn(), + mockDbWhere: vi.fn(), + mockCreateSchedulesForDeploy: vi.fn(), + mockDeployWorkflow: vi.fn(), + mockLoadWorkflowFromNormalizedTables: vi.fn(), + mockRemoveMcpToolsForWorkflow: vi.fn(), + mockSaveTriggerWebhooksForDeploy: vi.fn(), + mockSyncMcpToolsForWorkflow: vi.fn(), + mockUndeployWorkflow: vi.fn(), + mockValidatePublicApiAllowed: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), + mockValidateWorkflowPermissions: vi.fn(), +})) + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/lib/workflows/utils', () => ({ + validateWorkflowPermissions: (...args: unknown[]) => mockValidateWorkflowPermissions(...args), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { select: mockDbSelect, update: mockDbUpdate }, + workflow: { variables: 'variables', id: 'id' }, + workflowDeploymentVersion: { + state: 'state', + workflowId: 'workflowId', + isActive: 'isActive', + createdAt: 'createdAt', + id: 'id', + }, +})) + +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + desc: vi.fn(), + eq: vi.fn(), + } +}) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + loadWorkflowFromNormalizedTables: (...args: unknown[]) => + mockLoadWorkflowFromNormalizedTables(...args), + deployWorkflow: (...args: unknown[]) => mockDeployWorkflow(...args), + undeployWorkflow: (...args: unknown[]) => mockUndeployWorkflow(...args), +})) + +vi.mock('@/lib/workflows/comparison', () => ({ + hasWorkflowChanged: vi.fn().mockReturnValue(false), +})) + +vi.mock('@/lib/workflows/schedules', () => ({ + cleanupDeploymentVersion: vi.fn(), + createSchedulesForDeploy: (...args: unknown[]) => mockCreateSchedulesForDeploy(...args), + validateWorkflowSchedules: vi.fn().mockReturnValue({ isValid: true }), +})) + +vi.mock('@/lib/webhooks/deploy', () => ({ + cleanupWebhooksForWorkflow: (...args: unknown[]) => mockCleanupWebhooksForWorkflow(...args), + restorePreviousVersionWebhooks: vi.fn(), + saveTriggerWebhooksForDeploy: (...args: unknown[]) => mockSaveTriggerWebhooksForDeploy(...args), +})) + +vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ + removeMcpToolsForWorkflow: (...args: unknown[]) => mockRemoveMcpToolsForWorkflow(...args), + syncMcpToolsForWorkflow: (...args: unknown[]) => mockSyncMcpToolsForWorkflow(...args), +})) + +vi.mock('@/lib/audit/log', () => ({ + AuditAction: {}, + AuditResourceType: {}, + recordAudit: (...args: unknown[]) => mockRecordAudit(...args), +})) + +vi.mock('@/ee/access-control/utils/permission-check', () => ({ + PublicApiNotAllowedError: class PublicApiNotAllowedError extends Error {}, + validatePublicApiAllowed: (...args: unknown[]) => mockValidatePublicApiAllowed(...args), +})) + +import { DELETE, PATCH, POST } from '@/app/api/workflows/[id]/deploy/route' + +describe('Workflow deploy route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit, orderBy: mockDbOrderBy }) + mockDbOrderBy.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit.mockResolvedValue([]) + mockDbUpdate.mockReturnValue({ set: mockDbSet }) + mockDbSet.mockReturnValue({ where: mockDbWhere }) + mockCleanupWebhooksForWorkflow.mockResolvedValue(undefined) + mockCreateSchedulesForDeploy.mockResolvedValue({ success: true }) + mockLoadWorkflowFromNormalizedTables.mockResolvedValue({ + blocks: { 'block-1': { id: 'block-1', type: 'start_trigger', name: 'Start' } }, + edges: [], + loops: {}, + parallels: {}, + }) + mockSaveTriggerWebhooksForDeploy.mockResolvedValue({ success: true, warnings: [] }) + mockRemoveMcpToolsForWorkflow.mockResolvedValue(undefined) + mockSyncMcpToolsForWorkflow.mockResolvedValue(undefined) + mockValidatePublicApiAllowed.mockResolvedValue(undefined) + }) + + it('allows API-key auth for deploy using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { + success: true, + userId: 'api-user', + authType: 'api_key', + }, + }) + mockDeployWorkflow.mockResolvedValue({ + success: true, + deployedAt: '2024-01-01T00:00:00Z', + deploymentVersionId: 'dep-1', + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', { + method: 'POST', + headers: { 'x-api-key': 'test-key' }, + }) + const response = await POST(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.isDeployed).toBe(true) + expect(mockDeployWorkflow).toHaveBeenCalledWith({ + workflowId: 'wf-1', + deployedBy: 'api-user', + workflowName: 'Test Workflow', + }) + expect(mockValidateWorkflowPermissions).not.toHaveBeenCalled() + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: undefined, + actorEmail: undefined, + }) + ) + }) + + it('allows API-key auth for undeploy using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { + success: true, + userId: 'api-user', + authType: 'api_key', + }, + }) + mockUndeployWorkflow.mockResolvedValue({ success: true }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', { + method: 'DELETE', + headers: { 'x-api-key': 'test-key' }, + }) + const response = await DELETE(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.isDeployed).toBe(false) + expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' }) + expect(mockValidateWorkflowPermissions).not.toHaveBeenCalled() + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: undefined, + actorEmail: undefined, + }) + ) + }) + + it('checks public API restrictions against hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', { + method: 'PATCH', + headers: { 'content-type': 'application/json', 'x-api-key': 'test-key' }, + body: JSON.stringify({ isPublicApi: true }), + }) + const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidatePublicApiAllowed).toHaveBeenCalledWith('api-user') + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index 5ad26782382..407cac55064 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -2,6 +2,7 @@ import { db, workflow, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' import { and, desc, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { generateRequestId } from '@/lib/core/utils/request' import { removeMcpToolsForWorkflow, syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' @@ -21,7 +22,7 @@ import { createSchedulesForDeploy, validateWorkflowSchedules, } from '@/lib/workflows/schedules' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' import type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -35,15 +36,16 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ const { id } = await params try { - const { error, workflow: workflowData } = await validateWorkflowPermissions( - id, - requestId, - 'read' - ) - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const workflowData = access.workflow + if (!workflowData.isDeployed) { logger.info(`[${requestId}] Workflow is not deployed: ${id}`) return createSuccessResponse({ @@ -115,16 +117,18 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ const { id } = await params try { - const { - error, - session, - workflow: workflowData, - } = await validateWorkflowPermissions(id, requestId, 'admin') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } - const actorUserId: string | null = session?.user?.id ?? null + const auth = access.auth + const workflowData = access.workflow + + const actorUserId: string | null = auth?.userId ?? null if (!actorUserId) { logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment: ${id}`) return createErrorResponse('Unable to determine deploying user', 400) @@ -274,11 +278,13 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ // Sync MCP tools with the latest parameter schema await syncMcpToolsForWorkflow({ workflowId: id, requestId, context: 'deploy' }) + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowData?.workspaceId || null, actorId: actorUserId, - actorName: session?.user?.name, - actorEmail: session?.user?.email, + actorName, + actorEmail, action: AuditAction.WORKFLOW_DEPLOYED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, @@ -322,11 +328,16 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { id } = await params try { - const { error, session } = await validateWorkflowPermissions(id, requestId, 'admin') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const auth = access.auth + const body = await request.json() const { isPublicApi } = body @@ -338,8 +349,9 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise< const { validatePublicApiAllowed, PublicApiNotAllowedError } = await import( '@/ee/access-control/utils/permission-check' ) + const actorUserId = auth?.userId try { - await validatePublicApiAllowed(session?.user?.id) + await validatePublicApiAllowed(actorUserId) } catch (err) { if (err instanceof PublicApiNotAllowedError) { return createErrorResponse('Public API access is disabled', 403) @@ -368,13 +380,20 @@ export async function DELETE( const { id } = await params try { - const { - error, - session, - workflow: workflowData, - } = await validateWorkflowPermissions(id, requestId, 'admin') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) + } + + const auth = access.auth + const workflowData = access.workflow + + const actorUserId = auth?.userId ?? null + if (!actorUserId) { + return createErrorResponse('Unable to determine undeploying user', 400) } const result = await undeployWorkflow({ workflowId: id }) @@ -395,11 +414,13 @@ export async function DELETE( // Silently fail } + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowData?.workspaceId || null, - actorId: session!.user.id, - actorName: session?.user?.name, - actorEmail: session?.user?.email, + actorId: actorUserId, + actorName, + actorEmail, action: AuditAction.WORKFLOW_UNDEPLOYED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.test.ts b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts new file mode 100644 index 00000000000..6d903f62411 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts @@ -0,0 +1,61 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockValidateWorkflowAccess = vi.fn() +const mockVerifyInternalToken = vi.fn() +const mockLoadDeployedWorkflowState = vi.fn() + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/auth/internal', () => ({ + verifyInternalToken: (...args: unknown[]) => mockVerifyInternalToken(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + loadDeployedWorkflowState: (...args: unknown[]) => mockLoadDeployedWorkflowState(...args), +})) + +import { GET } from '@/app/api/workflows/[id]/deployed/route' + +describe('Workflow deployed-state route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockVerifyInternalToken.mockResolvedValue({ valid: false }) + mockLoadDeployedWorkflowState.mockResolvedValue({ + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + variables: [], + }) + }) + + it('uses hybrid workflow access when request is not internal bearer auth', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ workflow: { id: 'wf-1' } }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployed', { + headers: { 'x-api-key': 'test-key' }, + }) + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'read', + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.ts b/apps/sim/app/api/workflows/[id]/deployed/route.ts index 347e77eacb9..8a536784522 100644 --- a/apps/sim/app/api/workflows/[id]/deployed/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployed/route.ts @@ -3,7 +3,7 @@ import type { NextRequest, NextResponse } from 'next/server' import { verifyInternalToken } from '@/lib/auth/internal' import { generateRequestId } from '@/lib/core/utils/request' import { loadDeployedWorkflowState } from '@/lib/workflows/persistence/utils' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('WorkflowDeployedStateAPI') @@ -31,9 +31,12 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ } if (!isInternalCall) { - const { error } = await validateWorkflowPermissions(id, requestId, 'read') - if (error) { - const response = createErrorResponse(error.message, error.status) + const validation = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) + if (validation.error) { + const response = createErrorResponse(validation.error.message, validation.error.status) return addNoCacheHeaders(response) } } diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts new file mode 100644 index 00000000000..a90e79adee9 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts @@ -0,0 +1,143 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockDbFrom, + mockDbLimit, + mockDbSelect, + mockDbSet, + mockDbUpdate, + mockDbWhere, + mockDbWhereUpdate, + mockRecordAudit, + mockSaveWorkflowToNormalizedTables, + mockSyncMcpToolsForWorkflow, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockDbFrom: vi.fn(), + mockDbLimit: vi.fn(), + mockDbSelect: vi.fn(), + mockDbSet: vi.fn(), + mockDbUpdate: vi.fn(), + mockDbWhere: vi.fn(), + mockDbWhereUpdate: vi.fn(), + mockRecordAudit: vi.fn(), + mockSaveWorkflowToNormalizedTables: vi.fn(), + mockSyncMcpToolsForWorkflow: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) +const mockFetch = vi.fn() + +vi.stubGlobal('fetch', mockFetch) + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@/lib/core/config/env', () => ({ + env: { INTERNAL_API_SECRET: 'internal-secret', SOCKET_SERVER_URL: 'http://localhost:3002' }, +})) + +vi.mock('@sim/db', () => ({ + db: { + select: mockDbSelect, + update: mockDbUpdate, + }, + workflow: { id: 'id' }, + workflowDeploymentVersion: { + state: 'state', + workflowId: 'workflowId', + isActive: 'isActive', + version: 'version', + }, +})) + +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + eq: vi.fn(), + } +}) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + saveWorkflowToNormalizedTables: (...args: unknown[]) => + mockSaveWorkflowToNormalizedTables(...args), +})) + +vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ + syncMcpToolsForWorkflow: (...args: unknown[]) => mockSyncMcpToolsForWorkflow(...args), +})) + +vi.mock('@/lib/audit/log', () => ({ + AuditAction: { WORKFLOW_DEPLOYMENT_REVERTED: 'WORKFLOW_DEPLOYMENT_REVERTED' }, + AuditResourceType: { WORKFLOW: 'WORKFLOW' }, + recordAudit: (...args: unknown[]) => mockRecordAudit(...args), +})) + +import { POST } from '@/app/api/workflows/[id]/deployments/[version]/revert/route' + +describe('Workflow deployment version revert route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit.mockResolvedValue([ + { + state: { + blocks: { 'block-1': { id: 'block-1', type: 'start_trigger', name: 'Start' } }, + edges: [], + loops: {}, + parallels: {}, + }, + }, + ]) + mockDbUpdate.mockReturnValue({ set: mockDbSet }) + mockDbSet.mockReturnValue({ where: mockDbWhereUpdate }) + mockDbWhereUpdate.mockResolvedValue(undefined) + mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true }) + mockFetch.mockResolvedValue({ ok: true }) + }) + + it('allows API-key auth for revert using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments/3/revert', { + method: 'POST', + headers: { 'x-api-key': 'test-key' }, + }) + const response = await POST(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'admin', + }) + expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalled() + expect(mockSyncMcpToolsForWorkflow).toHaveBeenCalled() + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: undefined, + actorEmail: undefined, + }) + ) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts index d3762c9181f..58d52d0b01c 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.ts @@ -2,11 +2,13 @@ import { db, workflow, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { env } from '@/lib/core/config/env' import { generateRequestId } from '@/lib/core/utils/request' +import { syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('RevertToDeploymentVersionAPI') @@ -22,13 +24,23 @@ export async function POST( const { id, version } = await params try { - const { - error, - session, - workflow: workflowRecord, - } = await validateWorkflowPermissions(id, requestId, 'admin') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'admin', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) + } + + const auth = access.auth + const workflowRecord = access.workflow + + const actorUserId = auth?.userId + if (!actorUserId) { + logger.warn( + `[${requestId}] Unable to resolve actor user for workflow deployment revert: ${id}` + ) + return createErrorResponse('Unable to determine reverting user', 400) } const versionSelector = version === 'active' ? null : Number(version) @@ -90,6 +102,13 @@ export async function POST( .set({ lastSynced: new Date(), updatedAt: new Date() }) .where(eq(workflow.id, id)) + await syncMcpToolsForWorkflow({ + workflowId: id, + requestId, + state: deployedState, + context: 'revert', + }) + try { const socketServerUrl = env.SOCKET_SERVER_URL || 'http://localhost:3002' await fetch(`${socketServerUrl}/api/workflow-reverted`, { @@ -104,14 +123,16 @@ export async function POST( logger.error('Error sending workflow reverted event to socket server', e) } + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowRecord?.workspaceId ?? null, - actorId: session!.user.id, + actorId: actorUserId, action: AuditAction.WORKFLOW_DEPLOYMENT_REVERTED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, - actorName: session!.user.name ?? undefined, - actorEmail: session!.user.email ?? undefined, + actorName, + actorEmail, resourceName: workflowRecord?.name ?? undefined, description: `Reverted workflow to deployment version ${version}`, request, diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts new file mode 100644 index 00000000000..6b5a85a1230 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts @@ -0,0 +1,252 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockActivateWorkflowVersion, + mockAuthorizeWorkflowByWorkspacePermission, + mockCreateSchedulesForDeploy, + mockDbFrom, + mockDbLimit, + mockDbReturning, + mockDbSelect, + mockDbSet, + mockDbUpdate, + mockDbWhere, + mockDbWhereUpdate, + mockRecordAudit, + mockSaveTriggerWebhooksForDeploy, + mockSyncMcpToolsForWorkflow, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockActivateWorkflowVersion: vi.fn(), + mockAuthorizeWorkflowByWorkspacePermission: vi.fn(), + mockCreateSchedulesForDeploy: vi.fn(), + mockDbFrom: vi.fn(), + mockDbLimit: vi.fn(), + mockDbReturning: vi.fn(), + mockDbSelect: vi.fn(), + mockDbSet: vi.fn(), + mockDbUpdate: vi.fn(), + mockDbWhere: vi.fn(), + mockDbWhereUpdate: vi.fn(), + mockRecordAudit: vi.fn(), + mockSaveTriggerWebhooksForDeploy: vi.fn(), + mockSyncMcpToolsForWorkflow: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { + select: mockDbSelect, + update: mockDbUpdate, + }, + workflowDeploymentVersion: { + id: 'id', + state: 'state', + workflowId: 'workflowId', + version: 'version', + isActive: 'isActive', + name: 'name', + description: 'description', + }, +})) + +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + eq: vi.fn(), + } +}) + +vi.mock('@/lib/webhooks/deploy', () => ({ + restorePreviousVersionWebhooks: vi.fn(), + saveTriggerWebhooksForDeploy: (...args: unknown[]) => mockSaveTriggerWebhooksForDeploy(...args), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + activateWorkflowVersion: (...args: unknown[]) => mockActivateWorkflowVersion(...args), +})) + +vi.mock('@/lib/workflows/schedules', () => ({ + cleanupDeploymentVersion: vi.fn(), + createSchedulesForDeploy: (...args: unknown[]) => mockCreateSchedulesForDeploy(...args), + validateWorkflowSchedules: vi.fn(() => ({ isValid: true })), +})) + +vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ + syncMcpToolsForWorkflow: (...args: unknown[]) => mockSyncMcpToolsForWorkflow(...args), +})) + +vi.mock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: (...args: unknown[]) => + mockAuthorizeWorkflowByWorkspacePermission(...args), +})) + +vi.mock('@/lib/audit/log', () => ({ + AuditAction: { WORKFLOW_DEPLOYMENT_ACTIVATED: 'WORKFLOW_DEPLOYMENT_ACTIVATED' }, + AuditResourceType: { WORKFLOW: 'WORKFLOW' }, + recordAudit: (...args: unknown[]) => mockRecordAudit(...args), +})) + +import { PATCH } from '@/app/api/workflows/[id]/deployments/[version]/route' + +describe('Workflow deployment version route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit }) + mockDbLimit + .mockResolvedValueOnce([ + { + id: 'dep-3', + state: { + blocks: { 'block-1': { id: 'block-1', type: 'start_trigger', name: 'Start' } }, + }, + }, + ]) + .mockResolvedValueOnce([{ id: 'dep-2' }]) + mockDbUpdate.mockReturnValue({ set: mockDbSet }) + mockDbSet.mockReturnValue({ where: mockDbWhereUpdate }) + mockDbWhereUpdate.mockReturnValue({ returning: mockDbReturning }) + mockDbReturning.mockResolvedValue([]) + mockSaveTriggerWebhooksForDeploy.mockResolvedValue({ success: true, warnings: [] }) + mockCreateSchedulesForDeploy.mockResolvedValue({ success: true }) + mockActivateWorkflowVersion.mockResolvedValue({ + success: true, + deployedAt: '2024-01-17T12:00:00.000Z', + }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true, status: 200 }) + }) + + it('uses write permission for metadata-only patch updates', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'user-1', authType: 'session' }, + }) + mockDbReturning.mockResolvedValue([{ id: 'dep-3', name: 'Renamed', description: 'Updated' }]) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments/3', { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ name: 'Renamed', description: 'Updated' }), + }) + const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledTimes(1) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'write', + }) + expect(mockDbUpdate).toHaveBeenCalledTimes(1) + expect(mockActivateWorkflowVersion).not.toHaveBeenCalled() + }) + + it('allows API-key auth for activation using hybrid auth userId', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'api-user', authType: 'api_key' }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments/3', { + method: 'PATCH', + headers: { 'content-type': 'application/json', 'x-api-key': 'test-key' }, + body: JSON.stringify({ isActive: true }), + }) + const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenNthCalledWith(1, req, 'wf-1', { + requireDeployment: false, + action: 'write', + }) + expect(mockValidateWorkflowAccess).toHaveBeenCalledTimes(1) + expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: 'wf-1', + userId: 'api-user', + action: 'admin', + }) + expect(mockSaveTriggerWebhooksForDeploy).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'api-user' }) + ) + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: undefined, + actorEmail: undefined, + }) + ) + }) + + it('returns write auth failure before parsing or updating metadata', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + error: { message: 'Write permission required', status: 403 }, + }) + + const jsonSpy = vi.fn().mockResolvedValue({ name: 'Renamed' }) + const req = { + json: jsonSpy, + } as unknown as NextRequest + + const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) }) + + expect(response.status).toBe(403) + expect(jsonSpy).not.toHaveBeenCalled() + expect(mockDbUpdate).not.toHaveBeenCalled() + expect(mockActivateWorkflowVersion).not.toHaveBeenCalled() + }) + + it('returns admin auth failure before activation side effects', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' }, + auth: { success: true, userId: 'user-1', authType: 'session' }, + }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Admin permission required', + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments/3', { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ isActive: true }), + }) + const response = await PATCH(req, { params: Promise.resolve({ id: 'wf-1', version: '3' }) }) + + expect(response.status).toBe(403) + expect(mockValidateWorkflowAccess).toHaveBeenCalledTimes(1) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'write', + }) + expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: 'wf-1', + userId: 'user-1', + action: 'admin', + }) + expect(mockDbSelect).not.toHaveBeenCalled() + expect(mockSaveTriggerWebhooksForDeploy).not.toHaveBeenCalled() + expect(mockCreateSchedulesForDeploy).not.toHaveBeenCalled() + expect(mockActivateWorkflowVersion).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts index 56802840e95..b34f3fce46c 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts @@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { z } from 'zod' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' import { generateRequestId } from '@/lib/core/utils/request' import { syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' @@ -13,7 +14,8 @@ import { createSchedulesForDeploy, validateWorkflowSchedules, } from '@/lib/workflows/schedules' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' import type { BlockState } from '@/stores/workflows/workflow/types' @@ -53,9 +55,12 @@ export async function GET( const { id, version } = await params try { - const { error } = await validateWorkflowPermissions(id, requestId, 'read') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } const versionNum = Number(version) @@ -96,6 +101,14 @@ export async function PATCH( const { id, version } = await params try { + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'write', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) + } + const body = await request.json() const validation = patchBodySchema.safeParse(body) @@ -104,16 +117,22 @@ export async function PATCH( } const { name, description, isActive } = validation.data + const auth = access.auth + const workflowData = access.workflow + + if (isActive) { + if (!auth?.userId) { + return createErrorResponse('Unable to determine activating user', 400) + } - // Activation requires admin permission, other updates require write - const requiredPermission = isActive ? 'admin' : 'write' - const { - error, - session, - workflow: workflowData, - } = await validateWorkflowPermissions(id, requestId, requiredPermission) - if (error) { - return createErrorResponse(error.message, error.status) + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: id, + userId: auth.userId, + action: 'admin', + }) + if (!authorization.allowed) { + return createErrorResponse(authorization.message || 'Access denied', authorization.status) + } } const versionNum = Number(version) @@ -123,11 +142,7 @@ export async function PATCH( // Handle activation if (isActive) { - const actorUserId = session?.user?.id - if (!actorUserId) { - logger.warn(`[${requestId}] Unable to resolve actor user for deployment activation: ${id}`) - return createErrorResponse('Unable to determine activating user', 400) - } + const actorUserId = auth?.userId const [versionRow] = await db .select({ @@ -178,7 +193,7 @@ export async function PATCH( request, workflowId: id, workflow: workflowData as Record, - userId: actorUserId, + userId: actorUserId!, blocks, requestId, deploymentVersionId: versionRow.id, @@ -206,7 +221,7 @@ export async function PATCH( await restorePreviousVersionWebhooks({ request, workflow: workflowData as Record, - userId: actorUserId, + userId: actorUserId!, previousVersionId, requestId, }) @@ -226,7 +241,7 @@ export async function PATCH( await restorePreviousVersionWebhooks({ request, workflow: workflowData as Record, - userId: actorUserId, + userId: actorUserId!, previousVersionId, requestId, }) @@ -298,11 +313,13 @@ export async function PATCH( } } + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowData?.workspaceId, - actorId: actorUserId, - actorName: session?.user?.name, - actorEmail: session?.user?.email, + actorId: actorUserId!, + actorName, + actorEmail, action: AuditAction.WORKFLOW_DEPLOYMENT_ACTIVATED, resourceType: AuditResourceType.WORKFLOW, resourceId: id, diff --git a/apps/sim/app/api/workflows/[id]/deployments/route.test.ts b/apps/sim/app/api/workflows/[id]/deployments/route.test.ts new file mode 100644 index 00000000000..2b760dc7eb5 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/route.test.ts @@ -0,0 +1,97 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockDbFrom, + mockDbLeftJoin, + mockDbOrderBy, + mockDbSelect, + mockDbWhere, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockDbFrom: vi.fn(), + mockDbLeftJoin: vi.fn(), + mockDbOrderBy: vi.fn(), + mockDbSelect: vi.fn(), + mockDbWhere: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { select: mockDbSelect }, + user: { name: 'name', id: 'id' }, + workflowDeploymentVersion: { + id: 'id', + version: 'version', + name: 'name', + description: 'description', + isActive: 'isActive', + createdAt: 'createdAt', + createdBy: 'createdBy', + workflowId: 'workflowId', + }, +})) + +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + desc: vi.fn(), + eq: vi.fn(), + } +}) + +import { GET } from '@/app/api/workflows/[id]/deployments/route' + +describe('Workflow deployments list route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ leftJoin: mockDbLeftJoin }) + mockDbLeftJoin.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ orderBy: mockDbOrderBy }) + mockDbOrderBy.mockResolvedValue([ + { + id: 'dep-1', + version: 3, + name: 'Current active deployment', + description: 'Latest deployed state', + isActive: true, + createdAt: '2024-01-16T12:00:00.000Z', + createdBy: 'admin-api', + deployedBy: null, + }, + ]) + }) + + it('uses hybrid workflow access for read auth', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ workflow: { id: 'wf-1' } }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deployments', { + headers: { 'x-api-key': 'test-key' }, + }) + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'read', + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployments/route.ts b/apps/sim/app/api/workflows/[id]/deployments/route.ts index ac2e7e1015f..f4980b07c7a 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/route.ts @@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger' import { desc, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { generateRequestId } from '@/lib/core/utils/request' -import { validateWorkflowPermissions } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('WorkflowDeploymentsListAPI') @@ -16,9 +16,12 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ const { id } = await params try { - const { error } = await validateWorkflowPermissions(id, requestId, 'read') - if (error) { - return createErrorResponse(error.message, error.status) + const access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } const rawVersions = await db diff --git a/apps/sim/app/api/workflows/[id]/execute/route.async.test.ts b/apps/sim/app/api/workflows/[id]/execute/route.async.test.ts index 7d6c599dcfd..a33c7f282ff 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.async.test.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.async.test.ts @@ -6,24 +6,38 @@ import { createMockRequest } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' const { + mockAuthType, mockCheckHybridAuth, mockAuthorizeWorkflowByWorkspacePermission, mockPreprocessExecution, + mockGetJobQueue, + mockShouldExecuteInline, mockEnqueue, + mockLoggerInfo, + mockLoggerWarn, + mockLoggerError, + mockLoggerDebug, } = vi.hoisted(() => ({ + mockAuthType: { + SESSION: 'session', + API_KEY: 'api_key', + INTERNAL_JWT: 'internal_jwt', + }, mockCheckHybridAuth: vi.fn(), mockAuthorizeWorkflowByWorkspacePermission: vi.fn(), mockPreprocessExecution: vi.fn(), + mockGetJobQueue: vi.fn(), + mockShouldExecuteInline: vi.fn(), mockEnqueue: vi.fn().mockResolvedValue('job-123'), + mockLoggerInfo: vi.fn(), + mockLoggerWarn: vi.fn(), + mockLoggerError: vi.fn(), + mockLoggerDebug: vi.fn(), })) vi.mock('@/lib/auth/hybrid', () => ({ + AuthType: mockAuthType, checkHybridAuth: mockCheckHybridAuth, - AuthType: { - SESSION: 'session', - API_KEY: 'api_key', - INTERNAL_JWT: 'internal_jwt', - }, })) vi.mock('@/lib/workflows/utils', () => ({ @@ -37,13 +51,8 @@ vi.mock('@/lib/execution/preprocessing', () => ({ })) vi.mock('@/lib/core/async-jobs', () => ({ - getJobQueue: vi.fn().mockResolvedValue({ - enqueue: mockEnqueue, - startJob: vi.fn(), - completeJob: vi.fn(), - markJobFailed: vi.fn(), - }), - shouldExecuteInline: vi.fn().mockReturnValue(false), + getJobQueue: mockGetJobQueue, + shouldExecuteInline: mockShouldExecuteInline, })) vi.mock('@/lib/core/utils/request', () => ({ @@ -71,10 +80,10 @@ vi.mock('@/background/workflow-execution', () => ({ vi.mock('@sim/logger', () => ({ createLogger: vi.fn().mockReturnValue({ - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - debug: vi.fn(), + info: mockLoggerInfo, + warn: mockLoggerWarn, + error: mockLoggerError, + debug: mockLoggerDebug, }), })) @@ -85,33 +94,93 @@ vi.mock('uuid', () => ({ import { POST } from './route' +async function readJsonBody(response: Response) { + const text = await response.text() + + return { + text, + json: text ? JSON.parse(text) : null, + } +} + +function expectCheckpointOrder(checkpoints: string[], expected: string[]) { + expect(checkpoints).toEqual(expected) +} + describe('workflow execute async route', () => { + const asyncCheckpoints: string[] = [] + beforeEach(() => { vi.clearAllMocks() - mockCheckHybridAuth.mockResolvedValue({ - success: true, - userId: 'session-user-1', - authType: 'session', + asyncCheckpoints.length = 0 + + mockCheckHybridAuth.mockReset() + mockAuthorizeWorkflowByWorkspacePermission.mockReset() + mockPreprocessExecution.mockReset() + mockGetJobQueue.mockReset() + mockShouldExecuteInline.mockReset() + mockEnqueue.mockReset() + + mockEnqueue.mockResolvedValue('job-123') + mockGetJobQueue.mockResolvedValue({ + enqueue: mockEnqueue, + startJob: vi.fn(), + completeJob: vi.fn(), + markJobFailed: vi.fn(), + }) + mockShouldExecuteInline.mockImplementation(() => { + asyncCheckpoints.push('shouldExecuteInline') + return false }) - mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ - allowed: true, - workflow: { - id: 'workflow-1', - userId: 'owner-1', - workspaceId: 'workspace-1', - }, + mockCheckHybridAuth.mockImplementation(async () => { + asyncCheckpoints.push('authorization') + return { + success: true, + userId: 'session-user-1', + authType: mockAuthType.SESSION, + } }) - mockPreprocessExecution.mockResolvedValue({ - success: true, - actorUserId: 'actor-1', - workflowRecord: { - id: 'workflow-1', - userId: 'owner-1', - workspaceId: 'workspace-1', - }, + mockAuthorizeWorkflowByWorkspacePermission.mockImplementation(async () => { + asyncCheckpoints.push('authorizeWorkflowByWorkspacePermission') + return { + allowed: true, + workflow: { + id: 'workflow-1', + userId: 'owner-1', + workspaceId: 'workspace-1', + }, + } + }) + + mockPreprocessExecution.mockImplementation(async () => { + asyncCheckpoints.push('preprocessing') + return { + success: true, + actorUserId: 'actor-1', + workflowRecord: { + id: 'workflow-1', + userId: 'owner-1', + workspaceId: 'workspace-1', + }, + } + }) + + mockGetJobQueue.mockImplementation(async () => { + asyncCheckpoints.push('getJobQueue') + return { + enqueue: mockEnqueue, + startJob: vi.fn(), + completeJob: vi.fn(), + markJobFailed: vi.fn(), + } + }) + + mockEnqueue.mockImplementation(async () => { + asyncCheckpoints.push('enqueue') + return 'job-123' }) }) @@ -127,11 +196,23 @@ describe('workflow execute async route', () => { const params = Promise.resolve({ id: 'workflow-1' }) const response = await POST(req as any, { params }) - const body = await response.json() + const { json: body, text: bodyText } = await readJsonBody(response) - expect(response.status).toBe(202) + expect( + response.status, + `Expected async execute route to return 202, got ${response.status} with body: ${bodyText}` + ).toBe(202) + expectCheckpointOrder(asyncCheckpoints, [ + 'authorization', + 'authorizeWorkflowByWorkspacePermission', + 'preprocessing', + 'getJobQueue', + 'enqueue', + 'shouldExecuteInline', + ]) expect(body.executionId).toBe('execution-123') expect(body.jobId).toBe('job-123') + expect(mockLoggerError).not.toHaveBeenCalled() expect(mockEnqueue).toHaveBeenCalledWith( 'workflow-execution', expect.objectContaining({ @@ -162,4 +243,42 @@ describe('workflow execute async route', () => { } ) }) + + it('returns queue failure payload with checkpoint trail and logs the queue error', async () => { + const queueError = new Error('queue unavailable') + mockEnqueue.mockImplementationOnce(async () => { + asyncCheckpoints.push('enqueue') + throw queueError + }) + + const req = createMockRequest( + 'POST', + { input: { hello: 'world' } }, + { + 'Content-Type': 'application/json', + 'X-Execution-Mode': 'async', + } + ) + + const response = await POST(req as any, { params: Promise.resolve({ id: 'workflow-1' }) }) + const { json: body, text: bodyText } = await readJsonBody(response) + + expectCheckpointOrder(asyncCheckpoints, [ + 'authorization', + 'authorizeWorkflowByWorkspacePermission', + 'preprocessing', + 'getJobQueue', + 'enqueue', + ]) + expect( + response.status, + `Expected async execute route to return 500, got ${response.status} with body: ${bodyText}` + ).toBe(500) + expect(body).toEqual({ error: 'Failed to queue async execution: queue unavailable' }) + expect(mockShouldExecuteInline).not.toHaveBeenCalled() + expect(mockLoggerError).toHaveBeenCalledWith( + '[req-12345678] Failed to queue async execution', + queueError + ) + }) }) diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index 2000e5093ee..23686bfbede 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -24,6 +24,7 @@ const mockAuthorizeWorkflowByWorkspacePermission = vi.fn() const mockArchiveWorkflow = vi.fn() const mockDbUpdate = vi.fn() const mockDbSelect = vi.fn() +const mockValidateWorkflowAccess = vi.fn() /** * Helper to set mock auth state consistently across getSession and hybrid auth. @@ -32,9 +33,17 @@ function mockGetSession(session: { user: { id: string } } | null) { if (session) { mockCheckHybridAuth.mockResolvedValue({ success: true, userId: session.user.id }) mockCheckSessionOrInternalAuth.mockResolvedValue({ success: true, userId: session.user.id }) + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'workflow-123', workspaceId: 'workspace-456' }, + auth: { success: true, userId: session.user.id, authType: 'session' }, + }) } else { mockCheckHybridAuth.mockResolvedValue({ success: false }) mockCheckSessionOrInternalAuth.mockResolvedValue({ success: false }) + + mockValidateWorkflowAccess.mockResolvedValue({ + error: { message: 'Unauthorized', status: 401 }, + }) } } @@ -63,6 +72,10 @@ vi.mock('@/lib/workflows/persistence/utils', () => ({ mockLoadWorkflowFromNormalizedTables(workflowId), })) +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + vi.mock('@/lib/workflows/utils', () => ({ getWorkflowById: (workflowId: string) => mockGetWorkflowById(workflowId), authorizeWorkflowByWorkspacePermission: (params: { @@ -99,6 +112,7 @@ describe('Workflow By ID API Route', () => { afterEach(() => { vi.clearAllMocks() + vi.unstubAllGlobals() }) describe('GET /api/workflows/[id]', () => { @@ -130,7 +144,7 @@ describe('Workflow By ID API Route', () => { expect(data.error).toBe('Workflow not found') }) - it.concurrent('should allow access when user has admin workspace permission', async () => { + it('should allow access when user has admin workspace permission', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', @@ -168,7 +182,7 @@ describe('Workflow By ID API Route', () => { expect(data.data.id).toBe('workflow-123') }) - it.concurrent('should allow access when user has workspace permissions', async () => { + it('should allow access when user has workspace permissions', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'other-user', @@ -235,7 +249,7 @@ describe('Workflow By ID API Route', () => { expect(data.error).toBe('Unauthorized: Access denied to read this workflow') }) - it.concurrent('should use normalized tables when available', async () => { + it('should use normalized tables when available', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', @@ -363,16 +377,24 @@ describe('Workflow By ID API Route', () => { expect(data.success).toBe(true) }) - it('should prevent deletion of the last workflow in workspace', async () => { + it('should allow API-key-backed deletion when workflow access is validated', async () => { const mockWorkflow = { id: 'workflow-123', - userId: 'user-123', + userId: 'other-user', name: 'Test Workflow', workspaceId: 'workspace-456', } - mockGetSession({ user: { id: 'user-123' } }) - + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: mockWorkflow, + auth: { + success: true, + userId: 'api-user-1', + authType: 'api_key', + userName: 'API Key Actor', + userEmail: null, + }, + }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true, @@ -380,30 +402,36 @@ describe('Workflow By ID API Route', () => { workflow: mockWorkflow, workspacePermission: 'admin', }) - - // Mock db.select() to return only 1 workflow (the one being deleted) mockDbSelect.mockReturnValue({ from: vi.fn().mockReturnValue({ - where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }]), + where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }, { id: 'workflow-456' }]), }), }) + setupGlobalFetchMock({ ok: true }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { method: 'DELETE', + headers: { 'x-api-key': 'test-key' }, }) const params = Promise.resolve({ id: 'workflow-123' }) const response = await DELETE(req, { params }) - expect(response.status).toBe(400) - const data = await response.json() - expect(data.error).toBe('Cannot delete the only workflow in the workspace') + expect(response.status).toBe(200) + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + expect(auditMock.recordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user-1', + actorName: 'API Key Actor', + actorEmail: undefined, + }) + ) }) - it.concurrent('should deny deletion for non-admin users', async () => { + it('should prevent deletion of the last workflow in workspace', async () => { const mockWorkflow = { id: 'workflow-123', - userId: 'other-user', + userId: 'user-123', name: 'Test Workflow', workspaceId: 'workspace-456', } @@ -412,11 +440,37 @@ describe('Workflow By ID API Route', () => { mockGetWorkflowById.mockResolvedValue(mockWorkflow) mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ - allowed: false, - status: 403, - message: 'Unauthorized: Access denied to admin this workflow', + allowed: true, + status: 200, workflow: mockWorkflow, - workspacePermission: null, + workspacePermission: 'admin', + }) + + // Mock db.select() to return only 1 workflow (the one being deleted) + mockDbSelect.mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }]), + }), + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'DELETE', + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const response = await DELETE(req, { params }) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Cannot delete the only workflow in the workspace') + }) + + it('should deny deletion for non-admin users', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + error: { + message: 'Unauthorized: Access denied to admin this workflow', + status: 403, + }, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { @@ -485,6 +539,7 @@ describe('Workflow By ID API Route', () => { expect(response.status).toBe(200) const data = await response.json() expect(data.workflow.name).toBe('Updated Workflow') + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() }) it('should allow users with write permission to update workflow', async () => { @@ -532,24 +587,13 @@ describe('Workflow By ID API Route', () => { }) it('should deny update for users with only read permission', async () => { - const mockWorkflow = { - id: 'workflow-123', - userId: 'other-user', - name: 'Test Workflow', - workspaceId: 'workspace-456', - } - const updateData = { name: 'Updated Workflow' } - mockGetSession({ user: { id: 'user-123' } }) - - mockGetWorkflowById.mockResolvedValue(mockWorkflow) - mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ - allowed: false, - status: 403, - message: 'Unauthorized: Access denied to write this workflow', - workflow: mockWorkflow, - workspacePermission: 'read', + mockValidateWorkflowAccess.mockResolvedValue({ + error: { + message: 'Unauthorized: Access denied to write this workflow', + status: 403, + }, }) const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { @@ -565,7 +609,7 @@ describe('Workflow By ID API Route', () => { expect(data.error).toBe('Unauthorized: Access denied to write this workflow') }) - it.concurrent('should validate request data', async () => { + it('should validate request data', async () => { const mockWorkflow = { id: 'workflow-123', userId: 'user-123', @@ -718,7 +762,10 @@ describe('Workflow By ID API Route', () => { const updatedWorkflow = { ...mockWorkflow, folderId: 'folder-2', updatedAt: new Date() } - mockGetSession({ user: { id: 'user-123' } }) + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: mockWorkflow, + auth: { success: true, userId: 'user-123', authType: 'session' }, + }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true, @@ -760,7 +807,10 @@ describe('Workflow By ID API Route', () => { workspaceId: 'workspace-456', } - mockGetSession({ user: { id: 'user-123' } }) + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: mockWorkflow, + auth: { success: true, userId: 'user-123', authType: 'session' }, + }) mockGetWorkflowById.mockResolvedValue(mockWorkflow) mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true, @@ -827,7 +877,7 @@ describe('Workflow By ID API Route', () => { }) describe('Error handling', () => { - it.concurrent('should handle database errors gracefully', async () => { + it('should handle database errors gracefully', async () => { mockGetSession({ user: { id: 'user-123' } }) mockGetWorkflowById.mockRejectedValue(new Error('Database connection timeout')) diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 8b79fe2c287..558be1aca06 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -4,12 +4,14 @@ import { createLogger } from '@sim/logger' import { and, eq, isNull, ne } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log' -import { AuthType, checkHybridAuth, checkSessionOrInternalAuth } from '@/lib/auth/hybrid' +import { AuthType, checkHybridAuth } from '@/lib/auth/hybrid' import { generateRequestId } from '@/lib/core/utils/request' import { archiveWorkflow } from '@/lib/workflows/lifecycle' import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils' import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' const logger = createLogger('WorkflowByIdAPI') @@ -152,38 +154,35 @@ export async function DELETE( const { id: workflowId } = await params try { - const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!auth.success || !auth.userId) { + const validation = await validateWorkflowAccess(request, workflowId, { + requireDeployment: false, + action: 'admin', + }) + if (validation.error) { logger.warn(`[${requestId}] Unauthorized deletion attempt for workflow ${workflowId}`) - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + return NextResponse.json( + { error: validation.error.message }, + { status: validation.error.status } + ) } - const userId = auth.userId + const auth = validation.auth + const userId = auth?.userId + const workflowData = validation.workflow - const authorization = await authorizeWorkflowByWorkspacePermission({ - workflowId, - userId, - action: 'admin', - }) - const workflowData = authorization.workflow || (await getWorkflowById(workflowId)) + if (!userId) { + logger.warn(`[${requestId}] Missing user identity for workflow deletion ${workflowId}`) + return NextResponse.json( + { error: 'Workflow deletion requires a user-backed session or API key identity' }, + { status: 400 } + ) + } if (!workflowData) { logger.warn(`[${requestId}] Workflow ${workflowId} not found for deletion`) return NextResponse.json({ error: 'Workflow not found' }, { status: 404 }) } - const canDelete = authorization.allowed - - if (!canDelete) { - logger.warn( - `[${requestId}] User ${userId} denied permission to delete workflow ${workflowId}` - ) - return NextResponse.json( - { error: authorization.message || 'Access denied' }, - { status: authorization.status || 403 } - ) - } - // Check if this is the last workflow in the workspace if (workflowData.workspaceId) { const totalWorkflowsInWorkspace = await db @@ -255,11 +254,13 @@ export async function DELETE( const elapsed = Date.now() - startTime logger.info(`[${requestId}] Successfully archived workflow ${workflowId} in ${elapsed}ms`) + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowData.workspaceId || null, actorId: userId, - actorName: auth.userName, - actorEmail: auth.userEmail, + actorName, + actorEmail, action: AuditAction.WORKFLOW_DELETED, resourceType: AuditResourceType.WORKFLOW, resourceId: workflowId, @@ -290,42 +291,36 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ const { id: workflowId } = await params try { - const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!auth.success || !auth.userId) { + const validation = await validateWorkflowAccess(request, workflowId, { + requireDeployment: false, + action: 'write', + }) + if (validation.error) { logger.warn(`[${requestId}] Unauthorized update attempt for workflow ${workflowId}`) - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + return NextResponse.json( + { error: validation.error.message }, + { status: validation.error.status } + ) } - const userId = auth.userId + const userId = validation.auth?.userId + const workflowData = validation.workflow + if (!userId) { + logger.warn(`[${requestId}] Missing user identity for workflow update ${workflowId}`) + return NextResponse.json( + { error: 'Workflow update requires a user-backed session or API key identity' }, + { status: 400 } + ) + } const body = await request.json() const updates = UpdateWorkflowSchema.parse(body) - // Fetch the workflow to check ownership/access - 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 }) } - const canUpdate = authorization.allowed - - if (!canUpdate) { - logger.warn( - `[${requestId}] User ${userId} denied permission to update workflow ${workflowId}` - ) - return NextResponse.json( - { error: authorization.message || 'Access denied' }, - { status: authorization.status || 403 } - ) - } - const updateData: Record = { updatedAt: new Date() } if (updates.name !== undefined) updateData.name = updates.name if (updates.description !== undefined) updateData.description = updates.description diff --git a/apps/sim/app/api/workflows/[id]/status/route.test.ts b/apps/sim/app/api/workflows/[id]/status/route.test.ts new file mode 100644 index 00000000000..a7fbda1e8d1 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/status/route.test.ts @@ -0,0 +1,99 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockDbFrom, + mockDbLimit, + mockDbOrderBy, + mockDbSelect, + mockDbWhere, + mockHasWorkflowChanged, + mockLoadWorkflowFromNormalizedTables, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockDbFrom: vi.fn(), + mockDbLimit: vi.fn(), + mockDbOrderBy: vi.fn(), + mockDbSelect: vi.fn(), + mockDbWhere: vi.fn(), + mockHasWorkflowChanged: vi.fn(), + mockLoadWorkflowFromNormalizedTables: vi.fn(), + mockValidateWorkflowAccess: vi.fn(), +})) + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }), +})) + +vi.mock('@/app/api/workflows/middleware', () => ({ + validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + loadWorkflowFromNormalizedTables: (...args: unknown[]) => + mockLoadWorkflowFromNormalizedTables(...args), +})) + +vi.mock('@/lib/workflows/comparison', () => ({ + hasWorkflowChanged: (...args: unknown[]) => mockHasWorkflowChanged(...args), +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'req-123', +})) + +vi.mock('@sim/db', () => ({ + db: { + select: mockDbSelect, + }, + workflow: { variables: 'variables', id: 'id' }, + workflowDeploymentVersion: { + state: 'state', + workflowId: 'workflowId', + isActive: 'isActive', + createdAt: 'createdAt', + }, +})) + +vi.mock('drizzle-orm', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + and: vi.fn(), + desc: vi.fn(), + eq: vi.fn(), + } +}) + +import { GET } from '@/app/api/workflows/[id]/status/route' + +describe('Workflow status route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockDbSelect.mockReturnValue({ from: mockDbFrom }) + mockDbFrom.mockReturnValue({ where: mockDbWhere }) + mockDbWhere.mockReturnValue({ limit: mockDbLimit, orderBy: mockDbOrderBy }) + mockDbOrderBy.mockReturnValue({ limit: mockDbLimit }) + }) + + it('uses hybrid workflow access for read auth', async () => { + mockValidateWorkflowAccess.mockResolvedValue({ + workflow: { id: 'wf-1', isDeployed: false, deployedAt: null, isPublished: false }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/status', { + headers: { 'x-api-key': 'test-key' }, + }) + const response = await GET(req, { params: Promise.resolve({ id: 'wf-1' }) }) + + expect(response.status).toBe(200) + expect(mockValidateWorkflowAccess).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'read', + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/status/route.ts b/apps/sim/app/api/workflows/[id]/status/route.ts index f53fbe05e4d..7bc8eb059fd 100644 --- a/apps/sim/app/api/workflows/[id]/status/route.ts +++ b/apps/sim/app/api/workflows/[id]/status/route.ts @@ -17,7 +17,10 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ try { const { id } = await params - const validation = await validateWorkflowAccess(request, id, false) + const validation = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: 'read', + }) if (validation.error) { logger.warn(`[${requestId}] Workflow access validation failed: ${validation.error.message}`) return createErrorResponse(validation.error.message, validation.error.status) diff --git a/apps/sim/app/api/workflows/middleware.test.ts b/apps/sim/app/api/workflows/middleware.test.ts new file mode 100644 index 00000000000..f5ba64ae591 --- /dev/null +++ b/apps/sim/app/api/workflows/middleware.test.ts @@ -0,0 +1,264 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { validateWorkflowAccess } from './middleware' + +const { mockAuthenticateApiKeyFromHeader, mockUpdateApiKeyLastUsed, mockCheckHybridAuth } = + vi.hoisted(() => ({ + mockAuthenticateApiKeyFromHeader: vi.fn(), + mockUpdateApiKeyLastUsed: vi.fn(), + mockCheckHybridAuth: vi.fn(), + })) + +const { mockAuthorizeWorkflowByWorkspacePermission, mockGetWorkflowById, mockLoggerError } = + vi.hoisted(() => ({ + mockAuthorizeWorkflowByWorkspacePermission: vi.fn(), + mockGetWorkflowById: vi.fn(), + mockLoggerError: vi.fn(), + })) + +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: mockLoggerError }), +})) + +vi.mock('@/lib/api-key/service', () => ({ + authenticateApiKeyFromHeader: (...args: unknown[]) => mockAuthenticateApiKeyFromHeader(...args), + updateApiKeyLastUsed: (...args: unknown[]) => mockUpdateApiKeyLastUsed(...args), +})) + +vi.mock('@/lib/auth/hybrid', () => ({ + AuthType: { + SESSION: 'session', + API_KEY: 'api_key', + INTERNAL_JWT: 'internal_jwt', + }, + checkHybridAuth: (...args: unknown[]) => mockCheckHybridAuth(...args), +})) + +vi.mock('@/lib/core/config/env', () => ({ + env: {}, +})) + +vi.mock('@/lib/workflows/utils', () => ({ + authorizeWorkflowByWorkspacePermission: (...args: unknown[]) => + mockAuthorizeWorkflowByWorkspacePermission(...args), + getWorkflowById: (...args: unknown[]) => mockGetWorkflowById(...args), +})) + +const WORKFLOW_ID = 'wf-1' +const WORKSPACE_ID = 'ws-1' + +function createRequest() { + return new NextRequest(`http://localhost:3000/api/workflows/${WORKFLOW_ID}/status`) +} + +function createWorkflow(overrides: Record = {}) { + return { + id: WORKFLOW_ID, + workspaceId: WORKSPACE_ID, + isDeployed: false, + ...overrides, + } +} + +describe('validateWorkflowAccess', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCheckHybridAuth.mockResolvedValue({ success: true, userId: 'user-1', authType: 'session' }) + mockGetWorkflowById.mockResolvedValue(createWorkflow()) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: createWorkflow(), + workspacePermission: 'admin', + }) + }) + + it('returns 401 before workflow lookup when unauthenticated', async () => { + const request = createRequest() + + mockCheckHybridAuth.mockResolvedValue({ success: false, error: 'Unauthorized' }) + + const result = await validateWorkflowAccess(request, WORKFLOW_ID, { + requireDeployment: false, + action: 'read', + }) + + expect(result).toEqual({ + error: { + message: 'Unauthorized', + status: 401, + }, + }) + expect(mockCheckHybridAuth).toHaveBeenCalledWith(request, { requireWorkflowId: false }) + expect(mockGetWorkflowById).not.toHaveBeenCalled() + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + }) + + it('returns 404 for authenticated missing workflow', async () => { + mockGetWorkflowById.mockResolvedValue(null) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: false, + action: 'read', + }) + + expect(result).toEqual({ + error: { + message: 'Workflow not found', + status: 404, + }, + }) + expect(mockGetWorkflowById).toHaveBeenCalledWith(WORKFLOW_ID) + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + }) + + it('returns 403 for authenticated workflow without workspace', async () => { + mockGetWorkflowById.mockResolvedValue(createWorkflow({ workspaceId: null })) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: false, + action: 'read', + }) + + expect(result).toEqual({ + error: { + message: + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', + status: 403, + }, + }) + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + }) + + it('returns authorization denial status and message', async () => { + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to admin this workflow', + workflow: createWorkflow(), + workspacePermission: 'write', + }) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: false, + action: 'admin', + }) + + expect(result).toEqual({ + error: { + message: 'Unauthorized: Access denied to admin this workflow', + status: 403, + }, + }) + expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: WORKFLOW_ID, + userId: 'user-1', + action: 'admin', + }) + }) + + it('returns 403 for workspace api keys scoped to a different workspace', async () => { + const auth = { + success: true, + userId: 'user-1', + workspaceId: 'ws-2', + authType: 'api_key' as const, + apiKeyType: 'workspace' as const, + } + + mockCheckHybridAuth.mockResolvedValue(auth) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: false, + action: 'read', + }) + + expect(result).toEqual({ + error: { + message: 'Unauthorized: API key does not have access to this workspace', + status: 403, + }, + }) + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + }) + + it('allows workspace api keys scoped to the same workspace', async () => { + const workflow = createWorkflow({ name: 'Scoped Workflow' }) + const auth = { + success: true, + userId: 'user-1', + workspaceId: WORKSPACE_ID, + authType: 'api_key' as const, + apiKeyType: 'workspace' as const, + } + + mockCheckHybridAuth.mockResolvedValue(auth) + mockGetWorkflowById.mockResolvedValue(workflow) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: false, + action: 'read', + }) + + expect(result).toEqual({ workflow, auth }) + expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: WORKFLOW_ID, + userId: 'user-1', + action: 'read', + }) + }) + + it('returns workflow and auth on success', async () => { + const workflow = createWorkflow({ name: 'Test Workflow' }) + const auth = { success: true, userId: 'user-1', authType: 'session' as const } + + mockCheckHybridAuth.mockResolvedValue(auth) + mockGetWorkflowById.mockResolvedValue(workflow) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: false, + action: 'read', + }) + + expect(result).toEqual({ workflow, auth }) + }) + + it('returns 404 for deployed access when workflow is missing', async () => { + mockGetWorkflowById.mockResolvedValue(null) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: true, + }) + + expect(result).toEqual({ + error: { + message: 'Workflow not found', + status: 404, + }, + }) + expect(mockCheckHybridAuth).not.toHaveBeenCalled() + expect(mockAuthenticateApiKeyFromHeader).not.toHaveBeenCalled() + }) + + it('returns 403 for deployed access when workflow has no workspace', async () => { + mockGetWorkflowById.mockResolvedValue(createWorkflow({ workspaceId: null, isDeployed: true })) + + const result = await validateWorkflowAccess(createRequest(), WORKFLOW_ID, { + requireDeployment: true, + }) + + expect(result).toEqual({ + error: { + message: + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', + status: 403, + }, + }) + expect(mockCheckHybridAuth).not.toHaveBeenCalled() + expect(mockAuthenticateApiKeyFromHeader).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/workflows/middleware.ts b/apps/sim/app/api/workflows/middleware.ts index d6260002fe3..0ec440433e9 100644 --- a/apps/sim/app/api/workflows/middleware.ts +++ b/apps/sim/app/api/workflows/middleware.ts @@ -5,7 +5,7 @@ import { authenticateApiKeyFromHeader, updateApiKeyLastUsed, } from '@/lib/api-key/service' -import { type AuthResult, checkHybridAuth } from '@/lib/auth/hybrid' +import { type AuthResult, AuthType, checkHybridAuth } from '@/lib/auth/hybrid' import { env } from '@/lib/core/config/env' import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils' @@ -17,31 +17,47 @@ export interface ValidationResult { auth?: AuthResult } +export interface WorkflowAccessOptions { + requireDeployment?: boolean + action?: 'read' | 'write' | 'admin' + allowInternalSecret?: boolean +} + +async function getValidatedWorkflow(workflowId: string): Promise { + const workflow = await getWorkflowById(workflowId) + if (!workflow) { + return { + error: { + message: 'Workflow not found', + status: 404, + }, + } + } + + if (!workflow.workspaceId) { + return { + error: { + message: + 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', + status: 403, + }, + } + } + + return { workflow } +} + export async function validateWorkflowAccess( request: NextRequest, workflowId: string, - requireDeployment = true + options: boolean | WorkflowAccessOptions = true ): Promise { try { - const workflow = await getWorkflowById(workflowId) - if (!workflow) { - return { - error: { - message: 'Workflow not found', - status: 404, - }, - } - } - - if (!workflow.workspaceId) { - return { - error: { - message: - 'This workflow is not attached to a workspace. Personal workflows are deprecated and cannot be accessed.', - status: 403, - }, - } - } + const normalizedOptions: WorkflowAccessOptions = + typeof options === 'boolean' ? { requireDeployment: options } : options + const requireDeployment = normalizedOptions.requireDeployment ?? true + const action = normalizedOptions.action ?? 'read' + const allowInternalSecret = normalizedOptions.allowInternalSecret ?? false if (!requireDeployment) { const auth = await checkHybridAuth(request, { requireWorkflowId: false }) @@ -54,10 +70,29 @@ export async function validateWorkflowAccess( } } + const workflowResult = await getValidatedWorkflow(workflowId) + if (workflowResult.error || !workflowResult.workflow) { + return workflowResult + } + const workflow = workflowResult.workflow + + if ( + auth.authType === AuthType.API_KEY && + auth.apiKeyType === 'workspace' && + auth.workspaceId !== workflow.workspaceId + ) { + return { + error: { + message: 'Unauthorized: API key does not have access to this workspace', + status: 403, + }, + } + } + const authorization = await authorizeWorkflowByWorkspacePermission({ workflowId, userId: auth.userId, - action: 'read', + action, }) if (!authorization.allowed) { return { @@ -71,6 +106,12 @@ export async function validateWorkflowAccess( return { workflow, auth } } + const workflowResult = await getValidatedWorkflow(workflowId) + if (workflowResult.error || !workflowResult.workflow) { + return workflowResult + } + const workflow = workflowResult.workflow + if (requireDeployment) { if (!workflow.isDeployed) { return { @@ -82,7 +123,11 @@ export async function validateWorkflowAccess( } const internalSecret = request.headers.get('X-Internal-Secret') - if (env.INTERNAL_API_SECRET && internalSecret === env.INTERNAL_API_SECRET) { + if ( + allowInternalSecret && + env.INTERNAL_API_SECRET && + internalSecret === env.INTERNAL_API_SECRET + ) { return { workflow } } diff --git a/apps/sim/lib/audit/actor-metadata.test.ts b/apps/sim/lib/audit/actor-metadata.test.ts new file mode 100644 index 00000000000..68b3c4bebf2 --- /dev/null +++ b/apps/sim/lib/audit/actor-metadata.test.ts @@ -0,0 +1,31 @@ +/** + * @vitest-environment node + */ + +import { describe, expect, it } from 'vitest' +import { getAuditActorMetadata } from '@/lib/audit/actor-metadata' +import { AuthType } from '@/lib/auth/hybrid' + +describe('getAuditActorMetadata', () => { + it('preserves actor metadata for API key auth when present', () => { + expect( + getAuditActorMetadata({ + success: true, + userId: 'api-user', + userName: 'API Key Actor', + userEmail: 'api@example.com', + authType: AuthType.API_KEY, + }) + ).toEqual({ + actorName: 'API Key Actor', + actorEmail: 'api@example.com', + }) + }) + + it('returns undefined metadata when auth is missing', () => { + expect(getAuditActorMetadata(null)).toEqual({ + actorName: undefined, + actorEmail: undefined, + }) + }) +}) diff --git a/apps/sim/lib/audit/actor-metadata.ts b/apps/sim/lib/audit/actor-metadata.ts new file mode 100644 index 00000000000..e20298c45c5 --- /dev/null +++ b/apps/sim/lib/audit/actor-metadata.ts @@ -0,0 +1,18 @@ +import type { AuthResult } from '@/lib/auth/hybrid' + +export function getAuditActorMetadata(auth: AuthResult | null | undefined): { + actorName: string | undefined + actorEmail: string | undefined +} { + if (!auth) { + return { + actorName: undefined, + actorEmail: undefined, + } + } + + return { + actorName: auth.userName ?? undefined, + actorEmail: auth.userEmail ?? undefined, + } +}