From 0af29fe9f60ebfea75373ac0997d756e98cf3982 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Tue, 1 Mar 2022 12:16:50 -0500 Subject: [PATCH] Issue 5184 - memberOf does not work correctly with multiple include scopes Bug Description: MemberOf Plugin only looks at the first include scope, and the rest are ignored. So if multiple "memberOfEntryScope" attributes are set then the plugin will not work as expected. Fix Description: The fix is to read all the memberOfEntryScope attributes and update the group cache. relates: https://github.com/389ds/389-ds-base/issues/5184 Reviewed by: tbordaz(Thanks!) --- .../memberof_include_scopes_test.py | 105 ++++++++++++++++++ ldap/servers/plugins/memberof/memberof.c | 49 ++++---- 2 files changed, 128 insertions(+), 26 deletions(-) create mode 100644 dirsrvtests/tests/suites/memberof_plugin/memberof_include_scopes_test.py diff --git a/dirsrvtests/tests/suites/memberof_plugin/memberof_include_scopes_test.py b/dirsrvtests/tests/suites/memberof_plugin/memberof_include_scopes_test.py new file mode 100644 index 0000000000..025159ffa8 --- /dev/null +++ b/dirsrvtests/tests/suites/memberof_plugin/memberof_include_scopes_test.py @@ -0,0 +1,105 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2022 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# +import pytest +import os +import time +import ldap +from lib389.utils import ensure_str +from lib389.topologies import topology_st as topo +from lib389._constants import * +from lib389.plugins import MemberOfPlugin +from lib389.idm.user import UserAccounts +from lib389.idm.group import Groups +from lib389.idm.nscontainer import nsContainers + +SUBTREE_1 = 'cn=sub1,%s' % SUFFIX +SUBTREE_2 = 'cn=sub2,%s' % SUFFIX +SUBTREE_3 = 'cn=sub3,%s' % SUFFIX + +def add_container(inst, dn, name): + """Creates container entry""" + conts = nsContainers(inst, dn) + cont = conts.create(properties={'cn': name}) + return cont + +def add_member_and_group(server, cn, group_cn, subtree): + users = UserAccounts(server, subtree, rdn=None) + users.create(properties={'uid': f'test_{cn}', + 'cn': f'test_{cn}', + 'sn': f'test_{cn}', + 'description': 'member', + 'uidNumber': '1000', + 'gidNumber': '2000', + 'homeDirectory': '/home/testuser'}) + group = Groups(server, subtree, rdn=None) + group.create(properties={'cn': group_cn, + 'member': f'uid=test_{cn},{subtree}', + 'description': 'group'}) + +def check_membership(server, user_dn=None, group_dn=None, find_result=True): + ent = server.getEntry(user_dn, ldap.SCOPE_BASE, "(objectclass=*)", ['memberof']) + found = False + if ent.hasAttr('memberof'): + for val in ent.getValues('memberof'): + if ensure_str(val) == group_dn: + found = True + break + + if find_result: + assert found + else: + assert (not found) + +def test_multiple_scopes(topo): + """Specify memberOf works when multiple include scopes are defined + + :id: fbcd70cc-c83d-4c79-bd5b-2d8f017545ae + :setup: Standalone Instance + :steps: + 1. Set multiple include scopes + 2. Test members added to both scopes are correctly updated + 3. Test user outside of scope was not updated + :expectedresults: + 1. Success + 2. Success + 3. Success + """ + + inst = topo.standalone + + # configure plugin + memberof = MemberOfPlugin(inst) + memberof.enable() + memberof.add('memberOfEntryScope', SUBTREE_1) + memberof.add('memberOfEntryScope', SUBTREE_2) + inst.restart() + + # Add setup entries + add_container(inst, SUFFIX, 'sub1') + add_container(inst, SUFFIX, 'sub2') + add_container(inst, SUFFIX, 'sub3') + add_member_and_group(inst, 'm1', 'g1', SUBTREE_1) + add_member_and_group(inst, 'm2', 'g2', SUBTREE_2) + add_member_and_group(inst, 'm3', 'g3', SUBTREE_3) + + # Check users 1 and 2 were correctly updated + check_membership(inst, f'uid=test_m1,{SUBTREE_1}', f'cn=g1,{SUBTREE_1}', True) + check_membership(inst, f'uid=test_m2,{SUBTREE_2}', f'cn=g2,{SUBTREE_2}', True) + + # Check that user3, which is out of scope, was not updated + check_membership(inst, f'uid=test_m3,{SUBTREE_3}', f'cn=g1,{SUBTREE_1}', False) + check_membership(inst, f'uid=test_m3,{SUBTREE_3}', f'cn=g2,{SUBTREE_2}', False) + check_membership(inst, f'uid=test_m3,{SUBTREE_3}', f'cn=g3,{SUBTREE_3}', False) + + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main(["-s", CURRENT_FILE]) diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c index 190e65157a..08db890e4d 100644 --- a/ldap/servers/plugins/memberof/memberof.c +++ b/ldap/servers/plugins/memberof/memberof.c @@ -626,21 +626,6 @@ memberof_del_dn_type_callback(Slapi_Entry *e, void *callback_data) return rc; } -/* Check if the the entry include scope is a child of the sdn */ -static Slapi_DN * -memberof_scope_is_child_of_dn(MemberOfConfig *config, Slapi_DN *sdn) -{ - int i = 0; - - while (config->entryScopes && config->entryScopes[i]) { - if (slapi_sdn_issuffix(config->entryScopes[i], sdn)) { - return config->entryScopes[i]; - } - i++; - } - return NULL; -} - static void add_ancestors_cbdata(memberof_cached_value *ancestors, void *callback_data) { @@ -791,8 +776,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn search_pb = slapi_pblock_new(); be = slapi_get_first_backend(&cookie); while (be) { - Slapi_DN *scope_sdn = NULL; - + PRBool do_suffix_search = PR_TRUE; if (!all_backends) { be = slapi_be_select(sdn); if (be == NULL) { @@ -813,9 +797,19 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn if (memberof_entry_in_scope(config, base_sdn)) { /* do nothing, entry scope is spanning * multiple suffixes, start at suffix */ - } else if ((scope_sdn = memberof_scope_is_child_of_dn(config, base_sdn))) { - /* scope is below suffix, set search base */ - base_sdn = scope_sdn; + } else if (config->entryScopes) { + for (size_t i = 0; config->entryScopes[i]; i++) { + if (slapi_sdn_issuffix(config->entryScopes[i], base_sdn)) { + /* Search each include scope */ + slapi_search_internal_set_pb(search_pb, slapi_sdn_get_dn(config->entryScopes[i]), + LDAP_SCOPE_SUBTREE, filter_str, 0, 0, 0, 0, + memberof_get_plugin_id(), 0); + slapi_search_internal_callback_pb(search_pb, callback_data, 0, callback, 0); + /* We already did the search for this backend, don't + * do it again when we fall through */ + do_suffix_search = PR_FALSE; + } + } } else if (!all_backends) { break; } else { @@ -825,17 +819,20 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn } } - slapi_search_internal_set_pb(search_pb, slapi_sdn_get_dn(base_sdn), - LDAP_SCOPE_SUBTREE, filter_str, 0, 0, 0, 0, memberof_get_plugin_id(), 0); - slapi_search_internal_callback_pb(search_pb, callback_data, 0, callback, 0); - slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &rc); - if (rc != LDAP_SUCCESS) { - break; + if (do_suffix_search) { + slapi_search_internal_set_pb(search_pb, slapi_sdn_get_dn(base_sdn), + LDAP_SCOPE_SUBTREE, filter_str, 0, 0, 0, 0, memberof_get_plugin_id(), 0); + slapi_search_internal_callback_pb(search_pb, callback_data, 0, callback, 0); + slapi_pblock_get(search_pb, SLAPI_PLUGIN_INTOP_RESULT, &rc); + if (rc != LDAP_SUCCESS) { + break; + } } if (!all_backends) { break; } + /* Reset pb for next loop */ slapi_pblock_init(search_pb); be = slapi_get_next_backend(cookie); }