From c40f82217b6e9316ed49663e7ec4c154f26d7130 Mon Sep 17 00:00:00 2001 From: sezanzeb Date: Sat, 5 Dec 2020 18:49:52 +0100 Subject: [PATCH] more sophisticated permission checking --- keymapper/dev/permissions.py | 106 ++++++++++++++++++---------- keymapper/gtk/window.py | 37 +++------- tests/testcases/test_integration.py | 3 + tests/testcases/test_permissions.py | 2 + 4 files changed, 85 insertions(+), 63 deletions(-) diff --git a/keymapper/dev/permissions.py b/keymapper/dev/permissions.py index df4ed6fd..c850b5b5 100644 --- a/keymapper/dev/permissions.py +++ b/keymapper/dev/permissions.py @@ -23,7 +23,9 @@ import grp +import glob import getpass +import subprocess import os from keymapper.logger import logger @@ -31,48 +33,80 @@ from keymapper.paths import USER def check_group(group): - """Check if the user can access files of that group. - - Returns True if this group doesn't even exist. - """ + """Check if the required group is active and log if not.""" + # TODO test try: - return USER in grp.getgrnam(group).gr_mem + in_group = USER in grp.getgrnam(group).gr_mem except KeyError: - return True + # group doesn't exist. Ignore + return None + + # check if files exist with that group in /dev. Even if plugdev + # exists, that doesn't mean that it is needed. + used_groups = [os.stat(path).st_gid for path in glob.glob('/dev/input/*')] + if grp.getgrnam(group).gr_gid not in used_groups: + return None + + if not in_group: + msg = ( + 'Some devices may not be visible without being in the ' + f'"{group}" user group. Try `sudo usermod -a -G {group} {USER}` ' + 'and log out and back in or restart.', + ) + logger.warning(msg) + return msg + + try: + groups = subprocess.check_output('groups').decode().split() + group_active = group in groups + except FileNotFoundError: + # groups command missing. Idk if any distro doesn't have it + # but if so, cover the case. + return None + + if in_group and not group_active: + msg = ( + f'You are in the "{group}" group, but your session is not yet ' + 'using it. Some device may not be visible. Please log out and ' + 'back in or restart', + group + ) + logger.warning(msg) + return msg + + return None + + +def check_injection_rights(): + """Check if the user may write into /dev/uinput.""" + if not os.access('/dev/uinput', os.W_OK): + can_write = ( + 'Rights to write to /dev/uinput are missing, keycodes cannot ' + f'be injected. Try `sudo setfacl -m u:{USER}:rw- /dev/uinput`' + ) + logger.error(can_write) + return can_write + + return None def can_read_devices(): - """If the people ever looks into the console, make sure to help them.""" - is_root = getpass.getuser() == 'root' - is_input = check_group('input') - is_plugdev = check_group('plugdev') + """Help the user to setup correct permissions.""" + if getpass.getuser() == 'root': + return None + + input_check = check_group('input') + plugdev_check = check_group('plugdev') # ubuntu. funnily, individual devices in /dev/input/ have write permitted. - can_write = os.access('/dev/uinput', os.W_OK) + can_write = check_injection_rights() - def warn(group): - logger.warning( - 'Some devices may not be visible without being in the ' - '"%s" user group. Try `sudo usermod -a -G %s %s` ' - 'and log out and back in.', - group, - group, - USER - ) + ret = [] + if can_write is not None: + ret.append(can_write) + if input_check is not None: + ret.append(input_check) + if plugdev_check is not None: + ret.append(plugdev_check) - if not is_root: - if not is_plugdev: - warn('plugdev') - if not is_input: - warn('input') - if not can_write: - logger.error( - 'Injecting keycodes into /dev/uinput is not permitted. ' - 'Either use sudo or run ' - '`sudo setfacl -m u:%s:rw- /dev/uinput`', - {USER} - ) - - permitted = (is_root or (is_input and is_plugdev)) and can_write - - return permitted, is_root, is_input, is_plugdev, can_write + return ret diff --git a/keymapper/gtk/window.py b/keymapper/gtk/window.py index 0932abd7..8d6452d0 100755 --- a/keymapper/gtk/window.py +++ b/keymapper/gtk/window.py @@ -110,25 +110,14 @@ class Window: # already visible (without content) to make it look more responsive. gtk_iteration() - permitted, _, is_input, is_plugdev, can_write = can_read_devices() - if not permitted: - missing_groups = [] - if not is_input: - missing_groups.append('input') - if not is_plugdev: - missing_groups.append('plugdev') - - if len(missing_groups) > 0: - self.get('status_bar').push( - CTX_ERROR, - f'You are not in the {" and ".join(missing_groups)} ' - f'group{"s" if len(missing_groups) > 0 else ""}' - ) - elif not can_write: - self.get('status_bar').push( - CTX_ERROR, - 'Insufficient permissions on /dev/uinput' - ) + permission_errors = can_read_devices() + if len(permission_errors) > 0: + # TODO test + self.show_status( + CTX_ERROR, + 'Permission error. Hover for info', + '\n\n'.join(permission_errors) + ) self.populate_devices() @@ -235,10 +224,7 @@ class Window: # because the device is in grab mode by the daemon and # therefore the original keycode inaccessible logger.info('Cannot change keycodes while injecting') - self.get('status_bar').push( - CTX_ERROR, - 'Use "Apply Defaults" before editing' - ) + self.show_status(CTX_ERROR, 'Use "Apply Defaults" before editing') def get_focused_row(self): """Get the Row that is currently in focus.""" @@ -288,10 +274,7 @@ class Window: def on_apply_system_layout_clicked(self, _): """Load the mapping.""" self.dbus.stop_injecting(self.selected_device) - self.get('status_bar').push( - CTX_APPLY, - 'Applied the system default' - ) + self.show_status(CTX_APPLY, 'Applied the system default') GLib.timeout_add(10, self.show_device_mapping_status) def show_status(self, context_id, message, tooltip=None): diff --git a/tests/testcases/test_integration.py b/tests/testcases/test_integration.py index 0b7a2dc5..b98fd85f 100644 --- a/tests/testcases/test_integration.py +++ b/tests/testcases/test_integration.py @@ -518,6 +518,7 @@ class TestPermissions(unittest.TestCase): shutil.rmtree('/tmp/key-mapper-test') def test_check_groups_missing(self): + # TODO modify test class Grnam: def __init__(self, group): self.gr_mem = [] @@ -534,6 +535,7 @@ class TestPermissions(unittest.TestCase): self.assertIn('plugdev', labels) def test_check_plugdev_missing(self): + # TODO modify test class Grnam: def __init__(self, group): if group == 'input': @@ -553,6 +555,7 @@ class TestPermissions(unittest.TestCase): self.assertIn('plugdev', labels) def test_check_write_uinput(self): + # TODO modify test class Grnam: def __init__(self, group): self.gr_mem = [USER] diff --git a/tests/testcases/test_permissions.py b/tests/testcases/test_permissions.py index d0f22cae..8528dbc7 100644 --- a/tests/testcases/test_permissions.py +++ b/tests/testcases/test_permissions.py @@ -34,6 +34,7 @@ class TestPermissions(unittest.TestCase): grp.getgrnam = self.getgrnam def test_cannot_access(self): + # TODO modify test class Grnam: def __init__(self, group): self.gr_mem = [] @@ -42,6 +43,7 @@ class TestPermissions(unittest.TestCase): self.assertFalse(can_read_devices()[0]) def test_can_access(self): + # TODO modify test class Grnam: def __init__(self, group): self.gr_mem = [USER]