[SSSD] [PATCH] Retain group members in setups where id_provider=ad and ignore_group_members=True

Sumit Bose sbose at redhat.com
Fri May 29 16:24:06 UTC 2015


On Fri, May 22, 2015 at 05:35:49PM +0200, Jakub Hrozek wrote:
> On Fri, May 22, 2015 at 04:56:31PM +0200, Sumit Bose wrote:
> > On Fri, May 22, 2015 at 03:52:20PM +0200, Jakub Hrozek wrote:
> > > On Mon, May 11, 2015 at 12:27:03PM +0200, Jakub Hrozek wrote:
> > > > > I like with this direction more. I was even thinking we should not use name
> > > > > in RDN at all with the AD domain and always use objectSID. Then we would
> > > > > also have fewer problems changing numerical IDs..
> > > > > 
> > > > > My only concern is that this is an invasive solution that might bring
> > > > > bugs. Currently there is around 40 occurences of LDB_SCOPE_BASE in the
> > > > > code. At least those in sysdb_views.c and sysdb_asq_search(),
> > > > > sysdb_initgroups() and sysdb_initgroups_with_views() look like they
> > > > > would need to be converted. And currently the ignore_group_members bug
> > > > > is hitting users..
> > > > 
> > > > Oh and one more argument against renaming -- it is actually performance
> > > > intensive, because the memberof plugin needs to establish all the links
> > > > again..
> > > > 
> > > > So I would prefer to not use the name as RDN value at all, but the scope
> > > > seems out of question for 1.12 and even for 1.13 looks like a stretch
> > > > goal.
> > > 
> > > As also discussed elsewhere with Sumit, we need to first stop using the
> > > DN for searches in our cache and then stop using the name attribute in
> > > RDN -- at this point we won't need to rename the groups at all.
> > > 
> > > But that's a future goal, so as a short-term fix I implemented what
> > > Sumit proposed earlier -- if ignore_group_members is set to True, we
> > > fetch the group entry as if POSIX attributes were in use or as if the
> > > group was located in a subdomain.
> > 
> > > From dd1c23e5c0b197819eea793a118f89bc03fe9798 Mon Sep 17 00:00:00 2001
> > > From: Jakub Hrozek <jhrozek at redhat.com>
> > > Date: Fri, 22 May 2015 15:19:31 +0200
> > > Subject: [PATCH] Download complete groups if ignore_group_members is set with
> > >  tokengroups
> > > 
> > > Resolves:
> > >     https://fedorahosted.org/sssd/ticket/2644
> > > 
> > > When tokenGroups are enabled, we save groups using their SID as the RDN
> > > attribute during initgroups() and later, if the groups is requested and saved
> > > again with the full name, remove the original and save the new group entry.
> > > 
> > > Saving the new group entry would break if ignore_group_members is also
> > > set, because the new group entry would lack the "member" attribute, so the
> > > member/memberof links between the new group and the user entry wouldn't
> > > be established again.
> > > 
> > > This patch changes the initgroups processing so that the full group
> > > object is fetched when initgroups is enabled but together with
> > > ignore_group_members.
> > > 
> > > To reproduce the bug, set: ignore_group_members = True with a
> > > backend that uses:
> > >     id_provider = ad
> > > Then run:
> > >     $ id aduser at ad_domain.com
> > >     $ id aduser at ad_domain.com
> > > ---
> > >  src/providers/ldap/sdap_async_initgroups_ad.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c
> > > index 9915f1863f172d5d3f59afe03abbbfb87fdf3409..1db07049b21a65eb33ed5fb096ce606453504f38 100644
> > > --- a/src/providers/ldap/sdap_async_initgroups_ad.c
> > > +++ b/src/providers/ldap/sdap_async_initgroups_ad.c
> > > @@ -1445,7 +1445,9 @@ sdap_ad_tokengroups_initgroups_send(TALLOC_CTX *mem_ctx,
> > >      state->use_id_mapping = use_id_mapping;
> > >      state->domain = domain;
> > >  
> > > -    if (state->use_id_mapping && !IS_SUBDOMAIN(state->domain)) {
> > > +    if (state->use_id_mapping
> > > +            && !IS_SUBDOMAIN(state->domain)
> > > +            && state->domain->ignore_group_members == false) {
> > 
> > Since this is a workaround until a more general solution is available I
> > would recommend to add a comment with an explanation and maybe even a
> > link to a ticket to make sure that the workaround is not 'optimized'
> > away accidentally and that it is removed when the solution is available.
> > 
> > bye,
> > Sumit
> 
> Sure, good idea.

Hi,

I did a couple of tests and didn't found an issue (cause by the patch).
CI passes as well:

http://sssd-ci.duckdns.org/logs/job/16/38/summary.html

so ACK.

bye,
Sumit



More information about the sssd-devel mailing list