Skip to content

Commit 9caa083

Browse files
authored
fix(sharing): fixed folders not appearing when sharing workflows (#616)
* fix(sharing): fixed folders not appearing when sharing workflows * cleanup * fixed error case
1 parent 01eedb3 commit 9caa083

File tree

8 files changed

+399
-47
lines changed

8 files changed

+399
-47
lines changed

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

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ describe('Individual Folder API Route', () => {
4040
}
4141

4242
const { mockAuthenticatedUser, mockUnauthenticated } = mockAuth(TEST_USER)
43+
const mockGetUserEntityPermissions = vi.fn()
4344

4445
function createFolderDbMock(options: FolderDbMockOptions = {}) {
4546
const {
@@ -109,6 +110,12 @@ describe('Individual Folder API Route', () => {
109110
vi.resetModules()
110111
vi.clearAllMocks()
111112
setupCommonApiMocks()
113+
114+
mockGetUserEntityPermissions.mockResolvedValue('admin')
115+
116+
vi.doMock('@/lib/permissions/utils', () => ({
117+
getUserEntityPermissions: mockGetUserEntityPermissions,
118+
}))
112119
})
113120

114121
afterEach(() => {
@@ -181,6 +188,72 @@ describe('Individual Folder API Route', () => {
181188
expect(data).toHaveProperty('error', 'Unauthorized')
182189
})
183190

191+
it('should return 403 when user has only read permissions', async () => {
192+
mockAuthenticatedUser()
193+
mockGetUserEntityPermissions.mockResolvedValue('read') // Read-only permissions
194+
195+
const dbMock = createFolderDbMock()
196+
vi.doMock('@/db', () => dbMock)
197+
198+
const req = createMockRequest('PUT', {
199+
name: 'Updated Folder',
200+
})
201+
const params = Promise.resolve({ id: 'folder-1' })
202+
203+
const { PUT } = await import('./route')
204+
205+
const response = await PUT(req, { params })
206+
207+
expect(response.status).toBe(403)
208+
209+
const data = await response.json()
210+
expect(data).toHaveProperty('error', 'Write access required to update folders')
211+
})
212+
213+
it('should allow folder update for write permissions', async () => {
214+
mockAuthenticatedUser()
215+
mockGetUserEntityPermissions.mockResolvedValue('write') // Write permissions
216+
217+
const dbMock = createFolderDbMock()
218+
vi.doMock('@/db', () => dbMock)
219+
220+
const req = createMockRequest('PUT', {
221+
name: 'Updated Folder',
222+
})
223+
const params = Promise.resolve({ id: 'folder-1' })
224+
225+
const { PUT } = await import('./route')
226+
227+
const response = await PUT(req, { params })
228+
229+
expect(response.status).toBe(200)
230+
231+
const data = await response.json()
232+
expect(data).toHaveProperty('folder')
233+
})
234+
235+
it('should allow folder update for admin permissions', async () => {
236+
mockAuthenticatedUser()
237+
mockGetUserEntityPermissions.mockResolvedValue('admin') // Admin permissions
238+
239+
const dbMock = createFolderDbMock()
240+
vi.doMock('@/db', () => dbMock)
241+
242+
const req = createMockRequest('PUT', {
243+
name: 'Updated Folder',
244+
})
245+
const params = Promise.resolve({ id: 'folder-1' })
246+
247+
const { PUT } = await import('./route')
248+
249+
const response = await PUT(req, { params })
250+
251+
expect(response.status).toBe(200)
252+
253+
const data = await response.json()
254+
expect(data).toHaveProperty('folder')
255+
})
256+
184257
it('should return 400 when trying to set folder as its own parent', async () => {
185258
mockAuthenticatedUser()
186259

@@ -387,6 +460,68 @@ describe('Individual Folder API Route', () => {
387460
expect(data).toHaveProperty('error', 'Unauthorized')
388461
})
389462

463+
it('should return 403 when user has only read permissions for delete', async () => {
464+
mockAuthenticatedUser()
465+
mockGetUserEntityPermissions.mockResolvedValue('read') // Read-only permissions
466+
467+
const dbMock = createFolderDbMock()
468+
vi.doMock('@/db', () => dbMock)
469+
470+
const req = createMockRequest('DELETE')
471+
const params = Promise.resolve({ id: 'folder-1' })
472+
473+
const { DELETE } = await import('./route')
474+
475+
const response = await DELETE(req, { params })
476+
477+
expect(response.status).toBe(403)
478+
479+
const data = await response.json()
480+
expect(data).toHaveProperty('error', 'Admin access required to delete folders')
481+
})
482+
483+
it('should return 403 when user has only write permissions for delete', async () => {
484+
mockAuthenticatedUser()
485+
mockGetUserEntityPermissions.mockResolvedValue('write') // Write permissions (not enough for delete)
486+
487+
const dbMock = createFolderDbMock()
488+
vi.doMock('@/db', () => dbMock)
489+
490+
const req = createMockRequest('DELETE')
491+
const params = Promise.resolve({ id: 'folder-1' })
492+
493+
const { DELETE } = await import('./route')
494+
495+
const response = await DELETE(req, { params })
496+
497+
expect(response.status).toBe(403)
498+
499+
const data = await response.json()
500+
expect(data).toHaveProperty('error', 'Admin access required to delete folders')
501+
})
502+
503+
it('should allow folder deletion for admin permissions', async () => {
504+
mockAuthenticatedUser()
505+
mockGetUserEntityPermissions.mockResolvedValue('admin') // Admin permissions
506+
507+
const dbMock = createFolderDbMock({
508+
folderLookupResult: mockFolder,
509+
})
510+
vi.doMock('@/db', () => dbMock)
511+
512+
const req = createMockRequest('DELETE')
513+
const params = Promise.resolve({ id: 'folder-1' })
514+
515+
const { DELETE } = await import('./route')
516+
517+
const response = await DELETE(req, { params })
518+
519+
expect(response.status).toBe(200)
520+
521+
const data = await response.json()
522+
expect(data).toHaveProperty('success', true)
523+
})
524+
390525
it('should handle database errors during deletion', async () => {
391526
mockAuthenticatedUser()
392527

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

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { and, eq } from 'drizzle-orm'
22
import { type NextRequest, NextResponse } from 'next/server'
33
import { getSession } from '@/lib/auth'
44
import { createLogger } from '@/lib/logs/console-logger'
5+
import { getUserEntityPermissions } from '@/lib/permissions/utils'
56
import { db } from '@/db'
67
import { workflow, workflowFolder } from '@/db/schema'
78

@@ -19,17 +20,31 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{
1920
const body = await request.json()
2021
const { name, color, isExpanded, parentId } = body
2122

22-
// Verify the folder exists and belongs to the user
23+
// Verify the folder exists
2324
const existingFolder = await db
2425
.select()
2526
.from(workflowFolder)
26-
.where(and(eq(workflowFolder.id, id), eq(workflowFolder.userId, session.user.id)))
27+
.where(eq(workflowFolder.id, id))
2728
.then((rows) => rows[0])
2829

2930
if (!existingFolder) {
3031
return NextResponse.json({ error: 'Folder not found' }, { status: 404 })
3132
}
3233

34+
// Check if user has write permissions for the workspace
35+
const workspacePermission = await getUserEntityPermissions(
36+
session.user.id,
37+
'workspace',
38+
existingFolder.workspaceId
39+
)
40+
41+
if (!workspacePermission || workspacePermission === 'read') {
42+
return NextResponse.json(
43+
{ error: 'Write access required to update folders' },
44+
{ status: 403 }
45+
)
46+
}
47+
3348
// Prevent setting a folder as its own parent or creating circular references
3449
if (parentId && parentId === id) {
3550
return NextResponse.json({ error: 'Folder cannot be its own parent' }, { status: 400 })
@@ -81,19 +96,33 @@ export async function DELETE(
8196

8297
const { id } = await params
8398

84-
// Verify the folder exists and belongs to the user
99+
// Verify the folder exists
85100
const existingFolder = await db
86101
.select()
87102
.from(workflowFolder)
88-
.where(and(eq(workflowFolder.id, id), eq(workflowFolder.userId, session.user.id)))
103+
.where(eq(workflowFolder.id, id))
89104
.then((rows) => rows[0])
90105

91106
if (!existingFolder) {
92107
return NextResponse.json({ error: 'Folder not found' }, { status: 404 })
93108
}
94109

110+
// Check if user has admin permissions for the workspace (admin-only for deletions)
111+
const workspacePermission = await getUserEntityPermissions(
112+
session.user.id,
113+
'workspace',
114+
existingFolder.workspaceId
115+
)
116+
117+
if (workspacePermission !== 'admin') {
118+
return NextResponse.json(
119+
{ error: 'Admin access required to delete folders' },
120+
{ status: 403 }
121+
)
122+
}
123+
95124
// Recursively delete folder and all its contents
96-
const deletionStats = await deleteFolderRecursively(id, session.user.id)
125+
const deletionStats = await deleteFolderRecursively(id, existingFolder.workspaceId)
97126

98127
logger.info('Deleted folder and all contents:', {
99128
id,
@@ -113,41 +142,39 @@ export async function DELETE(
113142
// Helper function to recursively delete a folder and all its contents
114143
async function deleteFolderRecursively(
115144
folderId: string,
116-
userId: string
145+
workspaceId: string
117146
): Promise<{ folders: number; workflows: number }> {
118147
const stats = { folders: 0, workflows: 0 }
119148

120-
// Get all child folders first
149+
// Get all child folders first (workspace-scoped, not user-scoped)
121150
const childFolders = await db
122151
.select({ id: workflowFolder.id })
123152
.from(workflowFolder)
124-
.where(and(eq(workflowFolder.parentId, folderId), eq(workflowFolder.userId, userId)))
153+
.where(and(eq(workflowFolder.parentId, folderId), eq(workflowFolder.workspaceId, workspaceId)))
125154

126155
// Recursively delete child folders
127156
for (const childFolder of childFolders) {
128-
const childStats = await deleteFolderRecursively(childFolder.id, userId)
157+
const childStats = await deleteFolderRecursively(childFolder.id, workspaceId)
129158
stats.folders += childStats.folders
130159
stats.workflows += childStats.workflows
131160
}
132161

133-
// Delete all workflows in this folder
162+
// Delete all workflows in this folder (workspace-scoped, not user-scoped)
134163
const workflowsInFolder = await db
135164
.select({ id: workflow.id })
136165
.from(workflow)
137-
.where(and(eq(workflow.folderId, folderId), eq(workflow.userId, userId)))
166+
.where(and(eq(workflow.folderId, folderId), eq(workflow.workspaceId, workspaceId)))
138167

139168
if (workflowsInFolder.length > 0) {
140169
await db
141170
.delete(workflow)
142-
.where(and(eq(workflow.folderId, folderId), eq(workflow.userId, userId)))
171+
.where(and(eq(workflow.folderId, folderId), eq(workflow.workspaceId, workspaceId)))
143172

144173
stats.workflows += workflowsInFolder.length
145174
}
146175

147176
// Delete this folder
148-
await db
149-
.delete(workflowFolder)
150-
.where(and(eq(workflowFolder.id, folderId), eq(workflowFolder.userId, userId)))
177+
await db.delete(workflowFolder).where(eq(workflowFolder.id, folderId))
151178

152179
stats.folders += 1
153180

0 commit comments

Comments
 (0)