From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.13) Gecko/20080311 Firefox/2.0.0.13 Description of problem: Th fixup task of the plugin does not update the memberOf attribute by the value of the indirect group if it already contains the value of a nested group. Version-Release number of selected component (if applicable): FDS 1.1, memberOf rev 1.6 and after applying the bugfix for the bug 439628 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. Let us now manually delete the memberOf value corresponding to the group1 (indirect group) in the entry of user1 : dn: uid=user1,dc=example,dc=com changetype: modify delete: memberOf memberOf: cn=group1,dc=example,dc=com 6. Now let's launch the fixup task : ldapadd -x -D "cn=Directory Manager" -w `cat pass.txt` -f membertask.ldif the file membertask.ldif : dn: cn=Fix MemberOf, cn=memberof task, cn=tasks, cn=config objectClass: top objectClass: extensibleObject cn: Fix MemberOf basedn: <basedn> filter: (uid=*) Actual Results: The fixup task does not add the value of memberOf that corresponds to the group1: dn: uid=user1,dc=example,dc=com memberof: cn=group2,dc=example,dc=com Expected Results: The fixup task should correct the missing value of the attribute 'memberOf' : dn: uid=user1,dc=example,dc=com memberof: cn=group2,dc=example,dc=com memberof: cn=group1,dc=example,dc=com Additional info: The fixup task does not add the memberOf value corresponding to the indirect group group1 because this attribute contains already the value for the group nested in group1 (i.e. group2, "memberof: cn=group2,dc=example,dc=com"). The code of considers that it is a group looping and stops processing. So if we manually completely delete the attribute memberOf the fixup task works correctly and adds all the necessary values to it. The most simple way to fix this problem is to delete all the values of the memberOf attribute and than to add the necessary ones. At the same time it eliminates the step 3 ("Trim groups that have no relationship to entry"). The following patch implements this approach. At the same time I have found that eliminating the third step speeds up the fixup task - in my tests i have reduced the time by 5 (the original plugin fixup task took ~30 seconds, the patch reduces it to 6 seconds for ~7000 entries on x86_64 2.33GHz). Here is the the proposed patched code of the function 'memberof_fix_memberof_callback' : /* memberof_fix_memberof_callback() * Add initial and/or fix up broken group list in entry * * 0. Delete all the memberOf attribute values * 1. Make sure direct membership groups are in the entry * 2. Add all groups that current group list allows through nested membership */ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data) { int rc = 0; char *dn = slapi_entry_get_dn(e); memberof_add_groups data = {dn, dn}; /* step 0. */ slapi_entry_attr_delete(e, MEMBEROF_ATTR); /* step 1. and step 2. */ rc = memberof_call_foreach_dn(0, dn, MEMBEROF_GROUP_ATTR, memberof_add_groups_search_callback, &data); return rc; }
Created attachment 308743 [details] CVS Diffs The proposed approach of removing the memberOf attribute and then just generating the proper values seems to work well. This approach makes no assumptions about the state of the entry prior to the fixup task, which is a good thing.
Do you need to worry about deleted attributes and values?
(In reply to comment #2) > Do you need to worry about deleted attributes and values? Do you mean the values of the memberOf attribute that we're deleting as part of step 1? We don't need to worry about those since we generate all correct values by going through all of the present member attribute values in the directory. The memberOf values must be considered to be server maintained if you are using the memberOf plug-in. If someone manually added a memberOf attribute value that has no corresponding member attribute value in the group entry, then it should be removed.
I mean that slapi_entry keeps track of the deleted attributes and the deleted values of an attribute - for replication urp purposes - do you have to worry about those?
(In reply to comment #4) > I mean that slapi_entry keeps track of the deleted attributes and the deleted > values of an attribute - for replication urp purposes - do you have to worry > about those? I don't think I need to worry about them. If you are using the memberOf plug-in with MMR, the memberOf attribute must be fractionally excluded from replication (and will be doc'd as such). Due to this configuration, there will be no chance for a replication conflict due to the memberOf attribute itself.
Checked into ldapserver (HEAD). Thanks to Rich for the review! Checking in memberof.c; /cvs/dirsec/ldapserver/ldap/servers/plugins/memberof/memberof.c,v <-- memberof.c new revision: 1.8; previous revision: 1.7 done
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