#50 Test the case when container is a child of the target DN
Merged by abbra. Opened by abbra.
abbra/slapi-nis wide-basedn  into  master

Download 50.patch

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.

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.

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.

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.

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.

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

Metadata