Hi,
please find attched the patches I came up with so far to fix
https://fedorahosted.org/sssd/ticket/2868.
sdap_save_grpmem() is used to refresh the members of groups in the cache
based on the data received from LDAP, external entries which are
retained and the primary group stored in the user objects. If
ignore_group_members is set to True there is of course no member data
from LDAP and calling sdap_save_grpmem() in this case would remove all
members entries which where created during initgroups lookups.
So if ignore_group_members is True the members list of groups are
managed only indirectly by the initgroups calls and the list should not
be modified during group lookup to avoid loosing information. This is
what the first patch tries to achieve. Since I think sdap_save_grpmem()
shouldn't be called at all in this case I added the SSSDBG_CRIT_FAILURE
which should help to identify in which cases sdap_save_grpmem() is
called to see if it is valid or not.
This leads to a question I didn't found a good answer so far.
sdap_save_grpmem() is only called from sdap_save_groups() and only "if
(twopass && !populate_members)". I have to admit that I'm currently not
sure about the meaning and usage of the populate_members boolean. The
commit message introducing it says
"""
commit c4e120cec00807ef4fe7a0b7af96f5b0eece8a4b
Author: Ralf Haferkamp <rhafer(a)suse.de>
Date: Mon Oct 11 17:13:58 2010 +0200
Shortcut for save_group() to accept sysdb DNs as member attributes
Addtional parameter "populate_members" for save_group() and save_groups()
to indicate that the "member" attribute of the groups is populated with
sysdb DNs of the members (instead of LDAP DNs).
"""
while there is the call in sdap_get_groups_done():
ret = sdap_save_groups(state, state->sysdb, state->dom, state->opts,
state->groups, state->count,
!state->dom->ignore_group_members, NULL,
state->lookup_type == SDAP_LOOKUP_SINGLE,
&state->higher_usn);
where populate_members is directly related to ignore_group_members which
is imo not related to the original use case. populate_members might not
be a good choice of name in the original commit but I wonder if the
meaning has changed over time?
The second patch just avoids a use-less group lookup if the primary
group is already in the cache.
bye,
Sumit