[Freeipa-devel] [PATCH] 0040 certprofile: prevent rename (modrdn)

[Simo Sorce]

On Tue, 2015-08-25 at 21:49 +1000, Fraser Tweedale wrote:
> On Tue, Aug 25, 2015 at 01:39:42PM +0300, Alexander Bokovoy wrote:
> > On Tue, 25 Aug 2015, Petr Vobornik wrote:
> > >On 08/25/2015 07:37 AM, Alexander Bokovoy wrote:
> > >>On Tue, 25 Aug 2015, Fraser Tweedale wrote:
> > >>>The attached patch fixes
> > >>>https://fedorahosted.org/freeipa/ticket/5247.
> > >>>
> > >>>Thanks,
> > >>>Fraser
> > >>
> > >>>From 2cb4ab6eeedccc3471ed9bf983add4687ecd5c1a Mon Sep 17 00:00:00 2001
> > >>>From: Fraser Tweedale <ftweedal at redhat.com>
> > >>>Date: Mon, 24 Aug 2015 20:25:10 -0400
> > >>>Subject: [PATCH] certprofile: prevent rename (modrdn)
> > >>>
> > >>>Fixes: https://fedorahosted.org/freeipa/ticket/5247
> > >>>---
> > >>>ipalib/plugins/certprofile.py | 5 +++--
> > >>>1 file changed, 3 insertions(+), 2 deletions(-)
> > >>>
> > >>>diff --git a/ipalib/plugins/certprofile.py
> > >>>b/ipalib/plugins/certprofile.py
> > >>>index
> > >>>007cc543406b7e5705fd7474f3685cd6a9ce6aca..a0ffa38608400860994c771e4eba81304ead27be
> > >>>100644
> > >>>--- a/ipalib/plugins/certprofile.py
> > >>>+++ b/ipalib/plugins/certprofile.py
> > >>>@@ -323,8 +323,9 @@ class certprofile_mod(LDAPUpdate):
> > >>>   def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
> > >>>**options):
> > >>>       ca_enabled_check()
> > >>>       # Once a profile id is set it cannot be changed
> > >>>-        if 'cn' in entry_attrs:
> > >>>-            raise errors.ACIError(info=_('cn is immutable'))
> > >>>+        if 'rename' in options or 'cn' in entry_attrs:
> > >>>+            raise errors.ProtectedEntryError(label='certprofile',
> > >>>key=keys[0],
> > >>>+                reason=_('Certificate profiles cannot be renamed'))
> > >>>       if 'file' in options:
> > >>>           with self.api.Backend.ra_certprofile as profile_api:
> > >>>               profile_api.disable_profile(keys[0])
> > >>ACK
> > >
> > >can't we fix it by removing `rdn_is_primary_key = True`?
> > >
> > >That would also remove the --rename option. Yes it's an API change but if
> > >rename is forbidden than the option should not be even there, just the
> > >result error will different.
> > Well, that is another option, yes. Perhaps even a better one -- we have
> > plenty of places where rdn_is_primary_key is not actually used.
> > 
> I filed a ticket for this: https://fedorahosted.org/freeipa/ticket/5254
> 
> There are a bunch of commands that have this situation - not just
> certprofile - so if we're going to break API in one place IMO we
> should do them all at once.

Why do we need to break the API ?
Just deny it.

Simo.


-- 
Simo Sorce * Red Hat, Inc * New York