URL: https://github.com/SSSD/sssd/pull/128
Author: fidencio
 Title: #128: Fix group renaming issue when "id_provider = ldap" is set
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/128/head:pr128
git checkout pr128
From 2589ad216464e10cf63fed5b41e4d1823e3f7f81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 18 Jan 2017 16:39:22 +0100
Subject: [PATCH 1/3] SYSDB_OPS: Add a function to check if a group gid is
 unique
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The code has already been utilized sysdb_add_group() and now got split
into a new function in order to be re-utilized wherever is needed.

Related:
https://fedorahosted.org/sssd/ticket/3282

Signed-off-by: Fabiano FidĂȘncio <fiden...@redhat.com>
---
 src/db/sysdb_ops.c | 53 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 77e4c1a..214441c 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2041,6 +2041,41 @@ int sysdb_add_basic_group(struct sss_domain_info *domain,
 
 /* =Add-Group-Function==================================================== */
 
+static int check_group_gid_is_unique(struct sss_domain_info *domain,
+                                     gid_t gid)
+{
+    TALLOC_CTX *tmp_ctx;
+    struct ldb_message *msg = NULL;
+    int ret;
+
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) {
+        return ENOMEM;
+    }
+
+    /* check no other groups with the same gid exist */
+    if (gid != 0) {
+        ret = sysdb_search_group_by_gid(tmp_ctx, domain, gid, NULL, &msg);
+        if (ret != ENOENT) {
+            if (ret == EOK) {
+                DEBUG(SSSDBG_TRACE_LIBS,
+                      "Group with the same gid exists: [%"SPRIgid"].\n", gid);
+                ret = EEXIST;
+            } else {
+                DEBUG(SSSDBG_TRACE_LIBS,
+                      "sysdb_search_group_by_gid failed for gid: "
+                      "[%"SPRIgid"].\n", gid);
+            }
+            goto done;
+        }
+    }
+
+    ret = EOK;
+
+done:
+    return ret;
+}
+
 int sysdb_add_group(struct sss_domain_info *domain,
                     const char *name, gid_t gid,
                     struct sysdb_attrs *attrs,
@@ -2093,21 +2128,9 @@ int sysdb_add_group(struct sss_domain_info *domain,
         }
     }
 
-    /* check no other groups with the same gid exist */
-    if (gid != 0) {
-        ret = sysdb_search_group_by_gid(tmp_ctx, domain, gid, NULL, &msg);
-        if (ret != ENOENT) {
-            if (ret == EOK) {
-                DEBUG(SSSDBG_TRACE_LIBS,
-                      "Group with the same gid exists: [%"SPRIgid"].\n", gid);
-                ret = EEXIST;
-            } else {
-                DEBUG(SSSDBG_TRACE_LIBS,
-                      "sysdb_search_group_by_gid failed for gid: "
-                      "[%"SPRIgid"].\n", gid);
-            }
-            goto done;
-        }
+    ret = check_group_gid_is_unique(domain, gid);
+    if (ret != EOK) {
+        goto done;
     }
 
     /* try to add the group */

From 91bcaecd76e367f20e8da1480f483848236830c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 18 Jan 2017 16:43:52 +0100
Subject: [PATCH 2/3] SYSDB_OPS: Avoid adding incomplete groups with duplicated
 GID
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This situation can be hit when renaming a group. Without this patch, the
renamed group would be added to the cache while the old entry would be
there as well, causing some issues like not showing the groupname when
calling `groups user`.

Now, we take the a similar approach taken by sysdb_add_groupp(), checking
whether the group gid already exists and then deleting the old entry
before adding the new one.

Resolves:
https://fedorahosted.org/sssd/ticket/3282

Signed-off-by: Fabiano FidĂȘncio <fiden...@redhat.com>
---
 src/db/sysdb_ops.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 214441c..e985217 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2239,6 +2239,15 @@ int sysdb_add_incomplete_group(struct sss_domain_info *domain,
         return ENOMEM;
     }
 
+    ret = check_group_gid_is_unique(domain, gid);
+    if (ret != EOK) {
+        if (ret == EEXIST) {
+            sysdb_delete_group(domain, NULL, gid);
+        } else {
+            goto done;
+        }
+    }
+
     /* try to add the group */
     ret = sysdb_add_basic_group(domain, name, gid);
     if (ret) goto done;

From 984cf33775f551cba1faf1ffb4ab2581978f6314 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Mon, 23 Jan 2017 17:49:20 +0100
Subject: [PATCH 3/3] TESTS: Add sysdb test for renaming incomplete groups
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Related:
https://fedorahosted.org/sssd/ticket/3282

Signed-off-by: Fabiano FidĂȘncio <fiden...@redhat.com>
---
 src/tests/sysdb-tests.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index e011c4b..7c229f7 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -988,6 +988,53 @@ START_TEST (test_sysdb_add_incomplete_group)
 }
 END_TEST
 
+START_TEST (test_sysdb_rename_incomplete_group)
+{
+    struct sysdb_test_ctx *test_ctx;
+    struct test_data *data;
+    int ret;
+    struct ldb_message *msg = NULL;
+    const char *old_fqdn_groupname;
+    const char *stored_fqdn_groupname;
+    char *new_groupname;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    if (ret != EOK) {
+        fail("Could not set up the test");
+        return;
+    }
+
+    data = test_data_new_group(test_ctx, _i);
+    fail_if(data == NULL);
+
+    /* Add incomplete group */
+    ret = test_add_incomplete_group(data);
+    fail_if(ret != EOK, "Could not add incomplete group %s", data->groupname);
+
+    /* "Rename" the incomplete group by adding
+     * the very same group with a different groupname. */
+    old_fqdn_groupname = data->groupname;
+    new_groupname = talloc_asprintf(data, "foo%d", _i);
+    data->groupname = sss_create_internal_fqname(test_ctx,
+                                                 new_groupname,
+                                                 test_ctx->domain->name);
+
+    ret = test_add_incomplete_group(data);
+    fail_if(ret != EOK, "Could not add incomplete group %s: %s [%d]",
+           data->groupname, ret, sss_strerror(ret));
+
+    /* Check whether the name stored is the new one */
+    ret = sysdb_search_group_by_gid(test_ctx, test_ctx->domain, data->gid,
+                                    NULL, &msg);
+    stored_fqdn_groupname = ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
+    fail_if(strcmp(data->groupname, stored_fqdn_groupname) != 0,
+            "Failed renaming group \"%s\" to \"%s\". The stored name is \"%s\".",
+            old_fqdn_groupname, data->groupname, stored_fqdn_groupname);
+    talloc_free(test_ctx);
+}
+END_TEST
+
 START_TEST (test_sysdb_getpwnam)
 {
     struct sysdb_test_ctx *test_ctx;
@@ -6872,6 +6919,9 @@ Suite *create_sysdb_suite(void)
                         test_sysdb_add_incomplete_group,
                         28000, 28010);
     tcase_add_loop_test(tc_sysdb,
+                        test_sysdb_rename_incomplete_group,
+                        28000, 28010);
+    tcase_add_loop_test(tc_sysdb,
                         test_sysdb_remove_local_group_by_gid,
                         28000, 28010);
 
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to