Skip to content

Commit b56f3b5

Browse files
adrians5jadrians5j
committed
fix: non-owners can manage folder struct but not permissions
1 parent 6526c04 commit b56f3b5

File tree

4 files changed

+282
-88
lines changed

4 files changed

+282
-88
lines changed

‎packages/api-aco/__tests__/folder.flp.crud.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,14 +522,26 @@ describe("Folder Level Permissions", () => {
522522
{
523523
id: folderA.id,
524524
parentId: null,
525-
permissions: [],
525+
permissions: [
526+
{
527+
target: "admin:2",
528+
level: "public",
529+
inheritedFrom: "public"
530+
}
531+
],
526532
hasNonInheritedPermissions: false,
527533
canManagePermissions: false
528534
},
529535
{
530536
id: folderB.id,
531537
parentId: folderA.id,
532-
permissions: [],
538+
permissions: [
539+
{
540+
target: "admin:2",
541+
level: "public",
542+
inheritedFrom: `parent:${folderA.id}`
543+
}
544+
],
533545
hasNonInheritedPermissions: false,
534546
canManagePermissions: false
535547
}

‎packages/api-aco/__tests__/folder.flp.security.test.ts

Lines changed: 143 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,25 +68,61 @@ describe("Folder Level Permissions - Security Checks", () => {
6868
{
6969
id: createdFolders[0].id,
7070
parentId: null,
71-
permissions: [],
71+
permissions: [
72+
{
73+
inheritedFrom: "public",
74+
level: "public",
75+
target: "admin:2"
76+
}
77+
],
78+
canManageStructure: true,
79+
canManagePermissions: false,
80+
hasNonInheritedPermissions: false,
7281
slug: "folder-1"
7382
},
7483
{
7584
id: createdFolders[1].id,
7685
parentId: null,
77-
permissions: [],
86+
permissions: [
87+
{
88+
inheritedFrom: "public",
89+
level: "public",
90+
target: "admin:2"
91+
}
92+
],
93+
canManageStructure: true,
94+
canManagePermissions: false,
95+
hasNonInheritedPermissions: false,
7896
slug: "folder-2"
7997
},
8098
{
8199
id: createdFolders[2].id,
82100
parentId: null,
83-
permissions: [],
101+
permissions: [
102+
{
103+
inheritedFrom: "public",
104+
level: "public",
105+
target: "admin:2"
106+
}
107+
],
108+
canManageStructure: true,
109+
canManagePermissions: false,
110+
hasNonInheritedPermissions: false,
84111
slug: "folder-3"
85112
},
86113
{
87114
id: createdFolders[3].id,
88115
parentId: null,
89-
permissions: [],
116+
permissions: [
117+
{
118+
inheritedFrom: "public",
119+
level: "public",
120+
target: "admin:2"
121+
}
122+
],
123+
canManageStructure: true,
124+
canManagePermissions: false,
125+
hasNonInheritedPermissions: false,
90126
slug: "folder-4"
91127
}
92128
]);
@@ -143,7 +179,11 @@ describe("Folder Level Permissions - Security Checks", () => {
143179
acoIdentityB
144180
.updateFolder({
145181
id: folderA.id,
146-
data: { permissions: [] }
182+
data: {
183+
permissions: [
184+
{ level: "owner", target: `admin:${identityA.id}` } // Include previous permissions.
185+
]
186+
}
147187
})
148188
.then(([response]) => {
149189
return response.data.aco.updateFolder.error;
@@ -155,6 +195,97 @@ describe("Folder Level Permissions - Security Checks", () => {
155195
});
156196
});
157197

198+
it(`should reset folder access level back to "public"`, async () => {
199+
const folderA = await acoIdentityA
200+
.createFolder({
201+
data: {
202+
title: "Folder A",
203+
slug: "folder-a",
204+
type: FOLDER_TYPE
205+
}
206+
})
207+
.then(([response]) => {
208+
return response.data.aco.createFolder.data;
209+
});
210+
211+
await acoIdentityA.updateFolder({
212+
id: folderA.id,
213+
data: {
214+
permissions: [{ level: "owner", target: `admin:${identityB.id}` }]
215+
}
216+
});
217+
218+
// Should be allowed because the user is not loosing access.
219+
await expect(
220+
acoIdentityB
221+
.updateFolder({
222+
id: folderA.id,
223+
data: {
224+
permissions: [
225+
{ level: "owner", target: `admin:${identityB.id}` }, // Include previous permissions.
226+
{ level: "owner", target: `admin:random-id` } // Include new permissions.
227+
]
228+
}
229+
})
230+
.then(([response]) => {
231+
return response.data.aco.updateFolder.data;
232+
})
233+
).resolves.toMatchObject({
234+
canManagePermissions: true,
235+
hasNonInheritedPermissions: true,
236+
id: folderA.id,
237+
parentId: null,
238+
permissions: [
239+
{ inheritedFrom: null, level: "owner", target: "admin:2" },
240+
{ inheritedFrom: null, level: "owner", target: "admin:random-id" }
241+
]
242+
});
243+
244+
await expect(
245+
acoIdentityA
246+
.updateFolder({
247+
id: folderA.id,
248+
data: {
249+
permissions: []
250+
}
251+
})
252+
.then(([response]) => {
253+
return response.data.aco.updateFolder.data;
254+
})
255+
).resolves.toMatchObject({
256+
canManagePermissions: true,
257+
hasNonInheritedPermissions: false,
258+
id: folderA.id,
259+
parentId: null,
260+
permissions: [
261+
{
262+
inheritedFrom: "role:full-access",
263+
level: "owner",
264+
target: "admin:1"
265+
}
266+
]
267+
});
268+
269+
// Should not be allowed because the user is loosing access.
270+
await expect(
271+
acoIdentityB.getFolder({ id: folderA.id }).then(([response]) => {
272+
return response.data.aco.getFolder.data;
273+
})
274+
).resolves.toMatchObject({
275+
canManagePermissions: false,
276+
hasNonInheritedPermissions: false,
277+
id: folderA.id,
278+
parentId: null,
279+
permissions: [
280+
{
281+
inheritedFrom: "public",
282+
level: "public",
283+
target: "admin:2"
284+
}
285+
]
286+
});
287+
});
288+
158289
it("should not allow moving a folder to an inaccessible folder", async () => {
159290
const folderA = await acoIdentityA
160291
.createFolder({
@@ -326,7 +457,13 @@ describe("Folder Level Permissions - Security Checks", () => {
326457
canManagePermissions: false,
327458
hasNonInheritedPermissions: false,
328459
id: folderC.id,
329-
permissions: []
460+
permissions: [
461+
{
462+
target: "admin:2",
463+
level: "public",
464+
inheritedFrom: "public"
465+
}
466+
]
330467
}
331468
]);
332469
});

‎packages/api-aco/__tests__/graphql/folder.gql.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const DATA_FIELD = /* GraphQL */ `
1212
}
1313
hasNonInheritedPermissions
1414
canManagePermissions
15+
canManageStructure
1516
createdBy {
1617
id
1718
displayName

0 commit comments

Comments
 (0)