[SSSD] [PATCH] LDAP: tokengroups do not work with id_provider=ldap

Jakub Hrozek jhrozek at redhat.com
Mon Jul 7 20:25:23 UTC 2014


On Mon, Jul 07, 2014 at 07:03:01PM +0200, Pavel Reichl wrote:
> On Mon, 2014-06-23 at 11:41 +0200, Sumit Bose wrote:
> > On Mon, Jun 23, 2014 at 11:20:52AM +0200, Jakub Hrozek wrote:
> > > On Tue, Jun 17, 2014 at 06:30:31PM +0200, Pavel Reichl wrote:
> > > > Hello,
> > > > 
> > > > please see attached patch.
> > > > 
> > > > Regards,
> > > > 
> > > > PR
> > > 
> > > The patch solves the problem, but I think one part should be improved:
> > > 
> > > > @@ -875,7 +893,13 @@ static void sdap_ad_tokengroups_initgr_mapping_done(struct tevent_req *subreq)
> > > >          domain = find_subdomain_by_sid(get_domains_head(state->domain), sid);
> > > >          if (domain == NULL) {
> > > >              DEBUG(SSSDBG_MINOR_FAILURE, "Domain not found for SID %s\n", sid);
> > > > -            continue;
> > > > +            if (state->domain->parent == NULL &&
> > > > +                state->domain->subdomains == NULL) {
> > > > +                domain = state->domain;
> > > > +                DEBUG(SSSDBG_TRACE_FUNC, "Using domain %s\n", domain->name);
> > > > +            } else {
> > > > +                continue;
> > > > +            }
> > > >          }
> > > 
> > > I think this is a bit dangerous. I wonder if we should have some
> > > modification of find_subdomain_by_sid that would return the first
> > > configured domain if no subdomain provider was configured or if no
> > > domains had a SID. This could be a separate function.
> > 
> > This sounds even more dangerous to me.
> > 
> > > 
> > > Anyhow, find_subdomain_by_sid is misnamed, we routinely use the function
> > > to find the primary domain.
> > 
> > I think find_subdomain_by_sid() does what the name says and of course it
> > can return the primary domain as long as the SID of the domain is know
  ^^^^^^
fwiw, this was my concern, the function is named "find_subdomain" yet it
can find both main domain and subdomain. But I won't bikeshed any further.

> > which is the case for the IPA and AD provider.
> > 
> 
> > What about adding an explicit check if the running id provider is the
> > plain LDAP provider?
> Would that be acceptable with you Jakub?

It's a bit hackish but I don't see any other way.

> 
> >  As an alternative the LDAP provider can add a
> > special value in the id member of the sss_dom_info struct and then
> > find_subdomain_by_sid can handle this case specially?
> > 
> > bye,
> > Sumit
> > 
> > > _______________________________________________
> > > sssd-devel mailing list
> > > sssd-devel at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> 
> 



More information about the sssd-devel mailing list