[Freeipa-devel] [PATCH 0015] Add support for managing user auth types

[Nathaniel McCallum]

On Mon, 2013-09-09 at 15:35 +0200, Petr Viktorin wrote:
> On 09/05/2013 06:04 AM, Nathaniel McCallum wrote:
> > patch attached
> 
> Thanks, some comments below.
> 
> Git complains about trailing whitespace in the patch, please strip it.

Fixed.

> > freeipa-npmccallum-0015-Add-support-for-managing-user-auth-types.patch
> >
> >
> >>From 757436ccc431d26a3e62de830dad0b107a6c48ff Mon Sep 17 00:00:00 2001
> > From: Nathaniel McCallum<npmccallum at redhat.com>
> > Date: Wed, 4 Sep 2013 23:35:36 -0400
> > Subject: [PATCH] Add support for managing user auth types
> >
> > https://fedorahosted.org/freeipa/ticket/3368
> > ---
> >   ipalib/plugins/config.py | 16 ++++++++++++++++
> >   ipalib/plugins/user.py   | 32 ++++++++++++++++++++++----------
> >   2 files changed, 38 insertions(+), 10 deletions(-)
> >
> > diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py
> > index b9cf05016bf80cd48134cca5a50cdca7db423ca9..692ca22db70eb9a81a49eab6dc1e23284c8a9946 100644
> > @@ -210,6 +218,14 @@ class config_mod(LDAPUpdate):
> >
> >       def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
> >           assert isinstance(dn, DN)
> > +
> > +        if 'ipauserauthtype' in entry_attrs:
> > +            if 'objectclass' not in entry_attrs:
> > +                (_dn, _entry_attrs) = ldap.get_entry(dn, ['objectclass'])
> > +                entry_attrs['objectclass'] = _entry_attrs['objectclass']
> > +            if 'ipauserauthtypeclass' not in entry_attrs['objectclass']:
> > +                entry_attrs['objectclass'].append('ipauserauthtypeclass')
> 
> Shouldn't we rather add ipaUserAuthType to the ipaGuiConfig objectclass?

No. The ipaUserAuthTypeClass is generic and will be used several places.

> If not, we should still update ipaConfig on IPA update update rather 
> than here; install/updates/50-ipaconfig.update would be a good place.

Fixed.

> > diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
> > index 471981f48204209753eda2fb994d4c653dca0fa2..02f62120d281a873dfd9c21e1b855b112cca05a4 100644
> [...]
> > @@ -633,14 +640,19 @@ class user_mod(LDAPUpdate):
> >               entry_attrs['userpassword'] = ipa_generate_password(user_pwdchars)
> >               # save the password so it can be displayed in post_callback
> >               setattr(context, 'randompassword', entry_attrs['userpassword'])
> > +
> > +        if 'objectclass' not in entry_attrs:
> > +            (_dn, _entry_attrs) = ldap.get_entry(dn, ['objectclass'])
> > +            entry_attrs['objectclass'] = _entry_attrs['objectclass']
> 
> The framework is forcing some pretty ugly code here.
> I've filed https://fedorahosted.org/freeipa/ticket/3914 to simplify this 
> in the future.
> 
> 
> Just a note, it's no longer necessary to use (_dn, _entry_attrs) here; 
> ldap.get_entry() now returns a dict-like entry directly so you can use:
> 
>      _entry = ldap.get_entry(dn, ['objectclass'])
>      entry_attrs['objectclass'] = _entry['objectclass']
> 
> In fact, unpacking the entry into a tuple returns the DN and the entry 
> object itself. This:
>      (dn, entry) = ldap.get_entry(...)
> is exactly equivalent to:
>      entry = ldap.get_entry(...)
>      dn = entry.dn
> but the former is deprecated.

Fixed.

Nathaniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-npmccallum-0015-3-Add-support-for-managing-user-auth-types.patch
Type: text/x-patch
Size: 10235 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130918/d73e86d9/attachment.bin>