diff --git a/src/dialogs/permissions.css b/src/dialogs/permissions.css new file mode 100644 index 00000000..e28aa0e5 --- /dev/null +++ b/src/dialogs/permissions.css @@ -0,0 +1,3 @@ +.multiple-files-expandable { + margin-block-start: var(--pf-v5-global--spacer--md); +} diff --git a/src/dialogs/permissions.tsx b/src/dialogs/permissions.tsx index 074e40b5..e9a6589d 100644 --- a/src/dialogs/permissions.tsx +++ b/src/dialogs/permissions.tsx @@ -21,6 +21,8 @@ import React, { useState } from 'react'; import { Button } from '@patternfly/react-core/dist/esm/components/Button'; import { Checkbox } from '@patternfly/react-core/dist/esm/components/Checkbox'; +import { ExpandableSection, ExpandableSectionVariant } from + '@patternfly/react-core/dist/esm/components/ExpandableSection'; import { Form, FormGroup, FormSection } from '@patternfly/react-core/dist/esm/components/Form'; import { FormSelect, FormSelectOption } from '@patternfly/react-core/dist/esm/components/FormSelect'; import { Modal, ModalVariant } from '@patternfly/react-core/dist/esm/components/Modal'; @@ -45,6 +47,8 @@ import { inode_types } from '../common.ts'; // @ts-expect-error Cannot find module or its corresponding type declaration import read_selinux_context from './read-selinux.py'; +import "./permissions.css"; + const _ = cockpit.gettext; const PERMISSION_OPTIONS: Record = { @@ -115,11 +119,16 @@ function mode_to_args(mode: number) { return chmod_args.join(","); } -const EditPermissionsModal = ({ dialogResult, selected, path } : { +const EditPermissionsModal = ({ dialogResult, items, path } : { dialogResult: DialogResult, - selected: FolderFileInfo, + items: FolderFileInfo[], path: string, }) => { + cockpit.assert(items[0] !== undefined, "passed items cannot be empty"); + const selected = items[0]; + const is_user_equal = items.every((item) => item.user === items[0].user); + const is_group_equal = items.every((item) => item.group === items[0].group); + const [owner, setOwner] = useState(selected.user); const [mode, setMode] = useState(selected.mode ?? 0); const [group, setGroup] = useState(selected.group); @@ -147,13 +156,16 @@ const EditPermissionsModal = ({ dialogResult, selected, path } : { console.error("Cannot obtain users from getent group", exc); } - try { - const selinux_context = await python.spawn(read_selinux_context, [full_path]); - setSELinuxContext(selinux_context); - } catch (err) { - const e = err as python.PythonExitStatus; - if (e.exit_status !== 2) - console.error("Cannot obtain SELinux context", err); + // When editing multiple files determining the common selinux context might not be possible so don't show it. + if (items.length === 1) { + try { + const selinux_context = await python.spawn(read_selinux_context, [full_path]); + setSELinuxContext(selinux_context); + } catch (err) { + const e = err as python.PythonExitStatus; + if (e.exit_status !== 2) + console.error("Cannot obtain SELinux context", err); + } } }); @@ -179,16 +191,18 @@ const EditPermissionsModal = ({ dialogResult, selected, path } : { }; const spawnEditPermissions = async () => { - const permissionChanged = mode !== selected.mode; + const permissionChanged = items.some(item => item.mode !== mode); + // We only allow editing multiple files with the same owner:group. const ownerChanged = owner !== selected.user || group !== selected.group; + const file_paths = items.map(item => path + item.name); try { if (permissionChanged) - await cockpit.spawn(["chmod", mode.toString(8), full_path], + await cockpit.spawn(["chmod", mode.toString(8), ...file_paths], { superuser: "try", err: "message" }); if (ownerChanged) - await cockpit.spawn(["chown", owner + ":" + group, full_path], + await cockpit.spawn(["chown", owner + ":" + group, ...file_paths], { superuser: "try", err: "message" }); dialogResult.resolve(); @@ -255,13 +269,32 @@ const EditPermissionsModal = ({ dialogResult, selected, path } : { return a.name.localeCompare(b.name); } + let description; + if (items.length === 1) { + description = (selected.type) ? inode_types[selected.type] : _("Missing type"); + } else { + description = ( + + {items.map(itm => itm.name).join(", ")} + + ); + } + return ( {selected.name})} - description={(selected.type) ? inode_types[selected.type] : _("Missing type")} + title={items.length === 1 + ? fmt_to_fragments(_("$0 permissions"), {selected.name}) + : cockpit.format(_("Permissions for $0 files"), items.length)} + description={description} isOpen onClose={() => dialogResult.resolve()} footer={ @@ -291,7 +324,7 @@ const EditPermissionsModal = ({ dialogResult, selected, path } : { isInline />}
- {superuser.allowed && accounts && groups && + {superuser.allowed && accounts && groups && is_user_equal && is_group_equal && edit_permissions(dialogs, current_directory, base_path) + onClick: () => edit_permissions(dialogs, [current_directory], base_path) } ); if (supportsTerminal) { @@ -145,7 +144,7 @@ export function get_menu_items( { id: "edit-permissions", title: _("Edit permissions"), - onClick: () => edit_permissions(dialogs, item, path) + onClick: () => edit_permissions(dialogs, [item], path) }, { id: "rename-item", @@ -193,6 +192,20 @@ export function get_menu_items( onClick: () => confirm_delete(dialogs, path, selected, setSelected) } ); + + // Don't allow mixing regular files and folders when editing multiple + // permissions, it can be unclear if we are changing the folders + // permissions or the permissions of the files underneath. + if (selected.every(sel => sel.type === "reg")) { + menuItems.push( + { type: "divider" }, + { + id: "edit-permissions", + title: _("Edit permissions"), + onClick: () => edit_permissions(dialogs, selected, path) + } + ); + } } return menuItems; diff --git a/test/check-application b/test/check-application index 2d404806..d9fb732f 100755 --- a/test/check-application +++ b/test/check-application @@ -1569,6 +1569,93 @@ class TestFiles(testlib.MachineCase): b.click("li[data-location='/home/admin'] a") self.assert_last_breadcrumb("admin") + # Edit multiple files + create_multiple_dirs_sh = "mkdir -p /home/admin/multiple/testdir\n" + for i in range(20): + create_multiple_dirs_sh += f"touch /home/admin/multiple/filename{i}\n" + + self.write_file(f"{self.vm_tmpdir}/create-multiple-dirs.sh", create_multiple_dirs_sh, perm="755") + m.execute(f"runuser -u admin {self.vm_tmpdir}/create-multiple-dirs.sh") + b.mouse("[data-item='multiple']", "dblclick") + b.wait_not_present(".pf-v5-c-empty-state") + self.assert_last_breadcrumb("multiple") + + # Editing two files + b.click("[data-item='filename1']") + b.mouse("[data-item='filename2']", "click", ctrlKey=True) + b.mouse("[data-item='filename1']", "contextmenu") + b.click("button:contains('Edit permissions')") + + b.wait_text(".pf-v5-c-modal-box__title-text", "Permissions for 2 files") + b.wait_in_text("#edit-permissions-owner", "admin") + b.wait_in_text("#edit-permissions-group", "admin") + b.select_from_dropdown("#edit-permissions-owner-access", "read-only") + b.select_from_dropdown("#edit-permissions-group-access", "no-access") + b.select_from_dropdown("#edit-permissions-other-access", "no-access") + b.click(".pf-v5-c-modal-box button.pf-m-primary") + b.wait_not_present(".pf-v5-c-modal-box") + self.assertEqual(self.stat("%A", "/home/admin/multiple/filename1"), "-r--------") + self.assertEqual(self.stat("%A", "/home/admin/multiple/filename2"), "-r--------") + + # No owner/group shown if ownership differs + def verify_no_owner_group_shown() -> None: + # HACK: the selected state contains a copy of files state so permissions information is not updated. + b.mouse("[data-item='filename1']", "click", ctrlKey=True) + b.mouse("[data-item='filename1']", "click", ctrlKey=True) + b.mouse("[data-item='filename1']", "contextmenu") + b.click("button:contains('Edit permissions')") + + b.wait_visible("#edit-permissions-owner-access") + b.wait_not_present("#edit-permissions-owner") + b.wait_not_present("#edit-permissions-group") + b.select_from_dropdown("#edit-permissions-owner-access", "read-write") + b.select_from_dropdown("#edit-permissions-group-access", "read-only") + b.select_from_dropdown("#edit-permissions-other-access", "read-only") + + b.click(".pf-v5-c-modal-box button.pf-m-primary") + b.wait_not_present(".pf-v5-c-modal-box") + + self.assertEqual(self.stat("%A", "/home/admin/multiple/filename1"), "-rw-r--r--") + self.assertEqual(self.stat("%A", "/home/admin/multiple/filename2"), "-rw-r--r--") + + m.execute("chmod 600 /home/admin/multiple/filename1") + m.execute("chown root: /home/admin/multiple/filename1") + b.wait_in_text("[data-item='filename1'] .item-owner", "root") + verify_no_owner_group_shown() + + m.execute("chown admin:root /home/admin/multiple/filename1") + b.wait_in_text("[data-item='filename1'] .item-owner", "admin:root") + verify_no_owner_group_shown() + + # No Edit permissions when selecting a folder and files + b.mouse("[data-item='testdir']", "click", ctrlKey=True) + b.mouse("[data-item='filename1']", "contextmenu") + b.wait_visible("button:contains('Delete')") + b.wait_not_present("button:contains('Edit permissions')") + + # Editing > 10 files, select only files with the same owner/group + # Test that we see an expander and can toggle it. + b.mouse("[data-item='testdir']", "click", ctrlKey=True) + b.mouse("[data-item='filename1']", "click", ctrlKey=True) + for i in range(3, 15): + b.mouse(f"[data-item='filename{i}']", "click", ctrlKey=True) + + b.mouse("[data-item='filename3']", "contextmenu") + b.click("button:contains('Edit permissions')") + + b.wait_text(".pf-v5-c-modal-box__title-text", "Permissions for 13 files") + b.assert_pixels(".pf-v5-c-modal-box", "multiple-files-permissions-modal") + # Expand files + b.wait_text(".pf-v5-c-expandable-section button", "Show all files") + b.click(".pf-v5-c-expandable-section button") + b.wait_text(".pf-v5-c-expandable-section button", "Collapse") + b.click(".pf-v5-c-modal-box button.pf-m-link") + b.wait_not_present(".pf-v5-c-modal-box") + + b.go("/files#/?path=/home/admin") + self.assert_last_breadcrumb("admin") + b.wait_not_present(".pf-v5-c-empty-state") + # As normal user you cannot change user/group permissions b.drop_superuser() diff --git a/test/reference b/test/reference index bdb9f7ca..706094fb 160000 --- a/test/reference +++ b/test/reference @@ -1 +1 @@ -Subproject commit bdb9f7ca615425764935e8c89765ff972a25e39c +Subproject commit 706094fb506668547410343bc933207ee9075d93