URL: https://github.com/SSSD/sssd/pull/613 Author: fidencio Title: #613: cache_req: Keep the files provider as the first domain to be searched Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/613/head:pr613 git checkout pr613
From 28b1d6bf51f6c9bf85ec4a59f70143004b2edcb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 9 Jul 2018 12:58:34 +0200 Subject: [PATCH 1/3] cache_req: keep the files provider as the first domain to be searched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we can't guarantee any order on which domain will the first to be searched. More than that, in case domain_resolution_order is set, we actually enforce that the first domain searched will respect the option set. This behaviour is not exactly the expect, as the implicit files domain has to be searched first in order to avoid querying for local users in remote domains. In order to enforce this, let's just keep the files domain as the first to be searched, always! Resolves: https://pagure.io/SSSD/sssd/issue/3768 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/common/cache_req/cache_req_domain.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/responder/common/cache_req/cache_req_domain.c b/src/responder/common/cache_req/cache_req_domain.c index ad86d1252..d1621cbab 100644 --- a/src/responder/common/cache_req/cache_req_domain.c +++ b/src/responder/common/cache_req/cache_req_domain.c @@ -148,6 +148,7 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, int flag = SSS_GND_ALL_DOMAINS; int i; bool enforce_non_fqnames = false; + bool files_provider = false; errno_t ret; /* Firstly, in case a domains' resolution order is passed ... iterate over @@ -190,6 +191,8 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, continue; } + files_provider = is_files_provider(dom); + cr_domain = talloc_zero(mem_ctx, struct cache_req_domain); if (cr_domain == NULL) { ret = ENOMEM; @@ -207,11 +210,21 @@ cache_req_domain_new_list_from_string_list(TALLOC_CTX *mem_ctx, * NOTE: we do *not* want to use fully qualified names for the * files provider.*/ if (resolution_order != NULL) { - if (!is_files_provider(cr_domain->domain)) { + if (!files_provider) { sss_domain_info_set_output_fqnames(cr_domain->domain, true); } } + /* The implicit files provider should always be searched firstly, + * doesn't matter whether the domain_resolution_order set! + * + * By doing this we avoid querying other domains for local users. + */ + if (files_provider) { + DLIST_ADD(cr_domains, cr_domain); + continue; + } + DLIST_ADD_END(cr_domains, cr_domain, struct cache_req_domain *); } From 622f9c855e1ae8fd15581e924ef04b88f30a33b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Tue, 10 Jul 2018 11:05:30 +0200 Subject: [PATCH 2/3] tests: add basic tests for cache_req_domain_new_list_from_domain_resolution_order() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related: https://pagure.io/SSSD/sssd/issue/3768 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- Makefile.am | 14 ++- src/tests/domain_resolution_order-tests.c | 198 ++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 src/tests/domain_resolution_order-tests.c diff --git a/Makefile.am b/Makefile.am index 73e40def8..4786639f7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -219,7 +219,8 @@ if HAVE_CHECK ipa_hbac-tests \ sss_idmap-tests \ responder_socket_access-tests \ - safe-format-tests + safe-format-tests \ + domain_resolution_order-tests if BUILD_SSH non_interactive_check_based_tests += sysdb_ssh-tests @@ -2059,6 +2060,17 @@ files_tests_LDADD = \ $(SSSD_INTERNAL_LTLIBS) endif # HAVE_INOTIFY +domain_resolution_order_tests_SOURCES = \ + src/tests/domain_resolution_order-tests.c \ + src/responder/common/cache_req/cache_req_domain.c +domain_resolution_order_tests_CFLAGS = \ + $(AM_CFLAGS) \ + $(CHECK_CFLAGS) +domain_resolution_order_tests_LDADD = \ + $(CHECK_LIBS) \ + libsss_test_common.la \ + $(SSSD_INTERNAL_LTLIBS) + SSSD_RESOLV_TESTS_OBJ = \ $(SSSD_RESOLV_OBJ) diff --git a/src/tests/domain_resolution_order-tests.c b/src/tests/domain_resolution_order-tests.c new file mode 100644 index 000000000..79a63c568 --- /dev/null +++ b/src/tests/domain_resolution_order-tests.c @@ -0,0 +1,198 @@ +/* + Authors: + Fabiano Fidêncio <fiden...@redhat.com> + + Copyright (C) 2018 Red Hat + + SSSD tests: Domain Resolution Order tests + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. +*/ + +#include <stdlib.h> +#include <check.h> +#include <popt.h> + +#include "tests/common.h" +#include "responder/common/cache_req/cache_req_domain.h" + +#define DOMAIN_1 "one.domain.test" +#define DOMAIN_2 "two.domain.test" +#define DOMAIN_3 "three.domain.test" + +static TALLOC_CTX *test_ctx = NULL; + +static struct sss_domain_info *create_test_domains(TALLOC_CTX *mem_ctx, + bool with_files_provider) +{ + struct sss_domain_info *dom; + struct sss_domain_info *head; + const char *domains[3] = { DOMAIN_1, DOMAIN_2, DOMAIN_3 }; + const char *providers[3] = { "ldap", "ldap", "ldap" }; + + if (with_files_provider) { + providers[2] = "files"; + } + + for (int i = 0; i < 3; i++) { + dom = talloc_zero(mem_ctx, struct sss_domain_info); + fail_if(dom == NULL, "failed to allocate memory for sss_domain_info\n"); + + dom->name = discard_const(domains[i]); + dom->provider = discard_const(providers[i]); + + if (i == 0) { + head = dom; + continue; + } + + DLIST_ADD(head, dom); + } + + return head; +} + +static void setup_domain_resolution_order_test(void) +{ + + test_ctx = talloc_new(NULL); + fail_if(test_ctx == NULL, "failed to allocate memory for test_ctx\n"); +} + +static void teardown_domain_resolution_order_test(void) +{ + talloc_free(test_ctx); +} + +START_TEST(test_dro_with_files_provider) +{ + struct cache_req_domain *cr_domains = NULL; + struct cache_req_domain *cr_domain; + struct sss_domain_info *dom; + const char *dro = "two.domain.test:one.domain.test"; + const char *expected_order[3] = { "three.domain.test", + "two.domain.test", + "one.domain.test" }; + errno_t ret; + int i; + + dom = create_test_domains(test_ctx, true); + fail_if(dom == NULL, "failed to create the tests domains\n"); + + cr_domains = talloc_zero(test_ctx, struct cache_req_domain); + ret = cache_req_domain_new_list_from_domain_resolution_order(test_ctx, + dom, + dro, + &cr_domains); + fail_if(ret != EOK, "domain_new_list_from_domain_resolution_order() failed\n"); + + i = 0; + for (cr_domain = cr_domains; cr_domain != NULL; cr_domain = cr_domain->next) { + ret = strcmp(expected_order[i], cr_domain->domain->name); + fail_if(ret != 0, + "expected the \"%d\" domain to be \"%s\", but it's \"%s\"\n", + i, expected_order[i], cr_domain->domain->name); + i++; + } +} +END_TEST + +START_TEST(test_dro_without_files_provider) +{ + struct cache_req_domain *cr_domains = NULL; + struct cache_req_domain *cr_domain; + struct sss_domain_info *dom; + const char *dro = "two.domain.test:one.domain.test"; + const char *expected_order[3] = { "two.domain.test", + "one.domain.test", + "three.domain.test" }; + errno_t ret; + int i; + + dom = create_test_domains(test_ctx, false); + fail_if(dom == NULL, "failed to create the tests domains\n"); + + cr_domains = talloc_zero(test_ctx, struct cache_req_domain); + ret = cache_req_domain_new_list_from_domain_resolution_order(test_ctx, + dom, + dro, + &cr_domains); + fail_if(ret != EOK, "domain_new_list_from_domain_resolution_order() failed\n"); + + i = 0; + for (cr_domain = cr_domains; cr_domain != NULL; cr_domain = cr_domain->next) { + ret = strcmp(expected_order[i], cr_domain->domain->name); + fail_if(ret != 0, + "expected the \"%d\" domain to be \"%s\", but it's \"%s\"\n", + i, expected_order[i], cr_domain->domain->name); + i++; + } +} +END_TEST + +static Suite *dro_suite(void) +{ + Suite *s = suite_create("domain_resolution_order_suite"); + + TCase *tc_domain_resolution_order = tcase_create("domain_resolution_order"); + tcase_add_checked_fixture(tc_domain_resolution_order, + setup_domain_resolution_order_test, + teardown_domain_resolution_order_test); + + tcase_add_test(tc_domain_resolution_order, + test_dro_with_files_provider); + tcase_add_test(tc_domain_resolution_order, + test_dro_without_files_provider); + suite_add_tcase(s, tc_domain_resolution_order); + + return s; +} + +int main(int argc, const char *argv[]) +{ + int number_failed; + int opt; + poptContext pc; + int debug = 0; + + struct poptOption long_options[] = { + POPT_AUTOHELP + { "debug-level", 'd', POPT_ARG_INT, &debug, 0, "Set debug level", NULL }, + POPT_TABLEEND + }; + + /* Set debug level to invalid value so we can decide if -d 0 was used. */ + debug_level = SSSDBG_INVALID; + + pc = poptGetContext(argv[0], argc, (const char **) argv, long_options, 0); + while((opt = poptGetNextOpt(pc)) != -1) { + fprintf(stderr, "\nInvalid option %s: %s\n\n", + poptBadOption(pc, 0), poptStrerror(opt)); + poptPrintUsage(pc, stderr, 0); + return 1; + } + poptFreeContext(pc); + + DEBUG_CLI_INIT(debug); + + tests_set_cwd(); + + Suite *s = dro_suite(); + SRunner *sr = srunner_create(s); + /* If CK_VERBOSITY is set, use that, otherwise it defaults to CK_NORMAL */ + srunner_run_all(sr, CK_ENV); + number_failed = srunner_ntests_failed(sr); + srunner_free(sr); + return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE; +} From f243c668b9d9667291100513556f45704656c5dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Tue, 10 Jul 2018 14:40:09 +0200 Subject: [PATCH 3/3] tests: add a test to ensure the output_fqname is false for files provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related: https://pagure.io/SSSD/sssd/issue/3743 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/domain_resolution_order-tests.c | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/tests/domain_resolution_order-tests.c b/src/tests/domain_resolution_order-tests.c index 79a63c568..3ad8cb665 100644 --- a/src/tests/domain_resolution_order-tests.c +++ b/src/tests/domain_resolution_order-tests.c @@ -141,6 +141,39 @@ START_TEST(test_dro_without_files_provider) } END_TEST +START_TEST(test_dro_output_fqnames) +{ + struct cache_req_domain *cr_domains = NULL; + struct cache_req_domain *cr_domain; + struct sss_domain_info *dom; + const char *dro = "two.domain.test:one.domain.test"; + errno_t ret; + bool output_fqname; + bool expected; + + dom = create_test_domains(test_ctx, true); + fail_if(dom == NULL, "failed to create the tests domains\n"); + + cr_domains = talloc_zero(test_ctx, struct cache_req_domain); + ret = cache_req_domain_new_list_from_domain_resolution_order(test_ctx, + dom, + dro, + &cr_domains); + fail_if(ret != EOK, "domain_new_list_from_domain_resolution_order() failed\n"); + + for (cr_domain = cr_domains; cr_domain != NULL; cr_domain = cr_domain->next) { + output_fqname = sss_domain_info_get_output_fqnames(cr_domain->domain); + expected = !is_files_provider(cr_domain->domain); + + fail_if(output_fqname != expected, + "output_fqname for domain \"%s\" is expected to be \"%s\", but it's \"%s\"\n", + cr_domain->domain->name, + expected ? "true" : "false", + output_fqname ? "true" : "false"); + } +} +END_TEST + static Suite *dro_suite(void) { Suite *s = suite_create("domain_resolution_order_suite"); @@ -154,6 +187,8 @@ static Suite *dro_suite(void) test_dro_with_files_provider); tcase_add_test(tc_domain_resolution_order, test_dro_without_files_provider); + tcase_add_test(tc_domain_resolution_order, + test_dro_output_fqnames); suite_add_tcase(s, tc_domain_resolution_order); return s;
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/QU2JBPSIJMBAAXKCLQKMLY5VGVLHEQGB/
