From 90b1a3a9970e94ddb85c5b57434ac1f21f7852d0 Mon Sep 17 00:00:00 2001 From: Christopher O'Brien Date: Jul 09 2024 17:03:48 +0000 Subject: [PATCH 1/5] Make getGroupMembers anonymous and add getUserGroups --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 3f17e5e..7cc17e4 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -9553,7 +9553,7 @@ def drop_group_member(group, user): def get_group_members(group): """Get the members of a group""" - context.session.assertPerm('admin') + ginfo = get_user(group) if not ginfo or ginfo['usertype'] != koji.USERTYPES['GROUP']: raise koji.GenericError("No such group: %s" % group) @@ -13409,6 +13409,24 @@ class RootExports(object): dropGroupMember = staticmethod(drop_group_member) getGroupMembers = staticmethod(get_group_members) + def getUserGroups(self, user): + """ + The groups associated with the given user + + :param user: a str (Kerberos principal or name) or an int (user id) + or a dict: + - id: User's ID + - name: User's name + - krb_principal: Kerberos principal + + :returns: a dict mapping member's group IDs to group names + + :raises: GenericError if the specified user is not found + """ + + uinfo = get_user(user, strict=True) + return get_user_groups(uinfo["id"]) + def listUsers(self, userType=koji.USERTYPES['NORMAL'], prefix=None, queryOpts=None, perm=None, inherited_perm=False): """List users in the system diff --git a/tests/test_hub/test_get_user_groups.py b/tests/test_hub/test_get_user_groups.py new file mode 100644 index 0000000..3267561 --- /dev/null +++ b/tests/test_hub/test_get_user_groups.py @@ -0,0 +1,52 @@ +import mock + +import koji +import kojihub +from .utils import DBQueryTestCase + + +class TestGetUserGroups(DBQueryTestCase): + + def setUp(self): + super(TestGetUserGroups, self).setUp() + self.exports = kojihub.RootExports() + + self.context = mock.patch('kojihub.kojihub.context').start() + self.context_db = mock.patch('kojihub.db.context').start() + + def tearDown(self): + mock.patch.stopall() + + def test_no_such_user(self): + user = 'test-user' + self.qp_execute_return_value = [] + with self.assertRaises(koji.GenericError) as cm: + self.exports.getUserGroups(user) + self.assertEqual(f"No such user: {user!r}", str(cm.exception)) + self.assertEqual(len(self.queries), 1) + + def test_valid(self): + get_user = mock.patch('kojihub.kojihub.get_user').start() + get_user.return_value = {'id': 23, 'usertype': 0} + + user = 'test-user' + self.qp_execute_return_value = [{'group_id': 123, 'name': 'grp_123'}, + {'group_id': 456, 'name': 'grp_456'}] + + grps = self.exports.getUserGroups(user) + + get_user.assert_called_once_with(user, strict=True) + + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['user_groups']) + self.assertEqual(query.joins, ['users ON group_id = users.id']) + self.assertEqual(query.clauses, ['active IS TRUE', + 'user_id=%(user_id)i', + 'users.usertype=%(t_group)i']) + self.assertEqual(query.values, {'t_group': 2, + 'user_id': 23}) + self.assertEqual(query.columns, ['group_id', 'name']) + + self.assertEqual(grps, {123: 'grp_123', + 456: 'grp_456'}) diff --git a/tests/test_hub/test_user_groups.py b/tests/test_hub/test_user_groups.py index d007444..746afdc 100644 --- a/tests/test_hub/test_user_groups.py +++ b/tests/test_hub/test_user_groups.py @@ -262,11 +262,11 @@ class TestGrouplist(unittest.TestCase): def test_get_group_members(self): group, gid = 'test_group', 1 - # no permission - self.context.session.assertPerm.side_effect = koji.ActionNotAllowed - with self.assertRaises(koji.ActionNotAllowed): + # no permission needed, verify that it's not being checked + self.context.session.assertPerm.side_effect = Exception + with self.assertRaises(koji.GenericError): kojihub.get_group_members(group) - self.context.session.assertPerm.assert_called_with('admin') + self.context.session.assertPerm.assert_not_called() self.assertEqual(len(self.inserts), 0) self.assertEqual(len(self.updates), 0) From 97ed4239e0c66ad62042ea2dd32bc83c3c75bf56 Mon Sep 17 00:00:00 2001 From: Christopher O'Brien Date: Jul 09 2024 17:10:45 +0000 Subject: [PATCH 2/5] make the API tests pass --- diff --git a/tests/test_api/data/api.json b/tests/test_api/data/api.json index 1a0b1d6..a96dac1 100644 --- a/tests/test_api/data/api.json +++ b/tests/test_api/data/api.json @@ -2,7 +2,7 @@ "version": [ 1, 34, - 0 + 1 ], "lib": { "koji": { @@ -1421,7 +1421,7 @@ } }, "Retry": { - "type": "", + "type": "", "is_external": true }, "RetryError": { @@ -7457,6 +7457,16 @@ "varargs": null, "varkw": null }, + "getUserGroups": { + "desc": "(user)", + "args": [ + { + "name": "user" + } + ], + "varargs": null, + "varkw": null + }, "getUserPerms": { "desc": "(userID=None, with_groups=True)", "args": [ From a11e2d90e0ac02a10bccf1c743c84976591e6f2f Mon Sep 17 00:00:00 2001 From: Christopher O'Brien Date: Jul 09 2024 17:23:03 +0000 Subject: [PATCH 3/5] fix lingering mock patch --- diff --git a/tests/test_hub/test_get_user.py b/tests/test_hub/test_get_user.py index ba04035..5bc32e7 100644 --- a/tests/test_hub/test_get_user.py +++ b/tests/test_hub/test_get_user.py @@ -104,6 +104,9 @@ class TestGetUserByKrbPrincipal(unittest.TestCase): def setUp(self): self.get_user = mock.patch('kojihub.kojihub.get_user').start() + def tearDown(self): + mock.patch.stopall() + def test_wrong_type_krb_principal(self): krb_principal = ['test-user'] with self.assertRaises(koji.GenericError) as cm: From e67d3ce5ea09c3413d680218632c12c46a6bb75c Mon Sep 17 00:00:00 2001 From: Christopher O'Brien Date: Jul 10 2024 18:44:05 +0000 Subject: [PATCH 4/5] change return type to list of dicts --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 7cc17e4..51cc4bf 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -13419,13 +13419,15 @@ class RootExports(object): - name: User's name - krb_principal: Kerberos principal - :returns: a dict mapping member's group IDs to group names + :returns: a list of dicts, each containing the id and name of + a group :raises: GenericError if the specified user is not found """ uinfo = get_user(user, strict=True) - return get_user_groups(uinfo["id"]) + return [{'id': key, 'name': val} for key, val in + get_user_groups(uinfo["id"]).items()] def listUsers(self, userType=koji.USERTYPES['NORMAL'], prefix=None, queryOpts=None, perm=None, inherited_perm=False): diff --git a/tests/test_hub/test_get_user_groups.py b/tests/test_hub/test_get_user_groups.py index 3267561..4b23f3f 100644 --- a/tests/test_hub/test_get_user_groups.py +++ b/tests/test_hub/test_get_user_groups.py @@ -48,5 +48,5 @@ class TestGetUserGroups(DBQueryTestCase): 'user_id': 23}) self.assertEqual(query.columns, ['group_id', 'name']) - self.assertEqual(grps, {123: 'grp_123', - 456: 'grp_456'}) + self.assertEqual(grps, [{'id': 123, 'name': 'grp_123'}, + {'id': 456, 'name': 'grp_456'}]) From 3c4ed519b6678e80927f21a108bcd4fa011cd2f9 Mon Sep 17 00:00:00 2001 From: Christopher O'Brien Date: Jul 10 2024 18:47:17 +0000 Subject: [PATCH 5/5] undoing api json changes --- diff --git a/tests/test_api/data/api.json b/tests/test_api/data/api.json index a96dac1..1a0b1d6 100644 --- a/tests/test_api/data/api.json +++ b/tests/test_api/data/api.json @@ -2,7 +2,7 @@ "version": [ 1, 34, - 1 + 0 ], "lib": { "koji": { @@ -1421,7 +1421,7 @@ } }, "Retry": { - "type": "", + "type": "", "is_external": true }, "RetryError": { @@ -7457,16 +7457,6 @@ "varargs": null, "varkw": null }, - "getUserGroups": { - "desc": "(user)", - "args": [ - { - "name": "user" - } - ], - "varargs": null, - "varkw": null - }, "getUserPerms": { "desc": "(userID=None, with_groups=True)", "args": [