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.

Related: #192
  • Loading branch information
@jelly
jelly committed Oct 11, 2024
1 parent 8dc2256 commit c9d118d
Show file tree
Hide file tree
Showing 3 changed files with 173 additions and 35 deletions.
2 changes: 1 addition & 1 deletion src/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function get_permissions(n: number) {
}

export function * map_permissions<T>(func: (value: number, label: string) => T) {
for (const [value, label] of permissions.entries()) {
for (const [value, label] of simple_permissions.entries()) {
yield func(value, label);
}
}
110 changes: 92 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 @@ -47,12 +66,15 @@ const EditPermissionsModal = ({ dialogResult, selected, path }) => {
selected = { ...cwdInfo, isCwd: true, name: directory_name };
}

console.log(selected);

const [owner, setOwner] = useState(selected.user);
const [mode, setMode] = useState(selected.mode);
const [group, setGroup] = useState(selected.group);
const [errorMessage, setErrorMessage] = useState(undefined);
const [accounts, setAccounts] = useState(null);
const [groups, setGroups] = useState(null);
const [isExecutable, setIsExecutable] = useState((mode & 0b001001001) === 0b001001001);

useInit(async () => {
try {
Expand Down Expand Up @@ -99,22 +121,67 @@ 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) {
return a.name.localeCompare(b.name);
}

console.log((mode >> 6) & 7);
console.log((mode >> 3) & 7);
console.log((mode >> 0) & 7);

return (
<Modal
position="top"
Expand Down Expand Up @@ -183,38 +250,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" &&
<Checkbox
id="is-executable"
label={_("Set executable as program")}
isChecked={isExecutable}
onChange={() => setExecutableBits(!isExecutable)}
/>}
</Form>
</Stack>
</Modal>
Expand Down
96 changes: 80 additions & 16 deletions test/check-application
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1273,16 +1273,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 +1295,89 @@ 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}'])

b.wait_visible(f"[data-item='{shell_script}']")
b.click(f"[data-item='{shell_script}']")
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-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--")

# 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.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 +1399,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 +1413,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 c9d118d

Please sign in to comment.