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 138ac13459da69f6c48d17d9f7c639998a7f9c99 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/3202 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/db/sysdb_ops.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 7ca6575ce..f56347387 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -2289,12 +2289,62 @@ 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_sid_str = NULL; + const char *previous_uuid = NULL; + const char *previous_original_dn = NULL; + bool same_sid_str = false; + bool same_uuid = false; + bool same_original_dn = false; tmp_ctx = talloc_new(NULL); if (!tmp_ctx) { return ENOMEM; } + ret = sysdb_search_group_by_gid(tmp_ctx, domain, gid, NULL, &msg); + if (ret != ENOENT) { + previous_sid_str = ldb_msg_find_attr_as_string(msg, + SYSDB_SID_STR, + NULL); + if (previous_sid_str != NULL && sid_str != NULL) { + same_sid_str = strcmp(previous_sid_str, sid_str) == 0; + } else if (previous_sid_str == NULL && sid_str == NULL) { + same_sid_str = true; + } else { + same_sid_str = false; + } + + previous_uuid = ldb_msg_find_attr_as_string(msg, SYSDB_UUID, NULL); + if (previous_uuid != NULL && uuid != NULL) { + same_uuid = strcmp(previous_uuid, uuid) == 0; + } else if (previous_uuid == NULL && uuid == NULL) { + same_uuid = true; + } else { + same_uuid = false; + } + + previous_original_dn = ldb_msg_find_attr_as_string(msg, + SYSDB_ORIG_DN, + NULL); + if (previous_original_dn != NULL && original_dn != NULL) { + same_original_dn = strcmp(previous_original_dn, original_dn) == 0; + } else if (previous_original_dn == NULL && original_dn == NULL) { + same_original_dn = true; + } else { + same_original_dn = false; + } + + if (same_sid_str || same_uuid || same_original_dn) { + 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 24e5381cd56f32afc556f881f45832b76f3bd1a9 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/3202 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 c186ed2fb..3b30b6788 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; @@ -7045,6 +7110,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
