From 26654d3e5f5882dd1681116cb49228d108351d48 Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

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 <pbrezina@redhat.com>

---

 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