From 1dced7370e55be16154bbb649606f928765819d0 Mon Sep 17 00:00:00 2001

From: Jakub Hrozek <jhrozek@redhat.com>

Date: Tue, 3 Dec 2013 20:45:44 +0100

Subject: [PATCH 28/31] AD: Add a utility function to create list of

 connections

ad_id.c and ad_access.c used the same block of code. With the upcoming

option to disable GC lookups, we should unify the code in a function to

avoid breaking one of the code paths.

The same applies for the LDAP connection to the trusted AD DC.

Includes a unit test.

---

 Makefile.am                       |  28 +++++

 src/providers/ad/ad_access.c      |  16 +--

 src/providers/ad/ad_access.h      |   4 +-

 src/providers/ad/ad_common.c      |  52 +++++++++

 src/providers/ad/ad_common.h      |   7 ++

 src/providers/ad/ad_id.c          |  29 ++---

 src/providers/ad/ad_init.c        |   3 +-

 src/tests/cmocka/test_ad_common.c | 221 ++++++++++++++++++++++++++++++++++++++

 8 files changed, 319 insertions(+), 41 deletions(-)

 create mode 100644 src/tests/cmocka/test_ad_common.c

diff --git a/Makefile.am b/Makefile.am

index 583ccdb499306268640bfb894f673c42945e19ff..da407038089f3c010dea139735db9e0e2f000943 100644

--- a/Makefile.am

+++ b/Makefile.am

@@ -152,6 +152,7 @@ if HAVE_CMOCKA

         test_sss_idmap \

         test_utils \

         ad_access_filter_tests \

+        ad_common_tests \

         test_search_bases

 endif

@@ -1398,6 +1399,7 @@ ad_access_filter_tests_SOURCES = \

     src/util/sss_krb5.c \

     src/util/find_uid.c \

     src/util/user_info_msg.c \

+    src/providers/ad/ad_common.c \

     src/tests/cmocka/test_ad_access_filter.c

 ad_access_filter_tests_CFLAGS = \

     $(AM_CFLAGS) \

@@ -1416,6 +1418,32 @@ ad_access_filter_tests_LDADD = \

     libsss_krb5_common.la \

     libsss_test_common.la

+ad_common_tests_SOURCES = \

+    $(sssd_be_SOURCES) \

+    src/util/sss_ldap.c \

+    src/util/sss_krb5.c \

+    src/util/find_uid.c \

+    src/util/user_info_msg.c \

+    src/tests/cmocka/test_ad_common.c

+ad_common_tests_CFLAGS = \

+    $(AM_CFLAGS) \

+    $(SYSTEMD_LOGIN_CFLAGS) \

+    -DUNIT_TESTING

+ad_common_tests_LDFLAGS = \

+    -Wl,-wrap,sdap_set_sasl_options

+ad_common_tests_LDADD = \

+    $(PAM_LIBS) \

+    $(CMOCKA_LIBS) \

+    $(SSSD_LIBS) \

+    $(CARES_LIBS) \

+    $(KRB5_LIBS) \

+    $(SSSD_INTERNAL_LTLIBS) \

+    $(SYSTEMD_LOGIN_LIBS) \

+    libsss_ldap_common.la \

+    libsss_idmap.la \

+    libsss_krb5_common.la \

+    libsss_test_common.la

+

 endif

 noinst_PROGRAMS = pam_test_client

diff --git a/src/providers/ad/ad_access.c b/src/providers/ad/ad_access.c

index 6995172db304810899e538b37572e4ba953db3e7..68a292abc88daa2f10f6797db50cc75335e80483 100644

--- a/src/providers/ad/ad_access.c

+++ b/src/providers/ad/ad_access.c

@@ -274,26 +274,12 @@ ad_access_send(TALLOC_CTX *mem_ctx,

         goto done;

     }

-    state->clist = talloc_zero_array(state, struct sdap_id_conn_ctx *, 3);

+    state->clist = ad_gc_conn_list(state, ctx->ad_id_ctx, domain);

     if (state->clist == NULL) {

         ret = ENOMEM;

         goto done;

     }

-    /* Always try GC first */

-    ctx->gc_ctx->ignore_mark_offline = false;

-    state->clist[0] = ctx->gc_ctx;

-    if (IS_SUBDOMAIN(domain) == false) {

-        /* fall back to ldap if gc is not available */

-        state->clist[0]->ignore_mark_offline = true;

-

-        /* With root domain users we have the option to

-         * fall back to LDAP in case ie POSIX attributes

-         * are used but not replicated to GC

-         */

-        state->clist[1] = ctx->ldap_ctx;

-    }

-

     ret = ad_access_step(req, state->clist[state->cindex]);

     if (ret != EOK) {

         goto done;

diff --git a/src/providers/ad/ad_access.h b/src/providers/ad/ad_access.h

index ca5e69729c574be53b7da04df0ff89446da04c58..3bd19ccc508b43f7103c7041dcc8573a00235097 100644

--- a/src/providers/ad/ad_access.h

+++ b/src/providers/ad/ad_access.h

@@ -26,9 +26,7 @@

 struct ad_access_ctx {

     struct dp_option *ad_options;

     struct sdap_access_ctx *sdap_access_ctx;

-

-    struct sdap_id_conn_ctx *ldap_ctx;

-    struct sdap_id_conn_ctx *gc_ctx;

+    struct ad_id_ctx *ad_id_ctx;

 };

 void

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c

index f679c11ad18078b454b778ef30e40cca716412cb..af0ec839964233c7642205f4489e5b6462509848 100644

--- a/src/providers/ad/ad_common.c

+++ b/src/providers/ad/ad_common.c

@@ -1096,3 +1096,55 @@ ad_id_ctx_init(struct ad_options *ad_opts, struct be_ctx *bectx)

     return ad_ctx;

 }

+

+struct sdap_id_conn_ctx *

+ad_get_dom_ldap_conn(struct ad_id_ctx *ad_ctx, struct sss_domain_info *dom)

+{

+    struct sdap_id_conn_ctx *conn;

+    struct sdap_domain *sdom;

+    struct ad_id_ctx *subdom_id_ctx;

+

+    if (IS_SUBDOMAIN(dom)) {

+        sdom = sdap_domain_get(ad_ctx->sdap_id_ctx->opts, dom);

+        if (sdom == NULL || sdom->pvt == NULL) {

+            DEBUG(SSSDBG_CRIT_FAILURE, ("No ID ctx available for [%s].\n",

+                                        dom->name));

+            return NULL;

+        }

+        subdom_id_ctx = talloc_get_type(sdom->pvt, struct ad_id_ctx);

+        conn = subdom_id_ctx->ldap_ctx;

+    } else {

+        conn = ad_ctx->ldap_ctx;

+    }

+

+    return conn;

+}

+

+struct sdap_id_conn_ctx **

+ad_gc_conn_list(TALLOC_CTX *mem_ctx, struct ad_id_ctx *ad_ctx,

+                struct sss_domain_info *dom)

+{

+    struct sdap_id_conn_ctx **clist;

+

+    clist = talloc_zero_array(mem_ctx, struct sdap_id_conn_ctx *, 3);

+    if (clist == NULL) return NULL;

+

+    /* Always try GC first */

+    clist[0] = ad_ctx->gc_ctx;

+    if (IS_SUBDOMAIN(dom) == true) {

+        clist[0]->ignore_mark_offline = false;

+        /* Subdomain users are only present in GC. */

+        return clist;

+    }

+

+    /* fall back to ldap if gc is not available */

+    clist[0]->ignore_mark_offline = true;

+

+    /* With root domain users we have the option to

+     * fall back to LDAP in case ie POSIX attributes

+     * are used but not replicated to GC

+     */

+    clist[1] = ad_ctx->ldap_ctx;

+

+    return clist;

+}

diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h

index b8b73c042b8a5433f720c89c04447c07cd3eac43..ed5b8584dc5327a24e60985486c6155604271fd2 100644

--- a/src/providers/ad/ad_common.h

+++ b/src/providers/ad/ad_common.h

@@ -115,6 +115,13 @@ ad_get_dyndns_options(struct be_ctx *be_ctx,

 struct ad_id_ctx *

 ad_id_ctx_init(struct ad_options *ad_opts, struct be_ctx *bectx);

+struct sdap_id_conn_ctx **

+ad_gc_conn_list(TALLOC_CTX *mem_ctx, struct ad_id_ctx *ad_ctx,

+               struct sss_domain_info *dom);

+

+struct sdap_id_conn_ctx *

+ad_get_dom_ldap_conn(struct ad_id_ctx *ad_ctx, struct sss_domain_info *dom);

+

 /* AD dynamic DNS updates */

 errno_t ad_dyndns_init(struct be_ctx *be_ctx,

                        struct ad_options *ctx);

diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c

index cf71b172dd7c241a9280a7ea72ef2518f66a7435..e47c41863a14eed695907548d64f4559fbae629d 100644

--- a/src/providers/ad/ad_id.c

+++ b/src/providers/ad/ad_id.c

@@ -188,12 +188,6 @@ get_conn_list(struct be_req *breq, struct ad_id_ctx *ad_ctx,

               struct sss_domain_info *dom, struct be_acct_req *ar)

 {

     struct sdap_id_conn_ctx **clist;

-    struct sdap_domain *sdom;

-    struct ad_id_ctx *subdom_id_ctx;

-

-    /* LDAP, GC, sentinel */

-    clist = talloc_zero_array(breq, struct sdap_id_conn_ctx *, 3);

-    if (clist == NULL) return NULL;

     switch (ar->entry_type & BE_REQ_TYPE_MASK) {

     case BE_REQ_USER: /* user */

@@ -201,24 +195,17 @@ get_conn_list(struct be_req *breq, struct ad_id_ctx *ad_ctx,

     case BE_REQ_USER_AND_GROUP: /* get SID */

     case BE_REQ_GROUP: /* group */

     case BE_REQ_INITGROUPS: /* init groups for user */

-        /* Always try GC first */

-        clist[0] = ad_ctx->gc_ctx;

-        if (IS_SUBDOMAIN(dom) == true) {

-            clist[0]->ignore_mark_offline = false;

-            /* Subdomain users are only present in GC. */

-            break;

-        }

-        /* fall back to ldap if gc is not available */

-        clist[0]->ignore_mark_offline = true;

-

-        /* With root domain users we have the option to

-         * fall back to LDAP in case ie POSIX attributes

-         * are used but not replicated to GC

-         */

-        clist[1] = ad_ctx->ldap_ctx;

+        clist = ad_gc_conn_list(breq, ad_ctx, dom);

+        if (clist == NULL) return NULL;

         break;

+

     default:

+        /* Requests for other object should only contact LDAP by default */

+        clist = talloc_zero_array(breq, struct sdap_id_conn_ctx *, 2);

+        if (clist == NULL) return NULL;

+

         clist[0] = ad_ctx->ldap_ctx;

+        clist[1] = NULL;

         break;

     }

diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c

index 332bfda3801db3824ce1896d37e65e2c3a6b8b8b..ed69a7d9889bac1281b5ff7c7b0f290ab09173fb 100644

--- a/src/providers/ad/ad_init.c

+++ b/src/providers/ad/ad_init.c

@@ -377,8 +377,7 @@ sssm_ad_access_init(struct be_ctx *bectx,

     if (ret != EOK) {

         goto fail;

     }

-    access_ctx->ldap_ctx = ad_id_ctx->ldap_ctx;

-    access_ctx->gc_ctx = ad_id_ctx->gc_ctx;

+    access_ctx->ad_id_ctx = ad_id_ctx;

     ret = dp_copy_options(access_ctx, ad_options->basic, AD_OPTS_BASIC,

                           &access_ctx->ad_options);

diff --git a/src/tests/cmocka/test_ad_common.c b/src/tests/cmocka/test_ad_common.c

new file mode 100644

index 0000000000000000000000000000000000000000..648b68f2dc05947b1fbb4c680ec63d3c2c6275b3

--- /dev/null

+++ b/src/tests/cmocka/test_ad_common.c

@@ -0,0 +1,221 @@

+/*

+    Authors:

+        Jakub Hrozek <jhrozek@redhat.com>

+

+    Copyright (C) 2013 Red Hat

+

+    SSSD tests: AD access control filter 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 <talloc.h>

+#include <tevent.h>

+#include <errno.h>

+#include <popt.h>

+#include <unistd.h>

+#include <sys/types.h>

+#include <ifaddrs.h>

+#include <arpa/inet.h>

+

+/* In order to access opaque types */

+#include "providers/ad/ad_common.c"

+

+#include "tests/cmocka/common_mock.h"

+

+#define DOMNAME     "domname"

+#define SUBDOMNAME  "sub."DOMNAME

+#define REALMNAME   DOMNAME

+#define HOST_NAME   "ad."REALMNAME

+

+struct ad_common_test_ctx {

+    struct ad_id_ctx *ad_ctx;

+    struct ad_id_ctx *subdom_ad_ctx;

+

+    struct sss_domain_info *dom;

+    struct sss_domain_info *subdom;

+};

+

+static void

+ad_common_test_setup(void **state)

+{

+    struct ad_common_test_ctx *test_ctx;

+    errno_t ret;

+    struct sdap_domain *sdom;

+    struct ad_id_ctx *ad_ctx;

+    struct ad_id_ctx *subdom_ad_ctx;

+    struct sdap_id_conn_ctx *subdom_ldap_ctx;

+

+    assert_true(leak_check_setup());

+    check_leaks_push(global_talloc_context);

+

+    test_ctx = talloc_zero(global_talloc_context, struct ad_common_test_ctx);

+    assert_non_null(test_ctx);

+

+    test_ctx->dom = talloc_zero(test_ctx, struct sss_domain_info);

+    assert_non_null(test_ctx->dom);

+    test_ctx->dom->name = discard_const(DOMNAME);

+

+    test_ctx->subdom = talloc_zero(test_ctx, struct sss_domain_info);

+    assert_non_null(test_ctx->subdom);

+    test_ctx->subdom->name = discard_const(SUBDOMNAME);

+    test_ctx->subdom->parent = test_ctx->dom;

+

+    ad_ctx = talloc_zero(test_ctx, struct ad_id_ctx);

+    assert_non_null(ad_ctx);

+

+    ad_ctx->ad_options = ad_create_default_options(ad_ctx,

+                                                   REALMNAME, HOST_NAME);

+    assert_non_null(ad_ctx->ad_options);

+

+    ad_ctx->gc_ctx = talloc_zero(ad_ctx, struct sdap_id_conn_ctx);

+    assert_non_null(ad_ctx->gc_ctx);

+

+    ad_ctx->ldap_ctx = talloc_zero(ad_ctx, struct sdap_id_conn_ctx);

+    assert_non_null(ad_ctx->ldap_ctx);

+

+    ad_ctx->sdap_id_ctx = talloc_zero(ad_ctx, struct sdap_id_ctx);

+    assert_non_null(ad_ctx->sdap_id_ctx);

+

+    ad_ctx->sdap_id_ctx->opts = talloc_zero(ad_ctx->sdap_id_ctx,

+                                            struct sdap_options);

+    assert_non_null(ad_ctx->sdap_id_ctx->opts);

+

+    ret = sdap_domain_add(ad_ctx->sdap_id_ctx->opts, test_ctx->dom, &sdom;;

+    assert_int_equal(ret, EOK);

+

+    subdom_ad_ctx = talloc_zero(test_ctx, struct ad_id_ctx);

+    assert_non_null(subdom_ad_ctx);

+

+    subdom_ldap_ctx = talloc_zero(subdom_ad_ctx, struct sdap_id_conn_ctx);

+    assert_non_null(subdom_ldap_ctx);

+    subdom_ad_ctx->ldap_ctx = subdom_ldap_ctx;

+

+    ret = sdap_domain_add(ad_ctx->sdap_id_ctx->opts, test_ctx->subdom, &sdom;;

+    assert_int_equal(ret, EOK);

+    sdom->pvt = subdom_ad_ctx;

+

+    test_ctx->ad_ctx = ad_ctx;

+    test_ctx->subdom_ad_ctx = subdom_ad_ctx;

+

+    check_leaks_push(test_ctx);

+    *state = test_ctx;

+}

+

+static void

+ad_common_test_teardown(void **state)

+{

+    struct ad_common_test_ctx *test_ctx = talloc_get_type(*state,

+                                                  struct ad_common_test_ctx);

+    assert_non_null(test_ctx);

+

+    assert_true(check_leaks_pop(test_ctx) == true);

+    talloc_free(test_ctx);

+    assert_true(check_leaks_pop(global_talloc_context) == true);

+    assert_true(leak_check_teardown());

+}

+

+errno_t

+__wrap_sdap_set_sasl_options(struct sdap_options *id_opts,

+                             char *default_primary,

+                             char *default_realm,

+                             const char *keytab_path)

+{

+    /* Pretend SASL is fine */

+    return EOK;

+}

+

+void test_ldap_conn_list(void **state)

+{

+    struct sdap_id_conn_ctx *conn;

+

+    struct ad_common_test_ctx *test_ctx = talloc_get_type(*state,

+                                                     struct ad_common_test_ctx);

+    assert_non_null(test_ctx);

+

+    conn = ad_get_dom_ldap_conn(test_ctx->ad_ctx, test_ctx->dom);

+    assert_true(conn == test_ctx->ad_ctx->ldap_ctx);

+

+    conn = ad_get_dom_ldap_conn(test_ctx->ad_ctx, test_ctx->subdom);

+    assert_true(conn == test_ctx->subdom_ad_ctx->ldap_ctx);

+}

+

+void test_conn_list(void **state)

+{

+    struct sdap_id_conn_ctx **conn_list;

+

+    struct ad_common_test_ctx *test_ctx = talloc_get_type(*state,

+                                                     struct ad_common_test_ctx);

+    assert_non_null(test_ctx);

+

+    conn_list = ad_gc_conn_list(test_ctx, test_ctx->ad_ctx, test_ctx->dom);

+    assert_non_null(conn_list);

+

+    assert_true(conn_list[0] == test_ctx->ad_ctx->gc_ctx);

+    /* If there is a fallback, we should ignore the offline mode */

+    assert_true(conn_list[0]->ignore_mark_offline);

+    assert_true(conn_list[1] == test_ctx->ad_ctx->ldap_ctx);

+    assert_false(conn_list[1]->ignore_mark_offline);

+    assert_null(conn_list[2]);

+    talloc_free(conn_list);

+

+    conn_list = ad_gc_conn_list(test_ctx, test_ctx->ad_ctx, test_ctx->subdom);

+    assert_non_null(conn_list);

+

+    assert_true(conn_list[0] == test_ctx->ad_ctx->gc_ctx);

+    assert_false(conn_list[0]->ignore_mark_offline);

+    assert_null(conn_list[1]);

+    talloc_free(conn_list);

+}

+

+int main(int argc, const char *argv[])

+{

+    poptContext pc;

+    int opt;

+    struct poptOption long_options[] = {

+        POPT_AUTOHELP

+        SSSD_DEBUG_OPTS

+        POPT_TABLEEND

+    };

+

+    const UnitTest tests[] = {

+        unit_test_setup_teardown(test_ldap_conn_list,

+                                 ad_common_test_setup,

+                                 ad_common_test_teardown),

+        unit_test_setup_teardown(test_conn_list,

+                                 ad_common_test_setup,

+                                 ad_common_test_teardown),

+    };

+

+    /* Set debug level to invalid value so we can deside if -d 0 was used. */

+    debug_level = SSSDBG_INVALID;

+

+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);

+    while((opt = poptGetNextOpt(pc)) != -1) {

+        switch(opt) {

+        default:

+            fprintf(stderr, "\nInvalid option %s: %s\n\n",

+                    poptBadOption(pc, 0), poptStrerror(opt));

+            poptPrintUsage(pc, stderr, 0);

+            return 1;

+        }

+    }

+    poptFreeContext(pc);

+

+    DEBUG_INIT(debug_level);

+

+    tests_set_cwd();

+

+    return run_tests(tests);

+}

-- 

1.8.4.2