On Wed, Sep 16, 2015 at 04:29:20PM +0200, Jakub Hrozek wrote:
On Thu, Jul 23, 2015 at 02:34:52PM +0200, Lukas Slebodnik wrote:
> On (23/07/15 10:57), Jakub Hrozek wrote:
> >On Thu, Jul 23, 2015 at 09:39:00AM +0200, Lukas Slebodnik wrote:
> >> On (21/07/15 21:33), Jakub Hrozek wrote:
> >> >Hi,
> >> >
> >> >the attached patch fixes regression tracked by
> >> >https://fedorahosted.org/sssd/ticket/2723
> >> >
> >> >Please see the commit message for more details.
> >>
> >> >From 2f4e2e6d5e8f29f5b2cdd9f0b825edc172da57ea Mon Sep 17 00:00:00 2001
> >> >From: Jakub Hrozek <jhrozek(a)redhat.com>
> >> >Date: Tue, 21 Jul 2015 21:00:27 +0200
> >> >Subject: [PATCH] LDAP: imposing sizelimit=1 for single-entry searches
breaks
> >> > overlapping domains
> >> >
> >> >https://fedorahosted.org/sssd/ticket/2723
> >> >
> >> >In case there are overlapping sdap domains, a search for a single user
> >> >might match and return multiple entries. For instance, with AD domains
> >> >represented by search bases:
> >> > DC=win,DC=trust,DC=test
> >> > DC=child,DC=win,DC=trust,DC=test
> >> >
> >> >A search for user from win.trust.test would be based at:
> >> > DC=win,DC=trust,DC=test
> >> >but would match both search bases and return both users.
> >> >
> >> >Instead of performing complex filtering, just save both users. The
> >> >responder would select the entry that matches the user's search.
> >> Patch works
> >> but do we need to store all users?
> >
> >No, but the number of users we store is at maximum the number of
> >subdomains, so I didn't think it was an issue.
> >
> >>
> >> IIRC we have a code where we choose user based on the best match of
> >> domain dn and used dn.
> >
> >Yes, we have sdap_domain_get_by_dn(). I tried to remove regression with
> >as minimal patch as possible (restore previous behaviour).
> >
> It is a regression just in master caused by "wildcard patches" which was
not
> backported to stable branches. So we do not need to have a minimal patch.
>
> So we are not in hurry. So it's just a blocker for next upstream release.
>
> >I can also work on additional patch that matches the original DN based
> >on base DN of sdap_domain if you prefer.. Something like:
> >
> > entry_match = None
> > for entry in matched_entries:
> > if sdap_domain_get_by_dn(entry) == be_req_domain:
> > entry_match = entry
> > break
> >
> > if entry_match == None:
> > raise NoMatchError
> As you wish.
>
After just only a month since the nack, attached are updated patches. I
would prefer to only apply the first one to downstream, since the others
are just optimizations and tests.
CI runs revealed the tests didn't link correctly on Debian. New patches
are attached.