From c160e483ceb028ab792a13133bfc9680da5a9ab4 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Dec 09 2021 13:07:41 +0000 Subject: PR#3028: Add limits on name values Merges #3028 https://pagure.io/koji/pull-request/3028 Fixes: #2117 https://pagure.io/koji/issue/2117 limits on name values --- diff --git a/docs/source/hub_conf.rst b/docs/source/hub_conf.rst new file mode 100644 index 0000000..66734cb --- /dev/null +++ b/docs/source/hub_conf.rst @@ -0,0 +1,51 @@ +hub.conf +-------- +hub.conf is a standard .ini-like configuration file. Its main section is +called ``[hub]`` and contains the following options. They can occur anywhere. + +Incomplete document +^^^^^^^^^^^^^^^^^^^ + +This document is a stub and does not cover all options. +Work to complete this document is tracked in `Issue 3073 `_ + +The old :doc:`Server HOW TO ` doc also describes some hub configuration options. + +Name verification +^^^^^^^^^^^^^^^^^ +Currently we have two groups for name verification: + - internal names + - user names + +Group internal names is currently used for: + - archive type + - btype + - channel + - external repo + - group + - host + - kerberos + - permission + - tag + - target + - volume + +Group user names is currently used for: + - user + - host + +Host names are listed in both groups because hosts always have an associated user entry. + +.. glossary:: + MaxNameLengthInternal = 256 + Set length of internal names. By default there is allowed length set up to 256. + When length is set up to 0, length verifying is disabled. + + RegexNameInternal = ^[A-Za-z0-9/_.+-]+$ + Set regex for verify an internal names. When regex string is empty, verifying + is disabled. + + RegexUserName = ^[A-Za-z0-9/_.@-]+$ + Set regex for verify a user name and kerberos. User name and kerberos have + in default set up allowed '@' and '/' chars on top of basic name regex + for internal names. When regex string is empty, verifying is disabled. diff --git a/docs/source/index.rst b/docs/source/index.rst index e9a4704..cc6f717 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -41,6 +41,7 @@ Contents signing database_howto kojid_conf + hub_conf using_the_koji_build_system setting_rpm_macros profiles diff --git a/docs/source/server_howto.rst b/docs/source/server_howto.rst index b386688..09ef309 100644 --- a/docs/source/server_howto.rst +++ b/docs/source/server_howto.rst @@ -777,6 +777,8 @@ override all these values. So, you can use e.g. ``/etc/koji-hub/hub.conf.d/secret.conf`` for sensitive values. Typical usecase for separate config is :doc:`policy ` configuration file. +Doc page about hub options in :doc:`Hub conf `. (Currently in progress). + Authentication Configuration ---------------------------- diff --git a/hub/hub.conf b/hub/hub.conf index d8740b2..783cdd7 100644 --- a/hub/hub.conf +++ b/hub/hub.conf @@ -136,3 +136,8 @@ NotifyOnSuccess = True ## OfflineMessage as the fault string). ## If LockOut is True, the server will report a ServerOffline fault for all non-admin ## requests. + +## Determines rules for names (tag, user, ...) +# MaxNameLengthInternal = 256 +# RegexNameInternal = ^[A-Za-z0-9/_.+-]+$ +# RegexUserName = ^[A-Za-z0-9/_.@-]+$ diff --git a/hub/kojihub.py b/hub/kojihub.py index ec011f8..bb7e74d 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -2290,6 +2290,8 @@ def add_host_to_channel(hostname, channel_name, create=False, force=False): if host is None: raise koji.GenericError('host does not exist: %s' % hostname) host_id = host['id'] + if create: + verify_name_internal(channel_name) channel_id = get_channel_id(channel_name, create=create) if channel_id is None: raise koji.GenericError('channel does not exist: %s' % channel_name) @@ -2344,6 +2346,7 @@ def rename_channel(old, new): context.session.assertPerm('admin') if not isinstance(new, str): raise koji.GenericError("new channel name must be a string") + verify_name_internal(new) cinfo = get_channel(old, strict=True) dup_check = get_channel(new, strict=False) if dup_check: @@ -2376,6 +2379,7 @@ def edit_channel(channelInfo, **kw): if kw.get('name'): if not isinstance(kw['name'], str): raise koji.GenericError("new channel name must be a string") + verify_name_internal(kw['name']) dup_check = get_channel(kw['name'], strict=False) if dup_check: raise koji.GenericError("channel %(name)s already exists (id=%(id)i)" % dup_check) @@ -2431,6 +2435,7 @@ def add_channel(channel_name, description=None): context.session.assertPerm('admin') if not isinstance(channel_name, str): raise koji.GenericError("Channel name must be a string") + verify_name_internal(channel_name) dup_check = get_channel(channel_name, strict=False) if dup_check: raise koji.GenericError("channel %(name)s already exists (id=%(id)i)" % dup_check) @@ -3058,14 +3063,6 @@ def set_tag_update(tag_id, utype, event_id=None, user_id=None): insert.execute() -def _validate_build_target_name(name): - """ A helper function that validates a build target name. """ - max_name_length = 256 - if len(name) > max_name_length: - raise koji.GenericError("Build target name %s is too long. Max length " - "is %s characters" % (name, max_name_length)) - - def create_build_target(name, build_tag, dest_tag): """Create a new build target""" @@ -3075,7 +3072,7 @@ def create_build_target(name, build_tag, dest_tag): def _create_build_target(name, build_tag, dest_tag): """Create a new build target(no access check)""" - _validate_build_target_name(name) + verify_name_internal(name) # Does a target with this name already exist? if get_build_targets(info=name): @@ -3111,7 +3108,7 @@ def edit_build_target(buildTargetInfo, name, build_tag, dest_tag): def _edit_build_target(buildTargetInfo, name, build_tag, dest_tag): """Edit build target parameters, w/ no access checks""" - _validate_build_target_name(name) + verify_name_internal(name) target = lookup_build_target(buildTargetInfo) if not target: @@ -3343,11 +3340,7 @@ def create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_su 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, without access check""" - - max_name_length = 256 - if len(name) > max_name_length: - raise koji.GenericError("Tag name %s is too long. Max length is %s characters", - name, max_name_length) + verify_name_internal(name) arches = koji.parse_arches(arches, strict=True, allow_none=True) @@ -3549,6 +3542,7 @@ def _edit_tag(tagInfo, **kwargs): name = kwargs.get('name') if name and tag['name'] != name: + verify_name_internal(name) # attempt to update tag name # XXX - I'm not sure we should allow this sort of renaming anyway. # while I can see the convenience, it is an untracked change (granted @@ -3714,6 +3708,7 @@ def create_external_repo(name, url): of the new repo.""" context.session.assertPerm('admin') + verify_name_internal(name) if get_external_repos(info=name): raise koji.GenericError('An external repo named "%s" already exists' % name) @@ -3792,6 +3787,7 @@ def edit_external_repo(info, name=None, url=None): repo_id = repo['id'] if name and name != repo['name']: + verify_name_internal(name) existing_id = _singleValue("""SELECT id FROM external_repo WHERE name = %(name)s""", locals(), strict=False) if existing_id is not None: @@ -4127,6 +4123,7 @@ def _edit_user(userInfo, name=None, krb_principal_mappings=None): """Edit information for an existing user.""" user = get_user(userInfo, strict=True) if name and user['name'] != name: + verify_name_user(name=name) # attempt to update user name values = { 'name': name, @@ -4151,6 +4148,7 @@ def _edit_user(userInfo, name=None, krb_principal_mappings=None): if old: removed.add(old) if new: + verify_name_user(krb=new) added.add(new) dups = added & removed if dups: @@ -4820,6 +4818,7 @@ def list_btypes(query=None, queryOpts=None): def add_btype(name): """Add a new btype with the given name""" context.session.assertPerm('admin') + verify_name_internal(name) data = {'name': name} if list_btypes(data): raise koji.GenericError("btype already exists") @@ -5734,6 +5733,7 @@ def new_package(name, strict=True): def add_volume(name, strict=True): """Add a new storage volume in the database""" context.session.assertPerm('admin') + verify_name_internal(name) voldir = koji.pathinfo.volumedir(name) if not os.path.isdir(voldir): raise koji.GenericError('please create the volume directory first') @@ -7317,6 +7317,7 @@ def add_archive_type(name, description, extensions): :param str extensions: space-separated list of descriptions, eg. "yaml yml" """ context.session.assertPerm('admin') + verify_name_internal(name) data = {'name': name, 'description': description, 'extensions': extensions, @@ -8794,6 +8795,7 @@ def get_build_notification_blocks(user_id): def new_group(name): """Add a user group to the database""" context.session.assertPerm('admin') + verify_name_internal(name) if get_user(name): raise koji.GenericError('user/group already exists: %s' % name) return context.session.createUser(name, usertype=koji.USERTYPES['GROUP']) @@ -12405,6 +12407,8 @@ class RootExports(object): def grantPermission(self, userinfo, permission, create=False): """Grant a permission to a user""" context.session.assertPerm('admin') + if create: + verify_name_internal(permission) user_id = get_user(userinfo, strict=True)['id'] perm = lookup_perm(permission, strict=(not create), create=create) perm_id = perm['id'] @@ -12442,6 +12446,7 @@ class RootExports(object): exists. """ context.session.assertPerm('admin') + verify_name_user(username, krb_principal) if get_user(username): raise koji.GenericError('user already exists: %s' % username) if krb_principal and get_user_by_krb_principal(krb_principal): @@ -12965,6 +12970,8 @@ class RootExports(object): from the HostPrincipalFormat setting (if available). """ context.session.assertPerm('host') + verify_host_name(hostname) + # validate arches arches = " ".join(arches) arches = koji.parse_arches(arches, strict=True) @@ -15373,3 +15380,44 @@ def handle_upload(environ): context.session.id, context.session.callnum, time.time() - start, size, fn) return ret + + +def verify_name_internal(name): + if not isinstance(name, str): + raise koji.GenericError("Name should be string") + max_name_length_internal = context.opts['MaxNameLengthInternal'] + if max_name_length_internal != 0 and len(name) > max_name_length_internal: + raise koji.GenericError("Name %s is too long. Max length is %s characters" + % (name, max_name_length_internal)) + if context.opts.get('RegexNameInternal.compiled'): + regex_name_internal_compiled = context.opts['RegexNameInternal.compiled'] + if not regex_name_internal_compiled.match(name): + raise koji.GenericError("Name %s does not match RegexNameInternal value" % name) + + +def verify_name_user(name=None, krb=None): + if name and not isinstance(name, str): + raise koji.GenericError("Name should be string") + if krb and not isinstance(krb, str): + raise koji.GenericError("Kerberos principal should be string") + max_name_length_internal = context.opts['MaxNameLengthInternal'] + + if max_name_length_internal != 0: + if name and len(name) > max_name_length_internal: + raise koji.GenericError("Name %s is too long. Max length is %s characters" + % (name, max_name_length_internal)) + if krb and len(krb) > max_name_length_internal: + raise koji.GenericError("Kerberos principal %s is too long. Max length is " + "%s characters" % (krb, max_name_length_internal)) + if context.opts.get('RegexUserName.compiled'): + regex_user_name_compiled = context.opts['RegexUserName.compiled'] + if (name is not None) and (not regex_user_name_compiled.match(name)): + raise koji.GenericError("Name %s does not match RegexUserName value" % name) + if (krb is not None) and (not regex_user_name_compiled.match(krb)): + raise koji.GenericError("Kerberos principal %s does not match RegexUserName " + "value" % krb) + + +def verify_host_name(name): + verify_name_internal(name) + verify_name_user(name) diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index ebdfdc2..d9ffc69 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -28,6 +28,7 @@ import sys import threading import time import traceback +import re import koji import koji.auth @@ -467,6 +468,10 @@ def load_config(environ): ['LockOut', 'boolean', False], ['ServerOffline', 'boolean', False], ['OfflineMessage', 'string', None], + + ['MaxNameLengthInternal', 'integer', 256], + ['RegexNameInternal', 'string', r'^[A-Za-z0-9/_.+-]+$'], + ['RegexUserName', 'string', r'^[A-Za-z0-9/_.@-]+$'] ] opts = {} for name, dtype, default in cfgmap: @@ -495,6 +500,10 @@ def load_config(environ): if opts.get('KojiDir') is not None: koji.BASEDIR = opts['KojiDir'] koji.pathinfo.topdir = opts['KojiDir'] + if opts['RegexNameInternal'] != '': + opts['RegexNameInternal.compiled'] = re.compile(opts['RegexNameInternal']) + if opts['RegexUserName'] != '': + opts['RegexUserName.compiled'] = re.compile(opts['RegexUserName']) return opts diff --git a/koji/__init__.py b/koji/__init__.py index 073eaef..76e4ec1 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -431,6 +431,14 @@ class GSSAPIAuthError(AuthError): faultCode = 1023 +class NameValidationError(GenericError): + """Raised when name validation fails + + Currently not used, set up this error class into verify_name_internal, verify_name_user + instead of GenericError for Koji 1.31.""" + faultCode = 1024 + + class MultiCallInProgress(object): """ Placeholder class to be returned by method calls when in the process of diff --git a/tests/test_hub/test_add_archivetype.py b/tests/test_hub/test_add_archivetype.py index 21da183..49cd35a 100644 --- a/tests/test_hub/test_add_archivetype.py +++ b/tests/test_hub/test_add_archivetype.py @@ -1,4 +1,5 @@ import unittest + import mock import koji @@ -9,17 +10,19 @@ IP = kojihub.InsertProcessor class TestAddArchiveType(unittest.TestCase): + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub._multiRow') @mock.patch('kojihub.get_archive_type') @mock.patch('kojihub.InsertProcessor') - def test_add_archive_type(self, InsertProcessor, get_archive_type, - _multiRow): + def test_add_archive_type(self, InsertProcessor, get_archive_type, _multiRow, + verify_name_internal): # Not sure why mock can't patch kojihub.context, so we do this session = kojihub.context.session = mock.MagicMock() mocks = [InsertProcessor, get_archive_type, session] # It seems MagicMock will not automatically handle attributes that # start with "assert" session.assertPerm = mock.MagicMock() + verify_name_internal.return_value = None # expected case get_archive_type.return_value = None @@ -47,3 +50,14 @@ class TestAddArchiveType(unittest.TestCase): kojihub.add_archive_type('deb', 'Debian package', 'deb') InsertProcessor.assert_not_called() session.assertPerm.assert_called_with('admin') + + # name is longer as expected + new_archive_type = 'new-archive-type+' + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_archive_type(new_archive_type, 'Debian package', 'deb') + + # not except regex rules + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_archive_type(new_archive_type, 'Debian package', 'deb') diff --git a/tests/test_hub/test_add_btype.py b/tests/test_hub/test_add_btype.py index acb02b5..ce4494d 100644 --- a/tests/test_hub/test_add_btype.py +++ b/tests/test_hub/test_add_btype.py @@ -1,4 +1,5 @@ import unittest + import mock import koji @@ -9,15 +10,17 @@ IP = kojihub.InsertProcessor class TestAddBType(unittest.TestCase): + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub.list_btypes') @mock.patch('kojihub.InsertProcessor') - def test_add_btype(self, InsertProcessor, list_btypes): + def test_add_btype(self, InsertProcessor, list_btypes, verify_name_internal): # Not sure why mock can't patch kojihub.context, so we do this session = kojihub.context.session = mock.MagicMock() mocks = [InsertProcessor, list_btypes, session] # It seems MagicMock will not automatically handle attributes that # start with "assert" session.assertPerm = mock.MagicMock() + verify_name_internal.return_value = None # expected case list_btypes.return_value = None @@ -43,3 +46,14 @@ class TestAddBType(unittest.TestCase): kojihub.add_btype('new_btype') InsertProcessor.assert_not_called() session.assertPerm.assert_called_with('admin') + + # name is longer as expected + new_btype = 'new-btype+' + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_btype(new_btype) + + # not except regex rules + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_btype(new_btype) diff --git a/tests/test_hub/test_add_channel.py b/tests/test_hub/test_add_channel.py index 10509bb..5588c7b 100644 --- a/tests/test_hub/test_add_channel.py +++ b/tests/test_hub/test_add_channel.py @@ -32,9 +32,11 @@ class TestAddChannel(unittest.TestCase): self.inserts.append(insert) return insert + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub.get_channel') @mock.patch('kojihub._singleValue') - def test_add_channel_exists(self, _singleValue, get_channel): + def test_add_channel_exists(self, _singleValue, get_channel, verify_name_internal): + verify_name_internal.return_value = None get_channel.return_value = {'id': 123, 'name': self.channel_name} with self.assertRaises(koji.GenericError): self.exports.addChannel(self.channel_name) @@ -42,11 +44,13 @@ class TestAddChannel(unittest.TestCase): _singleValue.assert_not_called() self.assertEqual(len(self.inserts), 0) + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub.get_channel') @mock.patch('kojihub._singleValue') - def test_add_channel_valid(self, _singleValue, get_channel): + def test_add_channel_valid(self, _singleValue, get_channel, verify_name_internal): get_channel.return_value = {} _singleValue.side_effect = [12] + verify_name_internal.return_value = None r = self.exports.addChannel(self.channel_name, description=self.description) self.assertEqual(r, 12) @@ -63,3 +67,16 @@ class TestAddChannel(unittest.TestCase): _singleValue.assert_has_calls([ mock.call("SELECT nextval('channels_id_seq')", strict=True) ]) + + @mock.patch('kojihub.verify_name_internal') + def test_add_channel_wrong_format(self, verify_name_internal): + # name is longer as expected + channel_name = 'test-channel+' + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.addChannel(channel_name) + + # not except regex rules + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.addChannel(channel_name) diff --git a/tests/test_hub/test_add_host.py b/tests/test_hub/test_add_host.py index 0c2a8dc..93a0ae5 100644 --- a/tests/test_hub/test_add_host.py +++ b/tests/test_hub/test_add_host.py @@ -1,6 +1,7 @@ -import mock import unittest +import mock + import koji import kojihub @@ -39,10 +40,12 @@ class TestAddHost(unittest.TestCase): def tearDown(self): mock.patch.stopall() + @mock.patch('kojihub.verify_host_name') @mock.patch('kojihub._dml') @mock.patch('kojihub.get_host') @mock.patch('kojihub._singleValue') - def test_add_host_exists(self, _singleValue, get_host, _dml): + def test_add_host_exists(self, _singleValue, get_host, _dml, verify_host_name): + verify_host_name.return_value = None get_host.return_value = {'id': 123} with self.assertRaises(koji.GenericError): self.exports.addHost('hostname', ['i386', 'x86_64']) @@ -50,10 +53,12 @@ class TestAddHost(unittest.TestCase): get_host.assert_called_once_with('hostname') _singleValue.assert_not_called() + @mock.patch('kojihub.verify_host_name') @mock.patch('kojihub._dml') @mock.patch('kojihub.get_host') @mock.patch('kojihub._singleValue') - def test_add_host_valid(self, _singleValue, get_host, _dml): + def test_add_host_valid(self, _singleValue, get_host, _dml, verify_host_name): + verify_host_name.return_value = None get_host.return_value = {} _singleValue.side_effect = [333, 12] self.context.session.createUser.return_value = 456 @@ -63,22 +68,25 @@ class TestAddHost(unittest.TestCase): 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-') + self.context.session.createUser.assert_called_once_with( + 'hostname', usertype=koji.USERTYPES['HOST'], krb_principal='-hostname-') self.assertEqual(_singleValue.call_count, 2) _singleValue.assert_has_calls([ mock.call("SELECT id FROM channels WHERE name = 'default'"), mock.call("SELECT nextval('host_id_seq')", strict=True) ]) self.assertEqual(_dml.call_count, 1) - _dml.assert_called_once_with("INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s)", - {'hostID': 12, 'userID': 456, 'hostname': 'hostname'}) + _dml.assert_called_once_with("INSERT INTO host (id, user_id, name) " + "VALUES (%(hostID)i, %(userID)i, %(hostname)s)", + {'hostID': 12, 'userID': 456, 'hostname': 'hostname'}) + @mock.patch('kojihub.verify_host_name') @mock.patch('kojihub.get_user') @mock.patch('kojihub._dml') @mock.patch('kojihub.get_host') @mock.patch('kojihub._singleValue') - def test_add_host_wrong_user(self, _singleValue, get_host, _dml, get_user): + def test_add_host_wrong_user(self, _singleValue, get_host, _dml, get_user, verify_host_name): + verify_host_name.return_value = None get_user.return_value = { 'id': 1, 'name': 'hostname', @@ -94,11 +102,14 @@ class TestAddHost(unittest.TestCase): self.assertEqual(len(self.inserts), 0) self.assertEqual(len(self.updates), 0) + @mock.patch('kojihub.verify_host_name') @mock.patch('kojihub.get_user') @mock.patch('kojihub._dml') @mock.patch('kojihub.get_host') @mock.patch('kojihub._singleValue') - def test_add_host_wrong_user_forced(self, _singleValue, get_host, _dml, get_user): + def test_add_host_wrong_user_forced(self, _singleValue, get_host, _dml, get_user, + verify_host_name): + verify_host_name.return_value = None get_user.return_value = { 'id': 123, 'name': 'hostname', @@ -120,11 +131,14 @@ class TestAddHost(unittest.TestCase): self.assertEqual(update.clauses, ['id = %(userID)i']) self.assertEqual(update.data, {'usertype': koji.USERTYPES['HOST']}) + @mock.patch('kojihub.verify_host_name') @mock.patch('kojihub.get_user') @mock.patch('kojihub._dml') @mock.patch('kojihub.get_host') @mock.patch('kojihub._singleValue') - def test_add_host_superwrong_user_forced(self, _singleValue, get_host, _dml, get_user): + def test_add_host_superwrong_user_forced(self, _singleValue, get_host, _dml, get_user, + verify_host_name): + verify_host_name.return_value = None get_user.return_value = { 'id': 123, 'name': 'hostname', @@ -141,3 +155,16 @@ class TestAddHost(unittest.TestCase): _singleValue.assert_called() self.assertEqual(len(self.inserts), 0) self.assertEqual(len(self.updates), 0) + + @mock.patch('kojihub.verify_host_name') + def test_add_host_wrong_format(self, verify_host_name): + # name is longer as expected + hostname = 'host-name+' + verify_host_name.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.addHost(hostname, ['i386', 'x86_64'], force=True) + + # not except regex rules + verify_host_name.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.addHost(hostname, ['i386', 'x86_64'], force=True) diff --git a/tests/test_hub/test_add_host_to_channel.py b/tests/test_hub/test_add_host_to_channel.py index da0b953..51b0b91 100644 --- a/tests/test_hub/test_add_host_to_channel.py +++ b/tests/test_hub/test_add_host_to_channel.py @@ -1,6 +1,7 @@ -import mock import unittest +import mock + import koji import kojihub @@ -91,17 +92,20 @@ class TestAddHostToChannel(unittest.TestCase): get_channel_id.assert_called_once_with(cname, create=False) self.assertEqual(len(self.inserts), 0) + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub.get_channel') @mock.patch('kojihub.list_channels') @mock.patch('kojihub.get_channel_id') @mock.patch('kojihub.get_host') - def test_no_channel_create(self, get_host, get_channel_id, list_channels, get_channel): + def test_no_channel_create(self, get_host, get_channel_id, list_channels, get_channel, + verify_name_internal): name = 'hostname' cname = 'channel_name' get_host.return_value = {'id': 123, 'name': name} get_channel_id.return_value = 456 list_channels.return_value = [{'id': 1, 'name': 'default'}] get_channel.return_value = {'enabled': True} + verify_name_internal.return_value = None kojihub.add_host_to_channel(name, cname, create=True) @@ -142,3 +146,20 @@ class TestAddHostToChannel(unittest.TestCase): get_channel_id.assert_called_once_with(cname, create=False) list_channels.assert_called_once_with(123) self.assertEqual(len(self.inserts), 0) + + @mock.patch('kojihub.verify_name_internal') + @mock.patch('kojihub.get_host') + def test_channel_wrong_format(self, get_host, verify_name_internal): + name = 'hostname' + channel_name = 'test-channel+' + get_host.return_value = {'id': 123, 'name': name} + + # name is longer as expected + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_host_to_channel(name, channel_name, create=True) + + # not except regex rules + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_host_to_channel(name, channel_name, create=True) diff --git a/tests/test_hub/test_add_volume.py b/tests/test_hub/test_add_volume.py new file mode 100644 index 0000000..9aff770 --- /dev/null +++ b/tests/test_hub/test_add_volume.py @@ -0,0 +1,30 @@ +import unittest + +import mock + +import koji +import kojihub + + +class TestAddVolume(unittest.TestCase): + + def setUp(self): + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() + 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() + + def test_add_volume_wrong_format(self): + volume_name = 'volume-name+' + + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_volume(volume_name) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.add_volume(volume_name) diff --git a/tests/test_hub/test_create_build_target.py b/tests/test_hub/test_create_build_target.py new file mode 100644 index 0000000..dd0c399 --- /dev/null +++ b/tests/test_hub/test_create_build_target.py @@ -0,0 +1,82 @@ +# coding: utf-8 +import unittest + +import mock + +import koji +import kojihub + + +class TestCreateBuildTarget(unittest.TestCase): + + def setUp(self): + self.get_build_targets = mock.patch('kojihub.get_build_targets').start() + self.get_tag = mock.patch('kojihub.get_tag').start() + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() + 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.tag_name = 'test-tag' + self.destination_tag_name = 'dest-test-tag' + self.target_name = 'test_target' + self.build_target_info = { + 'build_tag': 2, + 'build_tag_name': self.tag_name, + 'dest_tag': 3, + 'dest_tag_name': self.destination_tag_name, + 'id': 1, + 'name': self.target_name + } + self.tag_info = {'arches': '', + 'extra': {}, + 'id': 1, + 'locked': False, + 'maven_include_all': False, + 'maven_support': False, + 'name': self.tag_name, + 'perm': None, + 'perm_id': None} + + def tearDown(self): + mock.patch.stopall() + + def test_target_wrong_format(self): + target_name = 'test-target+' + + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.create_build_target(target_name, self.tag_name, self.destination_tag_name) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.create_build_target(target_name, self.tag_name, self.destination_tag_name) + + def test_target_exists(self): + self.get_build_targets.return_value = self.build_target_info + self.verify_name_internal.return_value = None + expected = "A build target with the name '%s' already exists" % self.target_name + with self.assertRaises(koji.GenericError) as cm: + kojihub.create_build_target(self.target_name, self.tag_name, self.destination_tag_name) + self.assertEqual(expected, str(cm.exception)) + + def test_tag_not_exists(self): + self.get_build_targets.return_value = None + self.verify_name_internal.return_value = None + self.get_tag.return_value = None + expected = "build tag '%s' does not exist" % self.tag_name + with self.assertRaises(koji.GenericError) as cm: + kojihub.create_build_target(self.target_name, self.tag_name, self.destination_tag_name) + self.assertEqual(expected, str(cm.exception)) + + def test_dest_tag_not_exists(self): + self.get_build_targets.return_value = None + self.verify_name_internal.return_value = None + self.get_tag.side_effect = [self.tag_info, None] + expected = "destination tag '%s' does not exist" % self.destination_tag_name + with self.assertRaises(koji.GenericError) as cm: + kojihub.create_build_target(self.target_name, self.tag_name, self.destination_tag_name) + self.assertEqual(expected, str(cm.exception)) diff --git a/tests/test_hub/test_create_external_repo.py b/tests/test_hub/test_create_external_repo.py new file mode 100644 index 0000000..df54106 --- /dev/null +++ b/tests/test_hub/test_create_external_repo.py @@ -0,0 +1,47 @@ +# coding: utf-8 +import unittest + +import mock + +import koji +import kojihub + + +class TestCreateExternalRepo(unittest.TestCase): + + def setUp(self): + self.get_external_repos = mock.patch('kojihub.get_external_repos').start() + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() + 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.repo_url = 'http://path_to_ext_repo.com' + self.repo_name = 'test-repo' + self.repo_info = {'id': 1, 'name': self.repo_name, 'url': self.repo_url} + + def tearDown(self): + mock.patch.stopall() + + def test_create_external_repo_wrong_format(self): + repo_name = 'test-repo+' + + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.create_external_repo(repo_name, self.repo_url) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.create_external_repo(repo_name, self.repo_url) + + def test_create_external_repo_exists(self): + expected = 'An external repo named "%s" already exists' % self.repo_name + + self.verify_name_internal.return_value = None + self.get_external_repos.return_value = self.repo_info + with self.assertRaises(koji.GenericError) as cm: + kojihub.create_external_repo(self.repo_name, self.repo_url) + self.assertEqual(expected, str(cm.exception)) diff --git a/tests/test_hub/test_create_tag.py b/tests/test_hub/test_create_tag.py index f5d026e..22e2777 100644 --- a/tests/test_hub/test_create_tag.py +++ b/tests/test_hub/test_create_tag.py @@ -1,7 +1,8 @@ # coding: utf-8 -import mock import unittest +import mock + import koji import kojihub @@ -18,11 +19,12 @@ class TestCreateTag(unittest.TestCase): def setUp(self): self.InsertProcessor = mock.patch('kojihub.InsertProcessor', - side_effect=self.getInsert).start() + side_effect=self.getInsert).start() self.inserts = [] self._dml = mock.patch('kojihub._dml').start() self.get_tag = mock.patch('kojihub.get_tag').start() self.get_tag_id = mock.patch('kojihub.get_tag_id').start() + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() self.writeInheritanceData = mock.patch('kojihub.writeInheritanceData').start() self.context = mock.patch('kojihub.context').start() # It seems MagicMock will not automatically handle attributes that @@ -34,6 +36,7 @@ class TestCreateTag(unittest.TestCase): mock.patch.stopall() def test_duplicate(self): + self.verify_name_internal.return_value = None self.get_tag.return_value = {'name': 'duptag'} with self.assertRaises(koji.GenericError): kojihub.create_tag('duptag') @@ -41,6 +44,7 @@ class TestCreateTag(unittest.TestCase): def test_simple_create(self): self.get_tag.return_value = None self.get_tag_id.return_value = 99 + self.verify_name_internal.return_value = None self.context.event_id = 42 self.context.session.user_id = 23 kojihub.create_tag('newtag') @@ -66,6 +70,7 @@ class TestCreateTag(unittest.TestCase): def test_invalid_archs(self): self.get_tag.return_value = None self.get_tag_id.return_value = 99 + self.verify_name_internal.return_value = None self.context.event_id = 42 self.context.session.user_id = 23 @@ -79,3 +84,16 @@ class TestCreateTag(unittest.TestCase): kojihub.create_tag('newtag', arches=u'arch1,arch2') self.assertEqual(len(self.inserts), 0) + + def test_tag_wrong_format(self): + tag_name = 'test-tag+' + + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.create_tag(tag_name) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.create_tag(tag_name) diff --git a/tests/test_hub/test_create_user.py b/tests/test_hub/test_create_user.py new file mode 100644 index 0000000..2ba9f33 --- /dev/null +++ b/tests/test_hub/test_create_user.py @@ -0,0 +1,56 @@ +import unittest + +import mock + +import koji +import kojihub + + +class TestCreateUser(unittest.TestCase): + + def setUp(self): + self.verify_name_user = mock.patch('kojihub.verify_name_user').start() + self.get_user = mock.patch('kojihub.get_user').start() + self.get_user_by_krb_principal = mock.patch('kojihub.get_user_by_krb_principal').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.user_name = 'test_user' + self.user_info = {'id': 1, 'krb_principals': [], 'name': self.user_name, + 'status': 0, 'usertype': 0} + self.user_info_krb = {'id': 1, 'krb_principals': ['test_user@fedora.org'], + 'name': self.user_name, 'status': 0, 'usertype': 0} + + def test_create_user_wrong_format(self): + user_name = 'test-user+' + + # name is longer as expected + self.verify_name_user.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.createUser(user_name) + + # not except regex rules + self.verify_name_user.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.createUser(user_name) + + def test_create_user_exists(self): + expected = 'user already exists: %s' % self.user_name + self.verify_name_user.return_value = None + self.get_user.return_value = self.user_info + with self.assertRaises(koji.GenericError) as cm: + self.exports.createUser(self.user_name) + self.assertEqual(expected, str(cm.exception)) + + def test_create_user_exists_krb(self): + krb_principal = 'test_user@fedora.org' + expected = 'user with this Kerberos principal already exists: %s' % krb_principal + self.verify_name_user.return_value = None + self.get_user.return_value = None + self.get_user_by_krb_principal.return_value = self.user_info_krb + with self.assertRaises(koji.GenericError) as cm: + self.exports.createUser(self.user_name, krb_principal=krb_principal) + self.assertEqual(expected, str(cm.exception)) diff --git a/tests/test_hub/test_edit_build_target.py b/tests/test_hub/test_edit_build_target.py index 84e7bcf..3fb88f1 100644 --- a/tests/test_hub/test_edit_build_target.py +++ b/tests/test_hub/test_edit_build_target.py @@ -10,11 +10,13 @@ class TestEditBuildTarget(unittest.TestCase): def setUp(self): self.lookup_build_target = mock.patch('kojihub.lookup_build_target').start() + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() self.exports = kojihub.RootExports() def test_non_exist_build_target(self): session = kojihub.context.session = mock.MagicMock() session.assertPerm = mock.MagicMock() + self.verify_name_internal.return_value = None target_name = 'build-target' name = 'build-target-rename' build_tag = 'tag' @@ -25,3 +27,19 @@ class TestEditBuildTarget(unittest.TestCase): self.assertEqual("No such build target: %s" % target_name, str(cm.exception)) session.assertPerm.called_once_with('target') + + def test_target_wrong_format(self): + target_name = 'test-target' + name = 'build-target-rename+' + build_tag = 'tag' + dest_tag = 'dest-tag' + + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.editBuildTarget(target_name, name, build_tag, dest_tag) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.editBuildTarget(target_name, name, build_tag, dest_tag) diff --git a/tests/test_hub/test_edit_channel.py b/tests/test_hub/test_edit_channel.py index 39b6dcf..4196ba3 100644 --- a/tests/test_hub/test_edit_channel.py +++ b/tests/test_hub/test_edit_channel.py @@ -38,8 +38,9 @@ class TestEditChannel(unittest.TestCase): def tearDown(self): mock.patch.stopall() + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub.get_channel') - def test_edit_channel_missing(self, get_channel): + def test_edit_channel_missing(self, get_channel, verify_name_internal): expected = 'Invalid type for channelInfo: %s' % self.channel_name get_channel.side_effect = koji.GenericError(expected) with self.assertRaises(koji.GenericError) as ex: @@ -49,8 +50,10 @@ class TestEditChannel(unittest.TestCase): self.assertEqual(self.updates, []) self.assertEqual(expected, str(ex.exception)) + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub.get_channel') - def test_edit_channel_already_exists(self, get_channel): + def test_edit_channel_already_exists(self, get_channel, verify_name_internal): + verify_name_internal.return_value = None get_channel.side_effect = [ { 'id': 123, @@ -73,8 +76,10 @@ class TestEditChannel(unittest.TestCase): self.assertEqual('channel %s already exists (id=124)' % self.channel_name_new, str(ex.exception)) + @mock.patch('kojihub.verify_name_internal') @mock.patch('kojihub.get_channel') - def test_edit_channel_valid(self, get_channel): + def test_edit_channel_valid(self, get_channel, verify_name_internal): + verify_name_internal.return_value = None kojihub.get_channel.side_effect = [{ 'id': 123, 'name': self.channel_name, @@ -97,3 +102,22 @@ class TestEditChannel(unittest.TestCase): self.assertEqual(update.table, 'channels') self.assertEqual(update.values, values) self.assertEqual(update.clauses, clauses) + + @mock.patch('kojihub.verify_name_internal') + @mock.patch('kojihub.get_channel') + def test_edit_channel_wrong_format(self, get_channel, verify_name_internal): + channel_name_new = 'test-channel+' + get_channel.return_value = {'id': 123, + 'name': self.channel_name, + 'description': 'description', + } + + # name is longer as expected + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.editChannel(self.channel_name, name=channel_name_new) + + # not except regex rules + verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.editChannel(self.channel_name, name=channel_name_new) diff --git a/tests/test_hub/test_edit_external_repo.py b/tests/test_hub/test_edit_external_repo.py new file mode 100644 index 0000000..4146337 --- /dev/null +++ b/tests/test_hub/test_edit_external_repo.py @@ -0,0 +1,39 @@ +# coding: utf-8 +import unittest + +import mock + +import koji +import kojihub + + +class TestEditExternalRepo(unittest.TestCase): + + def setUp(self): + self.get_external_repo = mock.patch('kojihub.get_external_repo').start() + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() + 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.repo_url = 'http://path_to_ext_repo.com' + self.repo_name = 'test-repo' + self.repo_info = {'id': 1, 'name': self.repo_name, 'url': self.repo_url} + + def tearDown(self): + mock.patch.stopall() + + def test_edit_external_repo_wrong_format(self): + repo_name_new = 'test-repo+' + self.get_external_repo.return_value = self.repo_info + + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.edit_external_repo(self.repo_name, repo_name_new, self.repo_url) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.edit_external_repo(self.repo_name, repo_name_new, self.repo_url) diff --git a/tests/test_hub/test_edit_tag.py b/tests/test_hub/test_edit_tag.py index 118f84e..bc7aa1e 100644 --- a/tests/test_hub/test_edit_tag.py +++ b/tests/test_hub/test_edit_tag.py @@ -1,6 +1,8 @@ # coding: utf-8 -import mock import unittest + +import mock + import koji import kojihub @@ -32,6 +34,7 @@ class TestEditTag(unittest.TestCase): self._singleValue = mock.patch('kojihub._singleValue').start() self.get_tag = mock.patch('kojihub.get_tag').start() self.get_perm_id = mock.patch('kojihub.get_perm_id').start() + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() self.context = mock.patch('kojihub.context').start() # It seems MagicMock will not automatically handle attributes that # start with "assert" @@ -62,6 +65,7 @@ class TestEditTag(unittest.TestCase): 'exC': 3, 'exD': 4}} self._singleValue.return_value = None + self.verify_name_internal.return_value = None self.context.event_id = 42 self.context.session.user_id = 23 # no1 invoke @@ -168,7 +172,8 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333}) with self.assertRaises(koji.GenericError) as cm: kojihub._edit_tag('tag', **kwargs) - self.assertEqual(cm.exception.args[0], 'Can not both add/update and remove tag-extra: \'exC\'') + self.assertEqual(cm.exception.args[0], + 'Can not both add/update and remove tag-extra: \'exC\'') # no3 invoke kwargs = { @@ -231,3 +236,35 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333}) with self.assertRaises(koji.GenericError): kojihub._edit_tag('tag', **kwargs) + def test_edit_wrong_format_tag(self): + tag_name_new = 'new-test-tag+' + tag_name = 'tag' + self.get_tag.return_value = {'id': 333, + 'perm_id': 1, + 'name': tag_name, + 'arches': 'arch1 arch3', + 'locked': False, + 'maven_support': False, + 'maven_include_all': False, + 'extra': {'exA': 1, + 'exC': 3, + 'exD': 4}} + kwargs = { + 'perm': None, + 'name': tag_name_new, + 'arches': 'arch1 arch2', + 'locked': True, + 'maven_support': False, + 'maven_include_all': False, + 'extra': {}, + 'remove_extra': [] + } + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub._edit_tag('tag', **kwargs) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub._edit_tag('tag', **kwargs) diff --git a/tests/test_hub/test_edit_user.py b/tests/test_hub/test_edit_user.py index db6fc21..0452c86 100644 --- a/tests/test_hub/test_edit_user.py +++ b/tests/test_hub/test_edit_user.py @@ -1,11 +1,13 @@ -import mock import unittest +import mock + import koji import kojihub UP = kojihub.UpdateProcessor + class TestEditUser(unittest.TestCase): def getUpdate(self, *args, **kwargs): @@ -18,6 +20,7 @@ class TestEditUser(unittest.TestCase): self.updates = [] self._singleValue = mock.patch('kojihub._singleValue').start() self.get_user = mock.patch('kojihub.get_user').start() + self.verify_name_user = mock.patch('kojihub.verify_name_user').start() self.context = mock.patch('kojihub.context').start() self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', side_effect=self.getUpdate).start() @@ -33,6 +36,7 @@ class TestEditUser(unittest.TestCase): 'name': 'user', 'krb_principals': ['krb']} self._singleValue.return_value = None + self.verify_name_user.return_value = None kojihub._edit_user('user', name='newuser') # check the update @@ -43,8 +47,7 @@ class TestEditUser(unittest.TestCase): self.assertEqual(update.values, {'userID': 333}) self.assertEqual(update.clauses, ['id = %(userID)i']) - kojihub._edit_user('user', krb_principal_mappings=[{'old': 'krb', - 'new': 'newkrb'}]) + kojihub._edit_user('user', krb_principal_mappings=[{'old': 'krb', 'new': 'newkrb'}]) self.context.session.removeKrbPrincipal. \ assert_called_once_with(333, krb_principal='krb') self.context.session.setKrbPrincipal. \ @@ -85,7 +88,6 @@ class TestEditUser(unittest.TestCase): self.context.session.removeKrbPrincipal.assert_not_called() self.context.session.setKrbPrincipal.assert_not_called() - self._singleValue.reset_mock() self._singleValue.return_value = 2 with self.assertRaises(koji.GenericError) as cm: @@ -93,3 +95,14 @@ class TestEditUser(unittest.TestCase): self._singleValue.assert_called_once() self.assertEqual(cm.exception.args[0], 'Name newuser already taken by user 2') + + # name is longer as expected + new_username = 'new-user+' + self.verify_name_user.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub._edit_user('user', name=new_username) + + # not except regex rules + self.verify_name_user.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub._edit_user('user', name=new_username) diff --git a/tests/test_hub/test_grant_permissions.py b/tests/test_hub/test_grant_permissions.py new file mode 100644 index 0000000..8aef7dc --- /dev/null +++ b/tests/test_hub/test_grant_permissions.py @@ -0,0 +1,32 @@ +import unittest + +import mock + +import koji +import kojihub + + +class TestGrantPermission(unittest.TestCase): + + def setUp(self): + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').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.user_name = 'test_user' + + def test_grant_permission_wrong_format(self): + perms_name = 'test-perms+' + + # name is longer as expected + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.grantPermission(self.user_name, perms_name, create=True) + + # 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) diff --git a/tests/test_hub/test_user_groups.py b/tests/test_hub/test_user_groups.py index c8a4240..32acca1 100644 --- a/tests/test_hub/test_user_groups.py +++ b/tests/test_hub/test_user_groups.py @@ -1,6 +1,7 @@ -import mock import unittest +import mock + import koji import kojihub @@ -8,6 +9,7 @@ QP = kojihub.QueryProcessor IP = kojihub.InsertProcessor UP = kojihub.UpdateProcessor + class TestGrouplist(unittest.TestCase): def getQuery(self, *args, **kwargs): query = QP(*args, **kwargs) @@ -31,25 +33,27 @@ class TestGrouplist(unittest.TestCase): def setUp(self): self.context = mock.patch('kojihub.context').start() self.get_user = mock.patch('kojihub.get_user').start() + self.verify_name_internal = mock.patch('kojihub.verify_name_internal').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.QueryProcessor = mock.patch('kojihub.QueryProcessor', - side_effect=self.getQuery).start() + side_effect=self.getQuery).start() self.queries = [] self.InsertProcessor = mock.patch('kojihub.InsertProcessor', - side_effect=self.getInsert).start() + side_effect=self.getInsert).start() self.inserts = [] self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', - side_effect=self.getUpdate).start() + side_effect=self.getUpdate).start() self.updates = [] def tearDown(self): mock.patch.stopall() def test_new_group(self): + self.verify_name_internal.return_value = None name = 'test_group' # insufficient permissions @@ -74,6 +78,17 @@ class TestGrouplist(unittest.TestCase): self.context.session.assertPerm.assert_called_with('admin') self.context.session.createUser.called_with(name, usertype=koji.USERTYPES['GROUP']) + # name is longer as expected + name = 'new-group+' + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.new_group(name) + + # not except regex rules + self.verify_name_internal.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + kojihub.new_group(name) + def test_add_group_member(self): group, gid = 'test_group', 1 user, uid = 'username', 2 @@ -160,7 +175,6 @@ class TestGrouplist(unittest.TestCase): self.assertEqual(i.data['group_id'], gid) self.assertEqual(i.data['user_id'], uid) - @mock.patch('kojihub.get_group_members') def test_drop_group_member(self, get_group_members): group, gid = 'test_group', 1 @@ -244,7 +258,6 @@ class TestGrouplist(unittest.TestCase): self.assertEqual(u.values['user_id'], uid) self.assertEqual(u.values['group_id'], gid) - @mock.patch('kojihub._multiRow') def test_get_group_members(self, _multiRow): group, gid = 'test_group', 1 diff --git a/tests/test_hub/test_verify_names.py b/tests/test_hub/test_verify_names.py new file mode 100644 index 0000000..04c1350 --- /dev/null +++ b/tests/test_hub/test_verify_names.py @@ -0,0 +1,86 @@ +import unittest + +import mock +import re + +import koji +import kojihub + + +class TestVerifyNameInternal(unittest.TestCase): + + def setUp(self): + self.context = mock.patch('kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() + self.context.opts = {'MaxNameLengthInternal': 15, + 'RegexNameInternal.compiled': re.compile('^[A-Za-z0-9/_.+-]+$')} + + def test_verify_name_internal_integer_type(self): + expected_error = "Name should be string" + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_internal(1234) + self.assertEqual(expected_error, str(cm.exception)) + + def test_verify_name_internal_longer(self): + name = 'test-name-internal-longer' + expected_error = "Name %s is too long. Max length is 15 characters" % name + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_internal(name) + self.assertEqual(expected_error, str(cm.exception)) + + def test_verify_name_internal_wrong_chars(self): + name = 'test-name@#' + expected_error = "Name %s does not match RegexNameInternal value" % name + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_internal(name) + self.assertEqual(expected_error, str(cm.exception)) + + +class TestVerifyUser(unittest.TestCase): + def setUp(self): + self.context = mock.patch('kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() + self.context.opts = {'MaxNameLengthInternal': 15, + 'RegexUserName.compiled': re.compile('^[A-Za-z0-9/_.@-]+$')} + + def test_verify_user_type_name(self): + expected_error = "Name should be string" + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_user(name=1234) + self.assertEqual(expected_error, str(cm.exception)) + + def test_verify_user_type_kerberos(self): + expected_error = "Kerberos principal should be string" + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_user(name='test-name', krb=1234) + self.assertEqual(expected_error, str(cm.exception)) + + def test_verify_user_name_longer(self): + name = 'test-user-longer' + expected_error = "Name %s is too long. Max length is 15 characters" % name + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_user(name=name) + self.assertEqual(expected_error, str(cm.exception)) + + def test_verify_user_kerberos_longer(self): + name = 'test-user' + krb = 'testuser@kerberos-test.com' + expected_error = "Kerberos principal %s is too long. Max length is 15 characters" % krb + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_user(name=name, krb=krb) + self.assertEqual(expected_error, str(cm.exception)) + + def test_verify_user_name_wrong_chars(self): + name = 'test-name+@#' + expected_error = "Name %s does not match RegexUserName value" % name + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_user(name=name) + self.assertEqual(expected_error, str(cm.exception)) + + def test_verify_user_kerberos_wrong_chars(self): + name = 'test-name' + krb = 'user+@test.com' + expected_error = "Kerberos principal %s does not match RegexUserName value" % krb + with self.assertRaises(koji.GenericError) as cm: + kojihub.verify_name_user(name=name, krb=krb) + self.assertEqual(expected_error, str(cm.exception))