AD: Consolidate connection list construction on ad_common.c

Reviewed-by: Sumit Bose <sbose@redhat.com>

4 files changed, 71 insertions, 17 deletions

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 7d46af4a4..ffc135124 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -1287,3 +1287,34 @@ ad_ldap_conn_list(TALLOC_CTX *mem_ctx,
     clist[1] = NULL;
     return clist;
 }
+
+struct sdap_id_conn_ctx **
+ad_user_conn_list(TALLOC_CTX *mem_ctx,
+                  struct ad_id_ctx *ad_ctx,
+                  struct sss_domain_info *dom)
+{
+    struct sdap_id_conn_ctx **clist;
+    int cindex = 0;
+
+    clist = talloc_zero_array(ad_ctx, struct sdap_id_conn_ctx *, 3);
+    if (clist == NULL) {
+        return NULL;
+    }
+
+    /* Try GC first for users from trusted domains, but go to LDAP
+     * for users from non-trusted domains to get all POSIX attrs
+     */
+    if (dp_opt_get_bool(ad_ctx->ad_options->basic, AD_ENABLE_GC)
+            && IS_SUBDOMAIN(dom)) {
+        clist[cindex] = ad_ctx->gc_ctx;
+        clist[cindex]->ignore_mark_offline = true;
+        cindex++;
+    }
+
+    /* Users from primary domain can be just downloaded from LDAP.
+     * The domain's LDAP connection also works as a fallback
+     */
+    clist[cindex] = ad_get_dom_ldap_conn(ad_ctx, dom);
+
+    return clist;
+}
diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index 701e46198..0cefa1859 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -153,6 +153,11 @@ ad_ldap_conn_list(TALLOC_CTX *mem_ctx,
                   struct ad_id_ctx *ad_ctx,
                   struct sss_domain_info *dom);
 
+struct sdap_id_conn_ctx **
+ad_user_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);
 
diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c
index be0cb3b12..51d378863 100644
--- a/src/providers/ad/ad_id.c
+++ b/src/providers/ad/ad_id.c
@@ -244,25 +244,10 @@ 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;
-    int cindex = 0;
 
     switch (ar->entry_type & BE_REQ_TYPE_MASK) {
     case BE_REQ_USER: /* user */
-        clist = talloc_zero_array(ad_ctx, struct sdap_id_conn_ctx *, 3);
-        if (clist == NULL) return NULL;
-
-        /* Try GC first for users from trusted domains */
-        if (dp_opt_get_bool(ad_ctx->ad_options->basic, AD_ENABLE_GC)
-                && IS_SUBDOMAIN(dom)) {
-            clist[cindex] = ad_ctx->gc_ctx;
-            clist[cindex]->ignore_mark_offline = true;
-            cindex++;
-        }
-
-        /* Users from primary domain can be just downloaded from LDAP.
-         * The domain's LDAP connection also works as a fallback
-         */
-        clist[cindex] = ad_get_dom_ldap_conn(ad_ctx, dom);
+        clist = ad_user_conn_list(breq, ad_ctx, dom);
         break;
     case BE_REQ_BY_SECID:   /* by SID */
     case BE_REQ_USER_AND_GROUP: /* get SID */
@@ -270,7 +255,6 @@ get_conn_list(struct be_req *breq, struct ad_id_ctx *ad_ctx,
     case BE_REQ_INITGROUPS: /* init groups for user */
         clist = ad_gc_conn_list(breq, ad_ctx, dom);
         break;
-
     default:
         /* Requests for other object should only contact LDAP by default */
         clist = ad_ldap_conn_list(breq, ad_ctx, dom);
diff --git a/src/tests/cmocka/test_ad_common.c b/src/tests/cmocka/test_ad_common.c
index d2b59a23d..b0cf4b5e6 100644
--- a/src/tests/cmocka/test_ad_common.c
+++ b/src/tests/cmocka/test_ad_common.c
@@ -446,6 +446,37 @@ void test_ldap_conn_list(void **state)
     talloc_free(conn_list);
 }
 
+void test_user_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_user_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->ldap_ctx);
+    assert_false(conn_list[0]->ignore_mark_offline);
+    assert_null(conn_list[1]);
+    talloc_free(conn_list);
+
+    conn_list = ad_user_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_true(conn_list[0]->ignore_mark_offline);
+    assert_true(conn_list[1] == test_ctx->subdom_ad_ctx->ldap_ctx);
+    /* Subdomain error should not set the backend offline! */
+    assert_true(conn_list[1]->ignore_mark_offline);
+    talloc_free(conn_list);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -473,6 +504,9 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_ldap_conn_list,
                                         test_ldap_conn_setup,
                                         test_ldap_conn_teardown),
+        cmocka_unit_test_setup_teardown(test_user_conn_list,
+                                        test_ldap_conn_setup,
+                                        test_ldap_conn_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */