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.