On 09/18/2013 09:51 PM, Nathaniel McCallum wrote:
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<npmccal...@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.

Great, we're getting close!

Please send patches in `git format-patch` style (they include commit info).
Also, please bump the API revision in VERSION since API.txt was changed.

When adding the objectclass in user, it is possible that the user doesn't exist. You should call handle_not_found in this case so the appropriate error message is generated.
I ended up doing this for testing, squash in the patch if you want.

There's another test failure when trying to rename a manager user. I didn't investigate in detail why that happens.

I'm attaching the tests I used, do they look OK?

--
PetrĀ³

From 4582b32ba468dbae5321c617b3ef403dd557e54a Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Mon, 23 Sep 2013 15:02:07 +0200
Subject: [PATCH] Add tests for user auth type management

https://fedorahosted.org/freeipa/ticket/3368
---
 ipatests/test_xmlrpc/test_config_plugin.py | 30 +++++++++++++
 ipatests/test_xmlrpc/test_user_plugin.py   | 72 ++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_config_plugin.py b/ipatests/test_xmlrpc/test_config_plugin.py
index 3d9a31daf38b4b8a28febee90f5812cf78538ee7..56baf7f0bd5fb45d4d8c93028a875a47e84a3807 100644
--- a/ipatests/test_xmlrpc/test_config_plugin.py
+++ b/ipatests/test_xmlrpc/test_config_plugin.py
@@ -118,4 +118,34 @@ class test_config(Declarative):
                 error='SELinux user map default user not in order list'),
         ),
 
+        dict(
+            desc='Set user auth type',
+            command=('config_mod', [], dict(ipauserauthtype=u'password')),
+            expected=dict(
+                    result=lambda d: d['ipauserauthtype'] == (u'password',),
+                    value=u'',
+                    summary=None,
+                ),
+        ),
+
+        dict(
+            desc='Check user auth type',
+            command=('config_show', [], {}),
+            expected=dict(
+                    result=lambda d: d['ipauserauthtype'] == (u'password',),
+                    value=u'',
+                    summary=None,
+                ),
+        ),
+
+        dict(
+            desc='Unset user auth type',
+            command=('config_mod', [], dict(ipauserauthtype=None)),
+            expected=dict(
+                    result=lambda d: 'ipauserauthtype' not in d,
+                    value=u'',
+                    summary=None,
+                ),
+        ),
+
     ]
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 98e1965a4fbd3c2e77363495d0391be580bd0805..02d551cf00c832c2e3b63a03696073a1466c9406 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -1840,4 +1840,76 @@ class test_user(Declarative):
             extra_check = upg_check,
         ),
 
+        dict(
+            desc='Set ipauserauthtype for "%s"' % user1,
+            command=('user_mod', [user1], dict(ipauserauthtype=u'password')),
+            expected=dict(
+                result=dict(
+                    givenname=[u'Test'],
+                    homedirectory=[u'/home/tuser1'],
+                    loginshell=[u'/bin/sh'],
+                    sn=[u'User1'],
+                    uid=[user1],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[fuzzy_digits],
+                    mail=[u'%s@%s' % (user1, api.env.domain)],
+                    memberof_group=[u'ipausers'],
+                    nsaccountlock=False,
+                    has_keytab=False,
+                    has_password=False,
+                    ipauserauthtype=[u'password'],
+                ),
+                value=user1,
+                summary='Modified user "%s"' % user1,
+            ),
+        ),
+
+        dict(
+            desc='Retrieve "%s" to verify ipauserauthtype' % user1,
+            command=('user_show', [user1], {}),
+            expected=dict(
+                result=dict(
+                    dn=get_user_dn(user1),
+                    givenname=[u'Test'],
+                    homedirectory=[u'/home/tuser1'],
+                    loginshell=[u'/bin/sh'],
+                    sn=[u'User1'],
+                    uid=[user1],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[fuzzy_digits],
+                    mail=[u'%s@%s' % (user1, api.env.domain)],
+                    memberof_group=[u'ipausers'],
+                    nsaccountlock=False,
+                    has_keytab=False,
+                    has_password=False,
+                    ipauserauthtype=[u'password'],
+                ),
+                value=user1,
+                summary=None,
+            ),
+        ),
+
+        dict(
+            desc='Unset ipauserauthtype for "%s"' % user1,
+            command=('user_mod', [user1], dict(ipauserauthtype=None)),
+            expected=dict(
+                result=dict(
+                    givenname=[u'Test'],
+                    homedirectory=[u'/home/tuser1'],
+                    loginshell=[u'/bin/sh'],
+                    sn=[u'User1'],
+                    uid=[user1],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[fuzzy_digits],
+                    mail=[u'%s@%s' % (user1, api.env.domain)],
+                    memberof_group=[u'ipausers'],
+                    nsaccountlock=False,
+                    has_keytab=False,
+                    has_password=False,
+                ),
+                value=user1,
+                summary='Modified user "%s"' % user1,
+            ),
+        ),
+
     ]
-- 
1.8.3.1

From dfbc7e55178c613095910ede7a34010ed9773f28 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Mon, 23 Sep 2013 15:00:55 +0200
Subject: [PATCH] squash! Add support for managing user auth types

---
 ipalib/plugins/user.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 5c193ec82c900075ed73dd4ec960a5778eac823d..1ba1ab211d32f839ad35c0d723914b79f2f78aa1 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -642,7 +642,10 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             setattr(context, 'randompassword', entry_attrs['userpassword'])
 
         if 'objectclass' not in entry_attrs:
-            _entry = ldap.get_entry(dn, ['objectclass'])
+            try:
+                _entry = ldap.get_entry(dn, ['objectclass'])
+            except errors.NotFound:
+                self.obj.handle_not_found(*keys)
             entry_attrs['objectclass'] = _entry['objectclass']
 
         if 'ipasshpubkey' in entry_attrs:
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to