URL: https://github.com/SSSD/sssd/pull/5745
Author: sumit-bose
Title: #5745: cache_req: cache_first fix for fully-qualified names
Action: opened
PR body:
"""
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
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5745/head:pr5745
git checkout pr5745
From 356a0ab14bd104affc11fffa6f0ee7356fbb175f 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
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
---
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 750d655c19..56ec077f36 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 5cf7660e71..27a525f6ed 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);
