We can have target DN both inside or outside of a container. Previously, the code did not look into the latter one. When container is a child of the target DN (like using IPA's base DN instead of cn=compat,$BASE_DN), the check failed.
Fixes: rhbz#2168893
Signed-off-by: Alexander Bokovoy abokovoy@redhat.com
@tbordaz please review.
The code looks correct but the semantic looks weird. We are setting cbdata->answwer=TRUE if the target_dn (search base ?) is in the scope of map (that looks fine) but also if the map is in the scope of target_dn (I do not understand). My understanding is that a search(base=suffix, scope=one_level,filter=...) will get answer=TRUE because 'cn=compat,suffix' is a child of 'suffix' while the search should not go in cn=compat.
Also It looks like the function always return TRUE. So all SRCH should go into the map (correct ?).
I was wondering if the following code could help
.... cbdata->answer = FALSE; if (slapi_sdn_scope_test(cbdata->target_dn, set_data->container_sdn, cbdata->scope) != 0) { cbdata->answer = TRUE; } return cbdata->answer;
the target DN is a search's base DN. So if we have a subtree search done against $suffix, it should get into cn=compat,$suffix as well. With the current code we don't get there.
target DN
$suffix
cn=compat,$suffix
The fucntion will not return TRUE in case a search base DN does not include any of the compat tree containers. For example, a search for cn=users,$suffix would not match compat tree containers.
cn=users,$suffix
Okay but we need to set answer=FALSE for SRCH(base=$suffix, scope=one_level, filter...). So in the else branch, something like if (scope=subtree && slapi_sdn_issuffix(container, target_dn)
Weird I apply the patch and it looks backend_search_find_set_dn_in_group_cb always return TRUE
cbdata->answer is set to FALSE by default.
cbdata->answer
You are right that we can skip non-subtree searches from the second check:
diff --git a/src/back-sch.c b/src/back-sch.c index 094c6b9..e447bda 100644 --- a/src/back-sch.c +++ b/src/back-sch.c @@ -1342,7 +1342,8 @@ backend_search_find_set_dn_in_group_cb(const char *group, const char *set, bool_ set_data->container_sdn, cbdata->scope) != 0) { cbdata->answer = TRUE; - } else if (slapi_sdn_scope_test(set_data->container_sdn, + } else if ((cbdata->scope == LDAP_SCOPE_SUBTREE) && + slapi_sdn_scope_test(set_data->container_sdn, cbdata->target_dn, cbdata->scope) != 0) { cbdata->answer = TRUE;
I just realise that map_data_foreach_map callbacks needs to return TRUE. Please ignore my concern regarding backend_search_find_set_dn_in_group_cb returning TRUE
rebased onto 24eeccd408d9627299231d7843ca9e65e71af3de
It can also be directly 'slapi_sdn_issuffix(set_data->container_sdn, cbdata->target_dn)
It was using slapi_sdn_issuffix() in the previous change and that broke it.
slapi_sdn_issuffix()
Yes but the parameters had the wrong order
I am thinking that if we'd use slapi_sdn_scope_test in both cases, we don't need to do a scope test for subtree as well. Otherwise, slapi_sdn_suffix use would still need a scope test.
slapi_sdn_scope_test
slapi_sdn_suffix
Thanks. The patch LGTM. Ack
Pull-Request has been merged by abbra
Merged. I manually ran tests on Azure CI here: https://dev.azure.com/abbra1freeipa/slapi-nis/_build/results?buildId=697&view=results
We can have target DN both inside or outside of a container.
Previously, the code did not look into the latter one. When container is
a child of the target DN (like using IPA's base DN instead of
cn=compat,$BASE_DN), the check failed.
Fixes: rhbz#2168893
Signed-off-by: Alexander Bokovoy abokovoy@redhat.com