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/

Reply via email to