diff --git a/docs/source/NEWS.rst b/docs/source/NEWS.rst index 2f621ca6..27ad6923 100644 --- a/docs/source/NEWS.rst +++ b/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) diff --git a/ldaptor/entry.py b/ldaptor/entry.py index 2d8e23bf..40410aa3 100644 --- a/ldaptor/entry.py +++ b/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): diff --git a/ldaptor/inmemory.py b/ldaptor/inmemory.py index 27c883c7..4c2cd986 100644 --- a/ldaptor/inmemory.py +++ b/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) diff --git a/ldaptor/protocols/ldap/ldapserver.py b/ldaptor/protocols/ldap/ldapserver.py index 4bd8b4f8..d9b408fc 100644 --- a/ldaptor/protocols/ldap/ldapserver.py +++ b/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) diff --git a/ldaptor/protocols/ldap/ldapsyntax.py b/ldaptor/protocols/ldap/ldapsyntax.py index b45e424f..30430f07 100644 --- a/ldaptor/protocols/ldap/ldapsyntax.py +++ b/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) diff --git a/ldaptor/test/test_server.py b/ldaptor/test/test_server.py index 1e650b0a..7aaea54c 100644 --- a/ldaptor/test/test_server.py +++ b/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(