On Fri, 2013-11-15 at 21:01 +0100, Jakub Hrozek wrote: > On Fri, Nov 15, 2013 at 11:15:53AM +0100, Pavel Reichl wrote: > > Hello, > > > > First patch improves domain detection taking matched length into account > > as proposed and elaborated by Jakub. > > > > Second patch is a simple unit test testing sss_ldap_dn_in_search_bases > > and sdap_domain_get_by_dn. > > > > Pavel Reichl > > Hi Pavel, > > I haven't really read the full patches yet, just scrolled through them, > so I have only one comment so far: > > > +++ b/src/tests/cmocka/test_search_bases.c > > @@ -0,0 +1,214 @@ > > +/* > > + SSSD > > + > > + find_uid - Utilities tests > > + > > + Authors: > > + Abhishek Singh <abhishekkumarsingh....@gmail.com> > > + > > + Copyright (C) 2013 Red Hat > > You should credit yourself :-)
Hi Jakub, you are right, thanks for noticing. > _______________________________________________ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>From c7632a2310442cfc10708c5728bd63e6a8c916d2 Mon Sep 17 00:00:00 2001 From: Pavel Reichl <pavel.rei...@redhat.com> Date: Thu, 14 Nov 2013 21:34:51 +0000 Subject: [PATCH 1/2] SSSD: Improved domain detection A bit more elegant way of detection of what domain the group member belongs to Resolves: https://fedorahosted.org/sssd/ticket/2132 --- src/providers/ldap/ldap_common.c | 40 ++++++++++++++++++++++++++++------------ src/util/sss_ldap.c | 23 +++++++++++++++++++---- src/util/sss_ldap.h | 6 ++++++ 3 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index e29a5219768df154608aad9740dd9ed44c14e22e..6f6d93009ce97258c98948d077f2d0c60acc2945 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -68,23 +68,39 @@ sdap_domain_get_by_dn(struct sdap_options *opts, const char *dn) { struct sdap_domain *sditer = NULL; - char *dc = NULL; + struct sdap_domain *sdmatch = NULL; + TALLOC_CTX *tmp_ctx = NULL; + int match_len; + int best_match_len = 0; - dc = strstr(dn, "dc="); - if (dc == NULL) { - dc = strstr(dn, "DC="); - if (dc == NULL) { - return NULL; - } - } + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) + return NULL; DLIST_FOR_EACH(sditer, opts->sdom) { - if (strcasecmp(sditer->basedn, dc) == 0) { - return sditer; + if (sss_ldap_dn_in_search_bases_len(tmp_ctx, dn, sditer->search_bases, NULL, + &match_len) + || sss_ldap_dn_in_search_bases_len(tmp_ctx, dn, + sditer->user_search_bases, NULL, &match_len) + || sss_ldap_dn_in_search_bases_len(tmp_ctx, dn, + sditer->group_search_bases, NULL, &match_len) + || sss_ldap_dn_in_search_bases_len(tmp_ctx, dn, + sditer->netgroup_search_bases, NULL, &match_len) + || sss_ldap_dn_in_search_bases_len(tmp_ctx, dn, + sditer->sudo_search_bases, NULL, &match_len) + || sss_ldap_dn_in_search_bases_len(tmp_ctx, dn, + sditer->service_search_bases, NULL, &match_len) + || sss_ldap_dn_in_search_bases_len(tmp_ctx, dn, + sditer->autofs_search_bases, NULL, &match_len)) { + if (best_match_len < match_len) { + /*this is a longer match*/ + best_match_len = match_len; + sdmatch = sditer; + } } } - - return NULL; + talloc_free(tmp_ctx); + return sdmatch; } errno_t diff --git a/src/util/sss_ldap.c b/src/util/sss_ldap.c index 6d7b0907ca2fa48d9cff5257ab6bbba0ae7dd5c6..c9ab2159d557917bf460a61ec96f146fff4f88aa 100644 --- a/src/util/sss_ldap.c +++ b/src/util/sss_ldap.c @@ -470,10 +470,13 @@ int sss_ldap_init_recv(struct tevent_req *req, LDAP **ldap, int *sd) * _filter will contain combined filters from all possible search bases * or NULL if it should be empty */ -bool sss_ldap_dn_in_search_bases(TALLOC_CTX *mem_ctx, - const char *dn, - struct sdap_search_base **search_bases, - char **_filter) + + +bool sss_ldap_dn_in_search_bases_len(TALLOC_CTX *mem_ctx, + const char *dn, + struct sdap_search_base **search_bases, + char **_filter, + int *_match_len) { struct sdap_search_base *base; int basedn_len, dn_len; @@ -484,6 +487,7 @@ bool sss_ldap_dn_in_search_bases(TALLOC_CTX *mem_ctx, bool backslash_found = false; char *filter = NULL; bool ret = false; + int match_len; if (dn == NULL) { DEBUG(SSSDBG_FUNC_DATA, ("dn is NULL\n")); @@ -511,6 +515,7 @@ bool sss_ldap_dn_in_search_bases(TALLOC_CTX *mem_ctx, if (!base_confirmed) { continue; } + match_len = basedn_len; switch (base->scope) { case LDAP_SCOPE_BASE: @@ -558,6 +563,8 @@ bool sss_ldap_dn_in_search_bases(TALLOC_CTX *mem_ctx, * Append filter otherwise. */ ret = true; + if (_match_len) + *_match_len = match_len; if (base->filter == NULL || _filter == NULL) { goto done; @@ -589,6 +596,14 @@ done: return ret; } +bool sss_ldap_dn_in_search_bases(TALLOC_CTX *mem_ctx, + const char *dn, + struct sdap_search_base **search_bases, + char **_filter) +{ + return sss_ldap_dn_in_search_bases_len(mem_ctx, dn, search_bases, _filter, NULL); +} + char *sss_ldap_encode_ndr_uint32(TALLOC_CTX *mem_ctx, uint32_t flags) { char hex[9]; /* 4 bytes in hex + terminating zero */ diff --git a/src/util/sss_ldap.h b/src/util/sss_ldap.h index e5c30eb2115d422ef5a52cc5cd75c85be8fbe2d7..f298b2fbb30cf1532f8e94504ffb83ef73880b81 100644 --- a/src/util/sss_ldap.h +++ b/src/util/sss_ldap.h @@ -74,6 +74,12 @@ bool sss_ldap_dn_in_search_bases(TALLOC_CTX *mem_ctx, struct sdap_search_base **search_bases, char **_filter); +bool sss_ldap_dn_in_search_bases_len(TALLOC_CTX *mem_ctx, + const char *dn, + struct sdap_search_base **search_bases, + char **_filter, + int *_match_len); + char *sss_ldap_encode_ndr_uint32(TALLOC_CTX *mem_ctx, uint32_t flags); #endif /* __SSS_LDAP_H__ */ -- 1.8.3.1
>From cfcd83aca098784d9ab1ca59f0a1ad8ef3145b62 Mon Sep 17 00:00:00 2001 From: Pavel Reichl <pavel.rei...@redhat.com> Date: Thu, 14 Nov 2013 21:52:26 +0000 Subject: [PATCH] SSSD: Unit test - sss_ldap_dn_in_search_bases Unit test testing detection of the right domain when processing group with members from several domains Resolves: https://fedorahosted.org/sssd/ticket/2132 --- Makefile.am | 27 ++++- src/tests/cmocka/test_search_bases.c | 212 +++++++++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 src/tests/cmocka/test_search_bases.c diff --git a/Makefile.am b/Makefile.am index 992d5796ea3e729238a8d7664c128112291c40ab..ed155bf25c1d1adfe91a7c3c76cdbce62bf6ae59 100644 --- a/Makefile.am +++ b/Makefile.am @@ -154,7 +154,8 @@ if HAVE_CMOCKA fqnames-tests \ test_sss_idmap \ test_utils \ - ad_access_filter_tests + ad_access_filter_tests \ + test_search_bases endif check_PROGRAMS = \ @@ -1388,6 +1389,30 @@ test_utils_LDADD = \ $(SSSD_INTERNAL_LTLIBS) \ libsss_test_common.la +test_search_bases_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_search_bases.c +test_search_bases_CFLAGS = \ + $(AM_CFLAGS) \ + -DUNIT_TESTING +test_search_bases_LDADD = \ + $(PAM_LIBS) \ + $(CMOCKA_LIBS) \ + $(POPT_LIBS) \ + $(SSSD_LIBS) \ + $(CARES_LIBS) \ + $(KRB5_LIBS) \ + $(SSSD_INTERNAL_LTLIBS) \ + $(SYSTEMD_LOGIN_LIBS) \ + libsss_idmap.la \ + libsss_ldap_common.la \ + libsss_krb5_common.la \ + libsss_test_common.la + ad_access_filter_tests_SOURCES = \ $(sssd_be_SOURCES) \ src/util/sss_ldap.c \ diff --git a/src/tests/cmocka/test_search_bases.c b/src/tests/cmocka/test_search_bases.c new file mode 100644 index 0000000000000000000000000000000000000000..b5f186b22c7e0589391eda778774c353ff4f68c0 --- /dev/null +++ b/src/tests/cmocka/test_search_bases.c @@ -0,0 +1,212 @@ +/* + Authors: + Pavel Reichl <prei...@redhat.com> + + Copyright (C) 2013 Red Hat + + SSSD tests - Search bases + + 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 <stdarg.h> +#include <stdlib.h> +#include <stddef.h> +#include <setjmp.h> +#include <unistd.h> +#include <sys/types.h> +#include <cmocka.h> +#include <ldap.h> + +#include "util/find_uid.h" +#include "util/sss_ldap.h" +#include "tests/common.h" +#include "providers/ldap/ldap_common.h" +#include "providers/ldap/sdap.h" +#include "dhash.h" +#include "tests/common_check.h" + +struct sdap_search_base** generate_bases(TALLOC_CTX *mem_ctx, const char** dns, size_t n) +{ + struct sdap_search_base **search_bases; + errno_t err; + int i; + + search_bases = talloc_array(mem_ctx, struct sdap_search_base *, n + 1); + assert_non_null(search_bases); + + for(i=0; i < n; ++i) { + err = sdap_create_search_base(mem_ctx, dns[i], LDAP_SCOPE_SUBTREE, + NULL, &search_bases[i]); + if (err != EOK) { + fprintf(stderr, "Failed to create search base\n"); + } + assert_true(err == EOK); + } + search_bases[n] = NULL; + return search_bases; +} + +bool do_test_search_bases(const char* dn, const char** dns, size_t n) +{ + TALLOC_CTX *tmp_ctx; + struct sdap_search_base **search_bases; + bool ret; + + tmp_ctx = talloc_new(NULL); + assert_non_null(tmp_ctx); + + search_bases = generate_bases(tmp_ctx, dns, n); + ret = sss_ldap_dn_in_search_bases(tmp_ctx, dn, search_bases, NULL); + + talloc_free(tmp_ctx); + return ret; +} + +void test_search_bases_fail(void **state) +{ + const char *dn = "cn=user, dc=sub, dc=ad, dc=pb"; + const char *dns[] = {"dc=example, dc=com", "dc=subdom, dc=ad, dc=pb"}; + bool ret; + + ret = do_test_search_bases(dn, dns, 2); + assert_false(ret); +} + +void test_search_bases_success(void **state) +{ + const char *dn = "cn=user, dc=sub, dc=ad, dc=pb"; + const char *dns[] = {"", "dc=ad, dc=pb", "dc=sub, dc=ad, dc=pb"}; + bool ret; + + ret = do_test_search_bases(dn, dns, 3); + assert_true(ret); +} + +void test_search_empty_bases_fail(void **sstate) +{ + TALLOC_CTX *tmp_ctx; + struct sdap_search_base **search_bases; + const char *dn = "cn=user, dc=sub, dc=ad, dc=pb"; + errno_t err; + bool ret; + + tmp_ctx = talloc_new(NULL); + assert_non_null(tmp_ctx); + + search_bases = talloc_array(tmp_ctx, struct sdap_search_base*, 2); + assert_non_null(search_bases); + + err = sdap_create_search_base(tmp_ctx, "", LDAP_SCOPE_SUBTREE, NULL, + &search_bases[0]); + assert_true(err == EOK); + search_bases[1] = NULL; + + ret = sss_ldap_dn_in_search_bases(tmp_ctx, dn, search_bases, NULL); + assert_false(ret); + + talloc_free(tmp_ctx); +} + + +/* + * expected_result equal to 0 means that dn is not in any domain + * expected_result equal to 1 means that dn is in the domain based on dns + * expected_result equal to 2 means that dn is in the domain based on dns2 + */ + +void do_test_get_by_dn(const char *dn, const char **dns, size_t n, + const char **dns2, size_t n2, int expected_result) +{ + TALLOC_CTX *tmp_ctx; + struct sdap_options *opts; + struct sdap_domain *sdom; + struct sdap_domain *sdom2; + struct sdap_domain *res_sdom; + struct sdap_search_base **search_bases; + struct sdap_search_base **search_bases2; + tmp_ctx = talloc_new(NULL); + + search_bases = generate_bases(tmp_ctx, dns, n); + search_bases2 = generate_bases(tmp_ctx, dns2, n2); + sdom = talloc_zero(tmp_ctx, struct sdap_domain); + sdom2 = talloc_zero(tmp_ctx, struct sdap_domain); + + sdom->search_bases = search_bases; + sdom->next = sdom2; + sdom->prev = NULL; + sdom2->search_bases = search_bases2; + sdom2->next = NULL; + sdom2->prev = sdom; + + opts = talloc(tmp_ctx, struct sdap_options); + opts->sdom = sdom; + res_sdom = sdap_domain_get_by_dn(opts, dn); + + switch(expected_result) { + case 0: + assert_null(res_sdom); + break; + case 1: + assert_true(res_sdom == sdom); + break; + case 2: + assert_true(res_sdom == sdom2); + break; + } + talloc_free(tmp_ctx); +} + +void test_get_by_dn(void **state) +{ + const char *dn = "cn=user, dc=sub, dc=ad, dc=pb"; + const char *dns[] = {"dc=ad, dc=pb"}; + const char *dns2[] = {"dc=sub, dc=ad, dc=pb"}; + + do_test_get_by_dn(dn, dns, 1, dns2, 1, 2); +} + +void test_get_by_dn2(void **state) +{ + const char *dn = "cn=user, dc=ad, dc=com"; + const char *dns[] = {"dc=ad, dc=com"}; + const char *dns2[] = {"dc=sub, dc=ad, dc=pb"}; + + do_test_get_by_dn(dn, dns, 1, dns2, 1, 1); +} + +void test_get_by_dn_fail(void **state) +{ + const char *dn = "cn=user, dc=sub, dc=example, dc=com"; + const char *dns[] = {"dc=ad, dc=pb"}; + const char *dns2[] = {"dc=sub, dc=ad, dc=pb"}; + + do_test_get_by_dn(dn, dns, 1, dns2, 1, 0); +} + +int main(void) +{ + const UnitTest tests[] = { + unit_test(test_search_bases_fail), + unit_test(test_search_bases_success), +#if 0 + unit_test(test_search_empty_bases_fail), +#endif + unit_test(test_get_by_dn_fail), + unit_test(test_get_by_dn), + unit_test(test_get_by_dn2) + }; + + return run_tests(tests); +} -- 1.8.3.1
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
