On (22/07/14 18:43), Jakub Hrozek wrote:
On Thu, Mar 13, 2014 at 12:00:47PM +0100, Sumit Bose wrote:
On Wed, Mar 12, 2014 at 11:20:42PM +0100, Jakub Hrozek wrote:
Hi,
the attached two patches are not strictly related to tokenGroups processing, but it's very easy to reproduce the problem that way. The issue is only confusing DEBUG messages, but it has already cost me several hours in processing logs from an SSSD user, so I think a fix is due, at least for master.
See the patches and the commit messages for more details.
as a first note, the second patch makes nestedgroups-tests fail because sdap_idmap_domain_has_algorithmic_mapping() must be made available to the test and sdap_idmap_ctx must be initialized.
But since this 'only' influences the tests I will run some tests with the current version of the patches.
bye, Sumit
I completely forgot about these patches until I was doing cleanup of my git branches. Attached is a new revision..
From 3c4ce4a2b9f58e0b50f9ad48d07110b30bc74cd9 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Tue, 11 Mar 2014 17:39:31 +0100 Subject: [PATCH 2/2] Only check GID if ID-mapping
Makefile.am | 11 ++++++++--- src/providers/ldap/sdap_async_nested_groups.c | 9 ++++++++- src/tests/cmocka/test_nested_groups.c | 20 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/Makefile.am b/Makefile.am index e3592868ce29b569a71f6ad74bc24cc617301b34..783a8922ede16ea3cc3eb6df568773126c5661fb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1740,15 +1740,20 @@ fqnames_tests_LDADD = \ nestedgroups_tests_SOURCES = \ $(TEST_MOCK_OBJ) \ $(TEST_MOCK_PROVIDER_OBJ) \
- src/providers/ldap/sdap_idmap.c \ src/tests/cmocka/test_nested_groups.c \
- src/providers/ldap/sdap_async_nested_groups.c
- src/providers/ldap/sdap_async_nested_groups.c \
- $(NULL)
nestedgroups_tests_CFLAGS = \
- $(AM_CFLAGS)
- $(AM_CFLAGS) \
- $(NULL)
nestedgroups_tests_LDADD = \ $(CMOCKA_LIBS) \ $(SSSD_LIBS) \ $(SSSD_INTERNAL_LTLIBS) \
- libsss_test_common.la
- libsss_idmap.la \
- libsss_test_common.la \
- $(NULL)
test_sss_idmap_SOURCES = \ src/tests/cmocka/test_sss_idmap.c diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c index 305afbc9d4f153fe2daa6289cc318b1183be72ca..5398b14bc0e163c122def1c3d729b28d72873b82 100644 --- a/src/providers/ldap/sdap_async_nested_groups.c +++ b/src/providers/ldap/sdap_async_nested_groups.c @@ -34,6 +34,7 @@ #include "providers/ldap/ldap_common.h" #include "providers/ldap/sdap_async.h" #include "providers/ldap/sdap_async_private.h" +#include "providers/ldap/sdap_idmap.h"
#define sdap_nested_group_sysdb_search_users(domain, filter) \ sdap_nested_group_sysdb_search((domain), (filter), true) @@ -242,6 +243,7 @@ sdap_nested_group_hash_group(struct sdap_nested_group_ctx *group_ctx, errno_t ret; int32_t ad_group_type; bool posix_group = true;
bool use_id_mapping;
if (group_ctx->opts->schema_type == SDAP_SCHEMA_AD) { ret = sysdb_attrs_get_int32_t(group, SYSDB_GROUP_TYPE, &ad_group_type);
@@ -265,7 +267,12 @@ sdap_nested_group_hash_group(struct sdap_nested_group_ctx *group_ctx, } }
- if (posix_group) {
- use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
group_ctx->opts->idmap_ctx,group_ctx->domain->name,group_ctx->domain->domain_id);- if (posix_group && !use_id_mapping) { ret = sysdb_attrs_get_uint32_t(group, map[SDAP_AT_GROUP_GID].sys_name, &gid); }
270 use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping( 271 group_ctx->opts->idmap_ctx, 272 group_ctx->domain->name, 273 group_ctx->domain->domain_id); 274 275 if (posix_group && !use_id_mapping) { // 'posix_group' can be true and 'use_id_mapping' is not equal to 0 // sysdb_attrs_get_uint32_t will not be called. 276 ret = sysdb_attrs_get_uint32_t(group, map[SDAP_AT_GROUP_GID].sys_name, 277 &gid); 278 } 279 if (!posix_group || ret == ENOENT || (ret == EOK && gid == 0)) { ^^^^^^^^^^^^^ 'ret' needn't be ENOENT because function 'sysdb_attrs_get_int32_t' could be called on line 249
'gid' will not be initialised.
LS