From 26654d3e5f5882dd1681116cb49228d108351d48 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 12 Aug 2021 09:27:57 +0200 Subject: [PATCH] cache_req: cache_first fix for fully-qualified names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With commit b572871236a7f9059d375a5ab1bff8cbfd519956 "cache_req: introduce cache_behavior enumeration" the processing of cache and backend lookups was refactored. Unfortunately this introduce an issue when looking up users or groups with a fully-qualified name and the 'cache_first = True' option is set. In the old code the case when a domain name is available was handle before the cache_first first option was evaluated and cache_req was instructed to first look in the cache and then call the backend if the object is not available or expired, i.e. the default behavior. Since only a single domain is involved this is in agreement with 'cache_first = True' and only a single iteration is needed. In the new code the cache_first option is evaluated before the presence of a domain name is checked and as a result even for single domain searches the first cache_req iteration is only looking at the cache and will not call the backend. This means the now for searches with a fully-qualified name a second iteration is needed if the object was not found in the cache. Unfortunately the old exit condition that if a domain name is present only a single iteration is needed is still present in the new code which effectively makes requests with fully-qualified named only search the cache and never call the backends. This patch removes the exit condition and does a second iteration for fully-qualified names as well if 'cache_first = True' is set. Resolves: https://github.com/SSSD/sssd/issues/5744 Reviewed-by: Pavel Březina --- src/responder/common/cache_req/cache_req.c | 3 +- src/tests/cmocka/test_responder_cache_req.c | 53 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/responder/common/cache_req/cache_req.c b/src/responder/common/cache_req/cache_req.c index 750d655c1..56ec077f3 100644 --- a/src/responder/common/cache_req/cache_req.c +++ b/src/responder/common/cache_req/cache_req.c @@ -1331,8 +1331,7 @@ static errno_t cache_req_select_domains(struct tevent_req *req, state = tevent_req_data(req, struct cache_req_state); - if ((state->cr->cache_behavior != CACHE_REQ_CACHE_FIRST) - || (domain_name != NULL)) { + if (state->cr->cache_behavior != CACHE_REQ_CACHE_FIRST) { if (!state->first_iteration) { /* We're done here. */ diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c index 5cf7660e7..27a525f6e 100644 --- a/src/tests/cmocka/test_responder_cache_req.c +++ b/src/tests/cmocka/test_responder_cache_req.c @@ -992,6 +992,56 @@ void test_user_by_name_missing_notfound(void **state) assert_true(test_ctx->dp_called); } +void test_user_by_name_missing_notfound_cache_first(void **state) +{ + struct cache_req_test_ctx *test_ctx = NULL; + + test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx); + test_ctx->rctx->cache_first = true; + + /* Mock values. */ + will_return(__wrap_sss_dp_get_account_send, test_ctx); + mock_account_recv_simple(); + mock_parse_inp(users[0].short_name, NULL, ERR_OK); + + /* Test. */ + run_user_by_name(test_ctx, test_ctx->tctx->dom, 0, ENOENT); + assert_true(test_ctx->dp_called); +} + +void test_user_by_name_missing_notfound_full_name(void **state) +{ + struct cache_req_test_ctx *test_ctx = NULL; + + test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx); + + /* Mock values. */ + will_return(__wrap_sss_dp_get_account_send, test_ctx); + mock_account_recv_simple(); + mock_parse_inp(users[0].short_name, TEST_DOM_NAME, ERR_OK); + + /* Test. */ + run_user_by_name(test_ctx, test_ctx->tctx->dom, 0, ENOENT); + assert_true(test_ctx->dp_called); +} + +void test_user_by_name_missing_notfound_cache_first_full_name(void **state) +{ + struct cache_req_test_ctx *test_ctx = NULL; + + test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx); + test_ctx->rctx->cache_first = true; + + /* Mock values. */ + will_return(__wrap_sss_dp_get_account_send, test_ctx); + mock_account_recv_simple(); + mock_parse_inp(users[0].short_name, TEST_DOM_NAME, ERR_OK); + + /* Test. */ + run_user_by_name(test_ctx, test_ctx->tctx->dom, 0, ENOENT); + assert_true(test_ctx->dp_called); +} + void test_user_by_name_multiple_domains_requested_domains_found(void **state) { struct cache_req_test_ctx *test_ctx = NULL; @@ -4255,6 +4305,9 @@ int main(int argc, const char *argv[]) new_single_domain_test(user_by_name_ncache), new_single_domain_test(user_by_name_missing_found), new_single_domain_test(user_by_name_missing_notfound), + new_single_domain_test(user_by_name_missing_notfound_cache_first), + new_single_domain_test(user_by_name_missing_notfound_full_name), + new_single_domain_test(user_by_name_missing_notfound_cache_first_full_name), new_multi_domain_test(user_by_name_multiple_domains_found), new_multi_domain_test(user_by_name_multiple_domains_notfound), new_multi_domain_test(user_by_name_multiple_domains_parse), -- 2.26.3