From bdb4fce9eef2222078762e961850474903f0e73f Mon Sep 17 00:00:00 2001
From: Christopher O'Brien <cobrien@redhat.com>
Date: Oct 28 2019 11:42:12 +0000
Subject: [PATCH 1/2] check for tag or target permission in addition to admin for relevant CLI calls


---

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:

From 1407e0ed4ee11012d7aafdf6a3a6f641b38f62a8 Mon Sep 17 00:00:00 2001
From: Christopher O'Brien <cobrien@redhat.com>
Date: Oct 28 2019 11:42:40 +0000
Subject: [PATCH 2/2] make tests work


---

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']

