Skip to content

Commit c2a5964

Browse files
PlaneInABottletest
authored andcommitted
fix(workflows): allow app auth on workflow lifecycle actions
1 parent 568c128 commit c2a5964

File tree

4 files changed

+256
-17
lines changed

4 files changed

+256
-17
lines changed
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
5+
import { NextRequest } from 'next/server'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const mockValidateWorkflowAccess = vi.fn()
9+
const mockDbSelect = vi.fn()
10+
const mockDbFrom = vi.fn()
11+
const mockDbWhere = vi.fn()
12+
const mockDbLimit = vi.fn()
13+
const mockDbOrderBy = vi.fn()
14+
const mockDeployWorkflow = vi.fn()
15+
const mockUndeployWorkflow = vi.fn()
16+
const mockCleanupWebhooksForWorkflow = vi.fn()
17+
const mockRemoveMcpToolsForWorkflow = vi.fn()
18+
19+
vi.mock('@sim/logger', () => ({
20+
createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }),
21+
}))
22+
23+
vi.mock('@/lib/workflows/utils', () => ({
24+
validateWorkflowPermissions: vi.fn(),
25+
}))
26+
27+
vi.mock('@/app/api/workflows/middleware', () => ({
28+
validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args),
29+
}))
30+
31+
vi.mock('@/lib/core/utils/request', () => ({
32+
generateRequestId: () => 'req-123',
33+
}))
34+
35+
vi.mock('@sim/db', () => ({
36+
db: { select: mockDbSelect },
37+
workflow: { variables: 'variables', id: 'id' },
38+
workflowDeploymentVersion: { state: 'state', workflowId: 'workflowId', isActive: 'isActive', createdAt: 'createdAt', id: 'id' },
39+
}))
40+
41+
vi.mock('drizzle-orm', () => ({
42+
and: vi.fn(),
43+
desc: vi.fn(),
44+
eq: vi.fn(),
45+
}))
46+
47+
vi.mock('@/lib/workflows/persistence/utils', () => ({
48+
loadWorkflowFromNormalizedTables: vi.fn().mockResolvedValue(null),
49+
deployWorkflow: (...args: unknown[]) => mockDeployWorkflow(...args),
50+
undeployWorkflow: (...args: unknown[]) => mockUndeployWorkflow(...args),
51+
}))
52+
53+
vi.mock('@/lib/workflows/comparison', () => ({
54+
hasWorkflowChanged: vi.fn().mockReturnValue(false),
55+
}))
56+
57+
vi.mock('@/lib/workflows/schedules', () => ({
58+
cleanupDeploymentVersion: vi.fn(),
59+
createSchedulesForDeploy: vi.fn(),
60+
validateWorkflowSchedules: vi.fn().mockReturnValue({ isValid: true }),
61+
}))
62+
63+
vi.mock('@/lib/webhooks/deploy', () => ({
64+
cleanupWebhooksForWorkflow: (...args: unknown[]) => mockCleanupWebhooksForWorkflow(...args),
65+
restorePreviousVersionWebhooks: vi.fn(),
66+
saveTriggerWebhooksForDeploy: vi.fn(),
67+
}))
68+
69+
vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({
70+
removeMcpToolsForWorkflow: (...args: unknown[]) => mockRemoveMcpToolsForWorkflow(...args),
71+
syncMcpToolsForWorkflow: vi.fn(),
72+
}))
73+
74+
vi.mock('@/lib/audit/log', () => ({
75+
AuditAction: {},
76+
AuditResourceType: {},
77+
recordAudit: vi.fn(),
78+
}))
79+
80+
import { POST, DELETE } from '@/app/api/workflows/[id]/deploy/route'
81+
82+
describe('Workflow deploy route', () => {
83+
beforeEach(() => {
84+
vi.clearAllMocks()
85+
mockDbSelect.mockReturnValue({ from: mockDbFrom })
86+
mockDbFrom.mockReturnValue({ where: mockDbWhere })
87+
mockDbWhere.mockReturnValue({ limit: mockDbLimit, orderBy: mockDbOrderBy })
88+
mockDbOrderBy.mockReturnValue({ limit: mockDbLimit })
89+
mockDbLimit.mockResolvedValue([])
90+
mockCleanupWebhooksForWorkflow.mockResolvedValue(undefined)
91+
mockRemoveMcpToolsForWorkflow.mockResolvedValue(undefined)
92+
})
93+
94+
it('allows API-key auth for deploy using hybrid auth userId', async () => {
95+
mockValidateWorkflowAccess.mockResolvedValue({
96+
workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
97+
auth: { success: true, userId: 'api-user', authType: 'api_key' },
98+
})
99+
mockDeployWorkflow.mockResolvedValue({
100+
success: true,
101+
deployedAt: '2024-01-01T00:00:00Z',
102+
deploymentVersionId: 'dep-1',
103+
})
104+
105+
const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', {
106+
method: 'POST',
107+
headers: { 'x-api-key': 'test-key' },
108+
})
109+
const response = await POST(req, { params: Promise.resolve({ id: 'wf-1' }) })
110+
111+
expect(response.status).toBe(200)
112+
const data = await response.json()
113+
expect(data.isDeployed).toBe(true)
114+
expect(mockDeployWorkflow).toHaveBeenCalledWith({
115+
workflowId: 'wf-1',
116+
deployedBy: 'api-user',
117+
workflowName: 'Test Workflow',
118+
})
119+
})
120+
121+
it('allows API-key auth for undeploy using hybrid auth userId', async () => {
122+
mockValidateWorkflowAccess.mockResolvedValue({
123+
workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
124+
auth: { success: true, userId: 'api-user', authType: 'api_key' },
125+
})
126+
mockUndeployWorkflow.mockResolvedValue({ success: true })
127+
128+
const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', {
129+
method: 'DELETE',
130+
headers: { 'x-api-key': 'test-key' },
131+
})
132+
const response = await DELETE(req, { params: Promise.resolve({ id: 'wf-1' }) })
133+
134+
expect(response.status).toBe(200)
135+
const data = await response.json()
136+
expect(data.isDeployed).toBe(false)
137+
expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' })
138+
})
139+
})

apps/sim/app/api/workflows/[id]/deploy/route.ts

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
validateWorkflowSchedules,
2222
} from '@/lib/workflows/schedules'
2323
import { validateWorkflowPermissions } from '@/lib/workflows/utils'
24+
import { validateWorkflowAccess } from '@/app/api/workflows/middleware'
2425
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
2526
import type { WorkflowState } from '@/stores/workflows/workflow/types'
2627

@@ -29,20 +30,47 @@ const logger = createLogger('WorkflowDeployAPI')
2930
export const dynamic = 'force-dynamic'
3031
export const runtime = 'nodejs'
3132

33+
async function validateLifecycleAdminAccess(
34+
request: NextRequest,
35+
workflowId: string,
36+
requestId: string
37+
) {
38+
const hybridAccess = await validateWorkflowAccess(request, workflowId, {
39+
requireDeployment: false,
40+
action: 'admin',
41+
})
42+
43+
if (hybridAccess.error) {
44+
return hybridAccess
45+
}
46+
47+
if (hybridAccess.auth?.authType === 'session') {
48+
return await validateWorkflowPermissions(workflowId, requestId, 'admin')
49+
}
50+
51+
return {
52+
error: null,
53+
auth: hybridAccess.auth,
54+
session: null,
55+
workflow: hybridAccess.workflow,
56+
}
57+
}
58+
3259
export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) {
3360
const requestId = generateRequestId()
3461
const { id } = await params
3562

3663
try {
37-
const { error, workflow: workflowData } = await validateWorkflowPermissions(
38-
id,
39-
requestId,
40-
'read'
41-
)
42-
if (error) {
43-
return createErrorResponse(error.message, error.status)
64+
const access = await validateWorkflowAccess(request, id, {
65+
requireDeployment: false,
66+
action: 'read',
67+
})
68+
if (access.error) {
69+
return createErrorResponse(access.error.message, access.error.status)
4470
}
4571

72+
const workflowData = access.workflow
73+
4674
if (!workflowData.isDeployed) {
4775
logger.info(`[${requestId}] Workflow is not deployed: ${id}`)
4876
return createSuccessResponse({
@@ -115,15 +143,16 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
115143

116144
try {
117145
const {
146+
auth,
118147
error,
119148
session,
120149
workflow: workflowData,
121-
} = await validateWorkflowPermissions(id, requestId, 'admin')
150+
} = await validateLifecycleAdminAccess(request, id, requestId)
122151
if (error) {
123152
return createErrorResponse(error.message, error.status)
124153
}
125154

126-
const actorUserId: string | null = session?.user?.id ?? null
155+
const actorUserId: string | null = session?.user?.id ?? auth?.userId ?? null
127156
if (!actorUserId) {
128157
logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment: ${id}`)
129158
return createErrorResponse('Unable to determine deploying user', 400)
@@ -309,7 +338,7 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise<
309338
const { id } = await params
310339

311340
try {
312-
const { error, session } = await validateWorkflowPermissions(id, requestId, 'admin')
341+
const { error, session } = await validateLifecycleAdminAccess(request, id, requestId)
313342
if (error) {
314343
return createErrorResponse(error.message, error.status)
315344
}
@@ -356,14 +385,20 @@ export async function DELETE(
356385

357386
try {
358387
const {
388+
auth,
359389
error,
360390
session,
361391
workflow: workflowData,
362-
} = await validateWorkflowPermissions(id, requestId, 'admin')
392+
} = await validateLifecycleAdminAccess(request, id, requestId)
363393
if (error) {
364394
return createErrorResponse(error.message, error.status)
365395
}
366396

397+
const actorUserId = session?.user?.id ?? auth?.userId ?? null
398+
if (!actorUserId) {
399+
return createErrorResponse('Unable to determine undeploying user', 400)
400+
}
401+
367402
// Clean up external webhook subscriptions before undeploying
368403
await cleanupWebhooksForWorkflow(id, workflowData as Record<string, unknown>, requestId)
369404

@@ -385,7 +420,7 @@ export async function DELETE(
385420

386421
recordAudit({
387422
workspaceId: workflowData?.workspaceId || null,
388-
actorId: session!.user.id,
423+
actorId: actorUserId,
389424
actorName: session?.user?.name,
390425
actorEmail: session?.user?.email,
391426
action: AuditAction.WORKFLOW_UNDEPLOYED,

apps/sim/app/api/workflows/[id]/route.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const mockAuthorizeWorkflowByWorkspacePermission = vi.fn()
2424
const mockDbDelete = vi.fn()
2525
const mockDbUpdate = vi.fn()
2626
const mockDbSelect = vi.fn()
27+
const mockValidateWorkflowAccess = vi.fn()
2728

2829
/**
2930
* Helper to set mock auth state consistently across getSession and hybrid auth.
@@ -32,9 +33,16 @@ function mockGetSession(session: { user: { id: string } } | null) {
3233
if (session) {
3334
mockCheckHybridAuth.mockResolvedValue({ success: true, userId: session.user.id })
3435
mockCheckSessionOrInternalAuth.mockResolvedValue({ success: true, userId: session.user.id })
36+
mockValidateWorkflowAccess.mockResolvedValue({
37+
workflow: { id: 'workflow-123', workspaceId: 'workspace-456' },
38+
auth: { success: true, userId: session.user.id, authType: 'session' },
39+
})
3540
} else {
3641
mockCheckHybridAuth.mockResolvedValue({ success: false })
3742
mockCheckSessionOrInternalAuth.mockResolvedValue({ success: false })
43+
mockValidateWorkflowAccess.mockResolvedValue({
44+
error: { message: 'Unauthorized', status: 401 },
45+
})
3846
}
3947
}
4048

@@ -62,6 +70,10 @@ vi.mock('@/lib/workflows/persistence/utils', () => ({
6270
mockLoadWorkflowFromNormalizedTables(workflowId),
6371
}))
6472

73+
vi.mock('@/app/api/workflows/middleware', () => ({
74+
validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args),
75+
}))
76+
6577
vi.mock('@/lib/workflows/utils', () => ({
6678
getWorkflowById: (workflowId: string) => mockGetWorkflowById(workflowId),
6779
authorizeWorkflowByWorkspacePermission: (params: {
@@ -357,6 +369,46 @@ describe('Workflow By ID API Route', () => {
357369
expect(data.success).toBe(true)
358370
})
359371

372+
it('should allow API-key-backed deletion when workflow access is validated', async () => {
373+
const mockWorkflow = {
374+
id: 'workflow-123',
375+
userId: 'other-user',
376+
name: 'Test Workflow',
377+
workspaceId: 'workspace-456',
378+
}
379+
380+
mockValidateWorkflowAccess.mockResolvedValue({
381+
workflow: mockWorkflow,
382+
auth: { success: true, userId: 'api-user-1', authType: 'api_key' },
383+
})
384+
mockGetWorkflowById.mockResolvedValue(mockWorkflow)
385+
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
386+
allowed: true,
387+
status: 200,
388+
workflow: mockWorkflow,
389+
workspacePermission: 'admin',
390+
})
391+
mockDbSelect.mockReturnValue({
392+
from: vi.fn().mockReturnValue({
393+
where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }, { id: 'workflow-456' }]),
394+
}),
395+
})
396+
mockDbDelete.mockReturnValue({
397+
where: vi.fn().mockResolvedValue([{ id: 'workflow-123' }]),
398+
})
399+
setupGlobalFetchMock({ ok: true })
400+
401+
const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', {
402+
method: 'DELETE',
403+
headers: { 'x-api-key': 'test-key' },
404+
})
405+
const params = Promise.resolve({ id: 'workflow-123' })
406+
407+
const response = await DELETE(req, { params })
408+
409+
expect(response.status).toBe(200)
410+
})
411+
360412
it('should prevent deletion of the last workflow in workspace', async () => {
361413
const mockWorkflow = {
362414
id: 'workflow-123',

apps/sim/app/api/workflows/[id]/route.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ import { and, eq, isNull, ne } from 'drizzle-orm'
55
import { type NextRequest, NextResponse } from 'next/server'
66
import { z } from 'zod'
77
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
8-
import { checkHybridAuth, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
8+
import { checkHybridAuth } from '@/lib/auth/hybrid'
99
import { env } from '@/lib/core/config/env'
1010
import { PlatformEvents } from '@/lib/core/telemetry'
1111
import { generateRequestId } from '@/lib/core/utils/request'
1212
import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
1313
import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils'
14+
import { validateWorkflowAccess } from '@/app/api/workflows/middleware'
1415

1516
const logger = createLogger('WorkflowByIdAPI')
1617

@@ -146,13 +147,25 @@ export async function DELETE(
146147
const { id: workflowId } = await params
147148

148149
try {
149-
const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
150-
if (!auth.success || !auth.userId) {
150+
const validation = await validateWorkflowAccess(request, workflowId, {
151+
requireDeployment: false,
152+
action: 'admin',
153+
})
154+
if (validation.error) {
151155
logger.warn(`[${requestId}] Unauthorized deletion attempt for workflow ${workflowId}`)
152-
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
156+
return NextResponse.json({ error: validation.error.message }, { status: validation.error.status })
153157
}
154158

155-
const userId = auth.userId
159+
const auth = validation.auth
160+
const userId = auth?.userId
161+
162+
if (!userId) {
163+
logger.warn(`[${requestId}] Missing user identity for workflow deletion ${workflowId}`)
164+
return NextResponse.json(
165+
{ error: 'Workflow deletion requires a user-backed session or API key identity' },
166+
{ status: 400 }
167+
)
168+
}
156169

157170
const authorization = await authorizeWorkflowByWorkspacePermission({
158171
workflowId,

0 commit comments

Comments
 (0)