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(a)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(a)ad_domain.com
> > $ id aduser(a)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: