Re: [Freeipa-devel] [PATCH] 0040 certprofile: prevent rename (modrdn)
On 08/25/2015 04:19 PM, Simo Sorce wrote:
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
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.
OK, original patch pushed to:
master: 5c7d6a6a31daca8bf92d85d8c1895279be84c888
ipa-4-2: d943bf09799e6faf2dd83f630bcfd6f99575c5c8
--
Petr Vobornik
--
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
Re: [Freeipa-devel] [PATCH] 0040 certprofile: prevent rename (modrdn)
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
> > >>>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
--
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
Re: [Freeipa-devel] [PATCH] 0040 certprofile: prevent rename (modrdn)
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
> >>>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.
--
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
Re: [Freeipa-devel] [PATCH] 0040 certprofile: prevent rename (modrdn)
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
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.
--
/ Alexander Bokovoy
--
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
Re: [Freeipa-devel] [PATCH] 0040 certprofile: prevent rename (modrdn)
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
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.
--
Petr Vobornik
--
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
Re: [Freeipa-devel] [PATCH] 0040 certprofile: prevent rename (modrdn)
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
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
--
/ Alexander Bokovoy
--
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
