Re: [Freeipa-devel] [PATCH 0282] Prevent to rename certprofile profile id

2015-07-14 Thread Jan Cholasta 

Dne 10.7.2015 v 12:52 Simo Sorce napsal(a):

On Fri, 2015-07-10 at 11:28 +0200, Jan Cholasta wrote:

Dne 10.7.2015 v 11:10 Simo Sorce napsal(a):

On Fri, 2015-07-10 at 11:01 +0200, Jan Cholasta wrote:

Dne 10.7.2015 v 10:59 Jan Cholasta napsal(a):

Dne 10.7.2015 v 10:43 Martin Basti napsal(a):

On 10/07/15 07:29, Jan Cholasta wrote:

Hi,

Dne 9.7.2015 v 17:21 Martin Basti napsal(a):

https://fedorahosted.org/freeipa/ticket/5074

Patch attached.


NACK, you should remove the --rename option from certprofile-mod. You
can do it by removing "rdn_is_primary_key = True" from certprofile.

Honza


Updated patch attached.



What I meant was remove --rename *and* do the check from your previous
patch.

Anyway, I didn't realize we already released IPA with certprofile and
removing --rename would be a backward incompatible change, so I think
it's better to just keep it.

So ACK on the original patch.



Pushed to master: 67b2b3408579814f7ff307cfd20bc4250edbea15


I see no LDAP ACI that prevents a rename though, without that an admin
can simply issue a modrdn operation. If it is critical for us to not
allow renames we should rather have an ACI that prohibits them.


AFAIK there is no ACI to prevent renaming hosts (the check in this patch
is copied from the host plugin) or users either and so far nobody
complained. I'm not saying this is right, but the patch is consistent
with existing code.


Renaming users is explicitly allowed, renaming hosts is something we may
want to prevent too. Maybe we should add a ticket to take care of these
things ?




Forgot to push this patch to ipa-4-2:

Pushed to ipa-4-2: 62e30d007275a3051370006a7546a5b3158f9686

--
Jan Cholasta

--
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 0282] Prevent to rename certprofile profile id

2015-07-10 Thread Simo Sorce 
On Fri, 2015-07-10 at 11:28 +0200, Jan Cholasta wrote:
> Dne 10.7.2015 v 11:10 Simo Sorce napsal(a):
> > On Fri, 2015-07-10 at 11:01 +0200, Jan Cholasta wrote:
> >> Dne 10.7.2015 v 10:59 Jan Cholasta napsal(a):
> >>> Dne 10.7.2015 v 10:43 Martin Basti napsal(a):
>  On 10/07/15 07:29, Jan Cholasta wrote:
> > Hi,
> >
> > Dne 9.7.2015 v 17:21 Martin Basti napsal(a):
> >> https://fedorahosted.org/freeipa/ticket/5074
> >>
> >> Patch attached.
> >
> > NACK, you should remove the --rename option from certprofile-mod. You
> > can do it by removing "rdn_is_primary_key = True" from certprofile.
> >
> > Honza
> >
>  Updated patch attached.
> 
> >>>
> >>> What I meant was remove --rename *and* do the check from your previous
> >>> patch.
> >>>
> >>> Anyway, I didn't realize we already released IPA with certprofile and
> >>> removing --rename would be a backward incompatible change, so I think
> >>> it's better to just keep it.
> >>>
> >>> So ACK on the original patch.
> >>>
> >>
> >> Pushed to master: 67b2b3408579814f7ff307cfd20bc4250edbea15
> >
> > I see no LDAP ACI that prevents a rename though, without that an admin
> > can simply issue a modrdn operation. If it is critical for us to not
> > allow renames we should rather have an ACI that prohibits them.
> 
> AFAIK there is no ACI to prevent renaming hosts (the check in this patch 
> is copied from the host plugin) or users either and so far nobody 
> complained. I'm not saying this is right, but the patch is consistent 
> with existing code.

Renaming users is explicitly allowed, renaming hosts is something we may
want to prevent too. Maybe we should add a ticket to take care of these
things ?

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 0282] Prevent to rename certprofile profile id

2015-07-10 Thread Jan Cholasta 

Dne 10.7.2015 v 11:10 Simo Sorce napsal(a):

On Fri, 2015-07-10 at 11:01 +0200, Jan Cholasta wrote:

Dne 10.7.2015 v 10:59 Jan Cholasta napsal(a):

Dne 10.7.2015 v 10:43 Martin Basti napsal(a):

On 10/07/15 07:29, Jan Cholasta wrote:

Hi,

Dne 9.7.2015 v 17:21 Martin Basti napsal(a):

https://fedorahosted.org/freeipa/ticket/5074

Patch attached.


NACK, you should remove the --rename option from certprofile-mod. You
can do it by removing "rdn_is_primary_key = True" from certprofile.

Honza


Updated patch attached.



What I meant was remove --rename *and* do the check from your previous
patch.

Anyway, I didn't realize we already released IPA with certprofile and
removing --rename would be a backward incompatible change, so I think
it's better to just keep it.

So ACK on the original patch.



Pushed to master: 67b2b3408579814f7ff307cfd20bc4250edbea15


I see no LDAP ACI that prevents a rename though, without that an admin
can simply issue a modrdn operation. If it is critical for us to not
allow renames we should rather have an ACI that prohibits them.


AFAIK there is no ACI to prevent renaming hosts (the check in this patch 
is copied from the host plugin) or users either and so far nobody 
complained. I'm not saying this is right, but the patch is consistent 
with existing code.


--
Jan Cholasta

--
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 0282] Prevent to rename certprofile profile id

2015-07-10 Thread Simo Sorce 
On Fri, 2015-07-10 at 11:01 +0200, Jan Cholasta wrote:
> Dne 10.7.2015 v 10:59 Jan Cholasta napsal(a):
> > Dne 10.7.2015 v 10:43 Martin Basti napsal(a):
> >> On 10/07/15 07:29, Jan Cholasta wrote:
> >>> Hi,
> >>>
> >>> Dne 9.7.2015 v 17:21 Martin Basti napsal(a):
>  https://fedorahosted.org/freeipa/ticket/5074
> 
>  Patch attached.
> >>>
> >>> NACK, you should remove the --rename option from certprofile-mod. You
> >>> can do it by removing "rdn_is_primary_key = True" from certprofile.
> >>>
> >>> Honza
> >>>
> >> Updated patch attached.
> >>
> >
> > What I meant was remove --rename *and* do the check from your previous
> > patch.
> >
> > Anyway, I didn't realize we already released IPA with certprofile and
> > removing --rename would be a backward incompatible change, so I think
> > it's better to just keep it.
> >
> > So ACK on the original patch.
> >
> 
> Pushed to master: 67b2b3408579814f7ff307cfd20bc4250edbea15

I see no LDAP ACI that prevents a rename though, without that an admin
can simply issue a modrdn operation. If it is critical for us to not
allow renames we should rather have an ACI that prohibits them.

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 0282] Prevent to rename certprofile profile id

2015-07-10 Thread Jan Cholasta 

Dne 10.7.2015 v 10:59 Jan Cholasta napsal(a):

Dne 10.7.2015 v 10:43 Martin Basti napsal(a):

On 10/07/15 07:29, Jan Cholasta wrote:

Hi,

Dne 9.7.2015 v 17:21 Martin Basti napsal(a):

https://fedorahosted.org/freeipa/ticket/5074

Patch attached.


NACK, you should remove the --rename option from certprofile-mod. You
can do it by removing "rdn_is_primary_key = True" from certprofile.

Honza


Updated patch attached.



What I meant was remove --rename *and* do the check from your previous
patch.

Anyway, I didn't realize we already released IPA with certprofile and
removing --rename would be a backward incompatible change, so I think
it's better to just keep it.

So ACK on the original patch.



Pushed to master: 67b2b3408579814f7ff307cfd20bc4250edbea15

--
Jan Cholasta

--
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 0282] Prevent to rename certprofile profile id

2015-07-10 Thread Jan Cholasta 

Dne 10.7.2015 v 10:43 Martin Basti napsal(a):

On 10/07/15 07:29, Jan Cholasta wrote:

Hi,

Dne 9.7.2015 v 17:21 Martin Basti napsal(a):

https://fedorahosted.org/freeipa/ticket/5074

Patch attached.


NACK, you should remove the --rename option from certprofile-mod. You
can do it by removing "rdn_is_primary_key = True" from certprofile.

Honza


Updated patch attached.



What I meant was remove --rename *and* do the check from your previous 
patch.


Anyway, I didn't realize we already released IPA with certprofile and 
removing --rename would be a backward incompatible change, so I think 
it's better to just keep it.


So ACK on the original patch.

--
Jan Cholasta

--
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 0282] Prevent to rename certprofile profile id

2015-07-10 Thread Martin Basti 

On 10/07/15 07:29, Jan Cholasta wrote:

Hi,

Dne 9.7.2015 v 17:21 Martin Basti napsal(a):

https://fedorahosted.org/freeipa/ticket/5074

Patch attached.


NACK, you should remove the --rename option from certprofile-mod. You 
can do it by removing "rdn_is_primary_key = True" from certprofile.


Honza


Updated patch attached.

--
Martin Basti

From e395e3488fbd0eb80adbd2b3f9cbb82be331e5d9 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 9 Jul 2015 17:17:21 +0200
Subject: [PATCH] Prevent to rename certprofile profile id

https://fedorahosted.org/freeipa/ticket/5074
---
 ipalib/plugins/certprofile.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
index 6f9a41875b2a276b521219156e630817a9c41fdc..f8091a8e87038fe4ffed3f20f434873df8c1e992 100644
--- a/ipalib/plugins/certprofile.py
+++ b/ipalib/plugins/certprofile.py
@@ -92,7 +92,6 @@ class certprofile(LDAPObject):
 search_attributes = [
 'cn', 'description', 'ipacertprofilestoreissued'
 ]
-rdn_is_primary_key = True
 label = _('Certificate Profiles')
 label_singular = _('Certificate Profile')
 
-- 
2.4.3

-- 
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 0282] Prevent to rename certprofile profile id

2015-07-09 Thread Jan Cholasta 

Hi,

Dne 9.7.2015 v 17:21 Martin Basti napsal(a):

https://fedorahosted.org/freeipa/ticket/5074

Patch attached.


NACK, you should remove the --rename option from certprofile-mod. You 
can do it by removing "rdn_is_primary_key = True" from certprofile.


Honza

--
Jan Cholasta

--
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