URL: https://github.com/freeipa/freeipa/pull/1836 Author: rcritten Title: #1836: [Backport][ipa-4-6] certprofile: reject config with multiple profileIds Action: opened
PR body: """ This PR was opened automatically because PR #1830 was pushed to master and backport to ipa-4-6 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/1836/head:pr1836 git checkout pr1836
From b4ec44ea811d1f2d61ca7da19f96185efa43b9e0 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Wed, 18 Apr 2018 17:10:10 +1000 Subject: [PATCH 1/2] certprofile: reject config with multiple profileIds In certprofile-import if the config file contains two profileId directives with different values, with the first matching the profile ID CLI argument and the second differing, the profile gets imported under the second ID. This leads to: - failure to enable the profile - failure to add the IPA "tracking" certprofile object - inability to delete the misnamed profile from Dogtag (via ipa CLI) To avert this scenario, detect and reject profile configurations where profileId is specified multiple times (whether or not the values differ). https://pagure.io/freeipa/issue/7503 --- ipaserver/plugins/certprofile.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/ipaserver/plugins/certprofile.py b/ipaserver/plugins/certprofile.py index 4eab09f24f..5e8dbca046 100644 --- a/ipaserver/plugins/certprofile.py +++ b/ipaserver/plugins/certprofile.py @@ -236,14 +236,25 @@ def pre_callback(self, ldap, dn, entry, entry_attrs, *keys, **options): ca_enabled_check(self.api) context.profile = options['file'] - match = self.PROFILE_ID_PATTERN.search(options['file']) - if match is None: + matches = self.PROFILE_ID_PATTERN.findall(options['file']) + if len(matches) == 0: # no profileId found, use CLI value as profileId. context.profile = u'profileId=%s\n%s' % (keys[0], context.profile) - elif keys[0] != match.group(1): - raise errors.ValidationError(name='file', - error=_("Profile ID '%(cli_value)s' does not match profile data '%(file_value)s'") - % {'cli_value': keys[0], 'file_value': match.group(1)} + elif len(matches) > 1: + raise errors.ValidationError( + name='file', + error=_( + "Profile data specifies profileId multiple times: " + "%(values)s" + ) % dict(values=matches) + ) + elif keys[0] != matches[0]: + raise errors.ValidationError( + name='file', + error=_( + "Profile ID '%(cli_value)s' " + "does not match profile data '%(file_value)s'" + ) % dict(cli_value=keys[0], file_value=matches[0]) ) return dn From 50b1d1965b16da45683420130576e8f63b852320 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Wed, 18 Apr 2018 19:35:49 +1000 Subject: [PATCH 2/2] certprofile: add tests for config profileId scenarios Update the certprofile tests to cover the various scenarios concerning the profileId property in the profile configuration. The scenarios now explicitly tested are: - profileId not specified (should succeed) - mismatched profileId property (should fail) - multiple profileId properties (should fail) - one profileId property, matching given ID (should succeed) https://pagure.io/freeipa/issue/7503 --- .../test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl | 1 - ipatests/test_xmlrpc/test_certprofile_plugin.py | 33 ++++++++++++++++++++++ ipatests/test_xmlrpc/tracker/certprofile_plugin.py | 11 ++++++-- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl b/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl index cff1544627..f9e8ce441d 100644 --- a/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl +++ b/ipatests/test_xmlrpc/data/caIPAserviceCert_mod.cfg.tmpl @@ -1,6 +1,5 @@ auth.instance_id=raCertAuth classId=caEnrollImpl -profileId=caIPAserviceCert_mod visible=false desc=This certificate profile is for enrolling server certificates with IPA-RA agent authentication. enable=true diff --git a/ipatests/test_xmlrpc/test_certprofile_plugin.py b/ipatests/test_xmlrpc/test_certprofile_plugin.py index 1b3edda89c..7cefd0a8d7 100644 --- a/ipatests/test_xmlrpc/test_certprofile_plugin.py +++ b/ipatests/test_xmlrpc/test_certprofile_plugin.py @@ -222,3 +222,36 @@ class TestImportFromXML(XMLRPC_test): def test_import_xml(self, xmlprofile): with pytest.raises(errors.ExecutionError): xmlprofile.ensure_exists() + + +# The initial user_profile configuration does not specify profileId. +# This is fine (it gets derived from the profile-id CLI argument), +# but this case was already tested in TestProfileCRUD. +# +# This test case tests various scenarios where the profileId *is* +# specified in the profile configuration. These are: +# +# - mismatched profileId property (should fail) +# - multiple profileId properties (should fail) +# - one profileId property, matching given ID (should succeed) +# +@pytest.mark.tier1 +class TestImportProfileIdHandling(XMLRPC_test): + def test_import_with_mismatched_profile_id(self, user_profile): + command = user_profile.make_create_command( + extra_lines=['profileId=bogus'] + ) + with pytest.raises(errors.ValidationError): + command() + + def test_import_with_multiple_profile_id(self, user_profile): + # correct profile id, but two occurrences + prop = u'profileId={}'.format(user_profile.name) + command = user_profile.make_create_command(extra_lines=[prop, prop]) + with pytest.raises(errors.ValidationError): + command() + + def test_import_with_correct_profile_id(self, user_profile): + prop = u'profileId={}'.format(user_profile.name) + command = user_profile.make_create_command(extra_lines=[prop]) + command() diff --git a/ipatests/test_xmlrpc/tracker/certprofile_plugin.py b/ipatests/test_xmlrpc/tracker/certprofile_plugin.py index d9aa302025..a573c2288e 100644 --- a/ipatests/test_xmlrpc/tracker/certprofile_plugin.py +++ b/ipatests/test_xmlrpc/tracker/certprofile_plugin.py @@ -57,7 +57,14 @@ def profile(self): content = f.read() return unicode(content) - def make_create_command(self): + def make_create_command(self, extra_lines=None): + """ + :param extra_lines: list of extra lines to append to profile config. + + """ + if extra_lines is None: + extra_lines = [] + if not self.profile: raise RuntimeError('Tracker object without path to profile ' 'cannot be used to create profile entry.') @@ -65,7 +72,7 @@ def make_create_command(self): return self.make_command('certprofile_import', self.name, description=self.description, ipacertprofilestoreissued=self.store, - file=self.profile) + file=u'\n'.join([self.profile] + extra_lines)) def check_create(self, result): assert_deepequal(dict(
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
