On Mon, Sep 16, 2013 at 05:30:38PM +0200, Jakub Hrozek wrote:
> On Thu, Sep 12, 2013 at 02:01:43PM +0200, Pavel Březina wrote:
>> On 09/10/2013 03:46 PM, Jakub Hrozek wrote:
>>> On Tue, Sep 03, 2013 at 12:42:21PM +0200, Pavel Březina wrote:
>>>> Patch 0001: I haven't used this one after all, but I still think it
>>>> is nice to have. It reduces amount of code duplication.
>>>>
>>>> Patch 0005:
>>>>> + /* Try to get group SID and assign it a domain */ +
>>>>> group_sid = ldb_msg_find_attr_as_string(group, SYSDB_SID_STR,
>>>>> NULL); + if (group_sid == NULL) {
>>>>
>>>> I'm not sure if this branch isn't a dead code, but I kept it
there
>>>> because it mimics current behaviour.
>>>>
>>>>> + /* We will look it up in main domain. */ + domain
>>>>> = state->ctx->domain; + } else { + domain =
>>>>> find_subdomain_by_sid(state->ctx->domain, group_sid); +
if
>>>>> (domain == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE,
>>>>> ("There is no domain information for " +
>>>>> "SID %s\n", group_sid)); + return ENOENT; +
} +
>>>>> }
>>>
>>>> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> Date:
>>>> Mon, 2 Sep 2013 13:36:25 +0200 Subject: [PATCH 1/6] util: add
>>>> sss_idmap_talloc[_free]
>>>>
>>>> Remove code duplication.
>>>
>>> Removing code duplication added code :-) 76 insertions(+), 62
>>> deletions(-)
>>>
>>> But it's mostly the comment blobs, so ack.
>>>
>>>> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com> Date:
>>>> Tue, 27 Aug 2013 14:53:03 +0200 Subject: [PATCH 2/6] simple access
>>>> tests: fix typos
>>>
>>> ACK
>>>
>>> >From 5648c71b1a5a6bad5ce8e3361c7b4baf7d972c0b Mon Sep 17 00:00:00
>>>> 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
>>>> Date: Tue, 27 Aug 2013 14:02:42 +0200 Subject: [PATCH 3/6] simple
>>>> provider: support subdomain users
>>>>
>>>> Resolves:
https://fedorahosted.org/sssd/ticket/2034
>>>
>>> There is one thing that still needs fixing -- the case sensitivity
>>> flag is still read from the master domain through be_ctx->domain. In
>>> general, the providers should not access be_ctx->domain directly, but
>>> rather the handler should assign a sss_domain_info structure to some
>>> per-provider or per-request state and the code should operate on
>>> that.
>>>
>>> >From c1b31c3c2eefa71f5c17539fd615a785d7e1ccd0 Mon Sep 17 00:00:00
>>>> 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
>>>> Date: Mon, 2 Sep 2013 15:15:59 +0200 Subject: [PATCH 4/6] util: add
>>>> find_subdomain_by_sid()
>>>
>>> ACK, but please file a ticket to mass-namespace the functions in
>>> this module.
>>
>>
https://fedorahosted.org/sssd/ticket/2086
>>
>>> >From 78fff868aa9f1b5a8392328973e4ce3ba000a9e6 Mon Sep 17 00:00:00
>>>> 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
>>>> Date: Wed, 28 Aug 2013 11:35:27 +0200 Subject: [PATCH 5/6] simple
>>>> provider: support subdomain groups
>>>>
>>>
>>> Same comment about case sensitivity.
>>>
>>> >From 844b9cc30ad694116772f519791b3725a72d178a Mon Sep 17 00:00:00
>>>> 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
>>>> Date: Tue, 3 Sep 2013 12:34:07 +0200 Subject: [PATCH 6/6] simple
>>>> access test: initialize be_ctx for all tests
>>>>
>>>
>>> ACK
>>
>> Thanks. New patches are attached. I'm sending the case sensitivity
>> change in a separate patch, since I think it can stand on its own as
>> improvement of subdomains support in simple access provider.
>>
>> I also included patch that contains find_subdomain_by_object_name().
>>
>
> Hi,
>
> as part of review, I resolved conflict between your and Lukas' patches.
> Can you check if it's OK?
No it's not :-)
I'll prepare a local version for my review, but it would be nice if you
resent the whole set.