URL: https://github.com/freeipa/freeipa/pull/790 Author: martbab Title: #790: RFC: API for reporting PKINIT status Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/790/head:pr790 git checkout pr790
From 4f6876c27d467bfe1fc49ba960d1e2282335e946 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Thu, 11 May 2017 15:55:53 +0200 Subject: [PATCH 1/6] Allow for multivalued server attributes In order to achieve the task, the following changes were required: * vectorize the base class for server attributes * add a child class that enforces single-value attributes. It still accepts/returns single-value lists in order to not break Liskov substitution principle * Existing attributes inherit from the child class https://pagure.io/freeipa/issue/6937 --- ipaserver/plugins/serverroles.py | 4 +- ipaserver/servroles.py | 109 +++++++++++++++++++--------- ipatests/test_ipaserver/test_serverroles.py | 10 +-- 3 files changed, 79 insertions(+), 44 deletions(-) diff --git a/ipaserver/plugins/serverroles.py b/ipaserver/plugins/serverroles.py index e22eadd7b1..e81635c331 100644 --- a/ipaserver/plugins/serverroles.py +++ b/ipaserver/plugins/serverroles.py @@ -136,9 +136,7 @@ def config_retrieve(self, servrole): for name, attr in assoc_attributes.items(): attr_value = attr.get(self.api) - - if attr_value is not None: - result.update({name: attr_value}) + result.update({name: attr_value}) return result diff --git a/ipaserver/servroles.py b/ipaserver/servroles.py index cf45999951..84fed1046b 100644 --- a/ipaserver/servroles.py +++ b/ipaserver/servroles.py @@ -277,29 +277,33 @@ def get(self, api_instance): try: entries = ldap2.get_entries(search_base, filter=search_filter) except errors.EmptyResult: - return + return [] - master_cn = entries[0].dn[1]['cn'] + master_cns = {e.dn[1]['cn'] for e in entries} associated_role_providers = set( self._get_assoc_role_providers(api_instance)) - if master_cn not in associated_role_providers: + if not master_cns.issubset(associated_role_providers): raise errors.ValidationError( name=self.name, error=_("all masters must have %(role)s role enabled" % {'role': self.associated_role.name}) ) - return master_cn + return sorted(master_cns) - def _get_master_dn(self, api_instance, server): - return DN(('cn', server), api_instance.env.container_masters, - api_instance.env.basedn) + def _get_master_dns(self, api_instance, servers): + return [ + DN(('cn', server), api_instance.env.container_masters, + api_instance.env.basedn) for server in servers] + + def _get_masters_service_entries(self, ldap, master_dns): + service_dns = [ + DN(('cn', self.associated_service_name), master_dn) for master_dn + in master_dns] - def _get_masters_service_entry(self, ldap, master_dn): - service_dn = DN(('cn', self.associated_service_name), master_dn) - return ldap.get_entry(service_dn) + return [ldap.get_entry(service_dn) for service_dn in service_dns] def _add_attribute_to_svc_entry(self, ldap, service_entry): """ @@ -341,65 +345,98 @@ def _get_assoc_role_providers(self, api_instance): r[u'server_server'] for r in self.associated_role.status( api_instance) if r[u'status'] == ENABLED] - def _remove(self, api_instance, master): + def _remove(self, api_instance, masters): """ - remove attribute from the master + remove attribute from one or more masters :param api_instance: API instance - :param master: master FQDN + :param master: list or iterable containing master FQDNs """ ldap = api_instance.Backend.ldap2 - master_dn = self._get_master_dn(api_instance, master) - service_entry = self._get_masters_service_entry(ldap, master_dn) - self._remove_attribute_from_svc_entry(ldap, service_entry) + master_dns = self._get_master_dns(api_instance, masters) + service_entries = self._get_masters_service_entries(ldap, master_dns) + + for service_entry in service_entries: + self._remove_attribute_from_svc_entry(ldap, service_entry) - def _add(self, api_instance, master): + def _add(self, api_instance, masters): """ add attribute to the master :param api_instance: API instance - :param master: master FQDN + :param master: iterable containing master FQDNs :raises: * errors.ValidationError if the associated role is not enabled on the master """ - assoc_role_providers = self._get_assoc_role_providers(api_instance) + assoc_role_providers = set( + self._get_assoc_role_providers(api_instance)) + masters_set = set(masters) ldap = api_instance.Backend.ldap2 - if master not in assoc_role_providers: + masters_without_role = masters_set - assoc_role_providers + + if masters_without_role: raise errors.ValidationError( - name=master, + name=', '.join(sorted(masters_without_role)), error=_("must have %(role)s role enabled" % {'role': self.associated_role.name}) ) - master_dn = self._get_master_dn(api_instance, master) - service_entry = self._get_masters_service_entry(ldap, master_dn) - self._add_attribute_to_svc_entry(ldap, service_entry) + master_dns = self._get_master_dns(api_instance, masters) + service_entries = self._get_masters_service_entries(ldap, master_dns) + for service_entry in service_entries: + self._add_attribute_to_svc_entry(ldap, service_entry) - def set(self, api_instance, master): + def set(self, api_instance, masters): """ - set the attribute on master + set the attribute on masters :param api_instance: API instance - :param master: FQDN of the new master + :param masters: an interable with FQDNs of the new masters - the attribute is automatically unset from previous master if present + the attribute is automatically unset from previous masters if present :raises: errors.EmptyModlist if the new masters is the same as - the original on + the original ones """ - old_master = self.get(api_instance) + old_masters = self.get(api_instance) - if old_master == master: + if sorted(old_masters) == sorted(masters): raise errors.EmptyModlist - self._add(api_instance, master) + if old_masters: + self._remove(api_instance, old_masters) + + self._add(api_instance, masters) + + +class SingleValuedServerAttribute(ServerAttribute): + """ + Base class for server attributes that are forced to be single valued + + this means that `get` method will return a one-element list, and `set` + method will accept only one-element list + """ + + def set(self, api_instance, masters): + if len(masters) > 1: + raise errors.ValidationError( + name=self.attr_name, + error=_("must be enabled only on a single master")) + + super(SingleValuedServerAttribute, self).set(api_instance, masters) + + def get(self, api_instance): + masters = super(SingleValuedServerAttribute, self).get(api_instance) + num_masters = len(masters) + + if num_masters > 1: + raise errors.SingleMatchExpected(found=num_masters) - if old_master is not None: - self._remove(api_instance, old_master) + return masters _Service = namedtuple('Service', ['name', 'enabled']) @@ -574,14 +611,14 @@ def create_search_params(self, ldap, api_instance, server=None): ) attribute_instances = ( - ServerAttribute( + SingleValuedServerAttribute( u"ca_renewal_master_server", u"CA renewal master", u"ca_server_server", u"CA", u"caRenewalMaster", ), - ServerAttribute( + SingleValuedServerAttribute( u"dnssec_key_master_server", u"DNSSec key master", u"dns_server_server", diff --git a/ipatests/test_ipaserver/test_serverroles.py b/ipatests/test_ipaserver/test_serverroles.py index d8844df300..e671272783 100644 --- a/ipatests/test_ipaserver/test_serverroles.py +++ b/ipatests/test_ipaserver/test_serverroles.py @@ -706,7 +706,7 @@ def test_attribute_master(self, mock_api, mock_masters, actual_attr_masters = self.config_retrieve( assoc_role, mock_api)[attr_name] - assert actual_attr_masters == fqdn + assert actual_attr_masters == [fqdn] def test_set_attribute_on_the_same_provider_raises_emptymodlist( self, mock_api, mock_masters): @@ -727,7 +727,7 @@ def test_set_attribute_on_master_without_assoc_role_raises_validationerror( non_ca_fqdn = mock_masters.get_fqdn('trust-controller-dns') with pytest.raises(errors.ValidationError): - self.config_update(mock_api, **{attr_name: non_ca_fqdn}) + self.config_update(mock_api, **{attr_name: [non_ca_fqdn]}) def test_set_unknown_attribute_on_master_raises_notfound( self, mock_api, mock_masters): @@ -735,7 +735,7 @@ def test_set_unknown_attribute_on_master_raises_notfound( fqdn = mock_masters.get_fqdn('trust-controller-ca') with pytest.raises(errors.NotFound): - self.config_update(mock_api, **{attr_name: fqdn}) + self.config_update(mock_api, **{attr_name: [fqdn]}) def test_set_ca_renewal_master_on_other_ca_and_back(self, mock_api, mock_masters): @@ -747,7 +747,7 @@ def test_set_ca_renewal_master_on_other_ca_and_back(self, mock_api, other_ca_server = mock_masters.get_fqdn('trust-controller-ca') for host in (other_ca_server, original_renewal_master): - self.config_update(mock_api, **{attr_name: host}) + self.config_update(mock_api, **{attr_name: [host]}) assert ( - self.config_retrieve(role_name, mock_api)[attr_name] == host) + self.config_retrieve(role_name, mock_api)[attr_name] == [host]) From 3775face3aca0723427958a189123df4f9428d22 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Tue, 16 May 2017 17:29:39 +0200 Subject: [PATCH 2/6] Refactor the role/attribute member reporting code The `config` object now hosts a generic method for updating the config entry for desired server role configuration (if not empty). The duplicated code in dns/trust/vaultconfig commands was replaced by a call to a common method. https://pagure.io/freeipa/issue/6937 --- ipaserver/plugins/config.py | 24 ++++++++++++++++-------- ipaserver/plugins/dns.py | 16 ++++------------ ipaserver/plugins/trust.py | 22 ++++------------------ ipaserver/plugins/vault.py | 6 +++--- 4 files changed, 27 insertions(+), 41 deletions(-) diff --git a/ipaserver/plugins/config.py b/ipaserver/plugins/config.py index b50e7a4691..c88cb99b47 100644 --- a/ipaserver/plugins/config.py +++ b/ipaserver/plugins/config.py @@ -267,15 +267,21 @@ class config(LDAPObject): def get_dn(self, *keys, **kwargs): return DN(('cn', 'ipaconfig'), ('cn', 'etc'), api.env.basedn) - def show_servroles_attributes(self, entry_attrs, **options): + def update_entry_with_role_config(self, role_name, entry_attrs): + backend = self.api.Backend.serverroles + + role_config = backend.config_retrieve(role_name) + for key, value in role_config.items(): + if value: + entry_attrs.update({key: value}) + + + def show_servroles_attributes(self, entry_attrs, *roles, **options): if options.get('raw', False): return - backend = self.api.Backend.serverroles - - for role in ("CA server", "IPA master", "NTP server"): - config = backend.config_retrieve(role) - entry_attrs.update(config) + for role in roles: + self.update_entry_with_role_config(role, entry_attrs) def gather_trusted_domains(self): """ @@ -525,7 +531,8 @@ def exc_callback(self, keys, options, exc, call_func, keys, options, exc, call_func, *call_args, **call_kwargs) def post_callback(self, ldap, dn, entry_attrs, *keys, **options): - self.obj.show_servroles_attributes(entry_attrs, **options) + self.obj.show_servroles_attributes( + entry_attrs, "CA server", "IPA master", "NTP server", **options) return dn @@ -534,5 +541,6 @@ class config_show(LDAPRetrieve): __doc__ = _('Show the current configuration.') def post_callback(self, ldap, dn, entry_attrs, *keys, **options): - self.obj.show_servroles_attributes(entry_attrs, **options) + self.obj.show_servroles_attributes( + entry_attrs, "CA server", "IPA master", "NTP server", **options) return dn diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py index 05b4c00cac..b075f0bf97 100644 --- a/ipaserver/plugins/dns.py +++ b/ipaserver/plugins/dns.py @@ -4184,16 +4184,6 @@ def postprocess_result(self, result): if is_config_empty: result['summary'] = unicode(_('Global DNS configuration is empty')) - def show_servroles_attributes(self, entry_attrs, **options): - if options.get('raw', False): - return - - backend = self.api.Backend.serverroles - entry_attrs.update( - backend.config_retrieve("DNS server") - ) - - @register() class dnsconfig_mod(LDAPUpdate): __doc__ = _('Modify global DNS configuration.') @@ -4247,7 +4237,8 @@ def execute(self, *keys, **options): return result def post_callback(self, ldap, dn, entry_attrs, *keys, **options): - self.obj.show_servroles_attributes(entry_attrs, **options) + self.api.Object.config.show_servroles_attributes( + entry_attrs, "DNS server", **options) return dn @@ -4261,7 +4252,8 @@ def execute(self, *keys, **options): return result def post_callback(self, ldap, dn, entry_attrs, *keys, **options): - self.obj.show_servroles_attributes(entry_attrs, **options) + self.api.Object.config.show_servroles_attributes( + entry_attrs, "DNS server", **options) return dn diff --git a/ipaserver/plugins/trust.py b/ipaserver/plugins/trust.py index 0829f8c714..075b39dcc3 100644 --- a/ipaserver/plugins/trust.py +++ b/ipaserver/plugins/trust.py @@ -1278,22 +1278,6 @@ def _convert_groupdn(self, entry_attrs, options): entry_attrs['ipantfallbackprimarygroup'] = [groupdn[0][0].value] - def show_servroles(self, entry_attrs, **options): - if options.get('raw', False): - return - - backend = self.api.Backend.serverroles - - adtrust_agents = backend.config_retrieve( - "AD trust agent" - ) - adtrust_controllers = backend.config_retrieve( - "AD trust controller" - ) - - entry_attrs.update(adtrust_agents) - entry_attrs.update(adtrust_controllers) - @register() class trustconfig_mod(LDAPUpdate): @@ -1314,7 +1298,8 @@ def execute(self, *keys, **options): def post_callback(self, ldap, dn, entry_attrs, *keys, **options): self.obj._convert_groupdn(entry_attrs, options) - self.obj.show_servroles(entry_attrs, **options) + self.api.Object.config.show_servroles_attributes( + entry_attrs, "AD trust agent", "AD trust controller", **options) return dn @@ -1333,7 +1318,8 @@ def execute(self, *keys, **options): def post_callback(self, ldap, dn, entry_attrs, *keys, **options): self.obj._convert_groupdn(entry_attrs, options) - self.obj.show_servroles(entry_attrs, **options) + self.api.Object.config.show_servroles_attributes( + entry_attrs, "AD trust agent", "AD trust controller", **options) return dn diff --git a/ipaserver/plugins/vault.py b/ipaserver/plugins/vault.py index d46aca821d..d05a240c39 100644 --- a/ipaserver/plugins/vault.py +++ b/ipaserver/plugins/vault.py @@ -997,9 +997,9 @@ def execute(self, *args, **options): with self.api.Backend.kra.get_client() as kra_client: transport_cert = kra_client.system_certs.get_transport_cert() config = {'transport_cert': transport_cert.binary} - config.update( - self.api.Backend.serverroles.config_retrieve("KRA server") - ) + + self.api.Object.config.show_servroles_attributes( + config, "KRA server", **options) return { 'result': config, From 89c559785b49644443acdc93a9e9d5434aa8e4cb Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Fri, 12 May 2017 15:15:37 +0200 Subject: [PATCH 3/6] Add an attribute reporting client PKINIT-capable servers A new multi-valued server attribute `pkinit_server` was added which reports IPA masters that have PKINIT configuration usable by clients. The existing tests were modified to allow for testing the new attribute. https://pagure.io/freeipa/issue/6937 --- ipaserver/servroles.py | 7 ++ ipatests/test_ipaserver/test_serverroles.py | 109 +++++++++++++--------------- 2 files changed, 59 insertions(+), 57 deletions(-) diff --git a/ipaserver/servroles.py b/ipaserver/servroles.py index 84fed1046b..f6e79338b9 100644 --- a/ipaserver/servroles.py +++ b/ipaserver/servroles.py @@ -625,4 +625,11 @@ def create_search_params(self, ldap, api_instance, server=None): u"DNSSEC", u"dnssecKeyMaster", ), + ServerAttribute( + u"pkinit_server_server", + u"PKINIT enabled server", + u"ipa_master_server", + u"KDC", + u"pkinitEnabled" + ) ) diff --git a/ipatests/test_ipaserver/test_serverroles.py b/ipatests/test_ipaserver/test_serverroles.py index e671272783..b373a4d32f 100644 --- a/ipatests/test_ipaserver/test_serverroles.py +++ b/ipatests/test_ipaserver/test_serverroles.py @@ -58,7 +58,7 @@ def _make_master_entry_mods(ca=False): master_data = { - 'ca-dns-dnssec-keymaster': { + 'ca-dns-dnssec-keymaster-pkinit-server': { 'services': { 'CA': { 'enabled': True, @@ -72,14 +72,19 @@ def _make_master_entry_mods(ca=False): 'DNSSEC': { 'enabled': True, 'config': ['DNSSecKeyMaster'] + }, + 'KDC': { + 'enabled': True, + 'config': ['pkinitEnabled'] } }, 'expected_roles': { 'enabled': ['IPA master', 'CA server', 'DNS server'] }, - 'expected_attributes': {'DNS server': 'dnssec_key_master_server'} + 'expected_attributes': {'DNS server': 'dnssec_key_master_server', + 'IPA master': 'pkinit_server_server'} }, - 'ca-kra-renewal-master': { + 'ca-kra-renewal-master-pkinit-server': { 'services': { 'CA': { 'enabled': True, @@ -88,11 +93,16 @@ def _make_master_entry_mods(ca=False): 'KRA': { 'enabled': True, }, + 'KDC': { + 'enabled': True, + 'config': ['pkinitEnabled'] + }, }, 'expected_roles': { 'enabled': ['IPA master', 'CA server', 'KRA server'] }, - 'expected_attributes': {'CA server': 'ca_renewal_master_server'} + 'expected_attributes': {'CA server': 'ca_renewal_master_server', + 'IPA master': 'pkinit_server_server'} }, 'dns-trust-agent': { 'services': { @@ -234,7 +244,7 @@ def __init__(self, api_instance, domain_data): no_members=True, raw=True)['result']} - self.existing_attributes = self._check_test_host_attributes() + self.original_dns_configs = self._remove_test_host_attrs() def iter_domain_data(self): MasterData = namedtuple('MasterData', @@ -287,7 +297,6 @@ def _del_service_entry(self, service, fqdn): pass def _add_svc_entries(self, master_dn, svc_desc): - self._add_ipamaster_services(master_dn) for name in svc_desc: svc_dn = self.get_service_dn(name, master_dn) svc_mods = svc_desc[name] @@ -298,6 +307,8 @@ def _add_svc_entries(self, master_dn, svc_desc): enabled=svc_mods['enabled'], other_config=svc_mods.get('config', None))) + self._add_ipamaster_services(master_dn) + def _remove_svc_master_entries(self, master_dn): try: entries = self.ldap.connection.search_s( @@ -317,7 +328,11 @@ def _add_ipamaster_services(self, master_dn): """ for svc_name in self.ipamaster_services: svc_dn = self.get_service_dn(svc_name, master_dn) - self.ldap.add_entry(str(svc_dn), _make_service_entry_mods()) + try: + self.api.Backend.ldap2.get_entry(svc_dn) + except errors.NotFound: + self.ldap.add_entry( + str(svc_dn), _make_service_entry_mods()) def _add_members(self, dn, fqdn, member_attrs): _entry, attrs = self.ldap.connection.search_s( @@ -376,57 +391,36 @@ def _remove_members(self, dn, fqdn, member_attrs): except (ldap.NO_SUCH_OBJECT, ldap.NO_SUCH_ATTRIBUTE): pass - def _check_test_host_attributes(self): - existing_attributes = set() - - for service, value, attr_name in ( - ('CA', 'caRenewalMaster', 'ca renewal master'), - ('DNSSEC', 'DNSSecKeyMaster', 'dnssec key master')): + def _remove_test_host_attrs(self): + original_dns_configs = [] - svc_dn = DN(('cn', service), self.test_master_dn) + for attr_name in ( + 'caRenewalMaster', 'dnssecKeyMaster', 'pkinitEnabled'): try: - svc_entry = self.api.Backend.ldap2.get_entry(svc_dn) + svc_entry = self.api.Backend.ldap2.find_entry_by_attr( + 'ipaConfigString', attr_name, 'ipaConfigObject', + base_dn=self.test_master_dn) except errors.NotFound: continue else: - config_string_val = svc_entry.get('ipaConfigString', []) + original_dns_configs.append( + (svc_entry.dn, list(svc_entry.get('ipaConfigString', []))) + ) + svc_entry[u'ipaConfigString'].remove(attr_name) + self.api.Backend.ldap2.update_entry(svc_entry) - if value in config_string_val: - existing_attributes.add(attr_name) - - return existing_attributes - - def _remove_ca_renewal_master(self): - if 'ca renewal master' not in self.existing_attributes: - return + return original_dns_configs - ca_dn = DN(('cn', 'CA'), self.test_master_dn) - ca_entry = self.api.Backend.ldap2.get_entry(ca_dn) - - config_string_val = ca_entry.get('ipaConfigString', []) - try: - config_string_val.remove('caRenewalMaster') - except KeyError: - return - - ca_entry.update({'ipaConfigString': config_string_val}) - self.api.Backend.ldap2.update_entry(ca_entry) - - def _restore_ca_renewal_master(self): - if 'ca renewal master' not in self.existing_attributes: - return - - ca_dn = DN(('cn', 'CA'), self.test_master_dn) - ca_entry = self.api.Backend.ldap2.get_entry(ca_dn) - - config_string_val = ca_entry.get('ipaConfigString', []) - config_string_val.append('caRenewalMaster') - - ca_entry.update({'ipaConfigString': config_string_val}) - self.api.Backend.ldap2.update_entry(ca_entry) + def _restore_test_host_attrs(self): + for dn, config in self.original_dns_configs: + try: + svc_entry = self.api.Backend.ldap2.get_entry(dn) + svc_entry['ipaConfigString'] = config + self.api.Backend.ldap2.update_entry(svc_entry) + except (errors.NotFound, errors.EmptyModlist): + continue def setup_data(self): - self._remove_ca_renewal_master() for master_data in self.iter_domain_data(): # create host self._add_host_entry(master_data.fqdn) @@ -449,7 +443,6 @@ def setup_data(self): ) def teardown_data(self): - self._restore_ca_renewal_master() for master_data in self.iter_domain_data(): # first remove the master entries and service containers self._remove_svc_master_entries(master_data.dn) @@ -466,6 +459,8 @@ def teardown_data(self): # finally remove host entry self._del_host_entry(master_data.fqdn) + self._restore_test_host_attrs() + @pytest.fixture(scope='module') def mock_api(request): @@ -665,14 +660,14 @@ def test_provided_roles_on_master( def test_unknown_role_status_raises_notfound(self, mock_api, mock_masters): unknown_role = 'IAP maestr' - fqdn = mock_masters.get_fqdn('ca-dns-dnssec-keymaster') + fqdn = mock_masters.get_fqdn('ca-dns-dnssec-keymaster-pkinit-server') with pytest.raises(errors.NotFound): mock_api.Backend.serverroles.server_role_retrieve( fqdn, unknown_role) def test_no_servrole_queries_all_roles_on_server(self, mock_api, mock_masters): - master_name = 'ca-dns-dnssec-keymaster' + master_name = 'ca-dns-dnssec-keymaster-pkinit-server' enabled_roles = master_data[master_name]['expected_roles']['enabled'] result = self.find_role(None, mock_api, mock_masters, master=master_name) @@ -688,7 +683,7 @@ def test_invalid_substring_search_returns_nothing(self, mock_api, invalid_substr = 'fwfgbb' assert (not self.find_role(invalid_substr, mock_api, mock_masters, - 'ca-dns-dnssec-keymaster')) + 'ca-dns-dnssec-keymaster-pkinit-server')) class TestServerAttributes(object): @@ -706,7 +701,7 @@ def test_attribute_master(self, mock_api, mock_masters, actual_attr_masters = self.config_retrieve( assoc_role, mock_api)[attr_name] - assert actual_attr_masters == [fqdn] + assert fqdn in actual_attr_masters def test_set_attribute_on_the_same_provider_raises_emptymodlist( self, mock_api, mock_masters): @@ -744,10 +739,10 @@ def test_set_ca_renewal_master_on_other_ca_and_back(self, mock_api, original_renewal_master = self.config_retrieve( role_name, mock_api)[attr_name] - other_ca_server = mock_masters.get_fqdn('trust-controller-ca') + other_ca_server = [mock_masters.get_fqdn('trust-controller-ca')] for host in (other_ca_server, original_renewal_master): - self.config_update(mock_api, **{attr_name: [host]}) + self.config_update(mock_api, **{attr_name: host}) assert ( - self.config_retrieve(role_name, mock_api)[attr_name] == [host]) + self.config_retrieve(role_name, mock_api)[attr_name] == host) From 880c3db90b8d7c163f9e71a40eebfd7869797448 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Fri, 12 May 2017 15:27:36 +0200 Subject: [PATCH 4/6] Add the list of PKINIT servers as a virtual attribute to global config https://pagure.io/freeipa/issue/6937 --- ipaserver/plugins/config.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ipaserver/plugins/config.py b/ipaserver/plugins/config.py index c88cb99b47..df6bd466af 100644 --- a/ipaserver/plugins/config.py +++ b/ipaserver/plugins/config.py @@ -256,6 +256,12 @@ class config(LDAPObject): flags={'virtual_attribute', 'no_create'} ), Str( + 'pkinit_server_server*', + label=_('IPA master capable of PKINIT'), + doc=_('IPA master which can process PKINIT requests'), + flags={'virtual_attribute', 'no_create', 'no_update'} + ), + Str( 'ipadomainresolutionorder?', cli_name='domain_resolution_order', label=_('Domain resolution order'), From 204f0a2f15f78e71868f0bd9141cc06fe27fb3ab Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Fri, 12 May 2017 17:25:30 +0200 Subject: [PATCH 5/6] Add `pkinit-status` command This command is a more streamlined reporting tool for PKINIT feature status in the FreeIPA topology. It prints out whether PKINIT is enabled or disabled on individual masters in a topology. If a`--server` is specified, it reports status for an individual server. If `--status` is specified, it searches for all servers that have PKINIT enabled or disabled. https://pagure.io/freeipa/issue/6937 --- API.txt | 15 ++++++ VERSION.m4 | 4 +- ipaserver/plugins/pkinit.py | 109 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 125 insertions(+), 3 deletions(-) diff --git a/API.txt b/API.txt index a09da21aed..44567a22da 100644 --- a/API.txt +++ b/API.txt @@ -3736,6 +3736,20 @@ command: ping/1 args: 0,1,1 option: Str('version?') output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>]) +command: pkinit_status/1 +args: 1,7,4 +arg: Str('criteria?') +option: Flag('all', autofill=True, cli_name='all', default=False) +option: Flag('raw', autofill=True, cli_name='raw', default=False) +option: Str('server_server?', autofill=False, cli_name='server') +option: Int('sizelimit?', autofill=False) +option: StrEnum('status?', autofill=False, cli_name='status', values=[u'enabled', u'disabled']) +option: Int('timelimit?', autofill=False) +option: Str('version?') +output: Output('count', type=[<type 'int'>]) +output: ListOfEntries('result') +output: Output('summary', type=[<type 'unicode'>, <type 'NoneType'>]) +output: Output('truncated', type=[<type 'bool'>]) command: plugins/1 args: 0,3,3 option: Flag('all', autofill=True, cli_name='all', default=True) @@ -6804,6 +6818,7 @@ default: permission_remove_member/1 default: permission_show/1 default: ping/1 default: pkinit/1 +default: pkinit_status/1 default: plugins/1 default: privilege/1 default: privilege_add/1 diff --git a/VERSION.m4 b/VERSION.m4 index 6c768f0a6e..706c243739 100644 --- a/VERSION.m4 +++ b/VERSION.m4 @@ -73,8 +73,8 @@ define(IPA_DATA_VERSION, 20100614120000) # # ######################################################## define(IPA_API_VERSION_MAJOR, 2) -define(IPA_API_VERSION_MINOR, 226) -# Last change: Remove the pkinit-anonymous command +define(IPA_API_VERSION_MINOR, 227) +# Last change: Add `pkinit-status` command ######################################################## diff --git a/ipaserver/plugins/pkinit.py b/ipaserver/plugins/pkinit.py index e49b31091d..970f955c54 100644 --- a/ipaserver/plugins/pkinit.py +++ b/ipaserver/plugins/pkinit.py @@ -3,11 +3,33 @@ # from ipalib import Object -from ipalib import _ +from ipalib import _, ngettext +from ipalib.crud import Search +from ipalib.parameters import Int, Str, StrEnum from ipalib.plugable import Registry register = Registry() +__doc__ = _(""" +Kerberos PKINIT feature status reporting tools. + +Report IPA masters on which Kerberos PKINIT is enabled or disabled + +EXAMPLES: + List PKINIT status on all masters: + ipa pkinit-status + + Check PKINIT status on `ipa.example.com`: + ipa pkinit-status --server ipa.example.com + + List all IPA masters with disabled PKINIT: + ipa pkinit-status --status='disabled' + +For more info about PKINIT support see: + +https://www.freeipa.org/page/V4/Kerberos_PKINIT +""") + @register() class pkinit(Object): @@ -17,3 +39,88 @@ class pkinit(Object): object_name = _('pkinit') label = _('PKINIT') + + takes_params = ( + Str( + 'server_server?', + cli_name='server', + label=_('Server name'), + doc=_('IPA server hostname'), + ), + StrEnum( + 'status?', + cli_name='status', + label=_('PKINIT status'), + doc=_('Whether PKINIT is enabled or disabled'), + values=(u'enabled', u'disabled'), + flags={'virtual_attribute', 'no_create', 'no_update'} + ) + ) + + +@register() +class pkinit_status(Search): + __doc__ = _('Report PKINIT status on the IPA masters') + + msg_summary = ngettext('%(count)s server matched', + '%(count)s servers matched', 0) + + takes_options = Search.takes_options + ( + Int( + 'timelimit?', + label=_('Time Limit'), + doc=_('Time limit of search in seconds (0 is unlimited)'), + flags=['no_display'], + minvalue=0, + autofill=False, + ), + Int( + 'sizelimit?', + label=_('Size Limit'), + doc=_('Maximum number of entries returned (0 is unlimited)'), + flags=['no_display'], + minvalue=0, + autofill=False, + ), + ) + + def get_pkinit_status(self, server, status): + backend = self.api.Backend.serverroles + ipa_master_config = backend.config_retrieve("IPA master") + + if server is not None: + servers = [server] + else: + servers = ipa_master_config['ipa_master_server'] + + pkinit_servers = ipa_master_config['pkinit_server_server'] + + for s in servers: + pkinit_status = { + u'server_server': s, + u'status': ( + u'enabled' if s in pkinit_servers else u'disabled' + ) + } + if status is not None and pkinit_status[u'status'] != status: + continue + + yield pkinit_status + + def execute(self, *keys, **options): + if keys: + return dict( + result=[], + count=0, + truncated=False + ) + + server = options.get('server_server', None) + status = options.get('status', None) + + if server is not None: + self.api.Object.server_role.ensure_master_exists(server) + + result = sorted(self.get_pkinit_status(server, status)) + + return dict(result=result, count=len(result), truncated=False) From c8a14e9a3223a11ddd3f64bac591fb9185be52c9 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Thu, 18 May 2017 16:20:13 +0200 Subject: [PATCH 6/6] test_serverroles: Get rid of MockLDAP and use ldap2 instead The test fixture haphazardly intermixed MockLDAP and ldap2 calls in setup and teardown code, greatly hampering extension of the code and also porting efforts to Python 3. Get rid of MockLDAP and use ldap2 for all LDAP operations. https://pagure.io/freeipa/issue/6937 --- ipatests/test_ipaserver/test_serverroles.py | 109 +++++++++++++--------------- 1 file changed, 51 insertions(+), 58 deletions(-) diff --git a/ipatests/test_ipaserver/test_serverroles.py b/ipatests/test_ipaserver/test_serverroles.py index b373a4d32f..985c750b64 100644 --- a/ipatests/test_ipaserver/test_serverroles.py +++ b/ipatests/test_ipaserver/test_serverroles.py @@ -14,40 +14,39 @@ from ipaplatform.paths import paths from ipalib import api, create_api, errors from ipapython.dn import DN -from ipatests.util import MockLDAP -def _make_service_entry_mods(enabled=True, other_config=None): +def _make_service_entry(ldap_backend, dn, enabled=True, other_config=None): mods = { - b'objectClass': [b'top', b'nsContainer', b'ipaConfigObject'], + 'objectClass': ['top', 'nsContainer', 'ipaConfigObject'], } if enabled: - mods.update({b'ipaConfigString': [b'enabledService']}) + mods.update({'ipaConfigString': ['enabledService']}) if other_config is not None: - mods.setdefault(b'ipaConfigString', []) - mods[b'ipaConfigString'].extend(other_config) + mods.setdefault('ipaConfigString', []) + mods['ipaConfigString'].extend(other_config) - return mods + return ldap_backend.make_entry(dn, **mods) -def _make_master_entry_mods(ca=False): +def _make_master_entry(ldap_backend, dn, ca=False): mods = { - b'objectClass': [ - b'top', - b'nsContainer', - b'ipaReplTopoManagedServer', - b'ipaSupportedDomainLevelConfig', - b'ipaConfigObject', + 'objectClass': [ + 'top', + 'nsContainer', + 'ipaReplTopoManagedServer', + 'ipaSupportedDomainLevelConfig', + 'ipaConfigObject', ], - b'ipaMaxDomainLevel': [b'1'], - b'ipaMinDomainLevel': [b'0'], - b'ipaReplTopoManagedsuffix': [str(api.env.basedn)] + 'ipaMaxDomainLevel': ['1'], + 'ipaMinDomainLevel': ['0'], + 'ipaReplTopoManagedsuffix': [str(api.env.basedn)] } if ca: - mods[b'ipaReplTopoManagedsuffix'].append(b'o=ipaca') + mods['ipaReplTopoManagedsuffix'].append('o=ipaca') - return mods + return ldap_backend.make_entry(dn, **mods) _adtrust_agents = DN( ('cn', 'adtrust agents'), @@ -235,7 +234,7 @@ def __init__(self, api_instance, domain_data): ('cn', self.api.env.host), self.api.env.container_masters, self.api.env.basedn) - self.ldap = MockLDAP() + self.ldap = self.api.Backend.ldap2 self.existing_masters = { m['cn'][0] for m in self.api.Command.server_find( @@ -302,8 +301,9 @@ def _add_svc_entries(self, master_dn, svc_desc): svc_mods = svc_desc[name] self.ldap.add_entry( - str(svc_dn), - _make_service_entry_mods( + _make_service_entry( + self.ldap, + svc_dn, enabled=svc_mods['enabled'], other_config=svc_mods.get('config', None))) @@ -311,16 +311,16 @@ def _add_svc_entries(self, master_dn, svc_desc): def _remove_svc_master_entries(self, master_dn): try: - entries = self.ldap.connection.search_s( - str(master_dn), ldap.SCOPE_SUBTREE + entries = self.ldap.get_entries( + master_dn, ldap.SCOPE_SUBTREE ) - except ldap.NO_SUCH_OBJECT: + except errors.NotFound: return if entries: - entries.sort(key=lambda x: len(x[0]), reverse=True) - for entry_dn, _attrs in entries: - self.ldap.del_entry(str(entry_dn)) + entries.sort(key=lambda x: len(x.dn), reverse=True) + for entry in entries: + self.ldap.delete_entry(entry) def _add_ipamaster_services(self, master_dn): """ @@ -329,19 +329,14 @@ def _add_ipamaster_services(self, master_dn): for svc_name in self.ipamaster_services: svc_dn = self.get_service_dn(svc_name, master_dn) try: - self.api.Backend.ldap2.get_entry(svc_dn) + self.ldap.get_entry(svc_dn) except errors.NotFound: - self.ldap.add_entry( - str(svc_dn), _make_service_entry_mods()) + self.ldap.add_entry(_make_service_entry(self.ldap, svc_dn)) def _add_members(self, dn, fqdn, member_attrs): - _entry, attrs = self.ldap.connection.search_s( - str(dn), ldap.SCOPE_SUBTREE)[0] - mods = [] - value = attrs.get('member', []) - mod_op = ldap.MOD_REPLACE - if not value: - mod_op = ldap.MOD_ADD + entry_attrs = self.ldap.get_entry(dn) + + value = entry_attrs.get('member', []) for a in member_attrs: @@ -352,20 +347,18 @@ def _add_members(self, dn, fqdn, member_attrs): result = self._add_service_entry(a, fqdn)['result'] value.append(str(result['dn'])) - mods.append( - (mod_op, 'member', value) - ) - - self.ldap.connection.modify_s(str(dn), mods) + entry_attrs['member'] = value + self.ldap.update_entry(entry_attrs) def _remove_members(self, dn, fqdn, member_attrs): - _entry, attrs = self.ldap.connection.search_s( - str(dn), ldap.SCOPE_SUBTREE)[0] - mods = [] + entry_attrs = self.ldap.get_entry(dn) + + value = set(entry_attrs.get('member', [])) + + if not value: + return + for a in member_attrs: - value = set(attrs.get('member', [])) - if not value: - continue if a == 'host': try: @@ -382,13 +375,11 @@ def _remove_members(self, dn, fqdn, member_attrs): pass self._del_service_entry(a, fqdn) - mods.append( - (ldap.MOD_REPLACE, 'member', list(value)) - ) + entry_attrs['member'] = list(value) try: - self.ldap.connection.modify_s(str(dn), mods) - except (ldap.NO_SUCH_OBJECT, ldap.NO_SUCH_ATTRIBUTE): + self.ldap.update_entry(entry_attrs) + except (errors.NotFound, errors.EmptyModlist): pass def _remove_test_host_attrs(self): @@ -397,7 +388,7 @@ def _remove_test_host_attrs(self): for attr_name in ( 'caRenewalMaster', 'dnssecKeyMaster', 'pkinitEnabled'): try: - svc_entry = self.api.Backend.ldap2.find_entry_by_attr( + svc_entry = self.ldap.find_entry_by_attr( 'ipaConfigString', attr_name, 'ipaConfigObject', base_dn=self.test_master_dn) except errors.NotFound: @@ -407,7 +398,7 @@ def _remove_test_host_attrs(self): (svc_entry.dn, list(svc_entry.get('ipaConfigString', []))) ) svc_entry[u'ipaConfigString'].remove(attr_name) - self.api.Backend.ldap2.update_entry(svc_entry) + self.ldap.update_entry(svc_entry) return original_dns_configs @@ -416,7 +407,7 @@ def _restore_test_host_attrs(self): try: svc_entry = self.api.Backend.ldap2.get_entry(dn) svc_entry['ipaConfigString'] = config - self.api.Backend.ldap2.update_entry(svc_entry) + self.ldap.update_entry(svc_entry) except (errors.NotFound, errors.EmptyModlist): continue @@ -427,7 +418,9 @@ def setup_data(self): # create master self.ldap.add_entry( - str(master_data.dn), _make_master_entry_mods( + _make_master_entry( + self.ldap, + master_data.dn, ca='CA' in master_data.services)) # now add service entries
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
