URL: https://github.com/SSSD/sssd/pull/5663 Author: ikerexxe Title: #5663: cache_req: replace FQNs by shortnames Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5663/head:pr5663 git checkout pr5663
From 74b7ab6bcb56fd039e807804a11effaac2466a6c Mon Sep 17 00:00:00 2001 From: Iker Pedrosa <ipedr...@redhat.com> Date: Thu, 13 May 2021 12:24:09 +0200 Subject: [PATCH] cache_req: replace FQNs by shortnames Replace FQNs by shortnames in cache_req_XYZ_by_name_send() when the FQN already contains the domain name. This is done to avoid appending again the domain name to the FQN. Includes a unit test to check that the FQN to shortname replacement works correctly. It also includes an integration test to check that UpdateMemberList() and GetAll() return the correct users that are members of a group. This is done by first adding a member to a group and checking that it is returned correctly. Then, the member is deleted and the interface returns no members. Resolves: https://github.com/SSSD/sssd/issues/4255 --- .../common/cache_req/cache_req_private.h | 13 ++++ .../cache_req/plugins/cache_req_common.c | 52 ++++++++++++++++ .../plugins/cache_req_group_by_name.c | 17 ++++++ .../plugins/cache_req_user_by_name.c | 17 ++++++ src/tests/cmocka/test_responder_cache_req.c | 59 +++++++++++++++++++ src/tests/intg/test_infopipe.py | 54 +++++++++++++++++ 6 files changed, 212 insertions(+) diff --git a/src/responder/common/cache_req/cache_req_private.h b/src/responder/common/cache_req/cache_req_private.h index 2d52e7600b..501b1a3740 100644 --- a/src/responder/common/cache_req/cache_req_private.h +++ b/src/responder/common/cache_req/cache_req_private.h @@ -216,4 +216,17 @@ cache_req_common_get_acct_domain_recv(TALLOC_CTX *mem_ctx, errno_t cache_req_idminmax_check(struct cache_req_data *data, struct sss_domain_info *domain); + +/** + * Replace the FQN by the shortname if the domain is already included in the FQN. + * + * @param[in] mem_ctx The parent memory context for the confdb_ctx + * @param[inout] name The name to replace + * @param[in] domain The domain name + * + * @return EOK on success, errno on failure. + */ +//TODO: const +errno_t +cache_req_common_fqn_to_shortname(TALLOC_CTX *mem_ctx, const char **name, const char *domain); #endif /* _CACHE_REQ_PRIVATE_H_ */ diff --git a/src/responder/common/cache_req/plugins/cache_req_common.c b/src/responder/common/cache_req/plugins/cache_req_common.c index 6f4d27cb60..c66266ab03 100644 --- a/src/responder/common/cache_req/plugins/cache_req_common.c +++ b/src/responder/common/cache_req/plugins/cache_req_common.c @@ -190,3 +190,55 @@ cache_req_common_get_acct_domain_recv(TALLOC_CTX *mem_ctx, } return ret; } + +errno_t +cache_req_common_fqn_to_shortname(TALLOC_CTX *mem_ctx, const char **name, const char *domain) +{ + char *shortname = NULL; + char *lower_domain = NULL; + struct sss_names_ctx *names_ctx; + errno_t ret; + + names_ctx = talloc_zero(mem_ctx, struct sss_names_ctx); + if (!names_ctx) { + ret = ENOMEM; + goto done; + } + + names_ctx->re_pattern = talloc_strdup(names_ctx, "(?P<name>[^@]+)@?(?P<domain>[^@]*$)"); + if (names_ctx->re_pattern == NULL) { + ret = ENOMEM; + goto done; + } + + DEBUG(SSSDBG_CONF_SETTINGS, "Using re [%s].\n", names_ctx->re_pattern); + + ret = sss_regexp_new(names_ctx, + names_ctx->re_pattern, + 0, + &(names_ctx->re)); + if (ret != 0) { + DEBUG(SSSDBG_CONF_SETTINGS, "Error formatting regular expression\n"); + ret = EFAULT; + goto done; + } + + ret = sss_parse_name(mem_ctx, names_ctx, *name, NULL, &shortname); + if (ret == EOK && shortname) { + if (domain) { + lower_domain = sss_tc_utf8_str_tolower(mem_ctx, domain); + if (strstr(*name, lower_domain)) { + *name = talloc_steal(mem_ctx, shortname); + } + talloc_free(lower_domain); + } + talloc_free(shortname); + } + +done: + if (names_ctx) { + talloc_free(names_ctx); + } + + return ret; +} \ No newline at end of file diff --git a/src/responder/common/cache_req/plugins/cache_req_group_by_name.c b/src/responder/common/cache_req/plugins/cache_req_group_by_name.c index 3be0d5ea55..3186bb84a4 100644 --- a/src/responder/common/cache_req/plugins/cache_req_group_by_name.c +++ b/src/responder/common/cache_req/plugins/cache_req_group_by_name.c @@ -214,12 +214,29 @@ cache_req_group_by_name_send(TALLOC_CTX *mem_ctx, const char *name) { struct cache_req_data *data; + TALLOC_CTX *tmp_ctx; + errno_t ret; + + tmp_ctx = talloc_new(mem_ctx); + if (!tmp_ctx) { + return NULL; + } + + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + if (ret != EOK) { + talloc_free(tmp_ctx); + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get name [%d]: %s\n", ret, + sss_strerror(ret)); + return NULL; + } data = cache_req_data_name(mem_ctx, CACHE_REQ_GROUP_BY_NAME, name); if (data == NULL) { return NULL; } + talloc_free(tmp_ctx); + return cache_req_steal_data_and_send(mem_ctx, ev, rctx, ncache, cache_refresh_percent, req_dom_type, domain, diff --git a/src/responder/common/cache_req/plugins/cache_req_user_by_name.c b/src/responder/common/cache_req/plugins/cache_req_user_by_name.c index d24a2221b2..1e45126817 100644 --- a/src/responder/common/cache_req/plugins/cache_req_user_by_name.c +++ b/src/responder/common/cache_req/plugins/cache_req_user_by_name.c @@ -219,12 +219,29 @@ cache_req_user_by_name_send(TALLOC_CTX *mem_ctx, const char *name) { struct cache_req_data *data; + TALLOC_CTX *tmp_ctx; + errno_t ret; + + tmp_ctx = talloc_new(mem_ctx); + if (!tmp_ctx) { + return NULL; + } + + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + if (ret != EOK) { + talloc_free(tmp_ctx); + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get name [%d]: %s\n", ret, + sss_strerror(ret)); + return NULL; + } data = cache_req_data_name(mem_ctx, CACHE_REQ_USER_BY_NAME, name); if (data == NULL) { return NULL; } + talloc_free(tmp_ctx); + return cache_req_steal_data_and_send(mem_ctx, ev, rctx, ncache, cache_refresh_percent, req_dom_type, domain, diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c index 258bc78943..93232ca96a 100644 --- a/src/tests/cmocka/test_responder_cache_req.c +++ b/src/tests/cmocka/test_responder_cache_req.c @@ -27,6 +27,7 @@ #include "tests/cmocka/common_mock_resp.h" #include "db/sysdb.h" #include "responder/common/cache_req/cache_req.h" +#include "responder/common/cache_req/cache_req_private.h" #define TESTS_PATH "tp_" BASE_FILE_STEM #define TEST_CONF_DB "test_responder_cache_req_conf.ldb" @@ -4202,6 +4203,62 @@ void test_object_by_id_group_sub_domains_locator_cache_expired_two_calls(void ** talloc_free(tmp_ctx); } +void test_cache_req_common_fqn_to_shortname(void **state) +{ + TALLOC_CTX *tmp_ctx; + const char *name = NULL; + const char *domain = NULL; + errno_t ret; + + tmp_ctx = talloc_new(NULL); + assert_non_null(tmp_ctx); + check_leaks_push(tmp_ctx); + + // Test name=NULL and domain=NULL + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + assert_int_equal(ret, ERR_REGEX_NOMATCH); + assert_null(name); + + // Test name="name" and domain=NULL + name = strdup("name"); + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + assert_int_equal(ret, EOK); + assert_string_equal(name, "name"); + + // Test name="name@ldap" and domain=NULL + name = strdup("name@ldap"); + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + assert_int_equal(ret, EOK); + assert_string_equal(name, "name@ldap"); + + // Test name="name@ldap" and domain="ldap" + name = strdup("name@ldap"); + domain = talloc_strdup(tmp_ctx, "ldap"); + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + assert_int_equal(ret, EOK); + assert_string_equal(name, "name"); + talloc_free(domain); + + // Test name="name@ldap" and domain="LDAP" + name = strdup("name@ldap"); + domain = talloc_strdup(tmp_ctx, "LDAP"); + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + assert_int_equal(ret, EOK); + assert_string_equal(name, "name"); + talloc_free(domain); + + // Test name="name@ldap" and domain="ipa" + name = strdup("name@ldap"); + domain = talloc_strdup(tmp_ctx, "ipa"); + ret = cache_req_common_fqn_to_shortname(tmp_ctx, &name, domain); + assert_int_equal(ret, EOK); + assert_string_equal(name, "name@ldap"); + talloc_free(domain); + + assert_true(check_leaks_pop(tmp_ctx)); + talloc_free(tmp_ctx); +} + int main(int argc, const char *argv[]) { poptContext pc; @@ -4351,6 +4408,8 @@ int main(int argc, const char *argv[]) new_subdomain_test(object_by_id_group_sub_domains_locator_missing_found), new_subdomain_test(object_by_id_group_sub_domains_locator_missing_notfound), new_subdomain_test(object_by_id_group_sub_domains_locator_cache_expired_two_calls), + + cmocka_unit_test(test_cache_req_common_fqn_to_shortname), }; /* Set debug level to invalid value so we can decide if -d 0 was used. */ diff --git a/src/tests/intg/test_infopipe.py b/src/tests/intg/test_infopipe.py index 6cd7c49a8d..5aefcac9e9 100644 --- a/src/tests/intg/test_infopipe.py +++ b/src/tests/intg/test_infopipe.py @@ -615,3 +615,57 @@ def test_sssctl_domain_list_app_domain(dbus_system_bus, assert "Error" not in output assert output.find("LDAP") != -1 assert output.find("app") != -1 + + +def test_update_member_list_and_get_all(dbus_system_bus, + ldap_conn, + sanity_rfc2307): + ''' + Test that UpdateMemberList() and GetAll() return the correct users that are + members of a group + ''' + sssd_obj = dbus_system_bus.get_object( + 'org.freedesktop.sssd.infopipe', + '/org/freedesktop/sssd/infopipe/Groups') + groups_iface = dbus.Interface(sssd_obj, + 'org.freedesktop.sssd.infopipe.Groups') + group_id = 2011 + expected_user_result = "/org/freedesktop/sssd/infopipe/Users/LDAP/1001" + + group_path = groups_iface.FindByName('single_user_group') + + group_object = dbus_system_bus.get_object('org.freedesktop.sssd.infopipe', + group_path) + group_iface = dbus.Interface(group_object, + 'org.freedesktop.sssd.infopipe.Groups.Group') + + # update local cache for group + try: + group_iface.UpdateMemberList(group_id) + except dbus.exceptions.DBusException as ex: + assert False, "Unexpected DBusException raised" + + # check members of group + prop_iface = dbus.Interface(group_object, + 'org.freedesktop.DBus.Properties') + res = prop_iface.GetAll('org.freedesktop.sssd.infopipe.Groups.Group') + assert str(res.get("users")[0]) == expected_user_result + + # delete group (there's no other way of removing a user from a group) and + # wait change to propagate + ldap_conn.delete("cn=single_user_group,ou=Groups,dc=example,dc=com") + time.sleep(INTERACTIVE_TIMEOUT) + + # add group back but this time without any member + ldap_group = ldap_ent.group("dc=example,dc=com", "single_user_group", 2011) + ldap_conn.add_s(ldap_group[0], ldap_group[1]) + + # invalidate cache + subprocess.call(["sss_cache", "-E"]) + + # check that group has no members + group_iface.UpdateMemberList(group_id) + prop_interface = dbus.Interface(group_object, + 'org.freedesktop.DBus.Properties') + res = prop_interface.GetAll('org.freedesktop.sssd.infopipe.Groups.Group') + assert not res.get("users")
_______________________________________________ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
