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
> 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?
> 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@lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
