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

[Petr Viktorin]

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.

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

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

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

-- 
Petr³