Improve doc move permissions

src/backend/core/api/viewsets.py

@@ -906,6 +906,65 @@ def create_for_owner(self, request):
             {"id": str(document.id)}, status=status.HTTP_201_CREATED
         )
 
+    @drf.decorators.action(detail=True, methods=["post"])
+    @transaction.atomic
+    def detach(self, request, *args, **kwargs):
+        """
+        Detach a subdocument from it's parent
+
+        The user must be an administrator or owner of root document or an
+        editor (on target document) and creator of the document creator
+        """
+        user = request.user
+        document = self.get_object()
+
+        if document.is_root():
+            return drf.response.Response(
+                # mettre le message dans position est valid ?
+                {"message": "You cannot detach a root document"},
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+
+        target_document = document.get_root()
+        if not target_document:
+            return drf.response.Response(
+                {"message": "Parent document does not exist."},
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+
+        if not document.get_abilities(user).get("detach"):
+            return drf.response.Response(
+                {
+                    "message": (
+                        "You do not have permission to move documents "
+                        "as a sibling of this target document."
+                    )
+                },
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+
+        owner_accesses = document.get_root().accesses.filter(
+            role=models.RoleChoices.OWNER
+        )
+
+        document.move(target_document, pos=enums.MoveNodePositionChoices.LAST_SIBLING)
+
+        if (
+            owner_accesses
+            and not document.accesses.filter(role=models.RoleChoices.OWNER).exists()
+        ):
+            for owner_access in owner_accesses:
+                models.DocumentAccess.objects.update_or_create(
+                    document=document,
+                    user=owner_access.user,
+                    team=owner_access.team,
+                    defaults={"role": models.RoleChoices.OWNER},
+                )
+
+        return drf.response.Response(
+            {"message": "Document detached successfully."}, status=status.HTTP_200_OK
+        )
+
     @drf.decorators.action(detail=True, methods=["post"])
     @transaction.atomic
     def move(self, request, *args, **kwargs):
@@ -941,15 +1000,22 @@ def move(self, request, *args, **kwargs):
             enums.MoveNodePositionChoices.FIRST_CHILD,
             enums.MoveNodePositionChoices.LAST_CHILD,
         ]:
-            if not target_document.get_abilities(user).get("move"):
+            if not target_document.get_abilities(user).get("accesses_update"):
                 message = (
                     "You do not have permission to move documents "
                     "as a child to this target document."
                 )
         elif target_document.is_root():
-            owner_accesses = document.get_root().accesses.filter(
-                role=models.RoleChoices.OWNER
-            )
+            if not document.is_root():
+                message = (
+                    "You cannot move a document relative to this target document "
+                    "unless as its child."
+                )
+            elif not target_document.get_abilities(user).get("move"):
+                message = (
+                    "You do not have permission to move documents "
+                    "as a sibling of this target document."
+                )
         elif not target_document.get_parent().get_abilities(user).get("move"):
             message = (
                 "You do not have permission to move documents "

src/backend/core/models.py

@@ -1242,8 +1242,11 @@ def get_abilities(self, user):
 
         # Characteristics that are based only on specific access
         is_owner = role == RoleChoices.OWNER
+        is_creator = self.creator == user
+
         is_deleted = self.ancestors_deleted_at
         is_owner_or_admin = (is_owner or role == RoleChoices.ADMIN) and not is_deleted
+        is_editor_creator = (role == RoleChoices.EDITOR) and is_creator
 
         # Compute access roles before adding link roles because we don't
         # want anonymous users to access versions (we wouldn't know from
@@ -1280,7 +1283,7 @@ def get_abilities(self, user):
         can_destroy = (
             is_owner
             if self.is_root()
-            else (is_owner_or_admin or (user.is_authenticated and self.creator == user))
+            else (is_owner_or_admin or (user.is_authenticated and is_creator))
         ) and not is_deleted
 
         ai_allow_reach_from = settings.AI_ALLOW_REACH_FROM
@@ -1297,6 +1300,7 @@ def get_abilities(self, user):
 
         return {
             "accesses_manage": is_owner_or_admin,
+            "accesses_update": can_update_from_access,
             "accesses_view": has_access_role,
             "ai_proxy": ai_access,
             "ai_transform": ai_access,
@@ -1312,6 +1316,7 @@ def get_abilities(self, user):
             "cors_proxy": can_get,
             "descendants": can_get,
             "destroy": can_destroy,
+            "detach": (is_owner_or_admin or is_editor_creator) and not is_deleted,
             "duplicate": can_get and user.is_authenticated,
             "favorite": can_get and user.is_authenticated,
             "link_configuration": is_owner_or_admin,

src/backend/core/tests/documents/test_api_documents_detach.py

@@ -0,0 +1,207 @@
+"""
+Test moving documents within the document tree via an detail action API endpoint.
+"""
+
+import pytest
+from rest_framework.test import APIClient
+
+from core import factories, models
+
+pytestmark = pytest.mark.django_db
+
+
+def test_api_documents_detach_anonymous_user():
+    """Anonymous users should not be able to detach documents."""
+    target = factories.DocumentFactory()
+    document = factories.DocumentFactory(parent=target)
+
+    response = APIClient().post(f"/api/v1.0/documents/{document.id!s}/detach/")
+
+    assert response.status_code == 401
+    assert response.json() == {
+        "detail": "Authentication credentials were not provided."
+    }
+
+
+def test_api_documents_move_document_is_root():
+    """
+    Detaching a root document should not be allowed
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(users=[(user, "owner")])
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/")
+
+    assert response.status_code == 400
+    assert response.json() == {"message": "You cannot detach a root document"}
+
+
+@pytest.mark.parametrize("role", [None, "reader", "commenter", "editor"])
+def test_api_documents_detach_authenticated_document_no_permission(role):
+    """
+    Authenticated users should not be able to detach documents with insufficient
+    permissions on the root document.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    target = factories.DocumentFactory()
+    if role:
+        factories.UserDocumentAccessFactory(document=target, user=user, role=role)
+    document = factories.DocumentFactory(parent=target)
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/")
+
+    assert response.status_code == 403
+    assert response.json() == {
+        "detail": "You do not have permission to perform this action."
+    }
+
+
+@pytest.mark.parametrize("user_role", models.RoleChoices.values)
+@pytest.mark.parametrize("is_creator", [True, False])
+def test_api_documents_move_authenticated_detach(
+    is_creator,
+    user_role,
+):
+    """
+    Authenticated users with permissions should be allowed to detach documents
+    """
+
+    power_roles = ["administrator", "owner"]
+
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    target_owner = factories.UserFactory()
+    target = factories.DocumentFactory(
+        users=[(target_owner, "owner"), (user, user_role)]
+    )
+
+    document_args = {"parent": target}
+    if is_creator:
+        document_args["creator"] = user
+
+    document = factories.DocumentFactory(**document_args)
+    child = factories.DocumentFactory(parent=document)
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/")
+    document.refresh_from_db()
+
+    if user_role in power_roles or (
+        (
+            user_role == models.RoleChoices.EDITOR
+            or target.get_role(user) == models.RoleChoices.EDITOR
+        )
+        and is_creator
+    ):
+        assert response.status_code == 200
+
+        assert target in list(target.get_siblings())
+        assert document in list(target.get_siblings())
+        assert list(document.get_children()) == [child]
+    else:
+        assert response.status_code == 403
+
+
+def test_api_documents_move_authenticated_no_owner_user_and_team():
+    """
+    Detaching a document with no owner to the root of the tree should automatically declare
+    the owner of the previous root of the document as owner of the document itself.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    parent_owner = factories.UserFactory()
+    parent = factories.DocumentFactory(
+        users=[(parent_owner, "owner")], teams=[("lasuite", "owner")]
+    )
+    # A document with no owner
+    document = factories.DocumentFactory(parent=parent, users=[(user, "administrator")])
+    child = factories.DocumentFactory(parent=document)
+    target = factories.DocumentFactory()
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/")
+
+    assert response.status_code == 200
+    assert response.json() == {"message": "Document detached successfully."}
+    assert document in target.get_siblings()
+    assert parent in target.get_siblings()
+    assert target in target.get_siblings()
+
+    document.refresh_from_db()
+    assert list(document.get_children()) == [child]
+    assert document.accesses.count() == 3
+    assert document.accesses.get(user__isnull=False, role="owner").user == parent_owner
+    assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite"
+    assert document.accesses.get(role="administrator").user == user
+
+
+def test_api_documents_move_authenticated_no_owner_same_user():
+    """
+    Detaching a document should not fail if the user moving a document with no owner was
+    at the same time owner of the previous root and has a role on the document being moved.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    parent = factories.DocumentFactory(
+        users=[(user, "owner")], teams=[("lasuite", "owner")]
+    )
+    # A document with no owner
+    document = factories.DocumentFactory(parent=parent, users=[(user, "reader")])
+    child = factories.DocumentFactory(parent=document)
+    target = factories.DocumentFactory()
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/")
+
+    assert response.status_code == 200
+    assert response.json() == {"message": "Document detached successfully."}
+    assert document in target.get_siblings()
+    assert parent in target.get_siblings()
+    assert target in target.get_siblings()
+
+    document.refresh_from_db()
+    assert list(document.get_children()) == [child]
+    assert document.accesses.count() == 2
+    assert document.accesses.get(user__isnull=False, role="owner").user == user
+    assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite"
+
+
+def test_api_documents_move_authenticated_no_owner_same_team():
+    """
+    Detaching a document should not fail if the team that is owner of the document root was
+    already declared on the document with a different role.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    parent = factories.DocumentFactory(teams=[("lasuite", "owner")])
+    # A document with no owner but same team
+    document = factories.DocumentFactory(
+        parent=parent, users=[(user, "administrator")], teams=[("lasuite", "reader")]
+    )
+    child = factories.DocumentFactory(parent=document)
+    target = factories.DocumentFactory()
+
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/detach/")
+
+    assert response.status_code == 200
+    assert response.json() == {"message": "Document detached successfully."}
+    assert document in target.get_siblings()
+    assert parent in target.get_siblings()
+    assert target in target.get_siblings()
+
+    document.refresh_from_db()
+    assert list(document.get_children()) == [child]
+    assert document.accesses.count() == 2
+    assert document.accesses.get(user__isnull=False, role="administrator").user == user
+    assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite"