Permissions dialog re-design

The permissions dialog file/folder permissions was re-designed to make
it be able to support "changing permissions for enclosed files". The
available access permissions are simplified to `Read and write`,
`Read-only` and `No access` the executable bits are handled by a
checkbox for files and implicitly added for folders.

Furthermore setting a file as executable is now only possible for files
where this makes sense based on it's file extension (ie. code or unknown
files without an extension).

Related: #192

src/common.ts

@@ -45,9 +45,3 @@ export const inode_types = {
 export function get_permissions(n: number) {
     return permissions[n & 0o7];
 }
-
-export function * map_permissions<T>(func: (value: number, label: string) => T) {
-    for (const [value, label] of permissions.entries()) {
-        yield func(value, label);
-    }
-}

src/dialogs/permissions.jsx

@@ -20,6 +20,7 @@
 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 { 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';
@@ -34,10 +35,28 @@ import { superuser } from 'superuser';
 import { fmt_to_fragments } from 'utils.tsx';
 
 import { useFilesContext } from '../app.tsx';
-import { map_permissions, inode_types } from '../common.ts';
+import { inode_types } from '../common.ts';
 
 const _ = cockpit.gettext;
 
+const PERMISSION_OPTIONS = {
+    0: "no-access",
+    1: "no-access",
+    2: "write-only",
+    3: "write-only",
+    4: "read-only",
+    5: "read-only",
+    6: "read-write",
+    7: "read-write",
+};
+
+const OPTIONS_PERMISSIONS = {
+    "no-access": 0,
+    "write-only": 2,
+    "read-only": 4,
+    "read-write": 6,
+};
+
 const EditPermissionsModal = ({ dialogResult, selected, path }) => {
     const { cwdInfo } = useFilesContext();
 
@@ -53,6 +72,9 @@ const EditPermissionsModal = ({ dialogResult, selected, path }) => {
     const [errorMessage, setErrorMessage] = useState(undefined);
     const [accounts, setAccounts] = useState(null);
     const [groups, setGroups] = useState(null);
+    const [isExecutable, setIsExecutable] = useState((mode & 0b001001001) === 0b001001001);
+
+    const executable_file_types = ["code-file", "file"];
 
     useInit(async () => {
         try {
@@ -99,16 +121,57 @@ const EditPermissionsModal = ({ dialogResult, selected, path }) => {
         }
     };
 
-    function permissions_options() {
-        return [
-            ...map_permissions((value, label) => (
+    function permissions_options(mode) {
+        const options = [
+            <FormSelectOption
+              key="read-write"
+              value="read-write"
+              label={_("Read and write")}
+            />,
+            <FormSelectOption
+              key="read-only"
+              value="read-only"
+              label={_("Read-only")}
+            />,
+            <FormSelectOption
+              key="no-access"
+              value="no-access"
+              label={_("No access")}
+            />
+        ];
+
+        // Show write-only when such a file exists, but never offer this as a default option.
+        if (mode === 2 || mode === 3) {
+            options.push(
                 <FormSelectOption
-                  key={value}
-                  value={value}
-                  label={label}
+                  key="write-only"
+                  value="write-only"
+                  label={_("Write-only")}
                 />
-            ))
-        ];
+            );
+        }
+
+        return options;
+    }
+
+    function setPermissions(mask, shift, option) {
+        let val = OPTIONS_PERMISSIONS[option];
+        if ((selected.type === 'reg' && isExecutable) || (selected.type === 'dir' && option !== "no-access")) {
+            val += 1;
+        }
+
+        setMode((mode & mask) | (val << shift));
+    }
+
+    function setExecutableBits(shouldBeExecutable) {
+        setIsExecutable(shouldBeExecutable);
+
+        // Strip / add executable bits
+        if (shouldBeExecutable) {
+            setMode(mode | 0b001001001);
+        } else {
+            setMode(mode & ~0b001001001);
+        }
     }
 
     function sortByName(a, b) {
@@ -183,38 +246,45 @@ const EditPermissionsModal = ({ dialogResult, selected, path }) => {
                           fieldId="edit-permissions-owner-access"
                         >
                             <FormSelect
-                              value={(mode >> 6) & 7}
-                              onChange={(_, val) => { setMode((mode & 0o077) | (val << 6)) }}
+                              value={PERMISSION_OPTIONS[(mode >> 6) & 7]}
+                              onChange={(_, val) => { setPermissions(0o077, 6, val) }}
                               id="edit-permissions-owner-access"
                             >
-                                {permissions_options()}
+                                {permissions_options((mode >> 6) & 7)}
                             </FormSelect>
                         </FormGroup>
                         <FormGroup
                           label={_("Group access")}
                           fieldId="edit-permissions-group-access"
                         >
                             <FormSelect
-                              value={(mode >> 3) & 7}
-                              onChange={(_, val) => { setMode((mode & 0o707) | (val << 3)) }}
+                              value={PERMISSION_OPTIONS[(mode >> 3) & 7]}
+                              onChange={(_, val) => { setPermissions(0o707, 3, val) }}
                               id="edit-permissions-group-access"
                             >
-                                {permissions_options()}
+                                {permissions_options((mode >> 3) & 7)}
                             </FormSelect>
                         </FormGroup>
                         <FormGroup
                           label={_("Others access")}
                           fieldId="edit-permissions-other-access"
                         >
                             <FormSelect
-                              value={mode & 7}
-                              onChange={(_, val) => { setMode((mode & 0o770) | val) }}
+                              value={PERMISSION_OPTIONS[(mode) & 7]}
+                              onChange={(_, val) => { setPermissions(0o770, 0, val) }}
                               id="edit-permissions-other-access"
                             >
-                                {permissions_options()}
+                                {permissions_options(mode & 7)}
                             </FormSelect>
                         </FormGroup>
                     </FormSection>
+                    {selected.type === "reg" && executable_file_types.includes(selected?.category?.class || "file") &&
+                        <Checkbox
+                          id="is-executable"
+                          label={_("Set executable as program")}
+                          isChecked={isExecutable}
+                          onChange={() => setExecutableBits(!isExecutable)}
+                        />}
                 </Form>
             </Stack>
         </Modal>

test/check-application

@@ -1233,6 +1233,12 @@ class TestFiles(testlib.MachineCase):
             b.select_from_dropdown("#edit-permissions-group-access", access)
             b.select_from_dropdown("#edit-permissions-other-access", access)
 
+        def open_permissions_dialog(filename: str) -> None:
+            b.wait_visible(f"[data-item='{filename}']")
+            b.click(f"[data-item='{filename}']")
+            b.click("button:contains('Edit permissions')")
+            b.wait_in_text(".pf-v5-c-modal-box__title-text", filename)
+
         self.enter_files()
 
         # Check sidebar info
@@ -1273,16 +1279,16 @@ class TestFiles(testlib.MachineCase):
 
         # Test changing permissions
         b.click("button:contains('Edit permissions')")
-        select_access("0")
+        select_access("no-access")
         b.click("button.pf-m-primary")
         self.assertEqual(m.execute("ls -l /home/admin/newfile")[:10], "----------")
         wait_permissions("None")
 
         b.click("button:contains('Edit permissions')")
-        select_access("7")
+        select_access("read-write")
         b.click("button.pf-m-primary")
-        self.assertEqual(m.execute("ls -l /home/admin/newfile")[:10], "-rwxrwxrwx")
-        wait_permissions("Read, write, and execute")
+        self.assertEqual(m.execute("ls -l /home/admin/newfile")[:10], "-rw-rw-rw-")
+        wait_permissions("Read and write")
 
         # Test changing CWD permissions
         test_dir = "/home/admin/testdir"
@@ -1295,25 +1301,118 @@ class TestFiles(testlib.MachineCase):
         b.mouse("#files-card-parent", "contextmenu")
         b.click(".contextMenu button:contains('Edit permissions')")
         b.wait_in_text(".pf-v5-c-modal-box__title-text", "testdir")
-        b.select_from_dropdown("#edit-permissions-owner-access", "6")
-        b.select_from_dropdown("#edit-permissions-group-access", "4")
-        b.select_from_dropdown("#edit-permissions-other-access", "4")
+        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("button.pf-m-primary")
         b.wait_not_present(".pf-v5-c-modal-box")
 
-        self.assertEqual(m.execute(f"ls -ld {test_dir}")[:10], "drw-r--r--")
+        self.assertEqual(m.execute(f"ls -ld {test_dir}")[:10], "drwxr-xr-x")
 
         # Via kebab
         b.click("#dropdown-menu")
         b.click("button:contains('Edit permissions')")
         b.wait_in_text(".pf-v5-c-modal-box__title-text", "testdir")
-        b.select_from_dropdown("#edit-permissions-owner-access", "7")
-        b.select_from_dropdown("#edit-permissions-group-access", "5")
-        b.select_from_dropdown("#edit-permissions-other-access", "5")
+        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("button.pf-m-primary")
         b.wait_not_present(".pf-v5-c-modal-box")
 
-        self.assertEqual(m.execute(f"ls -ld {test_dir}")[:10], "drwxr-xr-x")
+        self.assertEqual(m.execute(f"ls -ld {test_dir}")[:10], "dr-x------")
+
+        # Can set a file as executable
+        b.click("li[data-location='/home/admin'] a")
+        self.assert_last_breadcrumb("admin")
+
+        shell_script = "install.sh"
+        m.execute(['runuser', '-u', 'admin', 'touch', f'/home/admin/{shell_script}'])
+        m.execute(['chmod', '644', f'/home/admin/{shell_script}'])
+
+        open_permissions_dialog(shell_script)
+        b.wait_val("#edit-permissions-owner-access", "read-write")
+        b.wait_val("#edit-permissions-group-access", "read-only")
+        b.wait_val("#edit-permissions-other-access", "read-only")
+        self.assertFalse(b.get_checked("#is-executable"))
+
+        b.set_checked("#is-executable", True)
+        b.click("button.pf-m-primary")
+        b.wait_not_present(".pf-v5-c-modal-box")
+
+        b.wait_text("#description-list-owner-permissions dd", "Read, write, and execute")
+        b.wait_text("#description-list-group-permissions dd", "Read and execute")
+        b.wait_text("#description-list-other-permissions dd", "Read and execute")
+        self.assertEqual(m.execute(f"ls -ld /home/admin/{shell_script}")[:10], "-rwxr-xr-x")
+
+        b.click("button:contains('Edit permissions')")
+        b.wait_in_text(".pf-v5-c-modal-box__title-text", shell_script)
+
+        # Permissions did not change visually only executable is checked now
+        b.wait_val("#edit-permissions-owner-access", "read-write")
+        b.wait_val("#edit-permissions-group-access", "read-only")
+        b.wait_val("#edit-permissions-other-access", "read-only")
+        self.assertTrue(b.get_checked("#is-executable"))
+
+        b.set_checked("#is-executable", False)
+        b.click("button.pf-m-primary")
+        b.wait_not_present(".pf-v5-c-modal-box")
+        self.assertEqual(m.execute(f"ls -l /home/admin/{shell_script}")[:10], "-rw-r--r--")
+
+        # Not executable for .png
+        img_file = "cockpit.png"
+        m.execute(['runuser', '-u', 'admin', 'touch', f'/home/admin/{img_file}'])
+
+        open_permissions_dialog(img_file)
+        b.wait_not_present("#is-executable")
+        b.click("div.pf-v5-c-modal-box__close button")
+        b.wait_not_present(".pf-v5-c-modal-box")
+
+        # fifo not executable
+        fifo_file = "test.fifo"
+        m.execute(['runuser', '-u', 'admin', 'mkfifo', f'/home/admin/{fifo_file}'])
+
+        open_permissions_dialog(fifo_file)
+        b.wait_not_present("#is-executable")
+        b.click("div.pf-v5-c-modal-box__close button")
+        b.wait_not_present(".pf-v5-c-modal-box")
+
+        # `uname` can be marked as executable
+        executable_file = "uname"
+        m.execute(['runuser', '-u', 'admin', 'touch', f'/home/admin/{executable_file}'])
+        m.execute(['chmod', '644', f'/home/admin/{executable_file}'])
+
+        open_permissions_dialog(executable_file)
+        b.set_checked("#is-executable", True)
+        b.click("button.pf-m-primary")
+        b.wait_not_present(".pf-v5-c-modal-box")
+
+        b.wait_text("#description-list-owner-permissions dd", "Read, write, and execute")
+        b.wait_text("#description-list-group-permissions dd", "Read and execute")
+        b.wait_text("#description-list-other-permissions dd", "Read and execute")
+
+        # Special file access permission cases
+
+        # We normally don't provide "write-only" as an option unless someone
+        # has misconfigured their access permissions. Also verify that +x bits
+        # are retained.
+        m.execute(['chmod', '532', f'/home/admin/{shell_script}'])
+        b.click(f"[data-item='{shell_script}']")
+        b.wait_text("#description-list-owner-permissions dd", "Read and execute")
+        b.wait_text("#description-list-group-permissions dd", "Write and execute")
+        b.wait_text("#description-list-other-permissions dd", "Write-only")
+        b.click("button:contains('Edit permissions')")
+
+        b.wait_in_text(".pf-v5-c-modal-box__title-text", shell_script)
+        b.wait_val("#edit-permissions-owner-access", "read-only")
+        b.wait_val("#edit-permissions-group-access", "write-only")
+        b.wait_val("#edit-permissions-other-access", "write-only")
+        self.assertFalse(b.get_checked("#is-executable"))
+
+        b.select_from_dropdown("#edit-permissions-group-access", "read-write")
+        b.select_from_dropdown("#edit-permissions-other-access", "no-access")
+        b.click("button.pf-m-primary")
+        b.wait_not_present(".pf-v5-c-modal-box")
+        self.assertEqual(m.execute(f"ls -l /home/admin/{shell_script}")[:10], "-r-xrw----")
 
         b.go("/files#/?path=/")
         b.wait_not_present(".pf-v5-c-empty-state")
@@ -1335,12 +1434,12 @@ class TestFiles(testlib.MachineCase):
         m.execute("touch /home/admin/adminfile; chown admin: /home/admin/adminfile")
         b.click("[data-item='adminfile']")
         b.click("button:contains('Edit permissions')")
-        select_access("7")
+        select_access("read-write")
         # A user cannot change ownership
         b.wait_not_in_text(".pf-v5-c-modal-box__body", "Ownership")
         b.click("button.pf-m-primary")
-        self.assertEqual(m.execute("ls -l /home/admin/adminfile")[:10], "-rwxrwxrwx")
-        wait_permissions("Read, write, and execute")
+        self.assertEqual(m.execute("ls -l /home/admin/adminfile")[:10], "-rw-rw-rw-")
+        wait_permissions("Read and write")
         # Does not change ownership
         b.wait_text("#description-list-owner dd", "admin")
         b.wait_text("#description-list-group dd", "admin")
@@ -1349,7 +1448,7 @@ class TestFiles(testlib.MachineCase):
         b.click("li[data-location='/'] a")
         b.click("[data-item='home']")
         b.click("button:contains('Edit permissions')")
-        select_access("7")
+        select_access("read-only")
         b.click("button.pf-m-primary")
         self.wait_modal_inline_alert("chmod: changing permissions of '/home': Operation not permitted")
         b.click("button.pf-m-link")