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? 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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel