On 04/09/2013 05:06 PM, Martin Kosek wrote:
On 04/09/2013 04:38 PM, Petr Vobornik wrote:
On 04/04/2013 12:02 PM, Martin Kosek wrote:
Thanks Tomas for your opinion, I can agree with that. To make it more in an
actual design, this is API following this discussion that I would propose:

This is API we already have in IPA:
ipa group-add --external
ipa group-add --nonposix
ipa group-find --private

This is API that I would propose to add to be consistent with what we already
have:
ipa group-find --nonposix
ipa group-find --posix
ipa group-find --external

--nonposix would only match groups added with --nonposix flag in group-add,
i.e. no --external groups.

As Tomas said, these should also be close together. We can even add a specific
option group for them, like there are with ipa dnsrecord-add, named for example
"Group Types". We may also raise OptionError when these option are used
together to make this less confusing - e.g. OptionError("group type options
(--nonposix, --posix and --external) are mutually exclusive").

Martin

New version attached.


1) default=False parameter for Flag is redundant:

+        Flag('external',
+             cli_name='external',
+             doc=_('search for groups with support of external non-IPA members
from trusted domains'),
+             default=False,
+        ),


2) No need to import StrEnum:
+from ipalib import Int, Str, StrEnum

3) This can be simplified:
+        if len(filters):
TO:
+        if filters:


Besides these minor issues, that patch works fine and I think we can push a
fixed version.

Thanks,
Martin


Additional self-nack.

4) original filter is ignored when some of the new options is used. It prevents from effective searching and also shows private groups when --posix is used.

All fixed, new unit test added. New version attached.

The fix doesn't touch usage of --private because it's a special case - private groups don't have ipausergroup oc and therefore they are incompatible with original filter.
--
Petr Vobornik
From b5ed783390b14df8482e246cb1a7771be942c102 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Mon, 11 Mar 2013 12:37:29 +0100
Subject: [PATCH] Filter groups by type (POSIX, non-POSIX, external)

Added flag for each groups type: --posix, --nonposix, --external to group-find command.

Group types:
* non-POSIX: not posix, not external
* POSIX: with objectclass posixgroup
* external: with objectclass ipaexternalgroup

https://fedorahosted.org/freeipa/ticket/3483
---
 API.txt                                |   5 +-
 ipalib/plugins/group.py                |  28 ++++++++
 tests/test_xmlrpc/objectclasses.py     |   1 +
 tests/test_xmlrpc/test_group_plugin.py | 116 ++++++++++++++++++++++++++++++++-
 tests/test_xmlrpc/xmlrpc_test.py       |   4 ++
 5 files changed, 151 insertions(+), 3 deletions(-)

diff --git a/API.txt b/API.txt
index 81a1f6187583029a4378e44d65bcc7b8d4496508..87db8d678510ebd58d8071ef4b1638085754aa0b 100644
--- a/API.txt
+++ b/API.txt
@@ -1307,11 +1307,12 @@ output: Output('result', <type 'bool'>, None)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('value', <type 'unicode'>, None)
 command: group_find
-args: 1,24,4
+args: 1,27,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='group_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=False)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
+option: Flag('external', autofill=True, cli_name='external', default=False)
 option: Int('gidnumber', attribute=True, autofill=False, cli_name='gid', minvalue=1, multivalue=False, query=True, required=False)
 option: Str('group*', cli_name='groups', csv=True)
 option: Str('in_group*', cli_name='in_groups', csv=True)
@@ -1321,12 +1322,14 @@ option: Str('in_role*', cli_name='in_roles', csv=True)
 option: Str('in_sudorule*', cli_name='in_sudorules', csv=True)
 option: Str('no_group*', cli_name='no_groups', csv=True)
 option: Str('no_user*', cli_name='no_users', csv=True)
+option: Flag('nonposix', autofill=True, cli_name='nonposix', default=False)
 option: Str('not_in_group*', cli_name='not_in_groups', csv=True)
 option: Str('not_in_hbacrule*', cli_name='not_in_hbacrules', csv=True)
 option: Str('not_in_netgroup*', cli_name='not_in_netgroups', csv=True)
 option: Str('not_in_role*', cli_name='not_in_roles', csv=True)
 option: Str('not_in_sudorule*', cli_name='not_in_sudorules', csv=True)
 option: Flag('pkey_only?', autofill=True, default=False)
+option: Flag('posix', autofill=True, cli_name='posix', default=False)
 option: Flag('private', autofill=True, cli_name='private', default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Int('sizelimit?', autofill=False, minvalue=0)
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index 21ee004905c044763bcb19c5feddeabaee2df6cc..02eeb10ca2ca2a5710e88d6e3c11f1d1cdaa4a7b 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -328,10 +328,35 @@ class group_find(LDAPSearch):
             cli_name='private',
             doc=_('search for private groups'),
         ),
+        Flag('posix',
+             cli_name='posix',
+             doc=_('search for POSIX groups'),
+        ),
+        Flag('external',
+             cli_name='external',
+             doc=_('search for groups with support of external non-IPA members from trusted domains'),
+        ),
+        Flag('nonposix',
+             cli_name='nonposix',
+             doc=_('search for non-POSIX groups'),
+        ),
     )
 
     def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **options):
         assert isinstance(base_dn, DN)
+
+        # filter groups by pseudo type
+        filters = []
+        if options['posix']:
+            search_kw = {'objectclass': ['posixGroup']}
+            filters.append(ldap.make_filter(search_kw, rules=ldap.MATCH_ALL))
+        if options['external']:
+            search_kw = {'objectclass': ['ipaExternalGroup']}
+            filters.append(ldap.make_filter(search_kw, rules=ldap.MATCH_ALL))
+        if options['nonposix']:
+            search_kw = {'objectclass': ['posixGroup' , 'ipaExternalGroup']}
+            filters.append(ldap.make_filter(search_kw, rules=ldap.MATCH_NONE))
+
         # if looking for private groups, we need to create a new search filter,
         # because private groups have different object classes
         if options['private']:
@@ -351,6 +376,9 @@ class group_find(LDAPSearch):
             cflt = ldap.make_filter(search_kw, exact=False)
 
             filter = ldap.combine_filters((oflt, cflt), rules=ldap.MATCH_ALL)
+        elif filters:
+            filters.append(filter)
+            filter = ldap.combine_filters(filters, rules=ldap.MATCH_ALL)
         return (filter, base_dn, scope)
 
 api.register(group_find)
diff --git a/tests/test_xmlrpc/objectclasses.py b/tests/test_xmlrpc/objectclasses.py
index d98a7ee64f6def12efcd6898b3e72bd1fdb2ad5e..75ac3eb174aaa50eecfda875024b62dbdab238a5 100644
--- a/tests/test_xmlrpc/objectclasses.py
+++ b/tests/test_xmlrpc/objectclasses.py
@@ -46,6 +46,7 @@ group = [
 ]
 
 externalgroup = group + [u'ipaexternalgroup']
+posixgroup = group + [u'posixgroup']
 
 host = [
     u'ipasshhost',
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 2d6d2014ae968e8fbe09a8862443b1de02ea804f..7a9b8b6d1b31b676a47327ccb2ceb7ca4eaa551c 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -23,7 +23,8 @@ Test the `ipalib/plugins/group.py` module.
 
 from ipalib import api, errors
 from tests.test_xmlrpc import objectclasses
-from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
+from tests.util import Fuzzy
+from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_set_ci
 from ipapython.dn import DN
 
 group1 = u'testgroup1'
@@ -248,7 +249,7 @@ class test_group(Declarative):
                     cn=[group2],
                     description=[u'Test desc 2'],
                     gidnumber=[fuzzy_digits],
-                    objectclass=objectclasses.group + [u'posixgroup'],
+                    objectclass=objectclasses.posixgroup,
                     ipauniqueid=[fuzzy_uuid],
                     dn=get_group_dn('testgroup2'),
                 ),
@@ -382,6 +383,98 @@ class test_group(Declarative):
             ),
         ),
 
+        dict(
+            desc='Search for non-POSIX groups',
+            command=('group_find', [], dict(nonposix=True, all=True)),
+            expected=dict(
+                summary=u'2 groups matched',
+                count=2,
+                truncated=False,
+                result=[
+                    {
+                        'dn': get_group_dn('ipausers'),
+                        'cn': [u'ipausers'],
+                        'description': [u'Default group for all users'],
+                        'objectclass': fuzzy_set_ci(objectclasses.group),
+                        'ipauniqueid': [fuzzy_uuid],
+                    },
+                    {
+                        'dn': get_group_dn('trust admins'),
+                        'member_user': [u'admin'],
+                        'cn': [u'trust admins'],
+                        'description': [u'Trusts administrators group'],
+                        'objectclass': fuzzy_set_ci(objectclasses.group),
+                        'ipauniqueid': [fuzzy_uuid],
+                    },
+                ],
+            ),
+        ),
+
+        dict(
+            desc='Search for non-POSIX groups with criteria filter',
+            command=('group_find', [u'users'], dict(nonposix=True, all=True)),
+            expected=dict(
+                summary=u'1 group matched',
+                count=1,
+                truncated=False,
+                result=[
+                    {
+                        'dn': get_group_dn('ipausers'),
+                        'cn': [u'ipausers'],
+                        'description': [u'Default group for all users'],
+                        'objectclass': fuzzy_set_ci(objectclasses.group),
+                        'ipauniqueid': [fuzzy_uuid],
+                    },
+                ],
+            ),
+        ),
+
+        dict(
+            desc='Search for POSIX groups',
+            command=('group_find', [], dict(posix=True, all=True)),
+            expected=dict(
+                summary=u'4 groups matched',
+                count=4,
+                truncated=False,
+                result=[
+                    {
+                        'dn': get_group_dn('admins'),
+                        'member_user': [u'admin'],
+                        'gidnumber': [fuzzy_digits],
+                        'cn': [u'admins'],
+                        'description': [u'Account administrators group'],
+                        'objectclass': fuzzy_set_ci(objectclasses.posixgroup),
+                        'ipauniqueid': [fuzzy_uuid],
+                    },
+                    {
+                        'dn': get_group_dn('editors'),
+                        'gidnumber': [fuzzy_digits],
+                        'cn': [u'editors'],
+                        'description': [u'Limited admins who can edit other users'],
+                        'objectclass': fuzzy_set_ci(objectclasses.posixgroup),
+                        'ipauniqueid': [fuzzy_uuid],
+                    },
+                    dict(
+                        dn=get_group_dn(group1),
+                        cn=[group1],
+                        description=[u'New desc 1'],
+                        gidnumber=[fuzzy_digits],
+                        objectclass=fuzzy_set_ci(objectclasses.posixgroup),
+                        ipauniqueid=[fuzzy_uuid],
+                    ),
+                    dict(
+                        dn=get_group_dn(group2),
+                        cn=[group2],
+                        description=[u'New desc 2'],
+                        gidnumber=[fuzzy_digits],
+                        objectclass=fuzzy_set_ci(objectclasses.posixgroup),
+                        ipauniqueid=[fuzzy_uuid],
+                    ),
+                ],
+            ),
+        ),
+
+
         ###############
         # test external SID members for group3:
         dict(
@@ -402,6 +495,25 @@ class test_group(Declarative):
             ),
         ),
 
+        dict(
+            desc='Search for external groups',
+            command=('group_find', [], dict(external=True, all=True)),
+            expected=dict(
+                summary=u'1 group matched',
+                count=1,
+                truncated=False,
+                result=[
+                    dict(
+                        cn=[group3],
+                        description=[u'Test desc 3'],
+                        objectclass=fuzzy_set_ci(objectclasses.externalgroup),
+                        ipauniqueid=[fuzzy_uuid],
+                        dn=get_group_dn(group3),
+                    ),
+                ],
+            ),
+        ),
+
 
         dict(
             desc='Convert posix group %r to support external membership' % (group2),
diff --git a/tests/test_xmlrpc/xmlrpc_test.py b/tests/test_xmlrpc/xmlrpc_test.py
index cfd7bd3393937e89392c080ad0192f3e89ffd3ed..89f1adc38d2d6408d8a562b13461efb3907297dd 100644
--- a/tests/test_xmlrpc/xmlrpc_test.py
+++ b/tests/test_xmlrpc/xmlrpc_test.py
@@ -83,6 +83,10 @@ fuzzy_dergeneralizedtime = Fuzzy('^[0-9]{14}Z$')
 # match any string
 fuzzy_string = Fuzzy(type=basestring)
 
+# case insensitive match of sets
+def fuzzy_set_ci(s):
+    return Fuzzy(test=lambda other: set(x.lower() for x in other) == set(y.lower() for y in s))
+
 try:
     if not api.Backend.xmlclient.isconnected():
         api.Backend.xmlclient.connect(fallback=False)
-- 
1.8.1.4

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

Reply via email to