On Mon, Jan 27, 2014 at 09:46:26AM +0100, Lukas Slebodnik wrote: > ehlo, > > two patches are attached. > The 1st one is almost he same like patch: > commit 16b27fcceebcbbaeefaf5b9bdf2dec3065adba4a > LDAP: Don't fail if subdomain cannot be found by sid > > I didn't notice that similar change was done in two separeted patches. > > > I am not sure if 2nd is right solution, because some conditions are litle bit > confusing for me. > > LS
> From 6fb4a41e7340f48565d5d81f079e9861b5b3c71e Mon Sep 17 00:00:00 2001 > From: Lukas Slebodnik <lsleb...@redhat.com> > Date: Fri, 24 Jan 2014 17:03:27 +0100 > Subject: [PATCH 1/2] LDAP: store group if subdomain cannot be found by sid > > Domain needn't contain sid if id_provider is ldap. > With enabled id mapping, group couldn't be stored, because domain > couldn't be found by sid. > > Resolves: > https://fedorahosted.org/sssd/ticket/2172 > --- > src/providers/ldap/sdap_async_groups.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/src/providers/ldap/sdap_async_groups.c > b/src/providers/ldap/sdap_async_groups.c > index > 4ae772636863acf4c1fd59a20a20d1ad3a28ace0..55813c0fe4ffe864e1725959ad2a6ff51b1ffc65 > 100644 > --- a/src/providers/ldap/sdap_async_groups.c > +++ b/src/providers/ldap/sdap_async_groups.c > @@ -450,6 +450,7 @@ static int sdap_save_group(TALLOC_CTX *memctx, > bool posix_group; > bool use_id_mapping; > char *sid_str; > + struct sss_domain_info *subdomain; I know that the function is called 'find_subdomain_by_sid' but if I read its code right, it can return also the main domain and is actually used this way on a couple of places. So I suggest we rename the function in a separate patch. Similarly, this variable shouldn't be called 'subdomain', but maybe 'sid_dom' or 'group_dom' ? (the same misnomer seems to be in save_user) > int32_t ad_group_type; > > tmpctx = talloc_new(NULL); > @@ -488,11 +489,12 @@ static int sdap_save_group(TALLOC_CTX *memctx, > /* If this object has a SID available, we will determine the correct > * domain by its SID. */ > if (sid_str != NULL) { > - dom = find_subdomain_by_sid(get_domains_head(dom), sid_str); > - if (dom == NULL) { > - DEBUG(SSSDBG_OP_FAILURE, ("SID %s does not belong to any known " > + subdomain = find_subdomain_by_sid(get_domains_head(dom), sid_str); > + if (subdomain) { > + dom = subdomain; > + } else { > + DEBUG(SSSDBG_TRACE_FUNC, ("SID %s does not belong to any known " > "domain\n", sid_str)); This change is correct. But I checked the other calls to find_subdomain_by_sid() and there are some other places where a similar change might be needed: src/providers/simple/simple_access_check.c:614 and several places in src/providers/ldap/sdap_async_initgroups_ad.c But I admit I only checked the code visually and didn't do any testing. Because there is a deadline coming up, we can commit this patch, but then we should file a ticket to inspect the other usages And one related idea -- some code in the LDAP provider really depends on having data like domain SID available. I wonder if we could add a new LDAP schema (SDAP_SCHEMA_AD_BE) that would only be used by the AD provider and code that depends on having the AD data be only called if this schema is selected. The current AD schema could be still used for cases where the LDAP ID provider is used. An alternative might be to split the code better during the upcoming refactoring of the LDAP provider. > - return ERR_DOMAIN_NOT_FOUND; > } > } > > -- > 1.8.5.3 > > From 8ee3656c5a24abf5520ea5c636935ff9b0c5fb2d Mon Sep 17 00:00:00 2001 > From: Lukas Slebodnik <lsleb...@redhat.com> > Date: Fri, 24 Jan 2014 19:11:57 +0100 > Subject: [PATCH 2/2] LDAP: Fix group handling with AD schema > > regression introduced in commit 8280c5213094a72fcaa499dda2f8647246185d45 Could we set the ldap_group_type in the gen_ad2008r2_group_map instead? Something like: - { "ldap_group_type", NULL, SYSDB_GROUP_TYPE, NULL }, + { "ldap_group_type", "groupType", SYSDB_GROUP_TYPE, NULL }, _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
