fix(workflows): preserve auth ordering and harden async tests

test · test · commit cd244435d98f · 2026-03-15T01:15:40.000+03:00
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
@@ -12,13 +12,21 @@ const {
   mockGetJobQueue,
   mockShouldExecuteInline,
   mockEnqueue,
+  mockLoggerInfo,
+  mockLoggerWarn,
+  mockLoggerError,
+  mockLoggerDebug,
 } = vi.hoisted(() => ({
   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', () => ({
@@ -70,10 +78,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,
   }),
 }))
 
@@ -84,10 +92,27 @@ 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()
 
+    asyncCheckpoints.length = 0
+
     mockCheckHybridAuth.mockReset()
     mockAuthorizeWorkflowByWorkspacePermission.mockReset()
     mockPreprocessExecution.mockReset()
@@ -102,31 +127,58 @@ describe('workflow execute async route', () => {
       completeJob: vi.fn(),
       markJobFailed: vi.fn(),
     })
-    mockShouldExecuteInline.mockReturnValue(false)
+    mockShouldExecuteInline.mockImplementation(() => {
+      asyncCheckpoints.push('shouldExecuteInline')
+      return false
+    })
+
+    mockCheckHybridAuth.mockImplementation(async () => {
+      asyncCheckpoints.push('authorization')
+      return {
+        success: true,
+        userId: 'session-user-1',
+        authType: 'session',
+      }
+    })
+
+    mockAuthorizeWorkflowByWorkspacePermission.mockImplementation(async () => {
+      asyncCheckpoints.push('authorizeWorkflowByWorkspacePermission')
+      return {
+        allowed: true,
+        workflow: {
+          id: 'workflow-1',
+          userId: 'owner-1',
+          workspaceId: 'workspace-1',
+        },
+      }
+    })
 
-    mockCheckHybridAuth.mockResolvedValue({
-      success: true,
-      userId: 'session-user-1',
-      authType: 'session',
+    mockPreprocessExecution.mockImplementation(async () => {
+      asyncCheckpoints.push('preprocessing')
+      return {
+        success: true,
+        actorUserId: 'actor-1',
+        workflowRecord: {
+          id: 'workflow-1',
+          userId: 'owner-1',
+          workspaceId: 'workspace-1',
+        },
+      }
     })
 
-    mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
-      allowed: true,
-      workflow: {
-        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(),
+      }
     })
 
-    mockPreprocessExecution.mockResolvedValue({
-      success: true,
-      actorUserId: 'actor-1',
-      workflowRecord: {
-        id: 'workflow-1',
-        userId: 'owner-1',
-        workspaceId: 'workspace-1',
-      },
+    mockEnqueue.mockImplementation(async () => {
+      asyncCheckpoints.push('enqueue')
+      return 'job-123'
     })
   })
 
@@ -142,11 +194,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({
@@ -177,4 +241,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/middleware.test.ts b/apps/sim/app/api/workflows/middleware.test.ts
@@ -0,0 +1,173 @@
+/**
+ * @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', () => ({
+  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<string, unknown> = {}) {
+  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 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 })
+  })
+})
diff --git a/apps/sim/app/api/workflows/middleware.ts b/apps/sim/app/api/workflows/middleware.ts
