more sophisticated permission checking

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

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):

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]

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]