From 353eefea1d2410fd77490eab67c25330d5762679 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Oct 31 2019 10:40:22 +0000 Subject: PR#1733: allow tag or target permissions as appropriate (on master) Merges #1733 https://pagure.io/koji/pull-request/1733 Fixes: #1729 https://pagure.io/koji/issue/1729 koji CLI still checks for admin permission instead of tag/target/host --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index efe43e2..a5dbc29 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -64,8 +64,8 @@ def handle_add_group(goptions, session, args): group = args[1] activate_session(session, goptions) - if not session.hasPerm('admin'): - print("This action requires admin privileges") + if not (session.hasPerm('admin') or session.hasPerm('tag')): + print("This action requires tag or admin privileges") return 1 dsttag = session.getTag(tag) @@ -95,8 +95,8 @@ def handle_block_group(goptions, session, args): group = args[1] activate_session(session, goptions) - if not session.hasPerm('admin'): - print("This action requires admin privileges") + if not (session.hasPerm('admin') or session.hasPerm('tag')): + print("This action requires tag or admin privileges") return 1 dsttag = session.getTag(tag) @@ -3397,8 +3397,8 @@ def handle_clone_tag(goptions, session, args): assert False # pragma: no cover activate_session(session, goptions) - if not session.hasPerm('admin') and not options.test: - parser.error(_("This action requires admin privileges")) + if not options.test and not (session.hasPerm('admin') or session.hasPerm('tag')): + parser.error(_("This action requires tag or admin privileges")) if args[0] == args[1]: parser.error(_('Source and destination tags must be different.')) @@ -3850,8 +3850,8 @@ def handle_add_target(goptions, session, args): #most targets have the same name as their destination dest_tag = name activate_session(session, goptions) - if not session.hasPerm('admin'): - print("This action requires admin privileges") + if not (session.hasPerm('admin') or session.hasPerm('target')): + print("This action requires target or admin privileges") return 1 chkbuildtag = session.getTag(build_tag) @@ -3885,8 +3885,8 @@ def handle_edit_target(goptions, session, args): assert False # pragma: no cover activate_session(session, goptions) - if not session.hasPerm('admin'): - print("This action requires admin privileges") + if not (session.hasPerm('admin') or session.hasPerm('target')): + print("This action requires target or admin privileges") return targetInfo = session.getBuildTarget(args[0]) @@ -3928,8 +3928,8 @@ def handle_remove_target(goptions, session, args): assert False # pragma: no cover activate_session(session, goptions) - if not session.hasPerm('admin'): - print("This action requires admin privileges") + if not (session.hasPerm('admin') or session.hasPerm('target')): + print("This action requires target or admin privileges") return target = args[0] @@ -3953,8 +3953,8 @@ def handle_remove_tag(goptions, session, args): assert False # pragma: no cover activate_session(session, goptions) - if not session.hasPerm('admin'): - print("This action requires admin privileges") + if not (session.hasPerm('admin') or session.hasPerm('tag')): + print("This action requires tag or admin privileges") return tag = args[0] @@ -4953,8 +4953,8 @@ def handle_add_tag(goptions, session, args): parser.error(_("Please specify a name for the tag")) assert False # pragma: no cover activate_session(session, goptions) - if not session.hasPerm('admin'): - print("This action requires admin privileges") + if not (session.hasPerm('admin') or session.hasPerm('tag')): + print("This action requires tag or admin privileges") return opts = {} if options.parent: diff --git a/tests/test_cli/test_add_group.py b/tests/test_cli/test_add_group.py index d9abda3..c14ea65 100644 --- a/tests/test_cli/test_add_group.py +++ b/tests/test_cli/test_add_group.py @@ -129,12 +129,13 @@ class TestAddGroup(unittest.TestCase): # Run it and check immediate output rv = handle_add_group(options, session, arguments) actual = stdout.getvalue() - expected = 'This action requires admin privileges\n' + expected = 'This action requires tag or admin privileges\n' self.assertMultiLineEqual(actual, expected) # Finally, assert that things were called as we expected. activate_session_mock.assert_called_once_with(session, options) - session.hasPerm.assert_called_once_with('admin') + session.hasPerm.assert_has_calls([mock.call('admin'), + mock.call('tag')]) session.getTag.assert_not_called() session.getTagGroups.assert_not_called() session.groupListAdd.assert_not_called() diff --git a/tests/test_cli/test_add_tag.py b/tests/test_cli/test_add_tag.py index 422b924..824941f 100644 --- a/tests/test_cli/test_add_tag.py +++ b/tests/test_cli/test_add_tag.py @@ -44,7 +44,7 @@ class TestAddTag(utils.CliTestCase): activate_session=None) # Case 2. not admin account - expected = "This action requires admin privileges\n" + expected = "This action requires tag or admin privileges\n" session.hasPerm.return_value = None handle_add_tag(options, session, ['test-tag']) self.assert_console_message(stdout, expected) diff --git a/tests/test_cli/test_block_group.py b/tests/test_cli/test_block_group.py index bf0a75a..feee8f9 100644 --- a/tests/test_cli/test_block_group.py +++ b/tests/test_cli/test_block_group.py @@ -123,6 +123,6 @@ class TestBlockGroup(utils.CliTestCase): session.hasPerm.return_value = False rv = handle_block_group(options, session, ['tag', 'grp']) self.assert_console_message( - stdout, 'This action requires admin privileges\n') + stdout, 'This action requires tag or admin privileges\n') self.assertEqual(rv, 1) activate_session_mock.assert_called_with(session, options) diff --git a/tests/test_cli/test_clone_tag.py b/tests/test_cli/test_clone_tag.py index e639749..3b3219b 100644 --- a/tests/test_cli/test_clone_tag.py +++ b/tests/test_cli/test_clone_tag.py @@ -59,10 +59,10 @@ clone-tag will create the destination tag if it does not already exist self.session, args, stderr=self.format_error_message( - "This action requires admin privileges"), + "This action requires tag or admin privileges"), activate_session=None) self.activate_session.assert_called_once() - self.session.hasPerm.assert_called_once_with('admin') + self.session.hasPerm.assert_has_calls([call('admin'), call('tag')]) def test_handle_clone_tag_same_tag(self): args = ['src-tag', 'src-tag']