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..1474d557df5 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts @@ -0,0 +1,232 @@ +/** + * @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 1dd8798a3f2..bd5fc9c418e 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' @@ -20,7 +21,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' @@ -34,15 +35,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({ @@ -114,16 +116,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) @@ -261,11 +265,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, @@ -309,11 +315,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 @@ -325,8 +336,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) @@ -355,13 +367,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) } // Clean up external webhook subscriptions before undeploying @@ -383,11 +402,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..8eaf6128be5 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/revert/route.test.ts @@ -0,0 +1,145 @@ +/** + * @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 6050bb4b253..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,12 +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') @@ -23,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) @@ -112,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..74c07587948 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.test.ts @@ -0,0 +1,161 @@ +/** + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockActivateWorkflowVersion, + mockCreateSchedulesForDeploy, + mockDbFrom, + mockDbLimit, + mockDbReturning, + mockDbSelect, + mockDbSet, + mockDbUpdate, + mockDbWhere, + mockDbWhereUpdate, + mockRecordAudit, + mockSaveTriggerWebhooksForDeploy, + mockSyncMcpToolsForWorkflow, + mockValidateWorkflowAccess, +} = vi.hoisted(() => ({ + mockActivateWorkflowVersion: 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/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', + }) + }) + + 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).toHaveBeenCalledWith(req, 'wf-1', { + requireDeployment: false, + action: 'admin', + }) + expect(mockSaveTriggerWebhooksForDeploy).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'api-user' }) + ) + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'api-user', + actorName: undefined, + actorEmail: undefined, + }) + ) + }) +}) 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..b6450b4beab 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,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 { BlockState } from '@/stores/workflows/workflow/types' @@ -53,9 +54,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) @@ -107,15 +111,17 @@ export async function PATCH( // 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 access = await validateWorkflowAccess(request, id, { + requireDeployment: false, + action: requiredPermission, + }) + if (access.error) { + return createErrorResponse(access.error.message, access.error.status) } + const auth = access.auth + const workflowData = access.workflow + const versionNum = Number(version) if (!Number.isFinite(versionNum)) { return createErrorResponse('Invalid version', 400) @@ -123,7 +129,7 @@ export async function PATCH( // Handle activation if (isActive) { - const actorUserId = session?.user?.id + const actorUserId = auth?.userId if (!actorUserId) { logger.warn(`[${requestId}] Unable to resolve actor user for deployment activation: ${id}`) return createErrorResponse('Unable to determine activating user', 400) @@ -298,11 +304,13 @@ export async function PATCH( } } + const { actorName, actorEmail } = getAuditActorMetadata(auth) + recordAudit({ workspaceId: workflowData?.workspaceId, actorId: actorUserId, - actorName: session?.user?.name, - actorEmail: session?.user?.email, + 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]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index d886e27d466..e48ed9bd6e8 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 mockDbDelete = 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: { @@ -358,16 +371,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, @@ -375,30 +396,39 @@ 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' }]), }), }) + mockDbDelete.mockReturnValue({ + where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }]), + }) + 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: undefined, + 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', } @@ -407,11 +437,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.concurrent('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', { @@ -480,6 +536,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 () => { @@ -527,24 +584,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', { @@ -713,7 +759,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, @@ -755,7 +804,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, diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 19d89e8eeb7..3c0a7977b88 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -4,13 +4,15 @@ 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 { 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 { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils' +import { validateWorkflowAccess } from '@/app/api/workflows/middleware' const logger = createLogger('WorkflowByIdAPI') @@ -146,38 +148,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 @@ -328,11 +327,13 @@ export async function DELETE( // Don't fail the deletion if Socket.IO notification fails } + 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, @@ -362,42 +363,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..8e2304737a0 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/status/route.test.ts @@ -0,0 +1,93 @@ +/** + * @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.ts b/apps/sim/app/api/workflows/middleware.ts index d6260002fe3..bfcaed902c2 100644 --- a/apps/sim/app/api/workflows/middleware.ts +++ b/apps/sim/app/api/workflows/middleware.ts @@ -17,12 +17,24 @@ export interface ValidationResult { auth?: AuthResult } +export interface WorkflowAccessOptions { + requireDeployment?: boolean + action?: 'read' | 'write' | 'admin' + allowInternalSecret?: boolean +} + export async function validateWorkflowAccess( request: NextRequest, workflowId: string, - requireDeployment = true + options: boolean | WorkflowAccessOptions = true ): Promise { try { + const normalizedOptions: WorkflowAccessOptions = + typeof options === 'boolean' ? { requireDeployment: options } : options + const requireDeployment = normalizedOptions.requireDeployment ?? true + const action = normalizedOptions.action ?? 'read' + const allowInternalSecret = normalizedOptions.allowInternalSecret ?? false + const workflow = await getWorkflowById(workflowId) if (!workflow) { return { @@ -57,7 +69,7 @@ export async function validateWorkflowAccess( const authorization = await authorizeWorkflowByWorkspacePermission({ workflowId, userId: auth.userId, - action: 'read', + action, }) if (!authorization.allowed) { return { @@ -82,7 +94,7 @@ 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.ts b/apps/sim/lib/audit/actor-metadata.ts new file mode 100644 index 00000000000..2ea44c3d41f --- /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?.authType !== 'session') { + return { + actorName: undefined, + actorEmail: undefined, + } + } + + return { + actorName: auth.userName ?? undefined, + actorEmail: auth.userEmail ?? undefined, + } +}