From 2a181763d0cff5f31dc18f3e71f79dd815906c09 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Fri, 22 Feb 2019 13:46:48 -0500
Subject: [PATCH] Ticket 50238 - Failed modrdn can corrupt entry cache

Bug Description:  Under certain conditions (found under IPA) when a backend
                  transaction plugin fails and causes a modrdn operation to
                  fail the entry cache no longer contains the original/pre
                  entry, but instead it has the post modrdn'ed entry with
                  the original entry's ID

Fix Description:  Upon failure, if the post entry is in the cache, then swap
                  it out with the original entry.

https://pagure.io/389-ds-base/issue/50238

Reviewed by: firstyear, spichugi, & tboardaz (Thanks!!!)
---
 dirsrvtests/tests/suites/betxns/betxn_test.py | 57 +++++++++++++++++++
 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c    | 16 ++++--
 2 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/dirsrvtests/tests/suites/betxns/betxn_test.py b/dirsrvtests/tests/suites/betxns/betxn_test.py
index 175496495..48181a9ea 100644
--- a/dirsrvtests/tests/suites/betxns/betxn_test.py
+++ b/dirsrvtests/tests/suites/betxns/betxn_test.py
@@ -8,6 +8,7 @@
 #
 import pytest
 import six
+import ldap
 from lib389.tasks import *
 from lib389.utils import *
 from lib389.topologies import topology_st
@@ -248,6 +249,62 @@ def test_betxn_memberof(topology_st, dynamic_plugins):
     log.info('test_betxn_memberof: PASSED')
 
 
+def test_betxn_modrdn_memberof(topology_st):
+    """Test modrdn operartions and memberOf
+
+    :id: 70d0b96e-b693-4bf7-bbf5-102a66ac5994
+
+    :setup: Standalone instance
+
+    :steps: 1. Enable and configure memberOf plugin
+            2. Set memberofgroupattr="member" and memberofAutoAddOC="nsContainer"
+            3. Create group and user outside of memberOf plugin scope
+            4. Do modrdn to move group into scope
+            5. Do modrdn to move group into scope (again)
+
+    :expectedresults:
+            1. memberOf plugin plugin should be ON
+            2. Set memberofgroupattr="member" and memberofAutoAddOC="nsContainer" should PASS
+            3. Creating group and user should PASS
+            4. Modrdn should fail with objectclass violation
+            5. Second modrdn should also fail with objectclass violation
+    """
+
+    peoplebase = 'ou=people,%s' % DEFAULT_SUFFIX
+    memberof = MemberOfPlugin(topology_st.standalone)
+    memberof.enable()
+    memberof.set_autoaddoc('nsContainer')  # Bad OC
+    memberof.set('memberOfEntryScope', peoplebase)
+    memberof.set('memberOfAllBackends', 'on')
+    topology_st.standalone.restart()
+
+    groups = Groups(topology_st.standalone, DEFAULT_SUFFIX)
+    group = groups.create(properties={
+        'cn': 'group',
+    })
+
+    # Create user and add it to group
+    users = UserAccounts(topology_st.standalone, basedn=DEFAULT_SUFFIX)
+    user = users.create(properties=TEST_USER_PROPERTIES)
+    if not ds_is_older('1.3.7'):
+        user.remove('objectClass', 'nsMemberOf')
+
+    group.add_member(user.dn)
+
+    # Attempt modrdn that should fail, but the original entry should stay in the cache
+    with pytest.raises(ldap.OBJECTCLASS_VIOLATION):
+        group.rename('cn=group_to_people', newsuperior=peoplebase)
+
+    # Should fail, but not with NO_SUCH_OBJECT as the original entry should still be in the cache
+    with pytest.raises(ldap.OBJECTCLASS_VIOLATION):
+        group.rename('cn=group_to_people', newsuperior=peoplebase)
+
+    #
+    # Done
+    #
+    log.info('test_betxn_modrdn_memberof: PASSED')
+
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index 684b040b8..e4d0337d4 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -1411,14 +1411,20 @@ common_return:
                                       "operation failed, the target entry is cleared from dncache (%s)\n", slapi_entry_get_dn(ec->ep_entry));
             CACHE_REMOVE(&inst->inst_dncache, bdn);
             CACHE_RETURN(&inst->inst_dncache, &bdn);
+            /*
+             * If the new/invalid entry (ec) is in the cache, that means we need to
+             * swap it out with the original entry (e) --> to undo the swap that
+             * modrdn_rename_entry_update_indexes() did.
+             */
+            if (cache_is_in_cache(&inst->inst_cache, ec)) {
+                if (cache_replace(&inst->inst_cache, ec, e) != 0) {
+                        slapi_log_err(SLAPI_LOG_ALERT, "ldbm_back_modrdn",
+                                "failed to replace cache entry after error\n");
+                 }
+            }
         }
 
-        /* remove the new entry from the cache if the op failed -
-           otherwise, leave it in */
         if (ec && inst) {
-            if (retval && cache_is_in_cache(&inst->inst_cache, ec)) {
-                CACHE_REMOVE(&inst->inst_cache, ec);
-            }
             CACHE_RETURN(&inst->inst_cache, &ec);
         }
         ec = NULL;
-- 
2.17.2