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 177ee2072ad328011f0bc7ff5903e89cf62deeac 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 1/2] 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_group(), checking whether the group gid already exists and then deleting the old entry before adding the new one. It's important to note that the new approach only happens in case the new added group has the same sid, uuid or original dn than the group already added to the cache. In case it doesn't happen, the previous behavior is preserved. Resolves: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/db/sysdb_ops.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 4cfef6823..765dc389e 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -2289,12 +2289,42 @@ int sysdb_add_incomplete_group(struct sss_domain_info *domain, TALLOC_CTX *tmp_ctx; int ret; struct sysdb_attrs *attrs; + struct ldb_message *msg; + const char *previous = NULL; + const char *group_attrs[] = { SYSDB_SID_STR, SYSDB_UUID, SYSDB_ORIG_DN, NULL }; + const char *values[] = { sid_str, uuid, original_dn, NULL }; + bool same = false; tmp_ctx = talloc_new(NULL); if (!tmp_ctx) { return ENOMEM; } + ret = sysdb_search_group_by_gid(tmp_ctx, domain, gid, group_attrs, &msg); + if (ret != ENOENT) { + for (int i = 0; !same && group_attrs[i] != NULL; i++) { + previous = ldb_msg_find_attr_as_string(msg, + group_attrs[i], + NULL); + if (previous != NULL && values[i] != NULL) { + same = strcmp(previous, values[i]) == 0; + } else if (previous == NULL && values[i] == NULL) { + same = true; + } else { + same = false; + } + } + } + + if (same) { + ret = sysdb_delete_group(domain, NULL, gid); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "sysdb_delete_group() failed [%d]: %s\n", + ret, sss_strerror(ret)); + } + } + /* try to add the group */ ret = sysdb_add_basic_group(domain, name, gid); if (ret) goto done; From d540d7ad1a15a876a1a2336868f1d93ded56018e 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 2/2] TESTS: Add sysdb test for renaming incomplete groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test added also test the negative case, when a group has been added but it has a different sid, uuid and original dn, which results in an EEXIST return. Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/sysdb-tests.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 63572e067..0130a428d 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -989,6 +989,71 @@ 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); + + /* "Rename" the already added incomplete group by adding + * the very same group with a different name. */ + old_fqdn_groupname = data->groupname; + new_groupname = talloc_asprintf(data, "foo%d", _i); + fail_if(new_groupname == NULL, "talloc_asprintf() failed\n"); + data->groupname = sss_create_internal_fqname(test_ctx, + new_groupname, + test_ctx->domain->name); + fail_if(data->groupname == NULL, + "sss_create_internal_fqname() failed\n"); + + ret = test_add_incomplete_group(data); + fail_if(ret != EOK, "Could not add incomplete group %s: %s [%d]", + data->groupname, sss_strerror(ret), 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); + fail_if(ret != EOK, + "Cannot find the group \"%s\" by its gid", + data->groupname); + 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); + + + /* + * Negative test: + * Change the group sid, uuid and original_fd ... + * So the old group must not be removed from cache as it's a case of + * id collision. + */ + ret = sysdb_add_incomplete_group(data->ctx->domain, data->groupname, + data->gid, "foo", "foo", "foo", + true, 0); + fail_if(ret != EEXIST, + "Adding a completely different group with the gid should not " + "delete the old group and EEXIST must be returned! ret [%d]: %s\n", + ret, sss_strerror(ret)); + + talloc_free(test_ctx); +} +END_TEST + START_TEST (test_sysdb_getpwnam) { struct sysdb_test_ctx *test_ctx; @@ -7113,6 +7178,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
