Bug 443241

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;
}