Re: [Freeipa-devel] [PATCH 0165] IPA API: Do not force setting krbCanonicalName on newly created entries

[Martin Babinsky]

On 06/24/2016 04:07 PM, Martin Babinsky wrote:

This patch reverts commits 705f66f7490c64de1adc129221b31927616c485 and
06d945a04607dc36e25af78688b4295420489fb9 responsible for
https://fedorahosted.org/freeipa/ticket/5996

This should unblock replica promotion.



self-NACK, disregard this patch, it should not be necessary to revert 
the whole commit


--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 0165] IPA API: Do not force setting krbCanonicalName on newly created entries

[Martin Babinsky]

This patch reverts commits 705f66f7490c64de1adc129221b31927616c485 and 
06d945a04607dc36e25af78688b4295420489fb9 responsible for 
https://fedorahosted.org/freeipa/ticket/5996


This should unblock replica promotion.

--
Martin^3 Babinsky
From ff2e26a41189d4192255e5244b5a5cb993b9b258 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 24 Jun 2016 14:31:31 +0200
Subject: [PATCH]  IPA API: Do not force setting krbCanonicalName on newly
 created entries

Commit 705f66f7490c64de1adc129221b31927616c485d forces unconditional setting
of krbCanonicaName attribute when adding new users, hosts, and services
through management framework. This may not always be desirable and can even
break replica promotion as the member of ipaservers group cannot create its
service principals due to misssing ACIs on master.

It is better to handle krbCanonicalName only during modification of entries
via API and let their creation be backwards compatible with previous IPA
servers.

Creation of entries by other means (e.g. kadmin.local) is done using
root/Directory Manager privileges so they are not subject to ACI evaluation.

This patch revert this commit and commit
06d945a04607dc36e25af78688b4295420489fb9 modifying tests.

https://fedorahosted.org/freeipa/ticket/5996
---
 ipalib/util.py   | 11 ---
 ipaserver/plugins/baseuser.py|  2 --
 ipaserver/plugins/host.py|  2 --
 ipaserver/plugins/service.py | 10 --
 ipaserver/plugins/stageuser.py   |  3 ---
 ipatests/test_xmlrpc/objectclasses.py|  1 +
 ipatests/test_xmlrpc/test_host_plugin.py |  1 -
 ipatests/test_xmlrpc/test_service_plugin.py  |  9 ++---
 ipatests/test_xmlrpc/test_user_plugin.py |  1 -
 ipatests/test_xmlrpc/tracker/host_plugin.py  |  4 +---
 ipatests/test_xmlrpc/tracker/stageuser_plugin.py |  5 +
 ipatests/test_xmlrpc/tracker/user_plugin.py  |  5 ++---
 12 files changed, 15 insertions(+), 39 deletions(-)

diff --git a/ipalib/util.py b/ipalib/util.py
index 67865eb04e85ffaf34475f0324cc9cc0703cf45b..8435f7ab6e8fd66caacb1641a4ef5409382637c5 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -901,14 +901,3 @@ def validate_bind_forwarder(ugettext, forwarder):
 return _('%(port)s is not a valid port' % dict(port=port))
 
 return None
-
-
-def set_krbcanonicalname(entry_attrs):
-objectclasses = set(i.lower() for i in entry_attrs['objectclass'])
-
-if 'krbprincipalaux' not in objectclasses:
-return
-
-if ('krbprincipalname' in entry_attrs
-and 'krbcanonicalname' not in entry_attrs):
-entry_attrs['krbcanonicalname'] = entry_attrs['krbprincipalname']
diff --git a/ipaserver/plugins/baseuser.py b/ipaserver/plugins/baseuser.py
index 7bb2e8a6360a6d04eaf0390239eafa0763f9d57c..bbea403d9782fcbe486af07215ad67ee83eb9b58 100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -39,7 +39,6 @@ from ipalib.util import (
 remove_sshpubkey_from_output_post,
 remove_sshpubkey_from_output_list_post,
 add_sshpubkey_to_attrs_pre,
-set_krbcanonicalname
 )
 
 if six.PY3:
@@ -498,7 +497,6 @@ class baseuser_add(LDAPCreate):
 def pre_common_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
 **options):
 assert isinstance(dn, DN)
-set_krbcanonicalname(entry_attrs)
 self.obj.convert_usercertificate_pre(entry_attrs)
 
 def post_common_callback(self, ldap, dn, entry_attrs, *keys, **options):
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index 0072431de3f130d09066100f12d9fcb34e9fb96b..919927c3dd4828810131f1ce7748a15064b1566b 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -50,7 +50,6 @@ from ipalib.util import (normalize_sshpubkey, validate_sshpubkey_no_options,
 remove_sshpubkey_from_output_list_post,
 normalize_hostname,
 hostname_validator,
-set_krbcanonicalname
 )
 from ipapython.ipautil import ipa_generate_password, CheckedIPAddress
 from ipapython.dnsutil import DNSName
@@ -633,7 +632,6 @@ class host_add(LDAPCreate):
 entry_attrs['objectclass'].append('krbprincipalaux')
 if 'krbprincipal' not in entry_attrs['objectclass']:
 entry_attrs['objectclass'].append('krbprincipal')
-set_krbcanonicalname(entry_attrs)
 else:
 if 'krbprincipalaux' in entry_attrs['objectclass']:
 entry_attrs['objectclass'].remove('krbprincipalaux')
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index cb9952d4479a543321999269cb4bd6ace0714436..24031eb429c1946f2ec730683f46c9cef35910ed 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -576,8 +576,14 @@ class service_add(LDAPCreate):
 if not 'managedby' in entry_attrs:
 entry_attrs['managedby'] =