[SSSD] [PATCH] AD: Set the bool value same as default value in opts

Jakub Hrozek jhrozek at redhat.com
Fri Jul 19 09:20:06 UTC 2013


On Tue, Jul 16, 2013 at 08:22:56PM +0200, Jakub Hrozek wrote:
> Hi,
> 
> this patch only affects the IPA server mode. Kindly see the commit
> message for more detailed explanation.

And now with a unit test..
-------------- next part --------------
>From 6f04bf4542afcb319546b4d826562ea496f14486 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 16 Jul 2013 19:07:09 +0200
Subject: [PATCH] AD: Set the bool value same as default value in opts

When the option values are copied using dp_opt_copy_map, the .val member
is used if it's not NULL. At the same time, the bool options are never
NULL, unlike integers or strings that can have special NULL-like values
such as NULL_STRING. This effectively means that when copying a bool
option, the .val member is always used.

But in the AD maps, some .val fields were set differently from the
.def_val fields. The effect was that when the AD subdomain provider was
initialized from IPA subdomain provider using only the defaults, some
options (notably referral chasing) were set to a value that didn't make
sense for the AD provider.

This patch makes sure that for all boolean option, the .val is always
the same as .def_val.
---
 Makefile.am                    |  1 +
 src/providers/ad/ad_opts.h     |  8 ++---
 src/tests/ipa_ldap_opt-tests.c | 69 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 515843420834bbbd0e5b4c86c114294c89afa752..0f90cc0fce95caaacb28b13ba5fdd73333990066 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1055,6 +1055,7 @@ auth_tests_LDADD = \
     libsss_test_common.la
 
 ipa_ldap_opt_tests_SOURCES = \
+    src/providers/data_provider_opts.c \			     
     src/tests/ipa_ldap_opt-tests.c
 ipa_ldap_opt_tests_CFLAGS = \
     $(AM_CFLAGS) \
diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h
index 2c60b761429b51f2c59a62c3f114953498791fae..197b97e222367ba80af017fe635fceffdd9a9faa 100644
--- a/src/providers/ad/ad_opts.h
+++ b/src/providers/ad/ad_opts.h
@@ -67,7 +67,7 @@ struct dp_option ad_def_ldap_opts[] = {
     { "ldap_autofs_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ldap_schema", DP_OPT_STRING, { "ad" }, NULL_STRING },
     { "ldap_offline_timeout", DP_OPT_NUMBER, { .number = 60 }, NULL_NUMBER },
-    { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
+    { "ldap_force_upper_case_realm", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "ldap_enumeration_refresh_timeout", DP_OPT_NUMBER, { .number = 300 }, NULL_NUMBER },
     { "ldap_purge_cache_timeout", DP_OPT_NUMBER, { .number = 10800 }, NULL_NUMBER },
     { "ldap_tls_cacert", DP_OPT_STRING, NULL_STRING, NULL_STRING },
@@ -76,7 +76,7 @@ struct dp_option ad_def_ldap_opts[] = {
     { "ldap_tls_key", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ldap_tls_cipher_suite", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ldap_id_use_start_tls", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
-    { "ldap_id_mapping", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
+    { "ldap_id_mapping", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "ldap_sasl_mech", DP_OPT_STRING, { "gssapi" }, NULL_STRING },
     { "ldap_sasl_authid", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ldap_sasl_realm", DP_OPT_STRING, NULL_STRING, NULL_STRING },
@@ -90,7 +90,7 @@ struct dp_option ad_def_ldap_opts[] = {
     { "krb5_canonicalize", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "krb5_use_kdcinfo", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "ldap_pwd_policy", DP_OPT_STRING, { "none" }, NULL_STRING },
-    { "ldap_referrals", DP_OPT_BOOL, BOOL_FALSE, BOOL_TRUE },
+    { "ldap_referrals", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "account_cache_expiration", DP_OPT_NUMBER, { .number = 0 }, NULL_NUMBER },
     { "ldap_dns_service_name", DP_OPT_STRING, { SSS_LDAP_SRV_NAME }, NULL_STRING },
     { "ldap_krb5_ticket_lifetime", DP_OPT_NUMBER, { .number = (24 * 60 * 60) }, NULL_NUMBER },
@@ -248,7 +248,7 @@ struct dp_option ad_dyndns_opts[] = {
     { "dyndns_refresh_interval", DP_OPT_NUMBER, { .number = 86400 }, NULL_NUMBER },
     { "dyndns_iface", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "dyndns_ttl", DP_OPT_NUMBER, { .number = 3600 }, NULL_NUMBER },
-    { "dyndns_update_ptr", DP_OPT_BOOL, BOOL_TRUE, BOOL_FALSE },
+    { "dyndns_update_ptr", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE },
     { "dyndns_force_tcp", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "dyndns_auth", DP_OPT_STRING, { "gss-tsig" }, NULL_STRING },
     DP_OPTION_TERMINATOR
diff --git a/src/tests/ipa_ldap_opt-tests.c b/src/tests/ipa_ldap_opt-tests.c
index ea4991c805c43403d6219678e9714baeda684a69..40afa5cba87d89bc6fa7345302991fe766b43314 100644
--- a/src/tests/ipa_ldap_opt-tests.c
+++ b/src/tests/ipa_ldap_opt-tests.c
@@ -161,6 +161,71 @@ START_TEST(test_compare_2307_with_2307bis)
 }
 END_TEST
 
+START_TEST(test_copy_opts)
+{
+    errno_t ret;
+    TALLOC_CTX *tmp_ctx;
+    struct dp_option *opts;
+
+    tmp_ctx = talloc_new(NULL);
+    fail_unless(tmp_ctx != NULL, "talloc_new failed");
+
+    ret = dp_copy_options(tmp_ctx, ad_def_ldap_opts, SDAP_OPTS_BASIC, &opts);
+    fail_unless(ret == EOK, "[%s]", strerror(ret));
+
+    for (int i=0; i < SDAP_OPTS_BASIC; i++) {
+        char *s1, *s2;
+        bool b1, b2;
+        int i1, i2;
+        struct dp_opt_blob  bl1, bl2;
+
+        switch (opts[i].type) {
+        case DP_OPT_STRING:
+            s1 = dp_opt_get_string(opts, i);
+            s2 = opts[i].def_val.string;
+
+            if (s1 != NULL || s2 != NULL) {
+                fail_unless(strcmp(s1, s2) == 0,
+                            "Option %s does not have default value after copy\n",
+                            opts[i].opt_name);
+            }
+            break;
+
+        case DP_OPT_NUMBER:
+            i1 = dp_opt_get_int(opts, i);
+            i2 = opts[i].def_val.number;
+
+            fail_unless(i1 == i2,
+                        "Option %s does not have default value after copy\n",
+                        opts[i].opt_name);
+            break;
+
+        case DP_OPT_BOOL:
+            b1 = dp_opt_get_bool(opts, i);
+            b2 = opts[i].def_val.boolean;
+
+            fail_unless(b1 == b2,
+                        "Option %s does not have default value after copy\n",
+                        opts[i].opt_name);
+            break;
+
+        case DP_OPT_BLOB:
+            bl1 = dp_opt_get_blob(opts, i);
+            bl2 = opts[i].def_val.blob;
+
+            fail_unless(bl1.length == bl2.length,
+                        "Blobs differ in size for option %s\n",
+                        opts[i].opt_name);
+            fail_unless(memcmp(bl1.data, bl2.data, bl1.length) == 0,
+                        "Blobs differ in value for option %s\n",
+                        opts[i].opt_name);
+        }
+    }
+
+    talloc_free(tmp_ctx);
+}
+END_TEST
+
 Suite *ipa_ldap_opt_suite (void)
 {
     Suite *s = suite_create ("ipa_ldap_opt");
@@ -176,6 +241,10 @@ Suite *ipa_ldap_opt_suite (void)
     tcase_add_test (tc_ipa_utils, test_domain_to_basedn);
     suite_add_tcase (s, tc_ipa_utils);
 
+    TCase *tc_dp_opts = tcase_create ("dp_opts");
+    tcase_add_test (tc_dp_opts, test_copy_opts);
+    suite_add_tcase (s, tc_dp_opts);
+
     return s;
 }
 
-- 
1.8.3.1



More information about the sssd-devel mailing list