Endi Sukma Dewata wrote:
On 6/13/2011 2:45 PM, Rob Crittenden wrote:
Indirect membership is calculated by looking at each member and pulling
all the memberof out of it. What was missing was doing nested searches
on any members in that member group.

So if group2 was a member of group1 and group3 was a member of group2 we
would miss group3 as being an indirect member of group1.

I updated the nesting test to do deeper nested testing. I confirmed that
this test failed with the old code and works with the new.

ticket https://fedorahosted.org/freeipa/ticket/1273

NACK. If a user is an indirect member of a group via 2 different paths,
the user will be listed twice. Here is a test scenario:

Group 1 has 2 members: group 2 and group 3.
User X is a member of both group 2 and group 3.
Group 1's indirect members should only list the user X once. Currently
it is listed twice.


Patch and test case updated.

rob
>From 8f9b36c40ffa5bf75fd8baba5f2185f940182607 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Mon, 13 Jun 2011 14:54:42 -0400
Subject: [PATCH] Fix indirect member calculation

Indirect membership is calculated by looking at each member and pulling
all the memberof out of it. What was missing was doing nested searches
on any members in that member group.

So if group2 was a member of group1 and group3 was a member of group2
we would miss group3 as being an indirect member of group1.

I updated the nesting test to do deeper nested testing. I confirmed
that this test failed with the old code and works with the new.

This also prevents duplicate indirect users.

ticket https://fedorahosted.org/freeipa/ticket/1273
---
 ipaserver/plugins/ldap2.py        |   24 ++-
 tests/test_xmlrpc/test_nesting.py |  293 ++++++++++++++++++++++++++++++++-----
 2 files changed, 270 insertions(+), 47 deletions(-)

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index b0a5c2c..a0b03c1 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -943,14 +943,20 @@ class ldap2(CrudBackend, Encoder):
         # Verify group membership
 
         results = []
-        for member in members:
-            try:
-                (result, truncated) = self.find_entries(searchfilter, attr_list,
-                    member, time_limit=time_limit,
-                    size_limit=size_limit, normalize=normalize)
-                results.append(list(result[0]))
-            except errors.NotFound:
-                pass
+        if membertype == MEMBERS_ALL or membertype == MEMBERS_INDIRECT:
+            checkmembers = copy.deepcopy(members)
+            for member in checkmembers:
+                try:
+                    (result, truncated) = self.find_entries(searchfilter,
+                        attr_list, member, time_limit=time_limit,
+                        size_limit=size_limit, normalize=normalize)
+                    results.append(list(result[0]))
+                    for m in result[0][1].get('member', []):
+                        # This member may contain other members, add it to our
+                        # candidate list
+                        checkmembers.append(m)
+                except errors.NotFound:
+                    pass
 
         if membertype == MEMBERS_ALL:
             entries = []
@@ -969,7 +975,7 @@ class ldap2(CrudBackend, Encoder):
 
         entries = []
         for e in results:
-            if unicode(e[0]) not in real_members:
+            if unicode(e[0]) not in real_members and unicode(e[0]) not in entries:
                 if membertype == MEMBERS_INDIRECT:
                     entries.append(e[0])
             else:
diff --git a/tests/test_xmlrpc/test_nesting.py b/tests/test_xmlrpc/test_nesting.py
index a7e6cb8..5418628 100644
--- a/tests/test_xmlrpc/test_nesting.py
+++ b/tests/test_xmlrpc/test_nesting.py
@@ -27,8 +27,11 @@ from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
 group1 = u'testgroup1'
 group2 = u'testgroup2'
 group3 = u'testgroup3'
+group4 = u'testgroup4'
 user1 = u'tuser1'
 user2 = u'tuser2'
+user3 = u'tuser3'
+user4 = u'tuser4'
 
 hostgroup1 = u'testhostgroup1'
 hgdn1 = u'cn=%s,cn=hostgroups,cn=accounts,%s' % (hostgroup1, api.env.basedn)
@@ -44,8 +47,11 @@ class test_nesting(Declarative):
         ('group_del', [group1], {}),
         ('group_del', [group2], {}),
         ('group_del', [group3], {}),
+        ('group_del', [group4], {}),
         ('user_del', [user1], {}),
         ('user_del', [user2], {}),
+        ('user_del', [user3], {}),
+        ('user_del', [user4], {}),
         ('host_del', [fqdn1], {}),
         ('hostgroup_del', [hostgroup1], {}),
         ('hostgroup_del', [hostgroup2], {}),
@@ -119,6 +125,26 @@ class test_nesting(Declarative):
 
 
         dict(
+            desc='Create %r' % group4,
+            command=(
+                'group_add', [group4], dict(description=u'Test desc 4')
+            ),
+            expected=dict(
+                value=group4,
+                summary=u'Added group "testgroup4"',
+                result=dict(
+                    cn=[group4],
+                    description=[u'Test desc 4'],
+                    gidnumber=[fuzzy_digits],
+                    objectclass=objectclasses.group + [u'posixgroup'],
+                    ipauniqueid=[fuzzy_uuid],
+                    dn=u'cn=testgroup4,cn=groups,cn=accounts,' + api.env.basedn,
+                ),
+            ),
+        ),
+
+
+        dict(
             desc='Create %r' % user1,
             command=(
                 'user_add', [user1], dict(givenname=u'Test', sn=u'User1')
@@ -176,38 +202,108 @@ class test_nesting(Declarative):
         ),
 
 
+        dict(
+            desc='Create %r' % user3,
+            command=(
+                'user_add', [user3], dict(givenname=u'Test', sn=u'User3')
+            ),
+            expected=dict(
+                value=user3,
+                summary=u'Added user "%s"' % user3,
+                result=dict(
+                    gecos=[u'Test User3'],
+                    givenname=[u'Test'],
+                    homedirectory=[u'/home/tuser3'],
+                    krbprincipalname=[u'tuser3@' + api.env.realm],
+                    loginshell=[u'/bin/sh'],
+                    objectclass=objectclasses.user,
+                    sn=[u'User3'],
+                    uid=[user3],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[fuzzy_digits],
+                    displayname=[u'Test User3'],
+                    cn=[u'Test User3'],
+                    initials=[u'TU'],
+                    ipauniqueid=[fuzzy_uuid],
+                    dn=u'uid=%s,cn=users,cn=accounts,%s' % (user3, api.env.basedn)
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Create %r' % user4,
+            command=(
+                'user_add', [user4], dict(givenname=u'Test', sn=u'User4')
+            ),
+            expected=dict(
+                value=user4,
+                summary=u'Added user "%s"' % user4,
+                result=dict(
+                    gecos=[u'Test User4'],
+                    givenname=[u'Test'],
+                    homedirectory=[u'/home/tuser4'],
+                    krbprincipalname=[u'tuser4@' + api.env.realm],
+                    loginshell=[u'/bin/sh'],
+                    objectclass=objectclasses.user,
+                    sn=[u'User4'],
+                    uid=[user4],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[fuzzy_digits],
+                    displayname=[u'Test User4'],
+                    cn=[u'Test User4'],
+                    initials=[u'TU'],
+                    ipauniqueid=[fuzzy_uuid],
+                    dn=u'uid=%s,cn=users,cn=accounts,%s' % (user4, api.env.basedn)
+                ),
+            ),
+        ),
+
+
         ###############
         # member stuff
         #
-        # Create 3 groups and 2 users and set the following membership:
+        # Create 4 groups and 4 users and set the following membership:
         #
         # g1:
-        #    member: g2
+        #    no direct memberships
         #
         # g2:
-        #    member: g3
-        #    member: user1
+        #    memberof: g1
+        #    member: user1, user2
         #
         # g3:
-        #    member: user2
+        #    memberof: g1
+        #    member: user3, g4
+        #
+        # g4:
+        #    memberof: g3
+        #    member: user1, user4
         #
         # So when we do a show it looks like:
         #
         # g1:
-        #    member: g2
-        #    indirect group: g3
-        #    indirect users: user1, user2
+        #    member groups: g2, g3
+        #    indirect member group: g4
+        #    indirect member users: user1, user2, tuser3, tuser4
         #
         # g2:
-        #    member group: g3
-        #    member user: tuser1
-        #    indirect users: user2
-        #    memberof: g1
+        #    member of group: g1
+        #    member users: tuser1, tuser2
         #
         # g3:
-        #    member: user2
-        #    memberof: g1, g2
-
+        #  member users: tuser3
+        #  member groups: g4
+        #  member of groups: g1
+        #  indirect member users: tuser4
+        #
+        # g4:
+        #  member users: tuser1, tuser4
+        #  member of groups: g3
+        #  indirect member of groups: g1
+        #
+        # Note that tuser1 is an indirect member of g1 both through
+        # g2 and g4. It should appear just once in the list.
 
         dict(
             desc='Add a group member %r to %r' % (group2, group1),
@@ -234,9 +330,9 @@ class test_nesting(Declarative):
 
 
         dict(
-            desc='Add a group member %r to %r' % (group3, group2),
+            desc='Add a group member %r to %r' % (group3, group1),
             command=(
-                'group_add_member', [group2], dict(group=group3)
+                'group_add_member', [group1], dict(group=group3)
             ),
             expected=dict(
                 completed=1,
@@ -247,12 +343,11 @@ class test_nesting(Declarative):
                     ),
                 ),
                 result={
-                        'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group2, api.env.basedn),
-                        'member_group': (group3,),
-                        'memberof_group': (u'testgroup1',),
+                        'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group1, api.env.basedn),
+                        'member_group': [group2, group3,],
                         'gidnumber': [fuzzy_digits],
-                        'cn': [group2],
-                        'description': [u'Test desc 2'],
+                        'cn': [group1],
+                        'description': [u'Test desc 1'],
                 },
             ),
         ),
@@ -274,7 +369,6 @@ class test_nesting(Declarative):
                 result={
                         'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group2, api.env.basedn),
                         'member_user': (u'tuser1',),
-                        'member_group': (group3,),
                         'memberof_group': (u'testgroup1',),
                         'gidnumber': [fuzzy_digits],
                         'cn': [group2],
@@ -285,9 +379,59 @@ class test_nesting(Declarative):
 
 
         dict(
-            desc='Add a user member %r to %r' % (user2, group3),
+            desc='Add a user member %r to %r' % (user2, group2),
+            command=(
+                'group_add_member', [group2], dict(user=user2)
+            ),
+            expected=dict(
+                completed=1,
+                failed=dict(
+                    member=dict(
+                        group=tuple(),
+                        user=tuple(),
+                    ),
+                ),
+                result={
+                        'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group2, api.env.basedn),
+                        'member_user': [user1, user2],
+                        'memberof_group': [group1],
+                        'gidnumber': [fuzzy_digits],
+                        'cn': [group2],
+                        'description': [u'Test desc 2'],
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Add a user member %r to %r' % (user3, group3),
+            command=(
+                'group_add_member', [group3], dict(user=user3)
+            ),
+            expected=dict(
+                completed=1,
+                failed=dict(
+                    member=dict(
+                        group=tuple(),
+                        user=tuple(),
+                    ),
+                ),
+                result={
+                        'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group3, api.env.basedn),
+                        'member_user': [user3],
+                        'memberof_group': [group1],
+                        'gidnumber': [fuzzy_digits],
+                        'cn': [group3],
+                        'description': [u'Test desc 3'],
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Add a group member %r to %r' % (group4, group3),
             command=(
-                'group_add_member', [group3], dict(user=user2)
+                'group_add_member', [group3], dict(group=group4)
             ),
             expected=dict(
                 completed=1,
@@ -299,9 +443,9 @@ class test_nesting(Declarative):
                 ),
                 result={
                         'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group3, api.env.basedn),
-                        'member_user': (u'tuser2',),
-                        'memberof_group': [u'testgroup2'],
-                        'memberofindirect_group': [u'testgroup1'],
+                        'member_user': [user3],
+                        'memberof_group': [group1],
+                        'member_group': [group4],
                         'gidnumber': [fuzzy_digits],
                         'cn': [group3],
                         'description': [u'Test desc 3'],
@@ -311,6 +455,58 @@ class test_nesting(Declarative):
 
 
         dict(
+            desc='Add a user member %r to %r' % (user1, group4),
+            command=(
+                'group_add_member', [group4], dict(user=user1)
+            ),
+            expected=dict(
+                completed=1,
+                failed=dict(
+                    member=dict(
+                        group=tuple(),
+                        user=tuple(),
+                    ),
+                ),
+                result={
+                        'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group4, api.env.basedn),
+                        'member_user': [user1],
+                        'memberof_group': [group3],
+                        'memberofindirect_group': [group1],
+                        'gidnumber': [fuzzy_digits],
+                        'cn': [group4],
+                        'description': [u'Test desc 4'],
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Add a user member %r to %r' % (user4, group4),
+            command=(
+                'group_add_member', [group4], dict(user=user4)
+            ),
+            expected=dict(
+                completed=1,
+                failed=dict(
+                    member=dict(
+                        group=tuple(),
+                        user=tuple(),
+                    ),
+                ),
+                result={
+                        'dn': u'cn=%s,cn=groups,cn=accounts,%s' % (group4, api.env.basedn),
+                        'member_user': [user1, user4],
+                        'memberof_group': [group3],
+                        'memberofindirect_group': [group1],
+                        'gidnumber': [fuzzy_digits],
+                        'cn': [group4],
+                        'description': [u'Test desc 4'],
+                },
+            ),
+        ),
+
+
+        dict(
             desc='Retrieve group %r' % group1,
             command=('group_show', [group1], {}),
             expected=dict(
@@ -320,9 +516,9 @@ class test_nesting(Declarative):
                     cn=[group1],
                     description=[u'Test desc 1'],
                     gidnumber= [fuzzy_digits],
-                    memberindirect_group = [u'testgroup3'],
-                    member_group = (u'testgroup2',),
-                    memberindirect_user = (u'tuser1',u'tuser2',),
+                    memberindirect_group = [group4],
+                    member_group = [group2, group3],
+                    memberindirect_user = [user1, user2, user3, user4],
                     dn=u'cn=testgroup1,cn=groups,cn=accounts,' + api.env.basedn,
                 ),
             ),
@@ -339,10 +535,8 @@ class test_nesting(Declarative):
                     cn=[group2],
                     description=[u'Test desc 2'],
                     gidnumber= [fuzzy_digits],
-                    memberof_group = (u'testgroup1',),
-                    member_group = (u'testgroup3',),
-                    member_user = (u'tuser1',),
-                    memberindirect_user = (u'tuser2',),
+                    memberof_group = [group1],
+                    member_user = [user1, user2],
                     dn=u'cn=testgroup2,cn=groups,cn=accounts,' + api.env.basedn,
                 ),
             ),
@@ -359,14 +553,35 @@ class test_nesting(Declarative):
                     cn=[group3],
                     description=[u'Test desc 3'],
                     gidnumber= [fuzzy_digits],
-                    memberof_group = (u'testgroup2',),
-                    member_user = (u'tuser2',),
-                    memberofindirect_group = (u'testgroup1',),
+                    memberof_group = [group1],
+                    member_user = [user3],
+                    member_group = [group4],
+                    memberindirect_user = [user1, user4],
                     dn=u'cn=testgroup3,cn=groups,cn=accounts,' + api.env.basedn,
                 ),
             ),
         ),
 
+
+        dict(
+            desc='Retrieve group %r' % group4,
+            command=('group_show', [group4], {}),
+            expected=dict(
+                value=group4,
+                summary=None,
+                result=dict(
+                    cn=[group4],
+                    description=[u'Test desc 4'],
+                    gidnumber= [fuzzy_digits],
+                    memberof_group = [group3],
+                    member_user = [user1, user4],
+                    memberofindirect_group = [group1],
+                    dn=u'cn=testgroup4,cn=groups,cn=accounts,' + api.env.basedn,
+                ),
+            ),
+        ),
+
+
         # Now do something similar with hosts and hostgroups
         dict(
             desc='Create host %r' % fqdn1,
@@ -393,6 +608,7 @@ class test_nesting(Declarative):
             ),
         ),
 
+
         dict(
             desc='Create %r' % hostgroup1,
             command=('hostgroup_add', [hostgroup1],
@@ -493,6 +709,7 @@ class test_nesting(Declarative):
             ),
         ),
 
+
         dict(
             desc='Retrieve %r' % fqdn1,
             command=('host_show', [fqdn1], {}),
-- 
1.7.4

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

Reply via email to