diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts new file mode 100644 index 000000000..9c4462f2a --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -0,0 +1,644 @@ +/** + * Integration tests for workflow by ID API route + * Tests the new centralized permissions system + * + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +describe('Workflow By ID API Route', () => { + const mockLogger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + } + + beforeEach(() => { + vi.resetModules() + + vi.stubGlobal('crypto', { + randomUUID: vi.fn().mockReturnValue('mock-request-id-12345678'), + }) + + vi.doMock('@/lib/logs/console-logger', () => ({ + createLogger: vi.fn().mockReturnValue(mockLogger), + })) + + vi.doMock('@/lib/workflows/db-helpers', () => ({ + loadWorkflowFromNormalizedTables: vi.fn().mockResolvedValue(null), + })) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe('GET /api/workflows/[id]', () => { + it('should return 401 when user is not authenticated', async () => { + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue(null), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(401) + const data = await response.json() + expect(data.error).toBe('Unauthorized') + }) + + it('should return 404 when workflow does not exist', async () => { + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(undefined), + }), + }), + }), + }, + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/nonexistent') + const params = Promise.resolve({ id: 'nonexistent' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(404) + const data = await response.json() + expect(data.error).toBe('Workflow not found') + }) + + it.concurrent('should allow access when user owns the workflow', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + name: 'Test Workflow', + workspaceId: null, + state: { blocks: {}, edges: [] }, + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + }, + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.data.id).toBe('workflow-123') + }) + + it.concurrent('should allow access when user has workspace permissions', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + name: 'Test Workflow', + workspaceId: 'workspace-456', + state: { blocks: {}, edges: [] }, + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + }, + })) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue('read'), + hasAdminPermission: vi.fn().mockResolvedValue(false), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.data.id).toBe('workflow-123') + }) + + it('should deny access when user has no workspace permissions', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + name: 'Test Workflow', + workspaceId: 'workspace-456', + state: { blocks: {}, edges: [] }, + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + }, + })) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue(null), + hasAdminPermission: vi.fn().mockResolvedValue(false), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(403) + const data = await response.json() + expect(data.error).toBe('Access denied') + }) + + it.concurrent('should use normalized tables when available', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + name: 'Test Workflow', + workspaceId: null, + state: { blocks: {}, edges: [] }, + } + + const mockNormalizedData = { + blocks: { 'block-1': { id: 'block-1', type: 'starter' } }, + edges: [{ id: 'edge-1', source: 'block-1', target: 'block-2' }], + loops: {}, + parallels: {}, + isFromNormalizedTables: true, + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + }, + })) + + vi.doMock('@/lib/workflows/db-helpers', () => ({ + loadWorkflowFromNormalizedTables: vi.fn().mockResolvedValue(mockNormalizedData), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.data.state.blocks).toEqual(mockNormalizedData.blocks) + expect(data.data.state.edges).toEqual(mockNormalizedData.edges) + }) + }) + + describe('DELETE /api/workflows/[id]', () => { + it('should allow owner to delete workflow', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + name: 'Test Workflow', + workspaceId: null, + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + const mockTransaction = vi.fn().mockImplementation(async (callback) => { + await callback({ + delete: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue(undefined), + }), + }) + }) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + transaction: mockTransaction, + }, + })) + + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'DELETE', + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { DELETE } = await import('./route') + const response = await DELETE(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.success).toBe(true) + }) + + it('should allow admin to delete workspace workflow', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + name: 'Test Workflow', + workspaceId: 'workspace-456', + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + const mockTransaction = vi.fn().mockImplementation(async (callback) => { + await callback({ + delete: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue(undefined), + }), + }) + }) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + transaction: mockTransaction, + }, + })) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue('admin'), + hasAdminPermission: vi.fn().mockResolvedValue(true), + })) + + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'DELETE', + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { DELETE } = await import('./route') + const response = await DELETE(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.success).toBe(true) + }) + + it.concurrent('should deny deletion for non-admin users', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + name: 'Test Workflow', + workspaceId: 'workspace-456', + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + }, + })) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue('read'), + hasAdminPermission: vi.fn().mockResolvedValue(false), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'DELETE', + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { DELETE } = await import('./route') + const response = await DELETE(req, { params }) + + expect(response.status).toBe(403) + const data = await response.json() + expect(data.error).toBe('Access denied') + }) + }) + + describe('PUT /api/workflows/[id]', () => { + it('should allow owner to update workflow', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + name: 'Test Workflow', + workspaceId: null, + } + + const updateData = { name: 'Updated Workflow' } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + update: vi.fn().mockReturnValue({ + set: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + returning: vi.fn().mockResolvedValue([{ ...mockWorkflow, ...updateData }]), + }), + }), + }), + }, + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'PUT', + body: JSON.stringify(updateData), + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { PUT } = await import('./route') + const response = await PUT(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.workflow.name).toBe('Updated Workflow') + }) + + it('should allow users with write permission to update workflow', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + name: 'Test Workflow', + workspaceId: 'workspace-456', + } + + const updateData = { name: 'Updated Workflow' } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + update: vi.fn().mockReturnValue({ + set: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + returning: vi.fn().mockResolvedValue([{ ...mockWorkflow, ...updateData }]), + }), + }), + }), + }, + })) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue('write'), + hasAdminPermission: vi.fn().mockResolvedValue(false), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'PUT', + body: JSON.stringify(updateData), + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { PUT } = await import('./route') + const response = await PUT(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.workflow.name).toBe('Updated Workflow') + }) + + 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' } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + }, + })) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue('read'), + hasAdminPermission: vi.fn().mockResolvedValue(false), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'PUT', + body: JSON.stringify(updateData), + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { PUT } = await import('./route') + const response = await PUT(req, { params }) + + expect(response.status).toBe(403) + const data = await response.json() + expect(data.error).toBe('Access denied') + }) + + it.concurrent('should validate request data', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + name: 'Test Workflow', + workspaceId: null, + } + + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockResolvedValue(mockWorkflow), + }), + }), + }), + }, + })) + + // Invalid data - empty name + const invalidData = { name: '' } + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123', { + method: 'PUT', + body: JSON.stringify(invalidData), + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { PUT } = await import('./route') + const response = await PUT(req, { params }) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Invalid request data') + }) + }) + + describe('Error handling', () => { + it.concurrent('should handle database errors gracefully', async () => { + vi.doMock('@/lib/auth', () => ({ + getSession: vi.fn().mockResolvedValue({ + user: { id: 'user-123' }, + }), + })) + + vi.doMock('@/db', () => ({ + db: { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + then: vi.fn().mockRejectedValue(new Error('Database connection timeout')), + }), + }), + }), + }, + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(500) + const data = await response.json() + expect(data.error).toBe('Internal server error') + expect(mockLogger.error).toHaveBeenCalled() + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 2068a2016..fd0286faa 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -1,21 +1,15 @@ -import { and, eq } from 'drizzle-orm' +import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' import { getSession } from '@/lib/auth' import { createLogger } from '@/lib/logs/console-logger' +import { getUserEntityPermissions, hasAdminPermission } from '@/lib/permissions/utils' import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/db-helpers' import { db } from '@/db' -import { - workflow, - workflowBlocks, - workflowEdges, - workflowSubflows, - workspaceMember, -} from '@/db/schema' +import { workflow, workflowBlocks, workflowEdges, workflowSubflows } from '@/db/schema' const logger = createLogger('WorkflowByIdAPI') -// Schema for workflow metadata updates const UpdateWorkflowSchema = z.object({ name: z.string().min(1, 'Name is required').optional(), description: z.string().optional(), @@ -63,20 +57,14 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ hasAccess = true } - // Case 2: Workflow belongs to a workspace the user is a member of + // Case 2: Workflow belongs to a workspace the user has permissions for if (!hasAccess && workflowData.workspaceId) { - const membership = await db - .select({ id: workspaceMember.id }) - .from(workspaceMember) - .where( - and( - eq(workspaceMember.workspaceId, workflowData.workspaceId), - eq(workspaceMember.userId, userId) - ) - ) - .then((rows) => rows[0]) - - if (membership) { + const userPermission = await getUserEntityPermissions( + userId, + 'workspace', + workflowData.workspaceId + ) + if (userPermission !== null) { hasAccess = true } } @@ -182,20 +170,10 @@ export async function DELETE( canDelete = true } - // Case 2: Workflow belongs to a workspace and user has admin/owner role + // Case 2: Workflow belongs to a workspace and user has admin permission if (!canDelete && workflowData.workspaceId) { - const membership = await db - .select({ role: workspaceMember.role }) - .from(workspaceMember) - .where( - and( - eq(workspaceMember.workspaceId, workflowData.workspaceId), - eq(workspaceMember.userId, userId) - ) - ) - .then((rows) => rows[0]) - - if (membership && (membership.role === 'owner' || membership.role === 'admin')) { + const hasAdmin = await hasAdminPermission(userId, workflowData.workspaceId) + if (hasAdmin) { canDelete = true } } @@ -300,20 +278,14 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{ canUpdate = true } - // Case 2: Workflow belongs to a workspace and user has admin/owner role + // Case 2: Workflow belongs to a workspace and user has write or admin permission if (!canUpdate && workflowData.workspaceId) { - const membership = await db - .select({ role: workspaceMember.role }) - .from(workspaceMember) - .where( - and( - eq(workspaceMember.workspaceId, workflowData.workspaceId), - eq(workspaceMember.userId, userId) - ) - ) - .then((rows) => rows[0]) - - if (membership && (membership.role === 'owner' || membership.role === 'admin')) { + const userPermission = await getUserEntityPermissions( + userId, + 'workspace', + workflowData.workspaceId + ) + if (userPermission === 'write' || userPermission === 'admin') { canUpdate = true } } diff --git a/apps/sim/app/api/workflows/[id]/variables/route.test.ts b/apps/sim/app/api/workflows/[id]/variables/route.test.ts new file mode 100644 index 000000000..b09cbb290 --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/variables/route.test.ts @@ -0,0 +1,325 @@ +/** + * Tests for workflow variables API route + * Tests the optimized permissions and caching system + * + * @vitest-environment node + */ + +import { NextRequest } from 'next/server' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createMockDatabase, + mockAuth, + mockCryptoUuid, + mockUser, + setupCommonApiMocks, +} from '@/app/api/__test-utils__/utils' + +describe('Workflow Variables API Route', () => { + let authMocks: ReturnType + let databaseMocks: ReturnType + + beforeEach(() => { + vi.resetModules() + setupCommonApiMocks() + mockCryptoUuid('mock-request-id-12345678') + authMocks = mockAuth(mockUser) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe('GET /api/workflows/[id]/variables', () => { + it('should return 401 when user is not authenticated', async () => { + authMocks.setUnauthenticated() + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(401) + const data = await response.json() + expect(data.error).toBe('Unauthorized') + }) + + it('should return 404 when workflow does not exist', async () => { + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[]] }, // No workflow found + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/nonexistent/variables') + const params = Promise.resolve({ id: 'nonexistent' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(404) + const data = await response.json() + expect(data.error).toBe('Workflow not found') + }) + + it('should allow access when user owns the workflow', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + workspaceId: null, + variables: { + 'var-1': { id: 'var-1', name: 'test', type: 'string', value: 'hello' }, + }, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.data).toEqual(mockWorkflow.variables) + }) + + it('should allow access when user has workspace permissions', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + workspaceId: 'workspace-456', + variables: { + 'var-1': { id: 'var-1', name: 'test', type: 'string', value: 'hello' }, + }, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + }) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue('read'), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.data).toEqual(mockWorkflow.variables) + + // Verify permissions check was called + const { getUserEntityPermissions } = await import('@/lib/permissions/utils') + expect(getUserEntityPermissions).toHaveBeenCalledWith( + 'user-123', + 'workspace', + 'workspace-456' + ) + }) + + it('should deny access when user has no workspace permissions', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + workspaceId: 'workspace-456', + variables: {}, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + }) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue(null), + })) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(401) + const data = await response.json() + expect(data.error).toBe('Unauthorized') + }) + + it.concurrent('should include proper cache headers', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + workspaceId: null, + variables: { + 'var-1': { id: 'var-1', name: 'test', type: 'string', value: 'hello' }, + }, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(200) + expect(response.headers.get('Cache-Control')).toBe('max-age=30, stale-while-revalidate=300') + expect(response.headers.get('ETag')).toMatch(/^"variables-workflow-123-\d+"$/) + }) + + it.concurrent('should return empty object for workflows with no variables', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + workspaceId: null, + variables: null, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.data).toEqual({}) + }) + }) + + describe('POST /api/workflows/[id]/variables', () => { + it('should allow owner to update variables', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + workspaceId: null, + variables: {}, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + update: { results: [{}] }, + }) + + const variables = [ + { id: 'var-1', workflowId: 'workflow-123', name: 'test', type: 'string', value: 'hello' }, + ] + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables', { + method: 'POST', + body: JSON.stringify({ variables }), + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { POST } = await import('./route') + const response = await POST(req, { params }) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.success).toBe(true) + }) + + it('should deny access for users without permissions', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'other-user', + workspaceId: 'workspace-456', + variables: {}, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + }) + + vi.doMock('@/lib/permissions/utils', () => ({ + getUserEntityPermissions: vi.fn().mockResolvedValue(null), + })) + + const variables = [ + { id: 'var-1', workflowId: 'workflow-123', name: 'test', type: 'string', value: 'hello' }, + ] + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables', { + method: 'POST', + body: JSON.stringify({ variables }), + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { POST } = await import('./route') + const response = await POST(req, { params }) + + expect(response.status).toBe(401) + const data = await response.json() + expect(data.error).toBe('Unauthorized') + }) + + it.concurrent('should validate request data schema', async () => { + const mockWorkflow = { + id: 'workflow-123', + userId: 'user-123', + workspaceId: null, + variables: {}, + } + + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { results: [[mockWorkflow]] }, + }) + + // Invalid data - missing required fields + const invalidData = { variables: [{ name: 'test' }] } + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables', { + method: 'POST', + body: JSON.stringify(invalidData), + }) + const params = Promise.resolve({ id: 'workflow-123' }) + + const { POST } = await import('./route') + const response = await POST(req, { params }) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Invalid request data') + }) + }) + + describe('Error handling', () => { + it.concurrent('should handle database errors gracefully', async () => { + authMocks.setAuthenticated({ id: 'user-123', email: 'test@example.com' }) + databaseMocks = createMockDatabase({ + select: { throwError: true, errorMessage: 'Database connection failed' }, + }) + + const req = new NextRequest('http://localhost:3000/api/workflows/workflow-123/variables') + const params = Promise.resolve({ id: 'workflow-123' }) + + const { GET } = await import('./route') + const response = await GET(req, { params }) + + expect(response.status).toBe(500) + const data = await response.json() + expect(data.error).toBe('Database connection failed') + }) + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/variables/route.ts b/apps/sim/app/api/workflows/[id]/variables/route.ts index 880593ec8..c93cb3884 100644 --- a/apps/sim/app/api/workflows/[id]/variables/route.ts +++ b/apps/sim/app/api/workflows/[id]/variables/route.ts @@ -1,10 +1,11 @@ -import { and, eq } from 'drizzle-orm' +import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { z } from 'zod' import { getSession } from '@/lib/auth' import { createLogger } from '@/lib/logs/console-logger' +import { getUserEntityPermissions } from '@/lib/permissions/utils' import { db } from '@/db' -import { workflow, workspaceMember } from '@/db/schema' +import { workflow } from '@/db/schema' import type { Variable } from '@/stores/panel/variables/types' const logger = createLogger('WorkflowVariablesAPI') @@ -47,23 +48,17 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id: const workflowData = workflowRecord[0] const workspaceId = workflowData.workspaceId - // Check authorization - either the user owns the workflow or is a member of the workspace + // Check authorization - either the user owns the workflow or has workspace permissions let isAuthorized = workflowData.userId === session.user.id - // If not authorized by ownership and the workflow belongs to a workspace, check workspace membership + // If not authorized by ownership and the workflow belongs to a workspace, check workspace permissions if (!isAuthorized && workspaceId) { - const membership = await db - .select() - .from(workspaceMember) - .where( - and( - eq(workspaceMember.workspaceId, workspaceId), - eq(workspaceMember.userId, session.user.id) - ) - ) - .limit(1) - - isAuthorized = membership.length > 0 + const userPermission = await getUserEntityPermissions( + session.user.id, + 'workspace', + workspaceId + ) + isAuthorized = userPermission !== null } if (!isAuthorized) { @@ -151,23 +146,17 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: const workflowData = workflowRecord[0] const workspaceId = workflowData.workspaceId - // Check authorization - either the user owns the workflow or is a member of the workspace + // Check authorization - either the user owns the workflow or has workspace permissions let isAuthorized = workflowData.userId === session.user.id - // If not authorized by ownership and the workflow belongs to a workspace, check workspace membership + // If not authorized by ownership and the workflow belongs to a workspace, check workspace permissions if (!isAuthorized && workspaceId) { - const membership = await db - .select() - .from(workspaceMember) - .where( - and( - eq(workspaceMember.workspaceId, workspaceId), - eq(workspaceMember.userId, session.user.id) - ) - ) - .limit(1) - - isAuthorized = membership.length > 0 + const userPermission = await getUserEntityPermissions( + session.user.id, + 'workspace', + workspaceId + ) + isAuthorized = userPermission !== null } if (!isAuthorized) { @@ -181,9 +170,10 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id: const variables = (workflowData.variables as Record) || {} // Add cache headers to prevent frequent reloading + const variableHash = JSON.stringify(variables).length const headers = new Headers({ - 'Cache-Control': 'max-age=60, stale-while-revalidate=300', // Cache for 1 minute, stale for 5 - ETag: `"${requestId}-${Object.keys(variables).length}"`, + 'Cache-Control': 'max-age=30, stale-while-revalidate=300', // Cache for 30 seconds, stale for 5 min + ETag: `"variables-${workflowId}-${variableHash}"`, }) return NextResponse.json( diff --git a/apps/sim/app/w/[id]/components/panel/components/variables/variables.tsx b/apps/sim/app/w/[id]/components/panel/components/variables/variables.tsx index 7954408c1..598a5aab2 100644 --- a/apps/sim/app/w/[id]/components/panel/components/variables/variables.tsx +++ b/apps/sim/app/w/[id]/components/panel/components/variables/variables.tsx @@ -42,12 +42,12 @@ export function Variables({ panelWidth }: VariablesProps) { // Get variables for the current workflow const workflowVariables = activeWorkflowId ? getVariablesByWorkflowId(activeWorkflowId) : [] - // Load variables when workflow changes + // Load variables when active workflow changes useEffect(() => { - if (activeWorkflowId && workflows[activeWorkflowId]) { + if (activeWorkflowId) { loadVariables(activeWorkflowId) } - }, [activeWorkflowId, workflows, loadVariables]) + }, [activeWorkflowId, loadVariables]) // Track editor references const editorRefs = useRef>({}) diff --git a/apps/sim/app/w/[id]/workflow.tsx b/apps/sim/app/w/[id]/workflow.tsx index e656b3fb1..a2e605a83 100644 --- a/apps/sim/app/w/[id]/workflow.tsx +++ b/apps/sim/app/w/[id]/workflow.tsx @@ -841,16 +841,15 @@ const WorkflowContent = React.memo(() => { return } - // Reset variables loaded state before setting active workflow - resetVariablesLoaded() - // Always call setActiveWorkflow when workflow ID changes to ensure proper state const { activeWorkflowId } = useWorkflowRegistry.getState() if (activeWorkflowId !== currentId) { + // Only reset variables when actually switching workflows + resetVariablesLoaded() setActiveWorkflow(currentId) } else { - // Even if the workflow is already active, call setActiveWorkflow to ensure state consistency + // Don't reset variables cache if we're not actually switching workflows setActiveWorkflow(currentId) } diff --git a/apps/sim/test-socket-integration.html b/apps/sim/test-socket-integration.html deleted file mode 100644 index 61af75221..000000000 --- a/apps/sim/test-socket-integration.html +++ /dev/null @@ -1,275 +0,0 @@ - - - - - - Socket Integration Test - - - - -
-

Socket.IO Collaborative Workflow Test

- -
- Disconnected -
- -
-

Presence Users:

-
None
-
- -
-

Test Workflow Operations:

- - - -
- -
-

Block Operations:

- - - -
- -
-

Edge Operations:

- - -
- -
-

Event Log:

-
-
-
- - - -