Bug 443241 - memberOf: Fixup task does not fix memberOf attribute of indirect groups
memberOf: Fixup task does not fix memberOf attribute of indirect groups
Status: CLOSED ERRATA
Product: 389
Classification: Community
Component: Server - memberOf Plug-in (Show other bugs)
1.1.0
All Linux
low Severity medium
: ---
: ---
Assigned To: Nathan Kinder
Chandrasekar Kannan
:
Depends On:
Blocks: 249650 FDS112
  Show dependency treegraph
 
Reported: 2008-04-19 13:11 EDT by Andrey Ivanov
Modified: 2015-01-04 18:31 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-08-04 14:21:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
CVS Diffs (1.36 KB, patch)
2008-06-09 14:48 EDT, Nathan Kinder
no flags Details | Diff

  None (edit)
Description Andrey Ivanov 2008-04-19 13:11:01 EDT
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;
}
Comment 1 Nathan Kinder 2008-06-09 14:48:42 EDT
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.
Comment 2 Rich Megginson 2008-06-09 15:57:39 EDT
Do you need to worry about deleted attributes and values?
Comment 3 Nathan Kinder 2008-06-09 16:21:45 EDT
(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.
Comment 4 Rich Megginson 2008-06-09 16:41:50 EDT
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?
Comment 5 Nathan Kinder 2008-06-09 16:48:45 EDT
(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.
Comment 6 Nathan Kinder 2008-06-09 17:44:42 EDT
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
Comment 10 errata-xmlrpc 2008-08-04 14:21:14 EDT
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

Note You need to log in before you can comment on or make changes to this bug.