Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-04 Thread Jan Cholasta

Dne 4.8.2015 v 13:50 Martin Babinsky napsal(a):

On 08/04/2015 10:27 AM, Martin Babinsky wrote:

On 08/03/2015 06:41 PM, Martin Babinsky wrote:

On 08/03/2015 03:39 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):

On 08/03/2015 02:46 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute
only.

Since a more general fix is out of 4.2.1 scope, I have implemented
some
workarounds in pre/post callbacks of user-* commands in order to
enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.


Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.



If you do the rename in `convert_usercertificate_post` only when
'--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).


Oops, I meant "rename only when --raw is *not* specified".




2)

+self.obj.convert_usercertificate_pre(entry_attrs,
**options)

Rather than calling this directly from user_add, this should be
called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().


3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.


Honza












Attaching updated patch.





I have realized that this patch also fixes
https://fedorahosted.org/freeipa/ticket/5173 so I have added the link to
the commit message.




Attaching updated patch.



Thanks, ACK.

Pushed to:
master: 3257ac6b876e9e62cae58060c96c525ff0df1ae3
ipa-4-2: 8b3ed42d6b2bab57793b9921a672ed8994469109

--
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 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-04 Thread Martin Babinsky

On 08/04/2015 10:27 AM, Martin Babinsky wrote:

On 08/03/2015 06:41 PM, Martin Babinsky wrote:

On 08/03/2015 03:39 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):

On 08/03/2015 02:46 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute
only.

Since a more general fix is out of 4.2.1 scope, I have implemented
some
workarounds in pre/post callbacks of user-* commands in order to
enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.


Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.



If you do the rename in `convert_usercertificate_post` only when
'--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).


Oops, I meant "rename only when --raw is *not* specified".




2)

+self.obj.convert_usercertificate_pre(entry_attrs, **options)

Rather than calling this directly from user_add, this should be
called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().


3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.


Honza












Attaching updated patch.





I have realized that this patch also fixes
https://fedorahosted.org/freeipa/ticket/5173 so I have added the link to
the commit message.




Attaching updated patch.

--
Martin^3 Babinsky
From 39058badbc22fe92b2a16a362a12f082085fa5b2 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 3 Aug 2015 13:36:29 +0200
Subject: [PATCH] store certificates issued for user entries as
 userCertificate;binary

This patch forces the user management CLI command to store certificates as
userCertificate;binary attribute. The code to retrieve of user information was
modified to enable outputting of userCertificate;binary attribute to the
command line.

The modification also fixes https://fedorahosted.org/freeipa/ticket/5173
---
 ipalib/plugins/baseuser.py | 23 ++-
 ipalib/plugins/user.py | 21 +
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/ipalib/plugins/baseuser.py b/ipalib/plugins/baseuser.py
index bd66cf5a3e3a4e6c18d1a54408f969668c834fab..5eede7a98e7e6d9bf31a6d553b0ce60c7cf3527c 100644
--- a/ipalib/plugins/baseuser.py
+++ b/ipalib/plugins/baseuser.py
@@ -187,7 +187,7 @@ class baseuser(LDAPObject):
 'telephonenumber', 'title', 'memberof', 'nsaccountlock',
 'memberofindirect', 'ipauserauthtype', 'userclass',
 'ipatokenradiusconfiglink', 'ipatokenradiususername',
-'krbprincipalexpiration', 'usercertificate',
+'krbprincipalexpiration', 'usercertificate;binary',
 ]
 search_display_attributes = [
 'uid', 'givenname', 'sn', 'homedirectory', 'loginshell',
@@ -465,10 +465,27 @@ class baseuser(LDAPObject):
 assert isinstance(user, DN)
 return self._user_status(user, DN(self.delete_container_dn, api.env.basedn))
 
+def convert_usercertificate_pre(self, entry_attrs):
+if 'usercertificate' in entry_attrs:
+entry_attrs['usercertificate;binary'] = entry_attrs.pop(
+'usercertificate')
+
+def convert_usercertificate_post(self, entry_attrs, **options):
+if 'usercertificate;binary' in entry_attrs:
+entry_attrs['usercertificate'] = entry_attrs.pop(
+'usercertificate;binary')
+
 class baseuser_add(LDAPCreate):
 """
 Prototype command plugin to be implemented by real plugin
 """
+def pre_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_pre(entry_attrs)
+
+def post_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_post(entry_attrs, **options)
 
 class baseuser_del(LDAPDelete):
 """
@@ -542,6 +559,7 @@ class baseuser_mod(LDAPUpdate):
 self.check_userpassword(entry_attrs, **options)
 
 self.check_objectclass(ldap, dn, entry_attrs)
+self.obj.convert_usercertificate_pre(entry_attrs)
 
 def post_common_callback(self, ldap, dn, entry_attrs, **options

Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-04 Thread Martin Babinsky

On 08/03/2015 06:41 PM, Martin Babinsky wrote:

On 08/03/2015 03:39 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):

On 08/03/2015 02:46 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.

Since a more general fix is out of 4.2.1 scope, I have implemented
some
workarounds in pre/post callbacks of user-* commands in order to
enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.


Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.



If you do the rename in `convert_usercertificate_post` only when '--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).


Oops, I meant "rename only when --raw is *not* specified".




2)

+self.obj.convert_usercertificate_pre(entry_attrs, **options)

Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().


3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.


Honza












Attaching updated patch.





I have realized that this patch also fixes 
https://fedorahosted.org/freeipa/ticket/5173 so I have added the link to 
the commit message.


--
Martin^3 Babinsky
From a244468cca764b959dff7c29fa39d6eddb4d9417 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 3 Aug 2015 13:36:29 +0200
Subject: [PATCH] store certificates issued for user entries as
 userCertificate;binary

This patch forces the user management CLI command to store certificates as
userCertificate;binary attribute. The code to retrieve of user information was
modified to enable outputting of userCertificate;binary attribute to the
command line.

The modification also fixes https://fedorahosted.org/freeipa/ticket/5173
---
 ipalib/plugins/baseuser.py | 24 +++-
 ipalib/plugins/user.py | 21 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/ipalib/plugins/baseuser.py b/ipalib/plugins/baseuser.py
index bd66cf5a3e3a4e6c18d1a54408f969668c834fab..bce427c02774f9dde2fe7d0925d04124add0daa4 100644
--- a/ipalib/plugins/baseuser.py
+++ b/ipalib/plugins/baseuser.py
@@ -187,7 +187,7 @@ class baseuser(LDAPObject):
 'telephonenumber', 'title', 'memberof', 'nsaccountlock',
 'memberofindirect', 'ipauserauthtype', 'userclass',
 'ipatokenradiusconfiglink', 'ipatokenradiususername',
-'krbprincipalexpiration', 'usercertificate',
+'krbprincipalexpiration', 'usercertificate;binary',
 ]
 search_display_attributes = [
 'uid', 'givenname', 'sn', 'homedirectory', 'loginshell',
@@ -465,10 +465,28 @@ class baseuser(LDAPObject):
 assert isinstance(user, DN)
 return self._user_status(user, DN(self.delete_container_dn, api.env.basedn))
 
+def convert_usercertificate_pre(self, entry_attrs):
+if 'usercertificate' in entry_attrs:
+entry_attrs['usercertificate;binary'] = entry_attrs.pop(
+'usercertificate')
+
+def convert_usercertificate_post(self, entry_attrs, **options):
+if ('usercertificate;binary' in entry_attrs
+and not options.get('raw', False)):
+entry_attrs['usercertificate'] = entry_attrs.pop(
+'usercertificate;binary')
+
 class baseuser_add(LDAPCreate):
 """
 Prototype command plugin to be implemented by real plugin
 """
+def pre_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_pre(entry_attrs)
+
+def post_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_post(entry_attrs, **options)
 
 class baseuser_del(LDAPDelete):
 """
@@ -542,6 +560,7 @@ class baseuser_mod(LDAPUpdate):
 self.check_userpassword(entry_attrs, **options)
 
 self.check_objectclass(ldap, dn, entry_attrs)
+self.obj.convert_usercertificate_pre(entry_attrs)
 
 def post_common_callback(self, ldap, dn, entry_attrs, **options):
 assert i

Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-03 Thread Martin Babinsky

On 08/03/2015 03:39 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):

On 08/03/2015 02:46 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.

Since a more general fix is out of 4.2.1 scope, I have implemented
some
workarounds in pre/post callbacks of user-* commands in order to
enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.


Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.



If you do the rename in `convert_usercertificate_post` only when '--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).


Oops, I meant "rename only when --raw is *not* specified".




2)

+self.obj.convert_usercertificate_pre(entry_attrs, **options)

Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().


3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.


Honza












Attaching updated patch.

--
Martin^3 Babinsky
From 76b3dc3fcd213ff8b6e16122f5c9220d52bbbd11 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 3 Aug 2015 13:36:29 +0200
Subject: [PATCH] store certificates issued for user entries as
 userCertificate;binary

This patch forces the user management CLI command to store certificates as
userCertificate;binary attribute. The code to retrieve of user information was
modified to enable outputting of userCertificate;binary attribute to the
command line.
---
 ipalib/plugins/baseuser.py | 24 +++-
 ipalib/plugins/user.py | 21 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/ipalib/plugins/baseuser.py b/ipalib/plugins/baseuser.py
index bd66cf5a3e3a4e6c18d1a54408f969668c834fab..bce427c02774f9dde2fe7d0925d04124add0daa4 100644
--- a/ipalib/plugins/baseuser.py
+++ b/ipalib/plugins/baseuser.py
@@ -187,7 +187,7 @@ class baseuser(LDAPObject):
 'telephonenumber', 'title', 'memberof', 'nsaccountlock',
 'memberofindirect', 'ipauserauthtype', 'userclass',
 'ipatokenradiusconfiglink', 'ipatokenradiususername',
-'krbprincipalexpiration', 'usercertificate',
+'krbprincipalexpiration', 'usercertificate;binary',
 ]
 search_display_attributes = [
 'uid', 'givenname', 'sn', 'homedirectory', 'loginshell',
@@ -465,10 +465,28 @@ class baseuser(LDAPObject):
 assert isinstance(user, DN)
 return self._user_status(user, DN(self.delete_container_dn, api.env.basedn))
 
+def convert_usercertificate_pre(self, entry_attrs):
+if 'usercertificate' in entry_attrs:
+entry_attrs['usercertificate;binary'] = entry_attrs.pop(
+'usercertificate')
+
+def convert_usercertificate_post(self, entry_attrs, **options):
+if ('usercertificate;binary' in entry_attrs
+and not options.get('raw', False)):
+entry_attrs['usercertificate'] = entry_attrs.pop(
+'usercertificate;binary')
+
 class baseuser_add(LDAPCreate):
 """
 Prototype command plugin to be implemented by real plugin
 """
+def pre_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_pre(entry_attrs)
+
+def post_common_callback(self, ldap, dn, entry_attrs, **options):
+assert isinstance(dn, DN)
+self.obj.convert_usercertificate_post(entry_attrs, **options)
 
 class baseuser_del(LDAPDelete):
 """
@@ -542,6 +560,7 @@ class baseuser_mod(LDAPUpdate):
 self.check_userpassword(entry_attrs, **options)
 
 self.check_objectclass(ldap, dn, entry_attrs)
+self.obj.convert_usercertificate_pre(entry_attrs)
 
 def post_common_callback(self, ldap, dn, entry_attrs, **options):
 assert isinstance(dn, DN)
@@ -554,6 +573,7 @@ class baseuser_mod(LDAPUpdate):
 convert_nsaccountlock(entry_attrs)
 self.obj.convert_manager(entry_attrs, **options)
 self.obj.get_password_attributes(ldap, dn, entry_attrs)
+self.obj.convert_use

Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-03 Thread Jan Cholasta

Dne 3.8.2015 v 14:58 Martin Babinsky napsal(a):

On 08/03/2015 02:46 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.

Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to
enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.


Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.



If you do the rename in `convert_usercertificate_post` only when '--raw'
is specified, then you get no certificate displayed when you do `ipa
user-show` on user with userCertificate;binary attribute. Is this
intended? (Keep in mind that `convert_usercertificate_post` should be
called in post-callback when returning results back to user/client).


Oops, I meant "rename only when --raw is *not* specified".




2)

+self.obj.convert_usercertificate_pre(entry_attrs, **options)

Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().


3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.


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


Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-03 Thread Martin Babinsky

On 08/03/2015 02:46 PM, Jan Cholasta wrote:

Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.

Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.


Actually, the attribute should be always renamed in
convert_usercertificate_pre, otherwise we would modify the wrong
attribute. In convert_usercertificate_post, it should actually be
renamed only when --raw is specified.



If you do the rename in `convert_usercertificate_post` only when '--raw' 
is specified, then you get no certificate displayed when you do `ipa 
user-show` on user with userCertificate;binary attribute. Is this 
intended? (Keep in mind that `convert_usercertificate_post` should be 
called in post-callback when returning results back to user/client).



2)

+self.obj.convert_usercertificate_pre(entry_attrs, **options)

Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().


3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.


Honza







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


Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-03 Thread Jan Cholasta

Dne 3.8.2015 v 14:14 Jan Cholasta napsal(a):

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.

Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all.
Same for convert_usercertificate_post.


Actually, the attribute should be always renamed in 
convert_usercertificate_pre, otherwise we would modify the wrong 
attribute. In convert_usercertificate_post, it should actually be 
renamed only when --raw is specified.





2)

+self.obj.convert_usercertificate_pre(entry_attrs, **options)

Rather than calling this directly from user_add, this should be called
from baseuser.pre_common_callback(), which should be called from
user_add.post_callback().


3) IMO you should change user_{add,remove}_cert to call
baseuser.convert_usercertificate_{pre,post} as well, to avoid code
duplication.


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


Re: [Freeipa-devel] [PATCH 0052] store user certificates in 'userCertificate; binary' attributes

2015-08-03 Thread Jan Cholasta

Hi,

Dne 3.8.2015 v 14:00 Martin Babinsky napsal(a):

This patch fixes the inconsistency between storing certificates in
'userCertificate'/'userCertificate;binary' attribute for the user
entries: the certificate must be stored in the latter attribute only.

Since a more general fix is out of 4.2.1 scope, I have implemented some
workarounds in pre/post callbacks of user-* commands in order to enforce
this behavior.


1)

+def convert_usercertificate_pre(self, entry_attrs, **options):
+if options.get('all', False):
+return

We don't want to do any renaming when --raw is specified, not --all. 
Same for convert_usercertificate_post.



2)

+self.obj.convert_usercertificate_pre(entry_attrs, **options)

Rather than calling this directly from user_add, this should be called 
from baseuser.pre_common_callback(), which should be called from 
user_add.post_callback().



3) IMO you should change user_{add,remove}_cert to call 
baseuser.convert_usercertificate_{pre,post} as well, to avoid code 
duplication.



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