From ed663f2e1ecec174bd34b0498c5e1b013e0cd9cc Mon Sep 17 00:00:00 2001 From: Mohamed El Amine BOUKERFA Date: Tue, 28 Apr 2026 15:13:42 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20Prevent=20moving=20docu?= =?UTF-8?q?ment=20to=20its=20own=20descendant=20or=20self?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When attempting to move a document to itself or to any of its descendants, the server would crash with a 500 Internal Server Error. Signed-off-by: Mohamed El Amine BOUKERFA --- CHANGELOG.md | 1 + src/backend/core/api/viewsets.py | 9 +- .../documents/test_api_documents_move.py | 89 +++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cde732765..0a881c139 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to - 🐛(frontend) fix interlinking modal clipping #2213 - 🛂(frontend) fix cannot manage member on small screen #2226 - 🐛(backend) load jwks url when OIDC_RS_PRIVATE_KEY_STR is set +- 🐛(backend) Prevent moving document to its own descendant or self #2208 ### Removed diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 6d053d18f..e9406cd40 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -44,6 +44,7 @@ from rest_framework import filters, status, viewsets from rest_framework import response as drf_response from rest_framework.permissions import AllowAny from rest_framework.views import APIView +from treebeard.exceptions import InvalidMoveToDescendant from core import authentication, choices, enums, models from core.api.filters import remove_accents @@ -961,7 +962,13 @@ class DocumentViewSet( status=status.HTTP_400_BAD_REQUEST, ) - document.move(target_document, pos=position) + try: + document.move(target_document, pos=position) + except InvalidMoveToDescendant: + return drf.response.Response( + {"target_document_id": "Cannot move a document to its own descendant."}, + status=status.HTTP_400_BAD_REQUEST, + ) # Make sure we have at least one owner if ( diff --git a/src/backend/core/tests/documents/test_api_documents_move.py b/src/backend/core/tests/documents/test_api_documents_move.py index ad4f68d41..4cd3727b0 100644 --- a/src/backend/core/tests/documents/test_api_documents_move.py +++ b/src/backend/core/tests/documents/test_api_documents_move.py @@ -438,3 +438,92 @@ def test_api_documents_move_authenticated_deleted_target_as_sibling(position): # Verify that the document has not moved document.refresh_from_db() assert document.is_root() is True + + +@pytest.mark.parametrize("position", enums.MoveNodePositionChoices.values) +def test_api_documents_move_to_descendant(position): + """ + Moving a document to one of its descendants should return a validation error. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + # Create a hierarchy: parent -> child -> grandchild + parent = factories.DocumentFactory(users=[(user, "owner")]) + child = factories.DocumentFactory(parent=parent, users=[(user, "owner")]) + grandchild = factories.DocumentFactory(parent=child, users=[(user, "owner")]) + + # Try moving parent to child (descendant) + response = client.post( + f"/api/v1.0/documents/{parent.id!s}/move/", + data={"target_document_id": str(child.id), "position": position}, + ) + + assert response.status_code == 400 + assert response.json() == { + "target_document_id": "Cannot move a document to its own descendant." + } + + # Try moving parent to grandchild + response = client.post( + f"/api/v1.0/documents/{parent.id!s}/move/", + data={"target_document_id": str(grandchild.id), "position": position}, + ) + + assert response.status_code == 400 + assert response.json() == { + "target_document_id": "Cannot move a document to its own descendant." + } + + # Try moving child to grandchild (still descendant) + response = client.post( + f"/api/v1.0/documents/{child.id!s}/move/", + data={"target_document_id": str(grandchild.id), "position": position}, + ) + + assert response.status_code == 400 + assert response.json() == { + "target_document_id": "Cannot move a document to its own descendant." + } + + # Ensure documents have not moved + parent.refresh_from_db() + child.refresh_from_db() + grandchild.refresh_from_db() + assert parent.is_root() is True + assert child.is_child_of(parent) is True + assert grandchild.is_child_of(child) is True + + +@pytest.mark.parametrize( + "position", + [ + enums.MoveNodePositionChoices.FIRST_CHILD, + enums.MoveNodePositionChoices.LAST_CHILD, + ], +) +def test_api_documents_move_to_self(position): + """ + Moving a document to itself should return a validation error. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(users=[(user, "owner")]) + + # Try moving document to itself + response = client.post( + f"/api/v1.0/documents/{document.id!s}/move/", + data={"target_document_id": str(document.id), "position": position}, + ) + + assert response.status_code == 400 + assert response.json() == { + "target_document_id": "Cannot move a document to its own descendant." + } + + # Ensure document has not moved + document.refresh_from_db() + assert document.is_root() is True