Support LDAPModifyDNRequest with deleteoldrdn=False on the server (fixes #89)

docs/source/NEWS.rst

@@ -76,6 +76,12 @@ Fixes
   document and expect. This fixes ``ValueError: too many values to
   unpack (expected 2)`` when round-tripping
   ``ModifyOp -> asLDAP -> fromLDAP`` (#223).
+- ``LDAPModifyDNRequest`` with ``deleteoldrdn=False`` is now honored by
+  the server: the old RDN's attribute value is retained on the moved
+  entry alongside the new RDN value, per RFC 4511 section 4.9. The
+  in-memory back end grew a ``deleteOldRDN`` kwarg on ``move`` and the
+  client-side ``ldapsyntax.LDAPEntry.move`` grew the same flag so
+  callers can request either behaviour (#89).
 
 
 21.2.0 (2021-02-28)

ldaptor/entry.py

@@ -268,7 +268,7 @@ def undo(self):
     def commit(self):
         raise NotImplementedError()
 
-    def move(self, newDN):
+    def move(self, newDN, deleteOldRDN=True):
         raise NotImplementedError()
 
     def delete(self):

ldaptor/inmemory.py

@@ -90,7 +90,7 @@ def _deleteChild(self, rdn):
     def deleteChild(self, rdn):
         return defer.maybeDeferred(self._deleteChild, rdn)
 
-    def _move(self, newDN):
+    def _move(self, newDN, deleteOldRDN=True):
         if not isinstance(newDN, distinguishedname.DistinguishedName):
             newDN = distinguishedname.DistinguishedName(stringValue=newDN)
         if newDN.up() != self.dn.up():
@@ -101,25 +101,28 @@ def _move(self, newDN):
             d = defer.maybeDeferred(root.lookup, newDN.up())
         else:
             d = defer.succeed(None)
-        d.addCallback(self._move2, newDN)
+        d.addCallback(self._move2, newDN, deleteOldRDN)
         return d
 
-    def _move2(self, newParent, newDN):
+    def _move2(self, newParent, newDN, deleteOldRDN=True):
         if newParent is not None:
             newParent._children[newDN.split()[0].getText()] = self
             del self._parent._children[self.dn.split()[0].getText()]
-        # remove old RDN attributes
-        for attr in self.dn.split()[0].split():
-            self[attr.attributeType].remove(attr.value)
-        # add new RDN attributes
+        if deleteOldRDN:
+            # remove old RDN attributes
+            for attr in self.dn.split()[0].split():
+                self[attr.attributeType].remove(attr.value)
+        # add new RDN attributes (idempotent when the value is already
+        # present, e.g. when deleteOldRDN is False and old and new RDN
+        # overlap)
         for attr in newDN.split()[0].split():
             # TODO what if the key does not exist?
             self[attr.attributeType].add(attr.value)
         self.dn = newDN
         return self
 
-    def move(self, newDN):
-        return defer.maybeDeferred(self._move, newDN)
+    def move(self, newDN, deleteOldRDN=True):
+        return defer.maybeDeferred(self._move, newDN, deleteOldRDN)
 
     def commit(self):
         return defer.succeed(True)

ldaptor/protocols/ldap/ldapserver.py

@@ -370,10 +370,6 @@ def handle_LDAPModifyDNRequest(self, request, controls, reply):
         dn = distinguishedname.DistinguishedName(request.entry)
         newrdn = distinguishedname.RelativeDistinguishedName(request.newrdn)
         deleteoldrdn = bool(request.deleteoldrdn)
-        if not deleteoldrdn:
-            raise ldaperrors.LDAPUnwillingToPerform(
-                "Cannot handle preserving old RDN yet."
-            )
         newSuperior = request.newSuperior
         if newSuperior is None:
             newSuperior = dn.up()
@@ -386,8 +382,7 @@ def handle_LDAPModifyDNRequest(self, request, controls, reply):
         d = root.lookup(dn)
 
         def _gotEntry(entry):
-            d = entry.move(newdn)
-            return d
+            return entry.move(newdn, deleteOldRDN=deleteoldrdn)
 
         def _report(entry):
             return pureldap.LDAPModifyDNResponse(resultCode=0)

ldaptor/protocols/ldap/ldapsyntax.py

@@ -358,7 +358,7 @@ def _cbMoveDone(self, msg, newDN):
         self.dn = newDN
         return self
 
-    def move(self, newDN):
+    def move(self, newDN, deleteOldRDN=True):
         self._checkState()
         newDN = distinguishedname.DistinguishedName(newDN)
 
@@ -368,7 +368,7 @@ def move(self, newDN):
         op = pureldap.LDAPModifyDNRequest(
             entry=self.dn.getText(),
             newrdn=newrdn.getText(),
-            deleteoldrdn=1,
+            deleteoldrdn=1 if deleteOldRDN else 0,
             newSuperior=newSuperior.getText(),
         )
         d = self.client.send(op)

ldaptor/test/test_server.py

@@ -717,6 +717,42 @@ def test_modifyDN_rdnOnly_deleteOldRDN_success(self):
         )
         return d
 
+    def test_modifyDN_rdnOnly_preserveOldRDN_success(self):
+        """
+        LDAPModifyDNRequest with deleteoldrdn=False must succeed and
+        retain the old RDN attribute value alongside the new one (#89).
+        """
+        newrdn = "cn=thingamagic"
+        self.server.dataReceived(
+            pureldap.LDAPMessage(
+                pureldap.LDAPModifyDNRequest(
+                    entry=self.thingie.dn.getText(),
+                    newrdn=newrdn,
+                    deleteoldrdn=False,
+                ),
+                id=2,
+            ).toWire()
+        )
+        self.assertEqual(
+            self.server.transport.value(),
+            pureldap.LDAPMessage(
+                pureldap.LDAPModifyDNResponse(resultCode=ldaperrors.Success.resultCode),
+                id=2,
+            ).toWire(),
+        )
+        d = self.stuff.children()
+
+        def _check(actual):
+            got = {str(e.dn): sorted(e.get("cn", [])) for e in actual}
+            expected = {
+                "cn=thingamagic,ou=stuff,dc=example,dc=com": ["thingamagic", "thingie"],
+                "cn=another,ou=stuff,dc=example,dc=com": ["another"],
+            }
+            self.assertEqual(got, expected)
+
+        d.addCallback(_check)
+        return d
+
     def test_modify(self):
         self.server.dataReceived(
             pureldap.LDAPMessage(