fix(flp):ensure permissions are correctly propagated to nested folders (

#3890)

Co-authored-by: Pavel Denisjuk <[email protected]>

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

@@ -432,20 +432,18 @@ describe("Folder Level Permissions - Security Checks", () => {
         });
 
         // Moving folder B to folder A, which is inaccessible to user B.
-        // User B should lose access to folder B.
+        // Still, user B should not lose access to folder B.
         await acoIdentityA.updateFolder({
             id: folderB.id,
             data: { parentId: folderA.id }
         });
 
         await expect(
             acoIdentityB.getFolder({ id: folderB.id }).then(([result]) => {
-                return result.data.aco.getFolder.error;
+                return result.data.aco.getFolder.data;
             })
-        ).resolves.toEqual({
-            code: "SECURITY_NOT_AUTHORIZED",
-            data: null,
-            message: "Not authorized!"
+        ).resolves.toMatchObject({
+            id: folderB.id
         });
 
         await expect(
@@ -454,9 +452,30 @@ describe("Folder Level Permissions - Security Checks", () => {
             })
         ).resolves.toMatchObject([
             {
+                id: folderB.id,
+                canManagePermissions: true,
+                canManageStructure: true,
+                canManageContent: true,
+                hasNonInheritedPermissions: true,
+                permissions: [
+                    {
+                        target: "admin:2",
+                        level: "owner",
+                        inheritedFrom: null
+                    },
+                    {
+                        inheritedFrom: `parent:${folderA.id}`,
+                        level: "owner",
+                        target: "admin:not-b"
+                    }
+                ]
+            },
+            {
+                id: folderC.id,
                 canManagePermissions: false,
+                canManageStructure: true,
+                canManageContent: true,
                 hasNonInheritedPermissions: false,
-                id: folderC.id,
                 permissions: [
                     {
                         target: "admin:2",
@@ -467,4 +486,99 @@ describe("Folder Level Permissions - Security Checks", () => {
             }
         ]);
     });
+
+    it("as an owner, I should be able to manage permissions on nested folders", async () => {
+        const folderA = await acoIdentityA
+            .createFolder({
+                data: {
+                    title: "Folder A",
+                    slug: "folder-a",
+                    type: FOLDER_TYPE
+                }
+            })
+            .then(([response]) => {
+                return response.data.aco.createFolder.data;
+            });
+
+        const folderB = await acoIdentityA
+            .createFolder({
+                data: {
+                    title: "Folder B",
+                    slug: "folder-b",
+                    type: FOLDER_TYPE,
+                    parentId: folderA.id
+                }
+            })
+            .then(([response]) => {
+                return response.data.aco.createFolder.data;
+            });
+
+        const folderC = await acoIdentityA
+            .createFolder({
+                data: {
+                    title: "Folder C",
+                    slug: "folder-c",
+                    type: FOLDER_TYPE,
+                    parentId: folderB.id
+                }
+            })
+            .then(([response]) => {
+                return response.data.aco.createFolder.data;
+            });
+
+        const folderD = await acoIdentityA
+            .createFolder({
+                data: {
+                    title: "Folder D",
+                    slug: "folder-d",
+                    type: FOLDER_TYPE,
+                    parentId: folderC.id
+                }
+            })
+            .then(([response]) => {
+                return response.data.aco.createFolder.data;
+            });
+
+        // We're updating the permissions on folder B. Meaning, identity B should have access to
+        // folders B, C and D.
+        await acoIdentityA.updateFolder({
+            id: folderB.id,
+            data: {
+                permissions: [{ level: "owner", target: `admin:${identityB.id}` }]
+            }
+        });
+
+        // User B should have access to folder B.
+        const [postFlpChangeFolderB] = await acoIdentityB.getFolder({ id: folderB.id });
+        const [postFlpChangeFolderC] = await acoIdentityB.getFolder({ id: folderC.id });
+        const [postFlpChangeFolderD] = await acoIdentityB.getFolder({ id: folderD.id });
+
+        const matchObject = {
+            canManagePermissions: true,
+            canManageStructure: true,
+            canManageContent: true
+        };
+
+        expect(postFlpChangeFolderB.data.aco.getFolder.data).toMatchObject({
+            slug: "folder-b",
+            ...matchObject
+        });
+        expect(postFlpChangeFolderC.data.aco.getFolder.data).toMatchObject({
+            slug: "folder-c",
+            ...matchObject
+        });
+        expect(postFlpChangeFolderD.data.aco.getFolder.data).toMatchObject({
+            slug: "folder-d",
+            ...matchObject
+        });
+
+        // Should not be able to change permissions on a public folder.
+        const [postFlpChangeFolderA] = await acoIdentityB.getFolder({ id: folderA.id });
+        expect(postFlpChangeFolderA.data.aco.getFolder.data).toMatchObject({
+            slug: "folder-a",
+            canManagePermissions: false,
+            canManageStructure: true,
+            canManageContent: true
+        });
+    });
 });

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

@@ -13,6 +13,7 @@ const DATA_FIELD = /* GraphQL */ `
         hasNonInheritedPermissions
         canManagePermissions
         canManageStructure
+        canManageContent
         createdBy {
             id
             displayName

packages/api-aco/src/utils/FolderLevelPermissions.ts

@@ -289,27 +289,6 @@ export class FolderLevelPermissions {
 
         const { folder } = params;
 
-        // We check for parent folder access first because the passed folder should be
-        // inaccessible if the parent folder is inaccessible.
-        if (folder.parentId) {
-            let foldersList = params.foldersList;
-            if (!foldersList) {
-                foldersList = await this.listAllFolders(folder.type);
-            }
-
-            const parentFolder = foldersList.find(f => f.id === folder.parentId);
-            if (parentFolder) {
-                const canAccessParentFolder = await this.canAccessFolder({
-                    ...params,
-                    folder: parentFolder
-                });
-
-                if (!canAccessParentFolder) {
-                    return false;
-                }
-            }
-        }
-
         const folderPermissions = await this.getFolderPermissions({
             folder,
             foldersList: params.foldersList