[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Freeipa-devel] [PATCH] 067 A new flag to disable creation of UPG



On Fri, 2011-05-20 at 10:58 -0400, Rob Crittenden wrote:
> Rob Crittenden wrote:
> > Martin Kosek wrote:
> >> On Mon, 2011-05-16 at 22:12 -0400, Rob Crittenden wrote:
> >>> Martin Kosek wrote:
> >>>> This patch is based on old Pavel's patch.
> >>>>
> >>>> I am considering applying the patch for master branch only as it
> >>>> changes
> >>>> an API (adds a new flag) and is a sort of new-functionality-ish.
> >>>>
> >>>> --
> >>>> Automatic creation may of User Private Groups (UPG) may not be
> >>>> wanted at all times. This patch adds a new flag --noprivate to
> >>>> ipa user-add command to disable it.
> >>>>
> >>>> https://fedorahosted.org/freeipa/ticket/1131
> >>>
> >>> Nack, setattr and addattr are removed from API.txt. I'm guessing it's a
> >>> side-effect of some change here.
> >>>
> >>> The approach generally looks good.
> >>>
> >>> rob
> >>
> >> You are right, this was a side-effect in user.py. I fixed the problem,
> >> updated patch is attached.
> >>
> >> Martin
> >
> > This looks good, just a couple of requests:
> >
> > 1. Bump the minor API version since we are adding a new flag
> > 2. Add a self-test for not creating a private group
> >
> > rob
> 
> Oh, and looking back at the user I create it still has the UPG magic in 
> the description attribute.
> 
> rob

Thanks for careful review, I missed this bug in the original patch. UPG
magic has been removed from the description and a test checking all this
has been added.

Martin
>From c6cf0ccd4ab05f4c2f2da3083b7e5670ef8dc711 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mkosek redhat com>
Date: Mon, 16 May 2011 12:56:04 +0200
Subject: [PATCH] A new flag to disable creation of UPG

Automatic creation may of User Private Groups (UPG) may not be
wanted at all times. This patch adds a new flag --noprivate to
ipa user-add command to disable it.

https://fedorahosted.org/freeipa/ticket/1131
---
 API.txt                                |    3 +-
 VERSION                                |    2 +-
 install/share/user_private_groups.ldif |    2 +-
 install/updates/50-suppress-upg.update |    2 +
 install/updates/Makefile.am            |    1 +
 ipalib/plugins/user.py                 |   53 ++++++++++++++++++++++++-------
 tests/test_xmlrpc/test_group_plugin.py |   44 ++++++++++++++++++++++++++
 7 files changed, 92 insertions(+), 15 deletions(-)
 create mode 100644 install/updates/50-suppress-upg.update

diff --git a/API.txt b/API.txt
index cd37b670304ce8675aacb619e13fe23d9779a138..c2034746dcbd639f8d768ca00fd0095b48667167 100644
--- a/API.txt
+++ b/API.txt
@@ -2524,7 +2524,7 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), 'User-friendly
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('value', <type 'unicode'>, "The primary_key value of the entry, e.g. 'jdoe' for a user")
 command: user_add
-args: 1,30,3
+args: 1,31,3
 arg: Str('uid', attribute=True, cli_name='login', default_from=DefaultFrom(<lambda>, 'givenname', 'sn'), label=Gettext('User login', domain='ipa', localedir=None), maxlength=255, multivalue=False, normalizer=<lambda>, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', pattern_errmsg='may only include letters, numbers, _, -, . and $', primary_key=True, required=True)
 option: Str('givenname', attribute=True, cli_name='first', label=Gettext('First name', domain='ipa', localedir=None), multivalue=False, required=True)
 option: Str('sn', attribute=True, cli_name='last', label=Gettext('Last name', domain='ipa', localedir=None), multivalue=False, required=True)
@@ -2553,6 +2553,7 @@ option: Str('manager', attribute=True, cli_name='manager', label=Gettext('Manage
 option: Str('carlicense', attribute=True, cli_name='carlicense', label=Gettext('Car License', domain='ipa', localedir=None), multivalue=False, required=False)
 option: Str('addattr*', validate_add_attribute, cli_name='addattr', exclude='webui')
 option: Str('setattr*', validate_set_attribute, cli_name='setattr', exclude='webui')
+option: Flag('noprivate', autofill=True, cli_name='noprivate', default=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui', flags=['no_output'])
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui', flags=['no_output'])
 option: Str('version?', exclude='webui', flags=['no_option', 'no_output'])
diff --git a/VERSION b/VERSION
index fc5718dd793c4e61d79d8648f0aab01168413607..44de4f5f575f96b6ce4346000ec7627afa24d354 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=1
+IPA_API_VERSION_MINOR=2
diff --git a/install/share/user_private_groups.ldif b/install/share/user_private_groups.ldif
index 9df729a47207d2ab3dd30f763a73d05b102d882b..41a78ba0b670693280e0bff77c9bc18201f7c34a 100644
--- a/install/share/user_private_groups.ldif
+++ b/install/share/user_private_groups.ldif
@@ -15,7 +15,7 @@ changetype: add
 objectclass: extensibleObject
 cn: UPG Definition
 originScope: cn=users,cn=accounts,$SUFFIX
-originFilter: objectclass=posixAccount
+originFilter: (&(objectclass=posixAccount)(!(description=__no_upg__)))
 managedBase: cn=groups,cn=accounts,$SUFFIX
 managedTemplate: cn=UPG Template,cn=etc,$SUFFIX
 
diff --git a/install/updates/50-suppress-upg.update b/install/updates/50-suppress-upg.update
new file mode 100644
index 0000000000000000000000000000000000000000..42e4257f2b63dccd6068c145ac07d4d0f6479232
--- /dev/null
+++ b/install/updates/50-suppress-upg.update
@@ -0,0 +1,2 @@
+dn: cn=UPG Definition,cn=Managed Entries,cn=plugins,cn=config
+replace: originFilter:objectclass=posixAccount:(&(objectclass=posixAccount)(!(description=__no_upg__)))
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index c9d1584b8b5a8b2c344244b5bf38169b1cf13073..eb864b6b031a2030e80718a55a238e891f61691d 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -17,6 +17,7 @@ app_DATA =				\
 	45-roles.update			\
 	50-lockout-policy.update	\
 	50-groupuuid.update		\
+	50-suppress-upg.update		\
 	$(NULL)
 
 EXTRA_DIST =				\
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index a058ff7ed7768f11e617b0c95b69347cc023986f..fa47cae8e8ba59cebd07f6684c8f8b0de648be3a 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -63,6 +63,9 @@ from ipalib import _, ngettext
 from ipalib.request import context
 from time import gmtime, strftime
 
+
+NO_UPG_MAGIC = '__no_upg__'
+
 def validate_nsaccountlock(entry_attrs):
     if 'nsaccountlock' in entry_attrs:
         if not isinstance(entry_attrs['nsaccountlock'], basestring):
@@ -70,6 +73,7 @@ def validate_nsaccountlock(entry_attrs):
         if entry_attrs['nsaccountlock'].lower() not in ('true','false'):
             raise errors.ValidationError(name='nsaccountlock', error='must be TRUE or FALSE')
 
+
 class user(LDAPObject):
     """
     User object.
@@ -289,22 +293,35 @@ class user_add(LDAPCreate):
     """
     Add a new user.
     """
-
     msg_summary = _('Added user "%(value)s"')
 
+    takes_options = LDAPCreate.takes_options + (
+        Flag('noprivate',
+            cli_name='noprivate',
+            doc=_('Don\'t create user private group'),
+        ),
+    )
+
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
-        try:
-            # The Managed Entries plugin will allow a user to be created
-            # even if a group has a duplicate name. This would leave a user
-            # without a private group. Check for both the group and the user.
-            self.api.Command['group_show'](keys[-1])
+        if not options.get('noprivate', False):
             try:
-                self.api.Command['user_show'](keys[-1])
-                self.obj.handle_duplicate_entry(*keys)
+                # The Managed Entries plugin will allow a user to be created
+                # even if a group has a duplicate name. This would leave a user
+                # without a private group. Check for both the group and the user.
+                self.api.Command['group_show'](keys[-1])
+                try:
+                    self.api.Command['user_show'](keys[-1])
+                    self.obj.handle_duplicate_entry(*keys)
+                except errors.NotFound:
+                    raise errors.ManagedGroupExistsError(group=keys[-1])
             except errors.NotFound:
-                raise errors.ManagedGroupExistsError(group=keys[-1])
-        except errors.NotFound:
-            pass
+                pass
+        else:
+            # we don't want an user private group to be created for this user
+            # add NO_UPG_MAGIC description attribute to let the DS plugin know
+            entry_attrs.setdefault('description', [])
+            entry_attrs['description'].append(NO_UPG_MAGIC)
+
         validate_nsaccountlock(entry_attrs)
         config = ldap.get_ipa_config()[1]
         if 'ipamaxusernamelength' in config:
@@ -330,7 +347,7 @@ class user_add(LDAPCreate):
 
         if 'gidnumber' not in entry_attrs:
             # gidNumber wasn't specified explicity, find out what it should be
-            if ldap.has_upg():
+            if not options.get('noprivate', False) and ldap.has_upg():
                 # User Private Groups - uidNumber == gidNumber
                 entry_attrs['gidnumber'] = entry_attrs['uidnumber']
             else:
@@ -360,6 +377,18 @@ class user_add(LDAPCreate):
         group_dn = self.api.Object['group'].get_dn(def_primary_group)
         ldap.add_entry_to_group(dn, group_dn)
         self.obj._convert_manager(entry_attrs, **options)
+        # delete description attribute NO_UPG_MAGIC if present
+        if options.get('noprivate', False):
+            if not options.get('all', False):
+                (dn, desc_attr) = ldap.get_entry(dn, ['description'])
+                entry_attrs.update(desc_attr)
+            if 'description' in entry_attrs and NO_UPG_MAGIC in entry_attrs['description']:
+                entry_attrs['description'].remove(NO_UPG_MAGIC)
+                kw = {'setattr': unicode('description=%s' % ','.join(entry_attrs['description']))}
+                try:
+                    self.api.Command['user_mod'](keys[-1], **kw)
+                except (errors.EmptyModlist, errors.NotFound):
+                    pass
         return dn
 
 api.register(user_add)
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index a49193261a527aa33ff7e6fde49c6ea2e65c5b91..8d61a5bdfe483e77c081ce947f94e5071a42c754 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -665,4 +665,48 @@ class test_group(Declarative):
             expected=errors.NotFound(reason='no such entry'),
         ),
 
+        dict(
+            desc='Delete %r' % user1,
+            command=('user_del', [user1], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                summary=u'Deleted user "tuser1"',
+                value=user1,
+            ),
+        ),
+
+        dict(
+            desc='Create %r without User Private Group' % 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,
+                    sn=[u'User1'],
+                    uid=[user1],
+                    uidnumber=[fuzzy_digits],
+                    displayname=[u'Test User1'],
+                    cn=[u'Test User1'],
+                    initials=[u'TU'],
+                    ipauniqueid=[fuzzy_uuid],
+                    dn=u'uid=tuser1,cn=users,cn=accounts,' + api.env.basedn,
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Verify the managed group %r was not created' % user1,
+            command=('group_show', [user1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
     ]
-- 
1.7.5.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]