Skip to content

Commit ec6176f

Browse files
leopuleoleopuleo
authored
fix: prevent folder move without permissions and add move confirmation dialog (#4662)
1 parent f5e8c70 commit ec6176f

File tree

14 files changed

+266
-7
lines changed

14 files changed

+266
-7
lines changed

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

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,119 @@ describe("Folder Level Permissions - Security Checks", () => {
371371
});
372372
});
373373

374+
it("should not allow moving a folder to an inaccessible folder, based on permission level (owner vs editor vs viewer)", async () => {
375+
const folderA = await acoIdentityA
376+
.createFolder({
377+
data: {
378+
title: "Folder A",
379+
slug: "folder-a",
380+
type: FOLDER_TYPE
381+
}
382+
})
383+
.then(([response]) => {
384+
return response.data.aco.createFolder.data;
385+
});
386+
387+
const folderB = await acoIdentityA
388+
.createFolder({
389+
data: {
390+
title: "Folder B",
391+
slug: "folder-b",
392+
type: FOLDER_TYPE
393+
}
394+
})
395+
.then(([response]) => {
396+
return response.data.aco.createFolder.data;
397+
});
398+
399+
const folderC = await acoIdentityA
400+
.createFolder({
401+
data: {
402+
title: "Folder C",
403+
slug: "folder-c",
404+
type: FOLDER_TYPE
405+
}
406+
})
407+
.then(([response]) => {
408+
return response.data.aco.createFolder.data;
409+
});
410+
411+
const subFolder = await acoIdentityA
412+
.createFolder({
413+
data: {
414+
title: "Sub Folder",
415+
slug: "subfolder",
416+
type: FOLDER_TYPE,
417+
parentId: folderA.id
418+
}
419+
})
420+
.then(([response]) => {
421+
return response.data.aco.createFolder.data;
422+
});
423+
424+
await acoIdentityA.updateFolder({
425+
id: folderA.id,
426+
data: {
427+
permissions: [{ level: "owner", target: `admin:${identityB.id}` }]
428+
}
429+
});
430+
431+
await acoIdentityA.updateFolder({
432+
id: folderB.id,
433+
data: {
434+
permissions: [{ level: "editor", target: `admin:${identityB.id}` }]
435+
}
436+
});
437+
438+
await acoIdentityA.updateFolder({
439+
id: folderC.id,
440+
data: {
441+
permissions: [{ level: "viewer", target: `admin:${identityB.id}` }]
442+
}
443+
});
444+
445+
// Should be allowed to move a subfolder from folder A to folder B, because user B has "owner" access to folder A and "editor" access to folder B.
446+
await expect(
447+
acoIdentityB
448+
.updateFolder({
449+
id: subFolder.id,
450+
data: { parentId: folderB.id }
451+
})
452+
.then(([response]) => {
453+
return response.data.aco.updateFolder.data;
454+
})
455+
).resolves.toMatchObject({ id: subFolder.id, parentId: folderB.id });
456+
457+
// Should be allowed to move a subfolder from folder B to folder A, because user B has "owner" access to folder A and "editor" access to folder B.
458+
await expect(
459+
acoIdentityB
460+
.updateFolder({
461+
id: subFolder.id,
462+
data: { parentId: folderA.id }
463+
})
464+
.then(([response]) => {
465+
return response.data.aco.updateFolder.data;
466+
})
467+
).resolves.toMatchObject({ id: subFolder.id, parentId: folderA.id });
468+
469+
// Should not be allowed to move a subfolder from folder A to folder C, because user A has "owner" access to folder A and "viewer" access to folder C.
470+
await expect(
471+
acoIdentityB
472+
.updateFolder({
473+
id: folderB.id,
474+
data: { parentId: folderC.id }
475+
})
476+
.then(([response]) => {
477+
return response.data.aco.updateFolder.error;
478+
})
479+
).resolves.toEqual({
480+
code: "CANNOT_MOVE_FOLDER_TO_NEW_PARENT",
481+
data: null,
482+
message:
483+
"Cannot move folder to a new parent because you don't have access to the new parent."
484+
});
485+
});
486+
374487
it("should still be be able to access a folder if its parent is inaccessible", async () => {
375488
const folderA = await acoIdentityA
376489
.createFolder({

‎packages/api-aco/src/folder/useCases/UpdateFolder/UpdateFolderWithFolderLevelPermissions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export class UpdateFolderWithFolderLevelPermissions implements IUpdateFolder {
7070

7171
await this.folderLevelPermissions.ensureCanAccessFolder({
7272
permissions: parentPermissions,
73-
rwd: "r"
73+
rwd: "w"
7474
});
7575
} catch (e) {
7676
if (e instanceof NotAuthorizedError) {

‎packages/app-aco/src/components/FolderTree/List/index.tsx‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import {
2121
import { ROOT_FOLDER } from "~/constants";
2222
import { DndFolderItemData, FolderItem } from "~/types";
2323
import { FolderProvider } from "~/contexts/folder";
24+
import { useAcoConfig } from "~/config";
25+
import { useConfirmMoveFolderDialog } from "~/dialogs";
2426

2527
interface ListProps {
2628
folders: FolderItem[];
@@ -45,6 +47,8 @@ export const List = ({
4547
const [treeData, setTreeData] = useState<NodeModel<DndFolderItemData>[]>([]);
4648
const [initialOpenList, setInitialOpenList] = useState<undefined | InitialOpen>();
4749
const [openFolderIds, setOpenFolderIds] = useState<string[]>([ROOT_FOLDER]);
50+
const { showDialog: showConfirmMoveFolderDialog } = useConfirmMoveFolderDialog();
51+
const { folder: folderConfigs } = useAcoConfig();
4852

4953
useEffect(() => {
5054
if (folders) {
@@ -63,6 +67,35 @@ export const List = ({
6367
setInitialOpenList(memoCreateInitialOpenList(focusedFolderId));
6468
}, [focusedFolderId]);
6569

70+
const onDrop = useCallback(
71+
async (newTree: NodeModel<DndFolderItemData>[], options: DropOptions) => {
72+
// Function to execute the drop logic
73+
const runDrop = () => handleDrop(newTree, options);
74+
75+
// If drop confirmation is enabled, show dialog before proceeding
76+
if (folderConfigs.dropConfirmation) {
77+
const { dragSourceId, dropTargetId } = options;
78+
const folder = folders.find(f => f.id === dragSourceId);
79+
const targetFolder = folders.find(f => f.id === dropTargetId);
80+
81+
// Abort if either folder is not found
82+
if (!folder || !targetFolder) {
83+
return;
84+
}
85+
86+
showConfirmMoveFolderDialog({
87+
folder,
88+
targetFolder,
89+
onAccept: runDrop
90+
});
91+
} else {
92+
// Otherwise, perform the drop immediately
93+
await runDrop();
94+
}
95+
},
96+
[folders, folderConfigs.dropConfirmation, showConfirmMoveFolderDialog]
97+
);
98+
6699
const handleDrop = async (
67100
newTree: NodeModel<DndFolderItemData>[],
68101
{ dragSourceId, dropTargetId }: DropOptions
@@ -139,7 +172,7 @@ export const List = ({
139172
<Tree
140173
tree={treeData}
141174
rootId={"0"}
142-
onDrop={handleDrop}
175+
onDrop={onDrop}
143176
onChangeOpen={ids => handleChangeOpen(ids as string[])}
144177
sort={sort}
145178
canDrag={item => canDrag(item!.id as string)}

‎packages/app-aco/src/config/AcoConfig.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ export function useAcoConfig() {
3939
},
4040
folder: {
4141
...folder,
42-
actions: [...(folder.actions || [])]
42+
actions: [...(folder.actions || [])],
43+
dropConfirmation: folder.dropConfirmation || false
4344
},
4445
record: {
4546
...record,

‎packages/app-aco/src/config/folder/DropConfirmation.tsx‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import React from "react";
2+
import { Property, useIdGenerator } from "@webiny/react-properties";
3+
4+
export interface DropConfirmationProps {
5+
value: boolean;
6+
}
7+
8+
export const DropConfirmation = ({ value }: DropConfirmationProps) => {
9+
const getId = useIdGenerator("dropConfirmation");
10+
11+
return (
12+
<Property id="folder" name={"folder"}>
13+
<Property id={getId()} name={"dropConfirmation"} value={value} />
14+
</Property>
15+
);
16+
};

‎packages/app-aco/src/config/folder/index.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { Action, ActionConfig } from "./Action";
2+
import { DropConfirmation } from "./DropConfirmation";
23

34
export interface FolderConfig {
45
actions: ActionConfig[];
6+
dropConfirmation: boolean;
57
}
68

79
export const Folder = {
8-
Action
10+
Action,
11+
DropConfirmation
912
};

‎packages/app-aco/src/dialogs/index.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
export * from "./useConfirmMoveFolderDialog";
12
export * from "./useCreateDialog";
23
export * from "./useDeleteDialog";
34
export * from "./useEditDialog";

‎packages/app-aco/src/dialogs/useConfirmMoveFolderDialog.tsx‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { useDialogs } from "@webiny/app-admin";
2+
import { FolderItem } from "~/types";
3+
4+
interface ShowDialogParams {
5+
folder: FolderItem;
6+
targetFolder: FolderItem;
7+
onAccept: (folder: FolderItem, targetFolder: FolderItem) => Promise<void>;
8+
}
9+
10+
interface UseConfirmMoveFolderDialogResponse {
11+
showDialog: (params: ShowDialogParams) => void;
12+
}
13+
14+
export const useConfirmMoveFolderDialog = (): UseConfirmMoveFolderDialogResponse => {
15+
const dialogs = useDialogs();
16+
17+
const showDialog = ({ folder, targetFolder, onAccept }: ShowDialogParams) => {
18+
dialogs.showDialog({
19+
title: "Move folder",
20+
content: `You are about to move the folder "${folder.title}" into "${targetFolder.title}"! Are you sure you want to continue?`,
21+
acceptLabel: "Move folder",
22+
cancelLabel: "Cancel",
23+
loadingLabel: "Moving folder...",
24+
onAccept: () => onAccept(folder, targetFolder)
25+
});
26+
};
27+
28+
return {
29+
showDialog
30+
};
31+
};

‎packages/app-file-manager/src/modules/FileManagerRenderer/FileManagerView/configComponents/Browser/FolderDropConfirmation.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import React from "react";
2+
import { AcoConfig } from "@webiny/app-aco";
3+
4+
const { Folder } = AcoConfig;
5+
6+
type FolderDropConfirmationProps = React.ComponentProps<typeof AcoConfig.Folder.DropConfirmation>;
7+
8+
export const FolderDropConfirmation = (props: FolderDropConfirmationProps) => {
9+
return (
10+
<AcoConfig>
11+
<Folder.DropConfirmation {...props} />
12+
</AcoConfig>
13+
);
14+
};

‎packages/app-file-manager/src/modules/FileManagerRenderer/FileManagerView/configComponents/Browser/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { GridConfig } from "./Grid";
1313
import { ActionButton } from "~/components/Grid/ActionButton";
1414
import { File } from "~/components/Grid/File";
1515
import { shouldDecorateFolderField } from "./FolderFieldDecorator";
16+
import { FolderDropConfirmation } from "./FolderDropConfirmation";
1617

1718
export interface BrowserConfig {
1819
bulkActions: BulkActionConfig[];
@@ -45,7 +46,8 @@ export const Browser = {
4546
shouldDecorate: shouldDecorateFolderField
4647
})
4748
},
48-
Action: FolderAction
49+
Action: FolderAction,
50+
DropConfirmation: FolderDropConfirmation
4951
},
5052
/**
5153
* @deprecated

0 commit comments

Comments
 (0)