Skip to content

Commit 2b05c74

Browse files
PlaneInABottletest
authored andcommitted
fix(workflows): allow app auth on workflow lifecycle actions
1 parent 507c84f commit 2b05c74

4 files changed

Lines changed: 270 additions & 16 deletions

File tree

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: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
validateWorkflowSchedules,
2323
} from '@/lib/workflows/schedules'
2424
import { validateWorkflowPermissions } from '@/lib/workflows/utils'
25+
import { validateWorkflowAccess } from '@/app/api/workflows/middleware'
2526
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
2627
import type { WorkflowState } from '@/stores/workflows/workflow/types'
2728

@@ -30,20 +31,59 @@ const logger = createLogger('WorkflowDeployAPI')
3031
export const dynamic = 'force-dynamic'
3132
export const runtime = 'nodejs'
3233

34+
async function validateLifecycleAdminAccess(
35+
request: NextRequest,
36+
workflowId: string,
37+
requestId: string
38+
) {
39+
const hybridAccess = await validateWorkflowAccess(request, workflowId, {
40+
requireDeployment: false,
41+
action: 'admin',
42+
})
43+
44+
if (hybridAccess.error) {
45+
return {
46+
error: hybridAccess.error,
47+
auth: null,
48+
session: null,
49+
workflow: null,
50+
}
51+
}
52+
53+
if (hybridAccess.auth?.authType === 'session') {
54+
const sessionAccess = await validateWorkflowPermissions(workflowId, requestId, 'admin')
55+
56+
return {
57+
error: sessionAccess.error,
58+
auth: null,
59+
session: sessionAccess.session,
60+
workflow: sessionAccess.workflow,
61+
}
62+
}
63+
64+
return {
65+
error: null,
66+
auth: hybridAccess.auth,
67+
session: null,
68+
workflow: hybridAccess.workflow,
69+
}
70+
}
71+
3372
export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) {
3473
const requestId = generateRequestId()
3574
const { id } = await params
3675

3776
try {
38-
const { error, workflow: workflowData } = await validateWorkflowPermissions(
39-
id,
40-
requestId,
41-
'read'
42-
)
43-
if (error) {
44-
return createErrorResponse(error.message, error.status)
77+
const access = await validateWorkflowAccess(request, id, {
78+
requireDeployment: false,
79+
action: 'read',
80+
})
81+
if (access.error) {
82+
return createErrorResponse(access.error.message, access.error.status)
4583
}
4684

85+
const workflowData = access.workflow
86+
4787
if (!workflowData.isDeployed) {
4888
logger.info(`[${requestId}] Workflow is not deployed: ${id}`)
4989
return createSuccessResponse({
@@ -116,15 +156,16 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
116156

117157
try {
118158
const {
159+
auth,
119160
error,
120161
session,
121162
workflow: workflowData,
122-
} = await validateWorkflowPermissions(id, requestId, 'admin')
163+
} = await validateLifecycleAdminAccess(request, id, requestId)
123164
if (error) {
124165
return createErrorResponse(error.message, error.status)
125166
}
126167

127-
const actorUserId: string | null = session?.user?.id ?? null
168+
const actorUserId: string | null = session?.user?.id ?? auth?.userId ?? null
128169
if (!actorUserId) {
129170
logger.warn(`[${requestId}] Unable to resolve actor user for workflow deployment: ${id}`)
130171
return createErrorResponse('Unable to determine deploying user', 400)
@@ -322,7 +363,7 @@ export async function PATCH(request: NextRequest, { params }: { params: Promise<
322363
const { id } = await params
323364

324365
try {
325-
const { error, session } = await validateWorkflowPermissions(id, requestId, 'admin')
366+
const { error, session } = await validateLifecycleAdminAccess(request, id, requestId)
326367
if (error) {
327368
return createErrorResponse(error.message, error.status)
328369
}
@@ -369,14 +410,20 @@ export async function DELETE(
369410

370411
try {
371412
const {
413+
auth,
372414
error,
373415
session,
374416
workflow: workflowData,
375-
} = await validateWorkflowPermissions(id, requestId, 'admin')
417+
} = await validateLifecycleAdminAccess(request, id, requestId)
376418
if (error) {
377419
return createErrorResponse(error.message, error.status)
378420
}
379421

422+
const actorUserId = session?.user?.id ?? auth?.userId ?? null
423+
if (!actorUserId) {
424+
return createErrorResponse('Unable to determine undeploying user', 400)
425+
}
426+
380427
const result = await undeployWorkflow({ workflowId: id })
381428
if (!result.success) {
382429
return createErrorResponse(result.error || 'Failed to undeploy workflow', 500)
@@ -397,7 +444,7 @@ export async function DELETE(
397444

398445
recordAudit({
399446
workspaceId: workflowData?.workspaceId || null,
400-
actorId: session!.user.id,
447+
actorId: actorUserId,
401448
actorName: session?.user?.name,
402449
actorEmail: session?.user?.email,
403450
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 mockArchiveWorkflow = 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

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

74+
vi.mock('@/app/api/workflows/middleware', () => ({
75+
validateWorkflowAccess: (...args: unknown[]) => mockValidateWorkflowAccess(...args),
76+
}))
77+
6678
vi.mock('@/lib/workflows/utils', () => ({
6779
getWorkflowById: (workflowId: string) => mockGetWorkflowById(workflowId),
6880
authorizeWorkflowByWorkspacePermission: (params: {
@@ -363,6 +375,46 @@ describe('Workflow By ID API Route', () => {
363375
expect(data.success).toBe(true)
364376
})
365377

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

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
1010
import { archiveWorkflow } from '@/lib/workflows/lifecycle'
1111
import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
1212
import { authorizeWorkflowByWorkspacePermission, getWorkflowById } from '@/lib/workflows/utils'
13+
import { validateWorkflowAccess } from '@/app/api/workflows/middleware'
1314

1415
const logger = createLogger('WorkflowByIdAPI')
1516

@@ -152,13 +153,28 @@ export async function DELETE(
152153
const { id: workflowId } = await params
153154

154155
try {
155-
const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
156-
if (!auth.success || !auth.userId) {
156+
const validation = await validateWorkflowAccess(request, workflowId, {
157+
requireDeployment: false,
158+
action: 'admin',
159+
})
160+
if (validation.error) {
157161
logger.warn(`[${requestId}] Unauthorized deletion attempt for workflow ${workflowId}`)
158-
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
162+
return NextResponse.json(
163+
{ error: validation.error.message },
164+
{ status: validation.error.status }
165+
)
159166
}
160167

161-
const userId = auth.userId
168+
const auth = validation.auth
169+
const userId = auth?.userId
170+
171+
if (!userId) {
172+
logger.warn(`[${requestId}] Missing user identity for workflow deletion ${workflowId}`)
173+
return NextResponse.json(
174+
{ error: 'Workflow deletion requires a user-backed session or API key identity' },
175+
{ status: 400 }
176+
)
177+
}
162178

163179
const authorization = await authorizeWorkflowByWorkspacePermission({
164180
workflowId,

0 commit comments

Comments
 (0)