[SSSD] [PATCH] ad: store group in correct tree on initgroups via tokenGroups

Pavel Březina pbrezina at redhat.com
Wed Sep 25 12:55:22 UTC 2013


On 09/25/2013 01:07 PM, Jakub Hrozek wrote:
> On Tue, Sep 24, 2013 at 03:14:59PM +0200, Pavel Březina wrote:
>> On 09/21/2013 07:41 PM, Jakub Hrozek wrote:
>>> On Fri, Sep 06, 2013 at 02:30:54PM +0200, Pavel Březina wrote:
>>>> https://fedorahosted.org/sssd/ticket/2066
>>>>
>>>> Please note, that these patches has to be applied atop
>>>> 0004-util-add-find_subdomain_by_sid.patch from my simple
>>>> provider patches. You can also pull it from branch ad-groups at
>>>> my fedorapeople repo.
>>>>
>>>> I still don't know why AD returns Domain Users from root domain
>>>> in tokenGroups, even though subaduser is not member of DU at ad.pb
>>>> (AD bug perhaps?), but with these patches we store it
>>>> correctly.
>>>>
>>>
>>> Did you check if the SID of root domain's Domain Users is also
>>> returned in the PAC?
>>>
>>>> This should be also a step forward to #2064.
>>>
>>>> From 3b07596669cac3ec57e7a20e210b6ccd1fc5fa3b Mon Sep 17
>>>> 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>> <pbrezina at redhat.com> Date: Fri, 6 Sep 2013 11:08:56 +0200
>>>> Subject: [PATCH 1/4] util: add get_domains_head()
>>>>
>>>> This function will return head of the domain list.
>>>>
>>>> Resolves: https://fedorahosted.org/sssd/ticket/2066 ---
>>>> src/util/domain_info_utils.c | 13 +++++++++++++ src/util/util.h
>>>> |  2 ++ 2 files changed, 15 insertions(+)
>>>>
>>>> diff --git a/src/util/domain_info_utils.c
>>>> b/src/util/domain_info_utils.c index
>>>> f9d9057a811f3e08c451d7f6b44bc14202559962..65f3c21f0ad3de2d560347260d317fa21b3e69b3
>>>> 100644 --- a/src/util/domain_info_utils.c +++
>>>> b/src/util/domain_info_utils.c @@ -27,6 +27,19 @@ /* the
>>>> directory domain - realm mappings are written to */ #define
>>>> KRB5_MAPPING_DIR PUBCONF_PATH"/krb5.include.d"
>>>>
>>>> +struct sss_domain_info *get_domains_head(struct
>>>> sss_domain_info *domain) +{ +    struct sss_domain_info *dom =
>>>> NULL; + +    /* get to the top level domain */ +    for (dom =
>>>> domain; dom->parent != NULL; dom = dom->parent); + +    /*
>>>> proceed to the list head */ +    for (; dom->prev != NULL; dom
>>>> = dom->prev);
>>>
>>> Can we currently have a setup with multiple top level domains?
>>
>> No. We create separate sssd_be process for each top level domain
>> and I just checked that be_ctx->domain contains only one top level
>> domain in multi-domain environment.
>
> OK, then what is the point of the loop that proceeds to the list head
> if there can only be one domain?

state->domain is not necessarily equal to be_ctx->domain in this case. I
would have pass it to _send as parameter and create state->be_domain for
example.

I'm not quite sure why I've gone this path. I remember that I've
actually created state->be_domain at first but then switched to
get_domains_head().

I'm also quite surprised that get_domains_head() is called at only one
place. I thought I used it more :-)

>>
>>>> + +    return dom; +} +
>
> Otherwise the patches work for me, so ACK.
>
> But I'm wondering whether (in a future patch, maybe during
> 1.12/1.13) change updating members to primarily work on DNs, not
> names?

Dunno. It may be a good idea. To be more general, we should completely
unify how we deal with objects from main domain and subdomain. At the
moment we have lots of code that goes like if (subdomain) then do this
else if (maindomain) then do this. My idea is to see main domain in the
same light as sub domain.

Also at the moment, subdomains are basically handled on sdap level. I'd
like to bring them more to back-end level.

It is something I'd like to talk about next time, when all devs are face
to face.




More information about the sssd-devel mailing list