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

Jakub Hrozek jhrozek at redhat.com
Wed Sep 25 15:21:30 UTC 2013


On Wed, Sep 25, 2013 at 02:55:22PM +0200, Pavel Březina wrote:
> 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.

I agree completely. I think we should start by always using FQDNs for
all users and storing the plain name separately. But I'm afraid a change
like this is outside the scope of even 1.12.



More information about the sssd-devel mailing list