[SSSD] [PATCH] Two minor patches for tokenGroups nested group processing

Jakub Hrozek jhrozek at redhat.com
Tue Jul 22 16:43:56 UTC 2014


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..
-------------- next part --------------
>From 6351b84e18b8b1a78d7360b9b9075c0ec4fbe1ab Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 11 Mar 2014 17:37:47 +0100
Subject: [PATCH 1/2] No point in searching for gid if we already know the
 group should be filtered

---
 src/providers/ldap/sdap_async_nested_groups.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c
index c521e08745db613cfa18d809c1afb4c9aef59463..305afbc9d4f153fe2daa6289cc318b1183be72ca 100644
--- a/src/providers/ldap/sdap_async_nested_groups.c
+++ b/src/providers/ldap/sdap_async_nested_groups.c
@@ -265,9 +265,11 @@ sdap_nested_group_hash_group(struct sdap_nested_group_ctx *group_ctx,
         }
     }
 
-    ret = sysdb_attrs_get_uint32_t(group, map[SDAP_AT_GROUP_GID].sys_name,
-                                   &gid);
-    if (ret == ENOENT || (ret == EOK && gid == 0) || !posix_group) {
+    if (posix_group) {
+        ret = sysdb_attrs_get_uint32_t(group, map[SDAP_AT_GROUP_GID].sys_name,
+                                       &gid);
+    }
+    if (!posix_group || ret == ENOENT || (ret == EOK && gid == 0)) {
         DEBUG(SSSDBG_TRACE_ALL,
              "The group's gid was %s\n", ret == ENOENT ? "missing" : "zero");
         DEBUG(SSSDBG_TRACE_INTERNAL,
-- 
1.9.3

-------------- next part --------------
>From 3c4ce4a2b9f58e0b50f9ad48d07110b30bc74cd9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at 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);
     }
diff --git a/src/tests/cmocka/test_nested_groups.c b/src/tests/cmocka/test_nested_groups.c
index 0c1a53b0e6f1291b3ee55d3a4d4396e864ae228a..b64d67d3cc802d9bb1e585a63da04b68ecc460c4 100644
--- a/src/tests/cmocka/test_nested_groups.c
+++ b/src/tests/cmocka/test_nested_groups.c
@@ -26,7 +26,9 @@
 #include "tests/cmocka/common_mock.h"
 #include "tests/cmocka/common_mock_sdap.h"
 #include "tests/cmocka/common_mock_sysdb_objects.h"
+#include "providers/ldap/ldap_common.h"
 #include "providers/ldap/sdap.h"
+#include "providers/ldap/sdap_idmap.h"
 #include "providers/ldap/sdap_async_private.h"
 
 #define TESTS_PATH "tests_ldap_nested_groups"
@@ -55,6 +57,8 @@ struct nested_groups_test_ctx {
     struct sdap_options *sdap_opts;
     struct sdap_handle *sdap_handle;
     struct sdap_domain *sdap_domain;
+    struct sdap_idmap_ctx *idmap_ctx;
+    struct sdap_id_ctx *sdap_id_ctx;
 
     struct sysdb_attrs **users;
     struct sysdb_attrs **groups;
@@ -391,6 +395,7 @@ static void nested_groups_test_one_group_dup_group_members(void **state)
 
 void nested_groups_test_setup(void **state)
 {
+    errno_t ret;
     struct nested_groups_test_ctx *test_ctx = NULL;
     static struct sss_test_conf_param params[] = {
         { "ldap_schema", "rfc2307bis" }, /* enable nested groups */
@@ -419,6 +424,21 @@ void nested_groups_test_setup(void **state)
     test_ctx->sdap_domain = test_ctx->sdap_opts->sdom;
     test_ctx->sdap_handle = mock_sdap_handle(test_ctx);
     assert_non_null(test_ctx->sdap_handle);
+
+    test_ctx->sdap_id_ctx = talloc_zero(test_ctx,
+                                        struct sdap_id_ctx);
+    assert_non_null(test_ctx->sdap_id_ctx);
+
+    test_ctx->sdap_id_ctx->be = talloc_zero(test_ctx->sdap_id_ctx,
+                                            struct be_ctx);
+    assert_non_null(test_ctx->sdap_id_ctx->be);
+
+    test_ctx->sdap_id_ctx->opts = test_ctx->sdap_opts;
+    test_ctx->sdap_id_ctx->be->domain = test_ctx->tctx->dom;
+
+    ret = sdap_idmap_init(test_ctx, test_ctx->sdap_id_ctx, &test_ctx->idmap_ctx);
+    assert_int_equal(ret, EOK);
+    test_ctx->sdap_opts->idmap_ctx = test_ctx->idmap_ctx;
 }
 
 void nested_groups_test_teardown(void **state)
-- 
1.9.3



More information about the sssd-devel mailing list