diff --git a/actions/users b/actions/users index 97c361235..d5059a4b1 100755 --- a/actions/users +++ b/actions/users @@ -264,11 +264,12 @@ def subcommand_set_user_password(arguments): def get_user_groups(username): - """Returns only the supplementary groups of the given user and doesn't include - the 'users' primary group""" + """Returns only the supplementary groups of the given user. + + Exclude the 'users' primary group from the returned list.""" process = _run( ['ldapid', username], stdout=subprocess.PIPE, check=False) - output = process.stdout.decode('utf-8').strip() + output = process.stdout.decode().strip() if output: groups_part = output.split(' ')[2] groups = groups_part.split('=')[1] @@ -276,13 +277,13 @@ def get_user_groups(username): for user in re.findall('\(.*?\)', groups)] group_names.remove('users') return group_names + return [] def subcommand_get_user_groups(arguments): """Return list of a given user's groups.""" - groups = [group - for group in get_user_groups(arguments.username)] + groups = get_user_groups(arguments.username) if groups: print(*groups, sep='\n') @@ -327,8 +328,7 @@ def subcommand_add_user_to_group(arguments): def remove_user_from_group(username, groupname): """Remove an LDAP user from an LDAP group.""" - _run( - ['ldapdeleteuserfromgroup', username, groupname]) + _run(['ldapdeleteuserfromgroup', username, groupname]) def subcommand_remove_user_from_group(arguments): diff --git a/plinth/modules/deluge/__init__.py b/plinth/modules/deluge/__init__.py index a2dae9316..55948a055 100644 --- a/plinth/modules/deluge/__init__.py +++ b/plinth/modules/deluge/__init__.py @@ -82,7 +82,7 @@ def setup(helper, old_version=None): disable=disable) helper.call('post', service.notify_enabled, None, True) helper.call('post', add_shortcut) - add_group("bittorrent") + add_group('bittorrent') def add_shortcut(): diff --git a/plinth/modules/users/__init__.py b/plinth/modules/users/__init__.py index a634ffc7c..12ef76d84 100644 --- a/plinth/modules/users/__init__.py +++ b/plinth/modules/users/__init__.py @@ -87,8 +87,10 @@ def _diagnose_ldap_entry(search_item): def add_group(group): + """Add an LDAP group.""" actions.superuser_run('users', options=['create-group', group]) def remove_group(group): + """Remove an LDAP group.""" actions.superuser_run('users', options=['remove-group', group]) diff --git a/plinth/modules/users/tests/test_actions.py b/plinth/modules/users/tests/test_actions.py index e75f1f92f..8d49587a9 100644 --- a/plinth/modules/users/tests/test_actions.py +++ b/plinth/modules/users/tests/test_actions.py @@ -96,6 +96,7 @@ class TestActions(unittest.TestCase): self.delete_user(user) except Exception: pass + for group in self.groups: self.delete_group(group) @@ -137,7 +138,7 @@ class TestActions(unittest.TestCase): """Return the list of groups for a user.""" process = self.call_action( ['get-user-groups', username], stdout=subprocess.PIPE) - return process.stdout.split() + return process.stdout.decode().split() def create_group(self, groupname=None): groupname = groupname or random_string() @@ -192,11 +193,9 @@ class TestActions(unittest.TestCase): new_groups = self.get_user_groups(new_username) old_users_groups = self.get_user_groups(old_username) - self.assertFalse(len(old_users_groups)) + self.assertFalse(old_users_groups) # empty self.assertEqual(old_groups, new_groups) - self.assertFalse(self.get_user_groups(old_username)) # is empty - with self.assertRaises(subprocess.CalledProcessError): self.rename_user(old_username) @@ -214,8 +213,6 @@ class TestActions(unittest.TestCase): def test_delete_user(self): """Test to check whether LDAP users can be deleted""" username, password = self.create_user(groups=[random_string()]) - groups_before = self.get_user_groups(username) - self.assertTrue(groups_before) self.delete_user(username) groups_after = self.get_user_groups(username) self.assertFalse(groups_after) # User gets removed from all groups @@ -256,6 +253,7 @@ class TestActions(unittest.TestCase): def test_user_group_interactions(self): group1 = random_string() user1, _ = self.create_user(groups=[group1]) + self.assertEqual([group1], self.get_user_groups(user1)) # add-user-to-group is not idempotent with self.assertRaises(subprocess.CalledProcessError): @@ -273,8 +271,7 @@ class TestActions(unittest.TestCase): # The expected groups got created and the user is part of them. expected_groups = [group1, group2, group3] - actual_groups = [g.decode() for g in self.get_user_groups(user1)] - self.assertEqual(expected_groups, actual_groups) + self.assertEqual(expected_groups, self.get_user_groups(user1)) # Remove user from group group_to_remove_from = random.choice(expected_groups) @@ -283,8 +280,7 @@ class TestActions(unittest.TestCase): # User is no longer in the group that they're removed from expected_groups.remove(group_to_remove_from) - actual_groups = [g.decode() for g in self.get_user_groups(user1)] - self.assertEqual(expected_groups, actual_groups) + self.assertEqual(expected_groups, self.get_user_groups(user1)) # User cannot be removed from a group that they're not part of random_group = random_string()