Allow changing permissions of multiple files

For our upload file(s) as administrator feature we want an easy way to
change permissions of multiple uploaded file(s). Editing multiple files
is restricted to regular files to not mix folder and file permissions
operations which might imply that all files under the folder change or
not.

Furthermore when the owner/group do not match it is not shown nor is the
SELinux context shown for the files as it is impossible to find a common
context.

src/dialogs/permissions.css

@@ -0,0 +1,3 @@
+.multiple-files-expandable {
+     margin-block-start: var(--pf-v5-global--spacer--md);
+}

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<number, string> = {
@@ -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<void>,
-    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 = (
+            <ExpandableSection
+              truncateMaxLines={1}
+              variant={ExpandableSectionVariant.truncate}
+              toggleTextExpanded={_("Collapse")}
+              toggleTextCollapsed={_("Show all files")}
+              className="multiple-files-expandable"
+            >
+                {items.map(itm => itm.name).join(", ")}
+            </ExpandableSection>
+        );
+    }
+
     return (
         <Modal
           position="top"
           variant={ModalVariant.small}
           /* Translators: $0 represents a filename */
-          title={fmt_to_fragments(_("$0 permissions"), <b>{selected.name}</b>)}
-          description={(selected.type) ? inode_types[selected.type] : _("Missing type")}
+          title={items.length === 1
+              ? fmt_to_fragments(_("$0 permissions"), <b>{selected.name}</b>)
+              : 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
                 />}
                 <Form isHorizontal>
-                    {superuser.allowed && accounts && groups &&
+                    {superuser.allowed && accounts && groups && is_user_equal && is_group_equal &&
                     <FormSection title={_("Ownership")}>
                         <FormGroup label={_("Owner")} fieldId="edit-permissions-owner">
                             <FormSelect
@@ -385,6 +418,6 @@ const EditPermissionsModal = ({ dialogResult, selected, path } : {
     );
 };
 
-export function edit_permissions(dialogs: Dialogs, selected: FolderFileInfo, path: string) {
-    dialogs.run(EditPermissionsModal, { selected, path });
+export function edit_permissions(dialogs: Dialogs, items: FolderFileInfo[], path: string) {
+    dialogs.run(EditPermissionsModal, { items, path });
 }

src/menu.tsx

@@ -80,8 +80,7 @@ export function get_menu_items(
     const supportsTerminal = cockpit.manifests.system?.tools?.terminal?.capabilities?.includes("path");
 
     if (selected.length === 0) {
-        // HACK: basename('/') is currently ""
-        const current_directory = { ...cwdInfo, name: basename(path) || "/", category: null, to: null };
+        const current_directory = { ...cwdInfo, name: basename(path), category: null, to: null };
         const base_path = get_base_path(path);
         menuItems.push(
             {
@@ -105,7 +104,7 @@ export function get_menu_items(
             {
                 id: "edit-permissions",
                 title: _("Edit permissions"),
-                onClick: () => 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;

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()