On 3.4.2012 13:04, Martin Kosek wrote:
On Tue, 2012-04-03 at 13:02 +0200, Martin Kosek wrote:
On Tue, 2012-04-03 at 11:58 +0200, Jan Cholasta wrote:
https://fedorahosted.org/freeipa/ticket/2572

Honza


NACK.

This creates a regression:

# ipa group-show foogroup
   Group name: foogroup
   Description: foo
   GID: 358800017

# ipa user-add --first=Foo --last=Bar fbar5 --gidnumber=358800017 --noprivate
------------------
Added user "fbar5"
------------------
   User login: fbar5
   First name: Foo
   Last name: Bar
   Full name: Foo Bar
   Display name: Foo Bar
   Initials: FB
   Home directory: /home/fbar5
   GECOS field: Foo Bar
   Login shell: /bin/sh
   Kerberos principal: fb...@idm.lab.bos.redhat.com
   UID: 358800019
   GID: 358800012
   Password: False
   Member of groups: ipausers
   Kerberos keys available: False

# id fbar5
uid=358800019(fbar5) gid=358800012(ipausers) groups=358800012(ipausers)

Custom user group (GID) was overwritten.

I think we also want a test case for this situation.

Martin


... and we also want to have the new error message(s) i18n-able.

Martin


Updated patch attached.

Honza

--
Jan Cholasta
>From 72258c8cf11ebe798db26782fa0d392972e556b6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 29 Mar 2012 09:12:36 -0400
Subject: [PATCH] Check whether the default user group is POSIX when adding
 new user with --noprivate.

ticket 2572
---
 API.txt                                |    6 +-
 ipalib/plugins/user.py                 |   12 ++-
 tests/test_xmlrpc/test_group_plugin.py |    4 +-
 tests/test_xmlrpc/test_user_plugin.py  |  163 +++++++++++++++++++++++++++++++-
 tests/util.py                          |    8 ++
 5 files changed, 183 insertions(+), 10 deletions(-)

diff --git a/API.txt b/API.txt
index e9eb1e1..cc0d1eb 100644
--- a/API.txt
+++ b/API.txt
@@ -3082,7 +3082,7 @@ option: Str('mail', attribute=True, cli_name='email', multivalue=True, required=
 option: Password('userpassword', attribute=True, cli_name='password', exclude='webui', multivalue=False, required=False)
 option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False)
 option: Int('uidnumber', attribute=True, autofill=True, cli_name='uid', default=999, minvalue=1, multivalue=False, required=True)
-option: Int('gidnumber', attribute=True, autofill=True, cli_name='gidnumber', minvalue=1, multivalue=False, required=True)
+option: Int('gidnumber', attribute=True, autofill=True, cli_name='gidnumber', default=999, minvalue=1, multivalue=False, required=True)
 option: Str('street', attribute=True, cli_name='street', multivalue=False, required=False)
 option: Str('l', attribute=True, cli_name='city', multivalue=False, required=False)
 option: Str('st', attribute=True, cli_name='state', multivalue=False, required=False)
@@ -3140,7 +3140,7 @@ option: Str('krbprincipalname', attribute=True, autofill=False, cli_name='princi
 option: Str('mail', attribute=True, autofill=False, cli_name='email', multivalue=True, query=True, required=False)
 option: Password('userpassword', attribute=True, autofill=False, cli_name='password', exclude='webui', multivalue=False, query=True, required=False)
 option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', default=999, minvalue=1, multivalue=False, query=True, required=False)
-option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', minvalue=1, multivalue=False, query=True, required=False)
+option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', default=999, minvalue=1, multivalue=False, query=True, required=False)
 option: Str('street', attribute=True, autofill=False, cli_name='street', multivalue=False, query=True, required=False)
 option: Str('l', attribute=True, autofill=False, cli_name='city', multivalue=False, query=True, required=False)
 option: Str('st', attribute=True, autofill=False, cli_name='state', multivalue=False, query=True, required=False)
@@ -3189,7 +3189,7 @@ option: Str('mail', attribute=True, autofill=False, cli_name='email', multivalue
 option: Password('userpassword', attribute=True, autofill=False, cli_name='password', exclude='webui', multivalue=False, required=False)
 option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False)
 option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', default=999, minvalue=1, multivalue=False, required=False)
-option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', minvalue=1, multivalue=False, required=False)
+option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', default=999, minvalue=1, multivalue=False, required=False)
 option: Str('street', attribute=True, autofill=False, cli_name='street', multivalue=False, required=False)
 option: Str('l', attribute=True, autofill=False, cli_name='city', multivalue=False, required=False)
 option: Str('st', attribute=True, autofill=False, cli_name='state', multivalue=False, required=False)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 64424e8..7b21270 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -76,6 +76,7 @@ EXAMPLES:
 
 
 NO_UPG_MAGIC = '__no_upg__'
+DNA_MAGIC = 999
 
 user_output_params = (
     Flag('has_keytab',
@@ -276,14 +277,14 @@ class user(LDAPObject):
             label=_('UID'),
             doc=_('User ID Number (system will assign one if not provided)'),
             autofill=True,
-            default=999,
+            default=DNA_MAGIC,
             minvalue=1,
         ),
         Int('gidnumber',
             label=_('GID'),
             doc=_('Group ID Number'),
             minvalue=1,
-            default_from=lambda uidnumber: uidnumber,
+            default=DNA_MAGIC,
             autofill=True,
         ),
         Str('street?',
@@ -455,7 +456,7 @@ class user_add(LDAPCreate):
         entry_attrs.setdefault('krbpwdpolicyreference', 'cn=global_policy,cn=%s,cn=kerberos,%s' % (api.env.realm, api.env.basedn))
         entry_attrs.setdefault('krbprincipalname', '%s@%s' % (entry_attrs['uid'], api.env.realm))
 
-        if 'gidnumber' not in entry_attrs:
+        if entry_attrs.get('gidnumber', DNA_MAGIC) == DNA_MAGIC:
             # gidNumber wasn't specified explicity, find out what it should be
             if not options.get('noprivate', False) and ldap.has_upg():
                 # User Private Groups - uidNumber == gidNumber
@@ -468,7 +469,10 @@ class user_add(LDAPCreate):
                 try:
                     (group_dn, group_attrs) = ldap.get_entry(group_dn, ['gidnumber'])
                 except errors.NotFound:
-                    error_msg = 'Default group for new users not found.'
+                    error_msg = _('Default group for new users not found.')
+                    raise errors.NotFound(reason=error_msg)
+                if 'gidnumber' not in group_attrs:
+                    error_msg = _('Default group for new users is not POSIX.')
                     raise errors.NotFound(reason=error_msg)
                 entry_attrs['gidnumber'] = group_attrs['gidnumber']
 
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 44682b6..d322c2d 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -752,7 +752,7 @@ class test_group(Declarative):
         dict(
             desc='Create %r without User Private Group' % user1,
             command=(
-                'user_add', [user1], dict(givenname=u'Test', sn=u'User1', noprivate=True)
+                'user_add', [user1], dict(givenname=u'Test', sn=u'User1', noprivate=True, gidnumber=1000)
             ),
             expected=dict(
                 value=user1,
@@ -768,7 +768,7 @@ class test_group(Declarative):
                     sn=[u'User1'],
                     uid=[user1],
                     uidnumber=[fuzzy_digits],
-                    gidnumber=[fuzzy_digits],
+                    gidnumber=[u'1000'],
                     displayname=[u'Test User1'],
                     cn=[u'Test User1'],
                     initials=[u'TU'],
diff --git a/tests/test_xmlrpc/test_user_plugin.py b/tests/test_xmlrpc/test_user_plugin.py
index b21737f..c9ab4d0 100644
--- a/tests/test_xmlrpc/test_user_plugin.py
+++ b/tests/test_xmlrpc/test_user_plugin.py
@@ -25,7 +25,7 @@ Test the `ipalib/plugins/user.py` module.
 
 from ipalib import api, errors
 from tests.test_xmlrpc import objectclasses
-from tests.util import assert_equal
+from tests.util import assert_equal, assert_not_equal
 from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_password, fuzzy_string, fuzzy_dergeneralizedtime
 from ipalib.dn import *
 
@@ -43,6 +43,12 @@ def upg_check(response):
                  response['result']['gidnumber'])
     return True
 
+def not_upg_check(response):
+    """Check that the user was not assigned to the corresponding private group."""
+    assert_not_equal(response['result']['uidnumber'],
+                     response['result']['gidnumber'])
+    return True
+
 class test_user(Declarative):
 
     cleanup_commands = [
@@ -1085,4 +1091,159 @@ class test_user(Declarative):
             expected=lambda x: True,
         ),
 
+        dict(
+            desc='Delete %r' % user1,
+            command=('user_del', [user1], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                summary=u'Deleted user "%s"' % user1,
+                value=user1,
+            ),
+        ),
+
+        dict(
+            desc='Create %r without UPG' % user1,
+            command=(
+                'user_add', [user1], dict(givenname=u'Test', sn=u'User1', noprivate=True)
+            ),
+            expected=errors.NotFound(reason='Default group for new users is not POSIX.'),
+        ),
+
+        dict(
+            desc='Create %r without UPG with GID explicitly set' % user2,
+            command=(
+                'user_add', [user2], dict(givenname=u'Test', sn=u'User2', noprivate=True, gidnumber=1000)
+            ),
+            expected=dict(
+                value=user2,
+                summary=u'Added user "tuser2"',
+                result=dict(
+                    gecos=[u'Test User2'],
+                    givenname=[u'Test'],
+                    description=[],
+                    homedirectory=[u'/home/tuser2'],
+                    krbprincipalname=[u'tuser2@' + api.env.realm],
+                    loginshell=[u'/bin/sh'],
+                    objectclass=objectclasses.user_base,
+                    sn=[u'User2'],
+                    uid=[user2],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[u'1000'],
+                    displayname=[u'Test User2'],
+                    cn=[u'Test User2'],
+                    initials=[u'TU'],
+                    ipauniqueid=[fuzzy_uuid],
+                    krbpwdpolicyreference=lambda x: [DN(i) for i in x] == \
+                        [DN(('cn','global_policy'),('cn',api.env.realm),
+                            ('cn','kerberos'),api.env.basedn)],
+                    memberof_group=[u'ipausers'],
+                    has_keytab=False,
+                    has_password=False,
+                    dn=lambda x: DN(x) == \
+                        DN(('uid','tuser2'),('cn','users'),('cn','accounts'),
+                           api.env.basedn),
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Delete %r' % user2,
+            command=('user_del', [user2], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                summary=u'Deleted user "%s"' % user2,
+                value=user2,
+            ),
+        ),
+
+        dict(
+            desc='Change default user group',
+            command=(
+                'config_mod', [], dict(ipadefaultprimarygroup=group1),
+            ),
+            expected=lambda x: True,
+        ),
+
+        dict(
+            desc='Create %r without UPG' % user1,
+            command=(
+                'user_add', [user1], dict(givenname=u'Test', sn=u'User1', noprivate=True)
+            ),
+            expected=dict(
+                value=user1,
+                summary=u'Added user "tuser1"',
+                result=dict(
+                    gecos=[u'Test User1'],
+                    givenname=[u'Test'],
+                    description=[],
+                    homedirectory=[u'/home/tuser1'],
+                    krbprincipalname=[u'tuser1@' + api.env.realm],
+                    loginshell=[u'/bin/sh'],
+                    objectclass=objectclasses.user_base,
+                    sn=[u'User1'],
+                    uid=[user1],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[fuzzy_digits],
+                    displayname=[u'Test User1'],
+                    cn=[u'Test User1'],
+                    initials=[u'TU'],
+                    ipauniqueid=[fuzzy_uuid],
+                    krbpwdpolicyreference=lambda x: [DN(i) for i in x] == \
+                        [DN(('cn','global_policy'),('cn',api.env.realm),
+                            ('cn','kerberos'),api.env.basedn)],
+                    memberof_group=[group1],
+                    has_keytab=False,
+                    has_password=False,
+                    dn=lambda x: DN(x) == \
+                        DN(('uid','tuser1'),('cn','users'),('cn','accounts'),
+                           api.env.basedn),
+                ),
+            ),
+            extra_check = not_upg_check,
+        ),
+
+        dict(
+            desc='Create %r without UPG with GID explicitly set' % user2,
+            command=(
+                'user_add', [user2], dict(givenname=u'Test', sn=u'User2', noprivate=True, gidnumber=1000)
+            ),
+            expected=dict(
+                value=user2,
+                summary=u'Added user "tuser2"',
+                result=dict(
+                    gecos=[u'Test User2'],
+                    givenname=[u'Test'],
+                    description=[],
+                    homedirectory=[u'/home/tuser2'],
+                    krbprincipalname=[u'tuser2@' + api.env.realm],
+                    loginshell=[u'/bin/sh'],
+                    objectclass=objectclasses.user_base,
+                    sn=[u'User2'],
+                    uid=[user2],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[u'1000'],
+                    displayname=[u'Test User2'],
+                    cn=[u'Test User2'],
+                    initials=[u'TU'],
+                    ipauniqueid=[fuzzy_uuid],
+                    krbpwdpolicyreference=lambda x: [DN(i) for i in x] == \
+                        [DN(('cn','global_policy'),('cn',api.env.realm),
+                            ('cn','kerberos'),api.env.basedn)],
+                    memberof_group=[group1],
+                    has_keytab=False,
+                    has_password=False,
+                    dn=lambda x: DN(x) == \
+                        DN(('uid','tuser2'),('cn','users'),('cn','accounts'),
+                           api.env.basedn),
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Reset default user group',
+            command=(
+                'config_mod', [], dict(ipadefaultprimarygroup=u'ipausers'),
+            ),
+            expected=lambda x: True,
+        ),
     ]
diff --git a/tests/util.py b/tests/util.py
index 5a365fb..01fe94c 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -107,6 +107,14 @@ def assert_equal(val1, val2):
     assert val1 == val2, '%r != %r' % (val1, val2)
 
 
+def assert_not_equal(val1, val2):
+    """
+    Assert ``val1`` and ``val2`` are the same type and of non-equal value.
+    """
+    assert type(val1) is type(val2), '%r != %r' % (val1, val2)
+    assert val1 != val2, '%r == %r' % (val1, val2)
+
+
 class Fuzzy(object):
     """
     Perform a fuzzy (non-strict) equality tests.
-- 
1.7.7.6

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

Reply via email to