From 594bf91f6b182e2b8d053109e330a8ccb67f395b Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: Jul 30 2020 12:41:14 +0000 Subject: Issue 51222 - It should not be allowed to delete Managed Entry manually Bug Description: It is possible to delete a managed entry and no error is raised. Also, while doing delete or modrdn peration on a managing entry and the managed entry doesn't exist, we should continue the operation. Fix Description: We should put an entry struct duplicate to SLAPI_ENTRY_PRE_OP pblock before we execute plugins PRE_OP. Also, we should allow to continue modrdn and delete managing entry operations execution even when managed entry doesn't exists. Allow 'cn=directory manager' to delete managed entry on direct update. Add a test. https://pagure.io/389-ds-base/issue/51222 Reviewed by: firstyear, tbordaz (Thanks!) --- diff --git a/dirsrvtests/tests/suites/plugins/managed_entry_test.py b/dirsrvtests/tests/suites/plugins/managed_entry_test.py index 4299509..43748c6 100644 --- a/dirsrvtests/tests/suites/plugins/managed_entry_test.py +++ b/dirsrvtests/tests/suites/plugins/managed_entry_test.py @@ -17,10 +17,12 @@ from lib389.config import Config from lib389.idm.organizationalunit import OrganizationalUnits, OrganizationalUnit from lib389.plugins import MEPTemplates, MEPConfigs, ManagedEntriesPlugin, MEPTemplate from lib389.idm.nscontainer import nsContainers +from lib389.idm.domain import Domain from lib389.tasks import Entry import ldap pytestmark = pytest.mark.tier1 +USER_PASSWORD = 'password' @pytest.fixture(scope="module") @@ -249,9 +251,100 @@ def test_mentry01(topo, _create_inital): assert user.get_attr_val_utf8('mepManagedEntry') != before +def test_managed_entry_removal(topo): + """Check that we can't remove managed entry manually + + :id: cf9c5be5-97ef-46fc-b199-8346acf4c296 + :setup: Standalone Instance + :steps: + 1. Enable the plugin + 2. Restart the instance + 3. Add our org units + 4. Set up config entry and template entry for the org units + 5. Add an entry that meets the MEP scope + 6. Check if a managed group entry was created + 7. Try to remove the entry while bound as Admin (non-DM) + 8. Remove the entry while bound as DM + 9. Check that the managing entry can be deleted too + :expectedresults: + 1. Success + 2. Success + 3. Success + 4. Success + 5. Success + 6. Success + 7. Should fail + 8. Success + 9. Success + """ + + inst = topo.standalone + + # Add ACI so we can test that non-DM user can't delete managed entry + domain = Domain(inst, DEFAULT_SUFFIX) + ACI_TARGET = f"(target = \"ldap:///{DEFAULT_SUFFIX}\")" + ACI_TARGETATTR = "(targetattr = *)" + ACI_ALLOW = "(version 3.0; acl \"Admin Access\"; allow (all) " + ACI_SUBJECT = "(userdn = \"ldap:///anyone\");)" + ACI_BODY = ACI_TARGET + ACI_TARGETATTR + ACI_ALLOW + ACI_SUBJECT + domain.add('aci', ACI_BODY) + + # stop the plugin, and start it + plugin = ManagedEntriesPlugin(inst) + plugin.disable() + plugin.enable() + + # Add our org units + ous = OrganizationalUnits(inst, DEFAULT_SUFFIX) + ou_people = ous.create(properties={'ou': 'managed_people'}) + ou_groups = ous.create(properties={'ou': 'managed_groups'}) + + mep_templates = MEPTemplates(inst, DEFAULT_SUFFIX) + mep_template1 = mep_templates.create(properties={ + 'cn': 'MEP template', + 'mepRDNAttr': 'cn', + 'mepStaticAttr': 'objectclass: groupOfNames|objectclass: extensibleObject'.split('|'), + 'mepMappedAttr': 'cn: $cn|uid: $cn|gidNumber: $uidNumber'.split('|') + }) + mep_configs = MEPConfigs(inst) + mep_configs.create(properties={'cn': 'config', + 'originScope': ou_people.dn, + 'originFilter': 'objectclass=posixAccount', + 'managedBase': ou_groups.dn, + 'managedTemplate': mep_template1.dn}) + inst.restart() + + # Add an entry that meets the MEP scope + test_users_m1 = UserAccounts(inst, DEFAULT_SUFFIX, rdn='ou={}'.format(ou_people.rdn)) + managing_entry = test_users_m1.create_test_user(1001) + managing_entry.reset_password(USER_PASSWORD) + user_bound_conn = managing_entry.bind(USER_PASSWORD) + + # Get the managed entry + managed_groups = Groups(inst, ou_groups.dn, rdn=None) + managed_entry = managed_groups.get(managing_entry.rdn) + + # Check that the managed entry was created + assert managed_entry.exists() + + # Try to remove the entry while bound as Admin (non-DM) + managed_groups_user_conn = Groups(user_bound_conn, ou_groups.dn, rdn=None) + managed_entry_user_conn = managed_groups_user_conn.get(managed_entry.rdn) + with pytest.raises(ldap.UNWILLING_TO_PERFORM): + managed_entry_user_conn.delete() + assert managed_entry_user_conn.exists() + + # Remove the entry while bound as DM + managed_entry.delete() + assert not managed_entry.exists() + + # Check that the managing entry can be deleted too + managing_entry.delete() + assert not managing_entry.exists() + + if __name__ == '__main__': # Run isolated # -s for DEBUG mode CURRENT_FILE = os.path.realpath(__file__) pytest.main("-s %s" % CURRENT_FILE) - diff --git a/ldap/servers/plugins/mep/mep.c b/ldap/servers/plugins/mep/mep.c index 401d95e..4f433ce 100644 --- a/ldap/servers/plugins/mep/mep.c +++ b/ldap/servers/plugins/mep/mep.c @@ -2158,6 +2158,7 @@ mep_pre_op(Slapi_PBlock *pb, int modop) Slapi_Mod *next_mod = NULL; char *origin_dn = NULL; Slapi_DN *origin_sdn = NULL; + char *requestor_dn = NULL; /* Fetch the target entry. */ if (sdn) { @@ -2249,11 +2250,19 @@ mep_pre_op(Slapi_PBlock *pb, int modop) slapi_ch_free_string(&origin_dn); } else { - errstr = slapi_ch_smprintf("%s a managed entry is not allowed. " - "It needs to be manually unlinked first.", - modop == LDAP_CHANGETYPE_DELETE ? "Deleting" - : "Renaming"); - ret = LDAP_UNWILLING_TO_PERFORM; + slapi_pblock_get(pb, SLAPI_REQUESTOR_DN, &requestor_dn); + if (slapi_dn_isroot(requestor_dn)) { + slapi_log_err(SLAPI_LOG_PLUGIN, MEP_PLUGIN_SUBSYSTEM, + "mep_pre_op - %s is %s a managed entry.", + requestor_dn, modop == LDAP_CHANGETYPE_DELETE ? "deleting" + : "renaming"); + } else { + errstr = slapi_ch_smprintf("%s a managed entry is not allowed. " + "It needs to be manually unlinked first.", + modop == LDAP_CHANGETYPE_DELETE ? "Deleting" + : "Renaming"); + ret = LDAP_UNWILLING_TO_PERFORM; + } } } } @@ -2587,10 +2596,18 @@ mep_del_post_op(Slapi_PBlock *pb) slapi_delete_internal_pb(mep_pb); slapi_pblock_get(mep_pb, SLAPI_PLUGIN_INTOP_RESULT, &result); if (result) { - slapi_log_err(SLAPI_LOG_ERR, MEP_PLUGIN_SUBSYSTEM, - "mep_del_post_op - Failed to delete managed entry " - "(%s) - error (%d)\n", - managed_dn, result); + if (result == LDAP_NO_SUCH_OBJECT) { + slapi_log_err(SLAPI_LOG_PLUGIN, MEP_PLUGIN_SUBSYSTEM, + "mep_del_post_op - Failed to delete managed entry " + "(%s) - it doesn't exist already)\n", + managed_dn); + result = SLAPI_PLUGIN_SUCCESS; + } else { + slapi_log_err(SLAPI_LOG_ERR, MEP_PLUGIN_SUBSYSTEM, + "mep_del_post_op - Failed to delete managed entry " + "(%s) - error (%d)\n", + managed_dn, result); + } } slapi_ch_free_string(&managed_dn); slapi_pblock_destroy(mep_pb); @@ -2702,11 +2719,19 @@ mep_modrdn_post_op(Slapi_PBlock *pb) slapi_delete_internal_pb(mep_pb); slapi_pblock_get(mep_pb, SLAPI_PLUGIN_INTOP_RESULT, &result); if (result) { - slapi_log_err(SLAPI_LOG_ERR, MEP_PLUGIN_SUBSYSTEM, - "mep_modrdn_post_op - Failed to delete managed entry " - "(%s) - error (%d)\n", - managed_dn, result); - goto bailmod; + if (result == LDAP_NO_SUCH_OBJECT) { + slapi_log_err(SLAPI_LOG_PLUGIN, MEP_PLUGIN_SUBSYSTEM, + "mep_modrdn_post_op - Failed to delete managed entry " + "(%s) - it doesn't exist already)\n", + managed_dn); + result = SLAPI_PLUGIN_SUCCESS; + } else { + slapi_log_err(SLAPI_LOG_ERR, MEP_PLUGIN_SUBSYSTEM, + "mep_modrdn_post_op - Failed to delete managed entry " + "(%s) - error (%d)\n", + managed_dn, result); + goto bailmod; + } } /* Clear out the pblock for reuse. */ slapi_pblock_init(mep_pb); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 3b7a321..f509196 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -404,6 +404,9 @@ replace_entry: delete_tombstone_entry = operation_is_flag_set(operation, OP_FLAG_TOMBSTONE_ENTRY); } + /* Save away a copy of the entry, before modifications */ + slapi_pblock_set(pb, SLAPI_ENTRY_PRE_OP, slapi_entry_dup(e->ep_entry)); + /* call the transaction pre delete plugins just after the * to-be-deleted entry is prepared. */ /* these should not need to modify the entry to be deleted - @@ -500,8 +503,6 @@ replace_entry: "entry: %s - flags: delete %d is_tombstone_entry %d create %d \n", dn, delete_tombstone_entry, is_tombstone_entry, create_tombstone_entry); #endif - /* Save away a copy of the entry, before modifications */ - slapi_pblock_set(pb, SLAPI_ENTRY_PRE_OP, slapi_entry_dup(e->ep_entry)); /* * Get the entry's parent. We do this here because index_read