Skip to content

Commit

Permalink
Permissions dialog re-design
Browse files Browse the repository at this point in the history 
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: cockpit-project#192
  • Loading branch information
@jelly
jelly committed Oct 14, 2024
1 parent 8dc2256 commit 920c64e
Show file tree
Hide file tree
Showing 3 changed files with 203 additions and 40 deletions.
6 changes: 0 additions & 6 deletions src/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
106 changes: 88 additions & 18 deletions src/dialogs/permissions.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();

Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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>
Expand Down
131 changes: 115 additions & 16 deletions test/check-application
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down

0 comments on commit 920c64e

Please sign in to comment.