From ae8c2d64b60a024b1d2c3c7f0a2ca98e2382f1fc Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Jun 04 2019 10:38:32 +0000
Subject: [PATCH 1/3] introduce 'host' permission + docs


Fixes: https://pagure.io/koji/issue/1453

---

diff --git a/docs/source/index.rst b/docs/source/index.rst
index b8a008a..162f558 100644
--- a/docs/source/index.rst
+++ b/docs/source/index.rst
@@ -22,6 +22,7 @@ Contents
     :maxdepth: 2
 
     HOWTO
+    permissions
     defining_hub_policies
     external_repo_server_bootstrap
     image_build
diff --git a/docs/source/permissions.rst b/docs/source/permissions.rst
new file mode 100644
index 0000000..b44dd46
--- /dev/null
+++ b/docs/source/permissions.rst
@@ -0,0 +1,76 @@
+=================
+Permission system
+=================
+
+Basic privileges for koji are handled by ``permissions``. These are granted
+and removed by ``admin`` user and allows other users to use different parts
+of koji. There are some default permissions, but new ones can be created by
+administrator and used in koji's :doc:`policies <defining_hub_policies>` or tag
+locks.
+
+Permission management
+=====================
+
+Admin user can use following koji CLI commands:
+
+  * ``koji grant-permission [--new] <permission> <user> [<user> ...]`` for
+    granting permission to one or more users. It can be also used to create
+    new permission class with ``--new``.
+  * ``koji revoke-permission <permission> <user> [<user> ...]`` for removing
+    such permission from users.
+  * ``koji list-permissions [--user <user>] [--mine]`` is self-descriptive.
+
+Default permissions
+===================
+
+Administration
+--------------
+
+``admin``
+  Basic permission, which can be delegated to other users. This
+  is superadmin without any limitations, so grant with caution. Especially
+  services should use some limited form instead of this.
+
+``host``
+  Restricted permission for handling host-related management tasks.
+
+Tasks
+-----
+
+``appliance``
+  appliance tasks (``koji spin-appliance``)
+
+``build``
+  currently unused
+
+``dist-repo``
+  distRepo tasks (``koji dist-repo``)
+
+``image``
+  image tasks (``koji image-build``)
+
+``livecd``
+  livecd tasks (``koji spin-livecd``)
+
+``repo``
+  newRepo tasks (``koji regen-repo``)
+
+``regen-repo``
+  same as ``repo`` for now
+
+Data
+----
+``image-import``
+  used for importing external maven artifacts
+  (``koji import-archive --type maven``)
+
+``maven-import``
+  used for importing external maven artifacts
+  (``koji import-archive --type maven``)
+
+``win-admin``
+  used in default policy for windows builds ('vm' channel)
+
+``win-import``
+  used for importing external maven artifacts
+  (``koji import-archive --type win``)
diff --git a/hub/kojihub.py b/hub/kojihub.py
index 6a03c8a..3462dfd 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -2108,7 +2108,7 @@ def readTagGroups(tag, event=None, inherit=True, incl_pkgs=True, incl_reqs=True,
         return [x for x in groups if not x['blocked']]
 
 def set_host_enabled(hostname, enabled=True):
-    context.session.assertPerm('admin')
+    context.session.assertPerm('host')
     host = get_host(hostname)
     if not host:
         raise koji.GenericError('host does not exist: %s' % hostname)
@@ -2128,7 +2128,7 @@ def add_host_to_channel(hostname, channel_name, create=False):
 
     Channel must already exist unless create option is specified
     """
-    context.session.assertPerm('admin')
+    context.session.assertPerm('host')
     host = get_host(hostname)
     if host == None:
         raise koji.GenericError('host does not exist: %s' % hostname)
@@ -2146,7 +2146,7 @@ def add_host_to_channel(hostname, channel_name, create=False):
     insert.execute()
 
 def remove_host_from_channel(hostname, channel_name):
-    context.session.assertPerm('admin')
+    context.session.assertPerm('host')
     host = get_host(hostname)
     if host == None:
         raise koji.GenericError('host does not exist: %s' % hostname)
@@ -4782,7 +4782,7 @@ def edit_host(hostInfo, **kw):
 
     Returns True if changes are made to the database, False otherwise.
     """
-    context.session.assertPerm('admin')
+    context.session.assertPerm('host')
 
     host = get_host(hostInfo, strict=True)
 
@@ -8848,7 +8848,7 @@ class RootExports(object):
     '''Contains functions that are made available via XMLRPC'''
 
     def restartHosts(self, priority=5, options=None):
-        context.session.assertPerm('admin')
+        context.session.assertPerm('host')
         if options is None:
             args = []
         else:
@@ -10987,7 +10987,7 @@ class RootExports(object):
         If krb_principal is not given then that field will be generated
         from the HostPrincipalFormat setting (if available).
         """
-        context.session.assertPerm('admin')
+        context.session.assertPerm('host')
         # validate arches
         arches = " ".join(arches)
         arches = koji.parse_arches(arches, strict=True)
diff --git a/tests/test_hub/test_add_host.py b/tests/test_hub/test_add_host.py
index 3746b2e..fa1978a 100644
--- a/tests/test_hub/test_add_host.py
+++ b/tests/test_hub/test_add_host.py
@@ -65,7 +65,7 @@ class TestAddHost(unittest.TestCase):
         r = self.exports.addHost('hostname', ['i386', 'x86_64'])
         self.assertEqual(r, 12)
 
-        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.context.session.assertPerm.assert_called_once_with('host')
         kojihub.get_host.assert_called_once_with('hostname')
         self.context.session.createUser.assert_called_once_with('hostname',
                 usertype=koji.USERTYPES['HOST'], krb_principal='-hostname-')

From d18cb4823f7bc83a0b5ba24afa165444b8395c3f Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Jun 04 2019 10:43:25 +0000
Subject: [PATCH 2/3] add "tag" permission


---

diff --git a/docs/source/permissions.rst b/docs/source/permissions.rst
index b44dd46..2d90f04 100644
--- a/docs/source/permissions.rst
+++ b/docs/source/permissions.rst
@@ -34,6 +34,9 @@ Administration
 ``host``
   Restricted permission for handling host-related management tasks.
 
+``tag``
+  Permission for adding/deleting/editing tags
+
 Tasks
 -----
 
diff --git a/hub/kojihub.py b/hub/kojihub.py
index 3462dfd..a4bbc18 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -688,7 +688,7 @@ def readDescendantsData(tag_id, event=None):
 
 def writeInheritanceData(tag_id, changes, clear=False):
     """Add or change inheritance data for a tag"""
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _writeInheritanceData(tag_id, changes, clear)
 
 
@@ -1638,7 +1638,7 @@ def _direct_untag_build(tag, build, user, strict=True, force=False):
 def grplist_add(taginfo, grpinfo, block=False, force=False, **opts):
     """Add to (or update) group list for tag"""
     #only admins....
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grplist_add(taginfo, grpinfo, block, force, **opts)
 
 
@@ -1696,7 +1696,7 @@ def grplist_remove(taginfo, grpinfo, force=False):
     Most of the time you really want to use the block or unblock functions
     """
     #only admins....
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grplist_remove(taginfo, grpinfo, force)
 
 
@@ -1724,7 +1724,7 @@ def grplist_unblock(taginfo, grpinfo):
     Otherwise, raise an error
     """
     # only admins...
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grplist_unblock(taginfo, grpinfo)
 
 
@@ -1758,7 +1758,7 @@ def _grplist_unblock(taginfo, grpinfo):
 def grp_pkg_add(taginfo, grpinfo, pkg_name, block=False, force=False, **opts):
     """Add package to group for tag"""
     #only admins....
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grp_pkg_add(taginfo, grpinfo, pkg_name, block, force, **opts)
 
 
@@ -1822,7 +1822,7 @@ def grp_pkg_remove(taginfo, grpinfo, pkg_name, force=False):
     Most of the time you really want to use the block or unblock functions
     """
     #only admins....
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grp_pkg_remove(taginfo, grpinfo, pkg_name, force)
 
 
@@ -1848,7 +1848,7 @@ def grp_pkg_unblock(taginfo, grpinfo, pkg_name):
     Otherwise, raise an error
     """
     # only admins...
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grp_pkg_unblock(taginfo, grpinfo, pkg_name)
 
 
@@ -1881,7 +1881,7 @@ def _grp_pkg_unblock(taginfo, grpinfo, pkg_name):
 def grp_req_add(taginfo, grpinfo, reqinfo, block=False, force=False, **opts):
     """Add group requirement to group for tag"""
     #only admins....
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grp_req_add(taginfo, grpinfo, reqinfo, block, force, **opts)
 
 
@@ -1946,7 +1946,7 @@ def grp_req_remove(taginfo, grpinfo, reqinfo, force=False):
     Most of the time you really want to use the block or unblock functions
     """
     #only admins....
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grp_req_remove(taginfo, grpinfo, reqinfo, force)
 
 
@@ -1973,7 +1973,7 @@ def grp_req_unblock(taginfo, grpinfo, reqinfo):
     Otherwise, raise an error
     """
     # only admins...
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _grp_req_unblock(taginfo, grpinfo, reqinfo)
 
 
@@ -3051,7 +3051,7 @@ def lookup_build_target(info, strict=False, create=False):
 
 def create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_support=False, maven_include_all=False, extra=None):
     """Create a new tag"""
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     return _create_tag(name, parent, arches, perm, locked, maven_support, maven_include_all, extra)
 
 
@@ -3206,7 +3206,7 @@ def edit_tag(tagInfo, **kwargs):
     :param list remove_extra: remove extra tag parameters.
     """
 
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _edit_tag(tagInfo, **kwargs)
 
 
@@ -3316,7 +3316,7 @@ def old_edit_tag(tagInfo, name, arches, locked, permissionID, extra=None):
 
 def delete_tag(tagInfo):
     """Delete the specified tag."""
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
     _delete_tag(tagInfo)
 
 
@@ -3469,7 +3469,7 @@ def delete_external_repo(info):
 def add_external_repo_to_tag(tag_info, repo_info, priority, merge_mode='koji'):
     """Add an external repo to a tag"""
 
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
 
     if merge_mode not in koji.REPO_MERGE_MODES:
         raise koji.GenericError('Invalid merge mode: %s' % merge_mode)
@@ -3496,7 +3496,7 @@ def add_external_repo_to_tag(tag_info, repo_info, priority, merge_mode='koji'):
 def remove_external_repo_from_tag(tag_info, repo_info):
     """Remove an external repo from a tag"""
 
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
 
     tag = get_tag(tag_info, strict=True)
     tag_id = tag['id']
@@ -3516,7 +3516,7 @@ def edit_tag_external_repo(tag_info, repo_info, priority):
     """Edit a tag<->external repo association
     This allows you to update the priority without removing/adding the repo."""
 
-    context.session.assertPerm('admin')
+    context.session.assertPerm('tag')
 
     tag = get_tag(tag_info, strict=True)
     tag_id = tag['id']
@@ -10185,7 +10185,7 @@ class RootExports(object):
         """
         # verify existence of tag and/or convert name to id
         tag = get_tag_id(tag, strict=True)
-        context.session.assertPerm('admin')
+        context.session.assertPerm('tag')
         return writeInheritanceData(tag, data, clear=clear)
 
     def getFullInheritance(self, tag, event=None, reverse=False, stops=None, jumps=None):
diff --git a/tests/test_hub/test_delete_tag.py b/tests/test_hub/test_delete_tag.py
index ef182d8..7fcf6b4 100644
--- a/tests/test_hub/test_delete_tag.py
+++ b/tests/test_hub/test_delete_tag.py
@@ -37,7 +37,7 @@ class TestDeleteTag(unittest.TestCase):
         with self.assertRaises(koji.GenericError):
             kojihub.delete_tag('badtag')
         self.assertEqual(self.updates, [])
-        self.context.session.assertPerm.assert_called_with('admin')
+        self.context.session.assertPerm.assert_called_with('tag')
 
     def test_good_tag(self):
         self.get_tag.return_value = {'id': 'TAGID'}
@@ -50,4 +50,4 @@ class TestDeleteTag(unittest.TestCase):
             self.assertEqual(u.values, {'value': 'TAGID'})
             self.assertEqual(u.rawdata, {'active': 'NULL'})
             self.assertEqual(u.data, data)
-        self.context.session.assertPerm.assert_called_with('admin')
+        self.context.session.assertPerm.assert_called_with('tag')
diff --git a/tests/test_hub/test_group_operations.py b/tests/test_hub/test_group_operations.py
index aee0095..e53101b 100644
--- a/tests/test_hub/test_group_operations.py
+++ b/tests/test_hub/test_group_operations.py
@@ -66,7 +66,7 @@ class TestGrouplist(unittest.TestCase):
         kojihub.grplist_add(tag, group)
 
         # what was called
-        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.context.session.assertPerm.assert_called_once_with('tag')
         self.get_tag.assert_called_once_with(tag, strict=True)
         self.lookup_group.assert_called_once_with(group, create=True)
         self.get_tag_groups.assert_called_with('tag_id', inherit=True,
@@ -100,7 +100,7 @@ class TestGrouplist(unittest.TestCase):
         self.context.session.assertPerm.side_effect = koji.GenericError
         with self.assertRaises(koji.GenericError):
             kojihub.grplist_add('tag', 'group')
-        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.context.session.assertPerm.assert_called_once_with('tag')
         self.assertEqual(len(self.inserts), 0)
         self.assertEqual(len(self.updates), 0)
 
@@ -108,7 +108,7 @@ class TestGrouplist(unittest.TestCase):
         self.get_tag.side_effect = koji.GenericError
         with self.assertRaises(koji.GenericError):
             kojihub.grplist_add('tag', 'group')
-        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.context.session.assertPerm.assert_called_once_with('tag')
         self.assertEqual(len(self.inserts), 0)
         self.assertEqual(len(self.updates), 0)
 
@@ -125,7 +125,7 @@ class TestGrouplist(unittest.TestCase):
         kojihub.grplist_block(tag, group)
 
         # what was called
-        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.context.session.assertPerm.assert_called_once_with('tag')
         self.get_tag.assert_called_once_with(tag, strict=True)
         self.lookup_group.assert_called_once_with(group, create=True)
         self.get_tag_groups.assert_called_with('tag_id', inherit=True,
@@ -166,7 +166,7 @@ class TestGrouplist(unittest.TestCase):
         kojihub.grplist_remove(tag, group)
 
         # what was called
-        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.context.session.assertPerm.assert_called_once_with('tag')
         self.get_tag.assert_called_once_with(tag, strict=True)
         self.lookup_group.assert_called_once_with(group, strict=True)
 
@@ -192,7 +192,7 @@ class TestGrouplist(unittest.TestCase):
             kojihub.grplist_unblock(tag, group)
 
         # what was called
-        self.context.session.assertPerm.assert_called_once_with('admin')
+        self.context.session.assertPerm.assert_called_once_with('tag')
         self.lookup_tag.assert_called_once_with(tag, strict=True)
         self.lookup_group.assert_called_once_with(group, strict=True)
 

From 38bae0a0b72570f9492ee65020ba7c33c3a06bd1 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Jun 04 2019 10:51:50 +0000
Subject: [PATCH 3/3] add 'target' permission


---

diff --git a/docs/source/permissions.rst b/docs/source/permissions.rst
index 2d90f04..e259e9e 100644
--- a/docs/source/permissions.rst
+++ b/docs/source/permissions.rst
@@ -37,6 +37,9 @@ Administration
 ``tag``
   Permission for adding/deleting/editing tags
 
+``target``
+  Permission for adding/deleting/editing targets
+
 Tasks
 -----
 
@@ -61,8 +64,8 @@ Tasks
 ``regen-repo``
   same as ``repo`` for now
 
-Data
-----
+Data Import
+-----------
 ``image-import``
   used for importing external maven artifacts
   (``koji import-archive --type maven``)
diff --git a/hub/kojihub.py b/hub/kojihub.py
index a4bbc18..e2006e3 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -2797,7 +2797,7 @@ def _validate_build_target_name(name):
 def create_build_target(name, build_tag, dest_tag):
     """Create a new build target"""
 
-    context.session.assertPerm('admin')
+    context.session.assertPerm('target')
     return _create_build_target(name, build_tag, dest_tag)
 
 
@@ -2833,7 +2833,7 @@ def _create_build_target(name, build_tag, dest_tag):
 
 def edit_build_target(buildTargetInfo, name, build_tag, dest_tag):
     """Set the build_tag and dest_tag of an existing build_target to new values"""
-    context.session.assertPerm('admin')
+    context.session.assertPerm('target')
     _edit_build_target(buildTargetInfo, name, build_tag, dest_tag)
 
 
@@ -2885,7 +2885,7 @@ def _edit_build_target(buildTargetInfo, name, build_tag, dest_tag):
 def delete_build_target(buildTargetInfo):
     """Delete the build target with the given name.  If no build target
     exists, raise a GenericError."""
-    context.session.assertPerm('admin')
+    context.session.assertPerm('target')
     _delete_build_target(buildTargetInfo)
 
 

