From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12 Description of problem: Imagine an entry that is a member of a direct group, we have an indirect group and we add this same entry as a DIRECT member to the indirect group. If we delete now the DIRECT membership in the INDIRECT group the indirect membership also disappears from the memberOf attribute values. Version-Release number of selected component (if applicable): FDS 1.1, memberOf rev. 1.4 How reproducible: Always Steps to Reproduce: 1. Create these three entries: dn: cn=group1,dc=example,dc=com objectclass: top objectClass: groupOfNames objectClass: inetUser cn: group1 dn: group2,dc=example,dc=com objectclass: top objectClass: groupOfNames objectClass: inetUser cn: group2 dn: cn=group2 dn: uid=user1,dc=example,dc=com uid: user1 objectClass: inetorgperson objectClass: organizationalPerson objectClass: person objectClass: top objectClass: inetUser cn: user sn: 1 2. Make group2 a member of group1: dn: cn=group1,dc=example,dc=com changetype: modify add: member member: cn=group2,dc=example,dc=com 3. Make user1 a member of group2: dn: cn=group2,dc=example,dc=com changetype: modify add: member member: uid=user1,dc=example,dc=com 4. At this point, the memberOf attribute should be correct in the three test entries: dn: cn=group1,dc=example,dc=com member: cn=group2,dc=example,dc=com dn: cn=group2,dc=example,dc=com member: uid=user1,dc=example,dc=com memberof: cn=group1,dc=example,dc=com dn: uid=user1,dc=example,dc=com memberof: cn=group2,dc=example,dc=com memberof: cn=group1,dc=example,dc=com 5. Now we make user1 a direct member of group1. I know we would not do it in a correctly written application but "to error is human" :) : dn: cn=group1,dc=example,dc=com changetype: modify add: member member: uid=user1,dc=example,dc=com 6. As for now the memberOf attribute is still correct everywhere. Notice however thet user1 is the member of group1 twice - once as a direct and once as an indirect member. 7. Now let's delete the direct membership of user1 in the group1: dn: cn=group1,dc=example,dc=com changetype: modify delete: member member: uid=user1,dc=example,dc=com Actual Results: The memberOf attributes values set is inconsistent, the entry of user1 contains only the memberOf of direct membership : dn: uid=user1,dc=example,dc=com memberof: cn=group2,dc=example,dc=com The indirect membership memberOf value is not present. Expected Results: To be consistent, the memberOf values should contain the DNs of both groups. Additional info: We should verify if the deleted direct 'member' is not at the same an indirect member of the group before actually deleting the DN of a group from the memberOf attribute of the entry. Otherwise we will arrive to a non-consistent memeberOf values set.
The verification should be sort of the one made in memberof_test_membership_callback(). By the way the comments to this function in the code incorrectly name it "memberof_memberof_search_callback()" : Line 1316 : /* memberof_memberof_search_callback() * for each attribute in the memberof attribute * determine if the entry is still a member * * test each for direct membership * move groups entry is memberof to member group * test remaining groups for membership in member groups * iterate until a pass fails to move a group over to member groups * remaining groups should be deleted */
Created attachment 302951 [details] CVS Diffs These diffs fix the reported issue. The fix is to check if an entry is an indirect group member before removing the memberOf attribute value form the entry. I had to add a few new helper functions to perform this check. The new utility functions allow you to check if a specific entry is a member of a specific group by looking at member values only. There is a function for checking if the entry is a direct member, and one that will check for direct or indirect membership. Both of these functions will not modify the entry, which is what was lacking in the previous code. These utility functions required a recursive function underneath to trace through indirect memberships. This recursive function will detect group loopings to ensure it doesn't endlessly recurse. I also did some refactoring and commenting of existing code to make things more clear. These changes have been checked for memory leaks with valgrind, and I've also checked for regressions of other recently fixed bugs in the memberOf plug-in.
Your fix looks good to me.
Checked into ldapserver (HEAD). Thanks to Noriko for her review! Checking in memberof.c; /cvs/dirsec/ldapserver/ldap/servers/plugins/memberof/memberof.c,v <-- memberof.c new revision: 1.7; previous revision: 1.6 done
*** Bug 454688 has been marked as a duplicate of this bug. ***
Bug verified, test on rhel5.2 64 bit Test 1: follow the same test as reported, and test result is pass Test 2: follow the same setup, but go further. Total 10 nested groups being created, user001 is direct member of each nested group, then delete the direct relation one by one, verify if user001 still memberof each nested group. The test result: pass Test output: server64[07/22/08 18:24]~ >./relation.sh u001 successfully added === create all === grp001 successfully added grp002 successfully added grp003 successfully added grp004 successfully added grp005 successfully added grp006 successfully added grp007 successfully added grp008 successfully added grp009 successfully added === append all === u001 successfully added to grp002 grp002 successfully added to grp001 u001 successfully added to grp003 grp003 successfully added to grp002 u001 successfully added to grp004 grp004 successfully added to grp003 u001 successfully added to grp005 grp005 successfully added to grp004 u001 successfully added to grp006 grp006 successfully added to grp005 u001 successfully added to grp007 grp007 successfully added to grp006 u001 successfully added to grp008 grp008 successfully added to grp007 u001 successfully added to grp009 grp009 successfully added to grp008 === remove all === u001 successfully removed from grp002 dn: cn=grp002,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1155 Full Name: grp002 Description: layer 2 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp003: cn=grp003,cn=groups,cn=accounts,dc=ipaqa,dc=com grp004: cn=grp004,cn=groups,cn=accounts,dc=ipaqa,dc=com grp005: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp003 dn: cn=grp003,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1156 Full Name: grp003 Description: layer 3 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp004: cn=grp004,cn=groups,cn=accounts,dc=ipaqa,dc=com grp005: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp004 dn: cn=grp004,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1157 Full Name: grp004 Description: layer 4 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp005: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp005 dn: cn=grp005,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1158 Full Name: grp005 Description: layer 5 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp006: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp006 dn: cn=grp006,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1159 Full Name: grp006 Description: layer 6 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp007: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp007 dn: cn=grp007,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1160 Full Name: grp007 Description: layer 7 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp008: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp008 dn: cn=grp008,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1161 Full Name: grp008 Description: layer 8 Members: user 001: uid=u001,cn=users,cn=accounts,dc=ipaqa,dc=com grp009: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com Continue? u001 successfully removed from grp009 dn: cn=grp009,cn=groups,cn=accounts,dc=ipaqa,dc=com GID: 1162 Full Name: grp009 Description: layer 9 Continue?
the test script is below: server64[07/22/08 18:27]~ >cat < relation.sh #!/bin/sh createAll(){ for N in 1 2 3 4 5 6 7 8 9 do ipa-addgroup -d "layer $N" grp00$N done } clearAll(){ for N in 1 2 3 4 5 6 7 8 9 do ipa-delgroup grp00$N done } appendAll(){ for N in 2 3 4 5 6 7 8 9 do M=$(expr $N - 1) ipa-modgroup -a u001 grp00$N ipa-modgroup -g grp00$N grp00$M done } removeAll(){ for N in 2 3 4 5 6 7 8 9 do ipa-modgroup -r u001 grp00$N ipa-findgroup grp00$N echo ipa-finduser -a u001 echo Continue? read done } findAll(){ for N in 1 2 3 4 5 6 7 8 9 do ipa-findgroup grp00$N ipa-finduser u001 echo "Continue?" read done } ipa-adduser -c GECOS -d /home/u001 -f user -l 001 -p redhat123 -s /bin/bash u001 echo "=== create all ===" createAll echo "=== append all ===" appendAll #echo "=== find all ===" #findAll echo "=== remove all ===" removeAll echo "===clear all ===" clearAll ipa-deluser u001
Gentlefolk, A quick question from the errata gallery. If I'm understanding the above a-right, when we have u001 is a direct member of grp001 u001 is a direct member of grp002 grp001 is a direct member of grp002 which means, u001 is an indirect member of grp002 if we remove u001 from direct membership of grp2 the old behaviour meant: u001 was not an indirect member of grp002 but the new behaviour means: u001 is still an indirect member of grp002 Correct? Also, is there any visible or otherwise meaningful difference between being an indirect or direct member of a group? The memberOf attribute value appears to be the same either way. BTW, these questions pertain to the following from the forthcoming errata note. If my reading above is correct and there is also no difference between a direct or indirect membership, I'm looking to go with: * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the memberOf attribute is not altered. OTOH, if my reading above is correct and there *is* a difference between a direct and indirect membership, I'm looking to go with: * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the direct membership is still deleted but the indirect membership remains intact. All responses, suggestions and recipes for really great pavlova gratefully accepted. Regards, Brian Forte.
(In reply to comment #10) > Gentlefolk, > > A quick question from the errata gallery. > > If I'm understanding the above a-right, when we have > > u001 is a direct member of grp001 > u001 is a direct member of grp002 > grp001 is a direct member of grp002 > > which means, > u001 is an indirect member of grp002 > > if we remove u001 from direct membership of grp2 > > the old behaviour meant: > u001 was not an indirect member of grp002 > > but the new behaviour means: > u001 is still an indirect member of grp002 > > Correct? Yes. The user was always supposed to be an indirect member, but the memberOf attribute in u001 for grp002 was missing before the fix. > > Also, is there any visible or otherwise meaningful difference between being an indirect or direct member of a group? > The memberOf attribute value appears to be the same either way. That is correct. As far as a user entry is concerned, there is no distinction between a direct group and an indirect group. All groups are equal, and the memberOf attribute will appear the same. > > BTW, these questions pertain to the following from the forthcoming errata note. > > If my reading above is correct and there is also no difference between a direct or indirect membership, I'm looking > to go with: > > * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the > indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package > now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the > memberOf attribute is not altered. This description is accurate. > > OTOH, if my reading above is correct and there *is* a difference between a direct and indirect membership, I'm > looking to go with: > > * if an entry was both directly and indirectly a member of a group and the direct membership was removed, the > indirect membership was, erroneously, also removed. When deleting a direct membership, this updated package > now checks to see if an entry is also an indirect group member. If an entry is an indirect member of a group, the > direct membership is still deleted but the indirect membership remains intact. > > All responses, suggestions and recipes for really great pavlova gratefully accepted. > > Regards, > > Brian Forte.
An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on therefore solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHBA-2008-0643.html