Add description for permissions

Fixes: https://pagure.io/koji/issue/335
This commit is contained in:
Jana Cupova 2021-11-23 07:29:08 +01:00 committed by Tomas Kopecek
parent e292fd8f2b
commit 1252129136
11 changed files with 371 additions and 68 deletions

View file

@ -2130,21 +2130,43 @@ def handle_list_permissions(goptions, session, args):
parser = OptionParser(usage=get_usage_str(usage))
parser.add_option("--user", help="List permissions for the given user")
parser.add_option("--mine", action="store_true", help="List your permissions")
parser.add_option("--quiet", action="store_true", default=goptions.quiet,
help="Do not print the header information")
(options, args) = parser.parse_args(args)
if len(args) > 0:
parser.error("This command takes no arguments")
activate_session(session, goptions)
perms = []
if options.user:
user = session.getUser(options.user)
if not user:
error("No such user: %s" % options.user)
perms = session.getUserPerms(user['id'])
for p in session.getUserPerms(user['id']):
perms.append({'name': p})
elif options.mine:
perms = session.getPerms()
for p in session.getPerms():
perms.append({'name': p})
else:
perms = [p['name'] for p in session.getAllPerms()]
for p in session.getAllPerms():
perms.append({'name': p['name'], 'description': p['description']})
if perms:
longest_perm = max([len(perm['name']) for perm in perms])
else:
longest_perm = 8
if longest_perm < len('Permission name '):
longest_perm = len('Permission name ')
if not options.quiet:
hdr = '{permname:<{longest_perm}}'
hdr = hdr.format(longest_perm=longest_perm, permname='Permission name')
if perms and perms[0].get('description'):
hdr += " Description".ljust(53)
print(hdr)
for perm in perms:
print(perm)
line = '{permname:<{longest_perm}}'
line = line.format(longest_perm=longest_perm, permname=perm['name'])
if perm.get('description'):
line += " %s" % perm['description']
print(line)
def handle_add_user(goptions, session, args):
@ -2417,10 +2439,12 @@ def handle_import_archive(options, session, args):
def handle_grant_permission(goptions, session, args):
"[admin] Grant a permission to a user"
usage = "usage: %prog grant-permission [--new] <permission> <user> [<user> ...]"
usage = "usage: %prog grant-permission [options] <permission> <user> [<user> ...]"
parser = OptionParser(usage=get_usage_str(usage))
parser.add_option("--new", action="store_true",
help="Create this permission if the permission does not exist")
parser.add_option("--description",
help="Add description about new permission")
(options, args) = parser.parse_args(args)
if len(args) < 2:
parser.error("Please specify a permission and at least one user")
@ -2436,6 +2460,10 @@ def handle_grant_permission(goptions, session, args):
kwargs = {}
if options.new:
kwargs['create'] = True
if options.description:
kwargs['description'] = options.description
if options.description and not options.new:
parser.error("Option new must be specified with option description.")
for user in users:
session.grantPermission(user['name'], perm, **kwargs)
@ -2460,6 +2488,19 @@ def handle_revoke_permission(goptions, session, args):
session.revokePermission(user['name'], perm)
def handle_edit_permission(goptions, session, args):
"[admin] Edit a permission description"
usage = "usage: %prog edit-permission <permission> <description>"
parser = OptionParser(usage=get_usage_str(usage))
(options, args) = parser.parse_args(args)
if len(args) < 2:
parser.error("Please specify a permission and a description")
activate_session(session, goptions)
perm = args[0]
description = args[1]
session.editPermission(perm, description)
def handle_grant_cg_access(goptions, session, args):
"[admin] Add a user to a content generator"
usage = "usage: %prog grant-cg-access <user> <content generator>"

View file

@ -0,0 +1,24 @@
-- upgrade script to migrate the Koji database schema
-- from version 1.27 to 1.28
BEGIN;
ALTER TABLE permissions ADD COLUMN description TEXT;
UPDATE permissions set description='Full administrator access. Perform all actions.' WHERE name = 'admin';
UPDATE permissions set description='Create appliance builds - deprecated.' WHERE name = 'appliance';
UPDATE permissions set description='Create a dist-repo.' WHERE name = 'dist-repo';
UPDATE permissions set description='Add, remove, enable, disable hosts and channels.' WHERE name = 'host';
UPDATE permissions set description='Start image tasks.' WHERE name = 'image';
UPDATE permissions set description='Import image archives.' WHERE name = 'image-import';
UPDATE permissions set description='Start livecd tasks.' WHERE name = 'livecd';
UPDATE permissions set description='Import maven archives.' WHERE name = 'maven-import';
UPDATE permissions set description='Manage repos: newRepo, repoExpire, repoDelete, repoProblem.' WHERE name = 'repo';
UPDATE permissions set description='Import RPM signatures and write signed RPMs.' WHERE name = 'sign';
UPDATE permissions set description='Manage packages in tags: add, block, remove, and clone tags.' WHERE name = 'tag';
UPDATE permissions set description='Add, edit, and remove targets.' WHERE name = 'target';
UPDATE permissions set description='The default hub policy rule for "vm" requires this permission to trigger Windows builds.' WHERE name = 'win-admin';
UPDATE permissions set description='Import win archives.' WHERE name = 'win-import';
COMMIT;

View file

@ -47,25 +47,25 @@ CREATE TABLE user_krb_principals (
CREATE TABLE permissions (
id SERIAL NOT NULL PRIMARY KEY,
name VARCHAR(50) UNIQUE NOT NULL
name VARCHAR(50) UNIQUE NOT NULL,
description TEXT
) WITHOUT OIDS;
-- Some basic perms
INSERT INTO permissions (name) VALUES ('admin');
INSERT INTO permissions (name) VALUES ('appliance');
INSERT INTO permissions (name) VALUES ('build');
INSERT INTO permissions (name) VALUES ('dist-repo');
INSERT INTO permissions (name) VALUES ('host');
INSERT INTO permissions (name) VALUES ('image');
INSERT INTO permissions (name) VALUES ('image-import');
INSERT INTO permissions (name) VALUES ('livecd');
INSERT INTO permissions (name) VALUES ('maven-import');
INSERT INTO permissions (name) VALUES ('repo');
INSERT INTO permissions (name) VALUES ('sign');
INSERT INTO permissions (name) VALUES ('tag');
INSERT INTO permissions (name) VALUES ('target');
INSERT INTO permissions (name) VALUES ('win-admin');
INSERT INTO permissions (name) VALUES ('win-import');
INSERT INTO permissions (name, description) VALUES ('admin', 'Full administrator access. Perform all actions.');
INSERT INTO permissions (name, description) VALUES ('appliance', 'Create appliance builds - deprecated.');
INSERT INTO permissions (name, description) VALUES ('dist-repo', 'Create a dist-repo.');
INSERT INTO permissions (name, description) VALUES ('host', 'Add, remove, enable, disable hosts and channels.');
INSERT INTO permissions (name, description) VALUES ('image', 'Start image tasks.');
INSERT INTO permissions (name, description) VALUES ('image-import', 'Import image archives.');
INSERT INTO permissions (name, description) VALUES ('livecd', 'Start livecd tasks.');
INSERT INTO permissions (name, description) VALUES ('maven-import', 'Import maven archives.');
INSERT INTO permissions (name, description) VALUES ('repo', 'Manage repos: newRepo, repoExpire, repoDelete, repoProblem.');
INSERT INTO permissions (name, description) VALUES ('sign', 'Import RPM signatures and write signed RPMs.');
INSERT INTO permissions (name, description) VALUES ('tag', 'Manage packages in tags: add, block, remove, and clone tags.');
INSERT INTO permissions (name, description) VALUES ('target', 'Add, edit, and remove targets.');
INSERT INTO permissions (name, description) VALUES ('win-admin', 'The default hub policy rule for "vm" requires this permission to trigger Windows builds.');
INSERT INTO permissions (name, description) VALUES ('win-import', 'Import win archives.');
CREATE TABLE user_perms (
user_id INTEGER NOT NULL REFERENCES users(id),

View file

@ -12417,14 +12417,21 @@ class RootExports(object):
getUser = staticmethod(get_user)
editUser = staticmethod(edit_user)
def grantPermission(self, userinfo, permission, create=False):
def grantPermission(self, userinfo, permission, create=False, description=None):
"""Grant a permission to a user"""
context.session.assertPerm('admin')
if create:
verify_name_internal(permission)
if description is not None and not create:
raise koji.GenericError('Description should be specified only with create.')
user_id = get_user(userinfo, strict=True)['id']
perm = lookup_perm(permission, strict=(not create), create=create)
perm_id = perm['id']
if create and description is not None:
update = UpdateProcessor('permissions', clauses=['id=%(perm_id)i'],
values={'perm_id': perm_id})
update.set(description=description)
update.execute()
if perm['name'] in koji.auth.get_user_perms(user_id):
raise koji.GenericError('user %s already has permission: %s' %
(userinfo, perm['name']))
@ -12447,6 +12454,16 @@ class RootExports(object):
update.make_revoke()
update.execute()
def editPermission(self, permission, description):
"""Edit a permission description"""
context.session.assertPerm('admin')
perm = lookup_perm(permission, strict=True)
perm_id = perm['id']
update = UpdateProcessor('permissions', clauses=['id=%(perm_id)i'],
values={'perm_id': perm_id})
update.set(description=description)
update.execute()
def createUser(self, username, status=None, krb_principal=None):
"""Add a user to the database
@ -13189,11 +13206,12 @@ class RootExports(object):
- id
- name
- description
"""
query = """SELECT id, name FROM permissions
query = """SELECT id, name, description FROM permissions
ORDER BY id"""
return _multiRow(query, {}, ['id', 'name'])
return _multiRow(query, {}, ['id', 'name', 'description'])
def getLoggedInUser(self):
"""Return information about the currently logged-in user. Returns data

View file

@ -26,6 +26,7 @@ admin commands:
edit-channel Edit a channel
edit-external-repo Edit data for an external repo
edit-host Edit a host
edit-permission Edit a permission description
edit-tag Alter tag information
edit-tag-inheritance Edit tag inheritance
edit-target Set the name, build_tag, and/or dest_tag of an existing build target to new values

View file

@ -26,6 +26,7 @@ admin commands:
edit-channel Edit a channel
edit-external-repo Edit data for an external repo
edit-host Edit a host
edit-permission Edit a permission description
edit-tag Alter tag information
edit-tag-inheritance Edit tag inheritance
edit-target Set the name, build_tag, and/or dest_tag of an existing build target to new values

View file

@ -0,0 +1,45 @@
from __future__ import absolute_import
import mock
import koji
from koji_cli.commands import handle_edit_permission
from . import utils
class TestEditPermission(utils.CliTestCase):
def setUp(self):
# Show long diffs in error output...
self.maxDiff = None
self.options = mock.MagicMock()
self.options.quiet = True
self.options.debug = False
self.session = mock.MagicMock()
self.session.getAPIVersion.return_value = koji.API_VERSION
self.activate_session_mock = mock.patch('koji_cli.commands.activate_session').start()
self.error_format = """Usage: %s edit-permission <permission> <description>
(Specify the --help global option for a list of other help options)
%s: error: {message}
""" % (self.progname, self.progname)
self.perm = 'test-perm'
self.description = 'test-description'
def test_handle_edit_permission_argument_error(self):
expected = self.format_error_message(
"Please specify a permission and a description")
for args in [[], [self.perm]]:
self.assert_system_exit(
handle_edit_permission,
self.options,
self.session,
args,
stderr=expected,
activate_session=None)
self.activate_session_mock.assert_not_called()
self.session.grantPermission.assert_not_called()
def test_handle_edit_permission_with_new_and_description(self):
handle_edit_permission(self.options, self.session, [self.perm, self.description])
self.session.editPermission.assert_called_once_with(self.perm, self.description)
self.activate_session_mock.assert_called_once_with(self.session, self.options)

View file

@ -1,7 +1,7 @@
from __future__ import absolute_import
import mock
import six
import unittest
import koji
from koji_cli.commands import handle_grant_permission
from . import utils
@ -9,72 +9,94 @@ from . import utils
class TestGrantPermission(utils.CliTestCase):
# Show long diffs in error output...
maxDiff = None
def setUp(self):
self.error_format = """Usage: %s grant-permission [--new] <permission> <user> [<user> ...]
# Show long diffs in error output...
self.maxDiff = None
self.options = mock.MagicMock()
self.options.quiet = True
self.options.debug = False
self.session = mock.MagicMock()
self.session.getAPIVersion.return_value = koji.API_VERSION
self.activate_session_mock = mock.patch('koji_cli.commands.activate_session').start()
self.error_format = """Usage: %s grant-permission [options] <permission> <user> [<user> ...]
(Specify the --help global option for a list of other help options)
%s: error: {message}
""" % (self.progname, self.progname)
self.perm = 'createuser'
self.user = 'user'
@mock.patch('sys.stdout', new_callable=six.StringIO)
@mock.patch('koji_cli.commands.activate_session')
def test_handle_grant_permission(
self,
activate_session_mock,
stdout):
"""Test handle_grant_permission function"""
session = mock.MagicMock()
options = mock.MagicMock()
perm = 'createrepo'
users = 'user'
# Case 1. argument error
def test_handle_grant_permission_argument_error(self):
expected = self.format_error_message(
"Please specify a permission and at least one user")
for args in [[], [perm]]:
for args in [[], [self.perm]]:
self.assert_system_exit(
handle_grant_permission,
options,
session,
self.options,
self.session,
args,
stderr=expected,
activate_session=None)
self.activate_session_mock.assert_not_called()
self.session.grantPermission.assert_not_called()
# Case 2. user not exists
expected = self.format_error_message(
"No such user: %s" % users)
session.getUser.return_value = None
def test_handle_grant_permission_non_exist_user(self):
expected = self.format_error_message("No such user: %s" % self.user)
self.session.getUser.return_value = None
self.assert_system_exit(
handle_grant_permission,
options,
session,
[perm, users],
self.options,
self.session,
[self.perm, self.user],
stderr=expected)
self.session.grantPermission.assert_not_called()
# Case 3. grant permission with --new
def test_handle_grant_permission_with_new(self):
users = ['user1', 'user2', 'user3']
perm = 'build_iso'
session.getUser.side_effect = [
self.session.getUser.side_effect = [
{'id': 101, 'name': users[0]},
{'id': 111, 'name': users[1]},
{'id': 121, 'name': users[2]},
]
handle_grant_permission(options, session, [perm, '--new'] + users)
handle_grant_permission(self.options, self.session, [perm, '--new'] + users)
calls = [mock.call(p, perm, create=True) for p in users]
session.grantPermission.assert_has_calls(calls)
self.session.grantPermission.assert_has_calls(calls)
self.activate_session_mock.assert_called_once_with(self.session, self.options)
def test_handle_grant_permission_description_without_new(self):
expected = self.format_error_message(
"Option new must be specified with option description.")
self.session.getUser.return_value = {'id': 101, 'name': self.user}
self.assert_system_exit(
handle_grant_permission,
self.options,
self.session,
[self.perm, self.user, '--description', 'test-description'],
stderr=expected)
self.session.grantPermission.assert_not_called()
def test_handle_grant_permission_with_new_and_description(self):
description = 'test-description'
self.session.getUser.return_value = {'id': 101, 'name': self.user}
handle_grant_permission(self.options, self.session,
['--new', '--description', description, self.perm, self.user])
self.session.grantPermission.assert_called_once_with(
self.user, self.perm, create=True, description=description)
self.activate_session_mock.assert_called_once_with(self.session, self.options)
def test_handle_grant_permission_help(self):
self.assert_help(
handle_grant_permission,
"""Usage: %s grant-permission [--new] <permission> <user> [<user> ...]
"""Usage: %s grant-permission [options] <permission> <user> [<user> ...]
(Specify the --help global option for a list of other help options)
Options:
-h, --help show this help message and exit
--new Create this permission if the permission does not exist
-h, --help show this help message and exit
--new Create this permission if the permission does not
exist
--description=DESCRIPTION
Add description about new permission
""" % self.progname)

View file

@ -32,12 +32,12 @@ class TestListPermissions(utils.CliTestCase):
options = mock.MagicMock()
user = 'tester'
all_perms = [
{'id': 0, 'name': 'admin'},
{'id': 1, 'name': 'build'},
{'id': 2, 'name': 'repo'},
{'id': 3, 'name': 'image'},
{'id': 4, 'name': 'livecd'},
{'id': 5, 'name': 'appliance'}
{'id': 0, 'name': 'admin', 'description': 'admin-description'},
{'id': 1, 'name': 'build', 'description': 'build-description'},
{'id': 2, 'name': 'repo', 'description': 'repo-description'},
{'id': 3, 'name': 'image', 'description': 'image-description'},
{'id': 4, 'name': 'livecd', 'description': 'livecd-description'},
{'id': 5, 'name': 'appliance', 'description': 'appliance-description'}
]
# case 1. argument error (no argument is required)
@ -62,7 +62,13 @@ class TestListPermissions(utils.CliTestCase):
perms = [p['name'] for p in all_perms[::1]]
session.getUserPerms.return_value = perms
session.getUser.return_value = {'id': 101, 'name': user}
expected = "\n".join([p for p in perms]) + "\n"
expected = """admin
build
repo
image
livecd
appliance
"""
handle_list_permissions(options, session, ['--user', user])
self.assert_console_message(stdout, expected)
@ -71,7 +77,9 @@ class TestListPermissions(utils.CliTestCase):
# case 4. List my permission
perms = [p['name'] for p in all_perms[1:3]]
session.getPerms.return_value = perms
expected = "\n".join([p for p in perms]) + "\n"
expected = """build
repo
"""
handle_list_permissions(options, session, ['--mine'])
self.assert_console_message(stdout, expected)
session.getUserPerms.assert_not_called()
@ -80,7 +88,13 @@ class TestListPermissions(utils.CliTestCase):
# case 5. List all permission
session.getAllPerms.return_value = all_perms
expected = "\n".join([p['name'] for p in all_perms]) + "\n"
expected = """admin admin-description
build build-description
repo repo-description
image image-description
livecd livecd-description
appliance appliance-description
"""
handle_list_permissions(options, session, [])
self.assert_console_message(stdout, expected)
session.getUserPerms.assert_not_called()
@ -97,6 +111,7 @@ Options:
-h, --help show this help message and exit
--user=USER List permissions for the given user
--mine List your permissions
--quiet Do not print the header information
""" % self.progname)

View file

@ -0,0 +1,44 @@
import unittest
import mock
import koji
import kojihub
UP = kojihub.UpdateProcessor
class TestEditPermission(unittest.TestCase):
def setUp(self):
self.lookup_perm = mock.patch('kojihub.lookup_perm').start()
self.update_processor = mock.patch('kojihub.UpdateProcessor').start()
self.exports = kojihub.RootExports()
self.context = mock.patch('kojihub.context').start()
# It seems MagicMock will not automatically handle attributes that
# start with "assert"
self.context.session.assertPerm = mock.MagicMock()
self.context.session.assertLogin = mock.MagicMock()
self.perm_name = 'test_perms'
self.perm_info = {'id': 1, 'name': self.perm_name}
self.description = 'test-description'
def test_edit_permission_non_exist_permission(self):
self.lookup_perm.side_effect = koji.GenericError
with self.assertRaises(koji.GenericError):
self.exports.editPermission(self.perm_name, self.description)
self.update_processor.assert_not_called()
self.context.session.assertPerm.assert_called_with('admin')
def test_edit_permission(self):
self.lookup_perm.return_value = self.perm_info
update = self.update_processor.return_value
self.exports.editPermission(self.perm_name, self.description)
self.update_processor.assert_called_once()
update.execute.assert_called_once()
args, kwargs = self.update_processor.call_args
up = UP(*args, **kwargs)
self.assertEqual(up.table, 'permissions')
self.assertEqual(up.rawdata, {})
self.context.session.assertPerm.assert_called_with('admin')

View file

@ -6,10 +6,19 @@ import koji
import kojihub
IP = kojihub.InsertProcessor
UP = kojihub.UpdateProcessor
class TestGrantPermission(unittest.TestCase):
def setUp(self):
self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start()
self.get_user = mock.patch('kojihub.get_user').start()
self.lookup_perm = mock.patch('kojihub.lookup_perm').start()
self.insert_processor = mock.patch('kojihub.InsertProcessor').start()
self.update_processor = mock.patch('kojihub.UpdateProcessor').start()
self.get_user_perms = mock.patch('koji.auth.get_user_perms').start()
self.exports = kojihub.RootExports()
self.context = mock.patch('kojihub.context').start()
# It seems MagicMock will not automatically handle attributes that
@ -17,6 +26,10 @@ class TestGrantPermission(unittest.TestCase):
self.context.session.assertPerm = mock.MagicMock()
self.context.session.assertLogin = mock.MagicMock()
self.user_name = 'test_user'
self.perms_name = 'test_perms'
self.userinfo = {'id': 1, 'krb_principals': [], 'name': self.user_name,
'status': 0, 'usertype': 0}
self.perm_info = {'id': 1, 'name': self.perms_name}
def test_grant_permission_wrong_format(self):
perms_name = 'test-perms+'
@ -25,8 +38,87 @@ class TestGrantPermission(unittest.TestCase):
self.verify_name_internal.side_effect = koji.GenericError
with self.assertRaises(koji.GenericError):
self.exports.grantPermission(self.user_name, perms_name, create=True)
self.insert_processor.assert_not_called()
# not except regex rules
self.verify_name_internal.side_effect = koji.GenericError
with self.assertRaises(koji.GenericError):
self.exports.grantPermission(self.user_name, perms_name, create=True)
self.insert_processor.assert_not_called()
self.context.session.assertPerm.assert_called_with('admin')
def test_grant_permission_description_without_create(self):
self.verify_name_internal.return_value = None
with self.assertRaises(koji.GenericError) as ex:
self.exports.grantPermission(self.user_name, self.perms_name,
description='test-description')
self.assertEqual("Description should be specified only with create.", str(ex.exception))
self.insert_processor.assert_not_called()
self.context.session.assertPerm.assert_called_with('admin')
def test_grant_permission_non_exist_user(self):
self.verify_name_internal.return_value = None
self.get_user.side_effect = koji.GenericError
with self.assertRaises(koji.GenericError):
self.exports.grantPermission(self.user_name, self.perms_name)
self.insert_processor.assert_not_called()
self.context.session.assertPerm.assert_called_with('admin')
def test_grant_permission_non_exist_permission_without_new(self):
self.verify_name_internal.return_value = None
self.get_user.return_value = self.userinfo
self.lookup_perm.side_effect = koji.GenericError
with self.assertRaises(koji.GenericError):
self.exports.grantPermission(self.user_name, self.perms_name)
self.insert_processor.assert_not_called()
self.context.session.assertPerm.assert_called_with('admin')
def test_grant_permission(self):
self.verify_name_internal.return_value = None
self.get_user.return_value = self.userinfo
self.lookup_perm.return_value = self.perm_info
self.get_user_perms.return_value = []
insert = self.insert_processor.return_value
self.exports.grantPermission(self.user_name, self.perms_name, create=True)
self.insert_processor.assert_called_once()
insert.execute.assert_called_once()
args, kwargs = self.insert_processor.call_args
ip = IP(*args, **kwargs)
self.assertEqual(ip.table, 'user_perms')
self.assertEqual(ip.rawdata, {})
self.context.session.assertPerm.assert_called_with('admin')
def test_grant_permission_exist_perms(self):
self.verify_name_internal.return_value = None
self.get_user.return_value = self.userinfo
self.lookup_perm.return_value = self.perm_info
self.get_user_perms.return_value = [self.perms_name]
with self.assertRaises(koji.GenericError) as ex:
self.exports.grantPermission(self.user_name, self.perms_name)
self.assertEqual(f'user {self.user_name} already has permission: {self.perm_info["name"]}',
str(ex.exception))
self.insert_processor.assert_not_called()
self.context.session.assertPerm.assert_called_with('admin')
def test_grant_permission_with_description(self):
self.verify_name_internal.return_value = None
self.get_user.return_value = self.userinfo
self.lookup_perm.return_value = self.perm_info
self.get_user_perms.return_value = []
insert = self.insert_processor.return_value
update = self.update_processor.return_value
self.exports.grantPermission(self.user_name, self.perms_name, create=True,
description='test-description')
self.update_processor.assert_called_once()
update.execute.assert_called_once()
args, kwargs = self.update_processor.call_args
up = UP(*args, **kwargs)
self.assertEqual(up.table, 'permissions')
self.assertEqual(up.rawdata, {})
self.insert_processor.assert_called_once()
insert.execute.assert_called_once()
args, kwargs = self.insert_processor.call_args
ip = IP(*args, **kwargs)
self.assertEqual(ip.table, 'user_perms')
self.assertEqual(ip.rawdata, {})
self.context.session.assertPerm.assert_called_with('admin')