diff --git a/src/common.ts b/src/common.ts index 43fb12b7..a935ddaa 100644 --- a/src/common.ts +++ b/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(func: (value: number, label: string) => T) { - for (const [value, label] of permissions.entries()) { - yield func(value, label); - } -} diff --git a/src/dialogs/permissions.jsx b/src/dialogs/permissions.jsx index 2473082e..46e4f4f8 100644 --- a/src/dialogs/permissions.jsx +++ b/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 = [ + , + , + + ]; + + // Show write-only when such a file exists, but never offer this as a default option. + if (mode === 2 || mode === 3) { + options.push( - )) - ]; + ); + } + + 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,11 +246,11 @@ const EditPermissionsModal = ({ dialogResult, selected, path }) => { fieldId="edit-permissions-owner-access" > > 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)} { fieldId="edit-permissions-group-access" > > 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)} { fieldId="edit-permissions-other-access" > { 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)} + {selected.type === "reg" && executable_file_types.includes(selected?.category?.class || "file") && + setExecutableBits(!isExecutable)} + />} diff --git a/test/check-application b/test/check-application index 3752d597..fa467e6f 100755 --- a/test/check-application +++ b/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")