🚸(backend) validate document access roles on submit

The frontend should only let users choose override roles that
make sense.

Author: sampaccoud

src/backend/core/api/serializers.py

@@ -339,12 +339,53 @@ def get_max_ancestors_role(self, instance):
         return getattr(instance, "max_ancestors_role", None)
 
     def get_max_role(self, instance):
-        """Return max_ancestors_role if annotated; else None."""
+        """Return max role."""
         return choices.RoleChoices.max(
             getattr(instance, "max_ancestors_role", None),
             instance.role,
         )
 
+    def validate(self, attrs):
+        """
+        Ensure the selected role is greater than or equal to the maximum role
+        found in any ancestor document for the same user/team.
+        """
+
+        if self.instance:
+            document = self.instance.document
+            user = self.instance.user
+            team = self.instance.team
+        else:
+            document_id = self.context["resource_id"]
+            document = models.Document.objects.get(id=document_id)
+            team = attrs.get("team")
+            user = attrs.get("user")
+
+        ancestors = document.get_ancestors().filter(ancestors_deleted_at__isnull=True)
+        access_qs = models.DocumentAccess.objects.filter(document__in=ancestors)
+
+        if user:
+            access_qs = access_qs.filter(user=user)
+        if team:
+            access_qs = access_qs.filter(team=team)
+
+        ancestor_roles = access_qs.values_list("role", flat=True)
+        inherited_role = choices.RoleChoices.max(*ancestor_roles)
+
+        role = attrs.get("role")
+        get_priority = choices.RoleChoices.get_priority
+        if inherited_role and get_priority(inherited_role) >= get_priority(role):
+            raise serializers.ValidationError(
+                {
+                    "role": (
+                        "Role overrides must be greater than the inherited role: "
+                        f"{inherited_role}/{role}"
+                    )
+                }
+            )
+
+        return super().validate(attrs)
+
     def update(self, instance, validated_data):
         """Make "user" field readonly but only on update."""
         validated_data.pop("team", None)

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

@@ -140,9 +140,9 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
             {
                 "id": str(access.id),
                 "document": {
+                    "depth": access.document.depth,
                     "id": str(access.document_id),
                     "path": access.document.path,
-                    "depth": access.document.depth,
                 },
                 "user": {
                     "full_name": access.user.full_name,
@@ -240,9 +240,9 @@ def test_api_document_accesses_list_authenticated_related_privileged(
             {
                 "id": str(access.id),
                 "document": {
+                    "depth": access.document.depth,
                     "id": str(access.document_id),
                     "path": access.document.path,
-                    "depth": access.document.depth,
                 },
                 "user": {
                     "id": str(access.user.id),
@@ -611,14 +611,15 @@ def test_api_document_accesses_retrieve_authenticated_related(
             "id": str(access.id),
             "abilities": access.get_abilities(user),
             "document": {
+                "depth": access.document.depth,
                 "id": str(access.document_id),
                 "path": access.document.path,
-                "depth": access.document.depth,
             },
             "user": access_user,
             "team": "",
             "role": access.role,
             "max_ancestors_role": None,
+            "max_role": access.role,
         }
 
 
@@ -963,6 +964,119 @@ def test_api_document_accesses_update_owner(
             assert updated_values == old_values
 
 
+@pytest.mark.parametrize("new_override_role", choices.RoleChoices.values)
+@pytest.mark.parametrize("parent_role", choices.RoleChoices.values)
+def test_api_document_accesses_update_higher_role_to_user(
+    parent_role,
+    new_override_role,
+    mock_reset_connections,  # pylint: disable=redefined-outer-name
+):
+    """
+    It should not be allowed to update the role of a document access override
+    for a user with a role lower or equal to the inherited role.
+    """
+    user, other_user = factories.UserFactory.create_batch(2)
+
+    client = APIClient()
+    client.force_login(user)
+
+    parent = factories.DocumentFactory(
+        users=[[user, "owner"], [other_user, parent_role]]
+    )
+    document = factories.DocumentFactory(parent=parent)
+
+    override_role = random.choice(choices.RoleChoices.values)
+    access = factories.UserDocumentAccessFactory(
+        document=document, user=other_user, role=override_role
+    )
+
+    get_priority = choices.RoleChoices.get_priority
+    if get_priority(new_override_role) > get_priority(parent_role):
+        with mock_reset_connections(document.id, str(access.user_id)):
+            response = client.put(
+                f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/",
+                data={"role": new_override_role},
+                format="json",
+            )
+
+        assert response.status_code == 200
+        access.refresh_from_db()
+        assert access.role == new_override_role
+    else:
+        response = client.put(
+            f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/",
+            data={"role": new_override_role},
+            format="json",
+        )
+        assert response.status_code == 400
+        access.refresh_from_db()
+        assert access.role == override_role
+        assert response.json() == {
+            "role": [
+                "Role overrides must be greater than the inherited role: "
+                f"{parent_role}/{new_override_role}"
+            ],
+        }
+
+
+@pytest.mark.skip(
+    reason="Pending fix on https://github.com/suitenumerique/docs/issues/969"
+)
+@pytest.mark.parametrize("new_override_role", choices.RoleChoices.values)
+@pytest.mark.parametrize("parent_role", choices.RoleChoices.values)
+def test_api_document_accesses_update_higher_role_to_team(
+    parent_role,
+    new_override_role,
+    mock_reset_connections,  # pylint: disable=redefined-outer-name
+):
+    """
+    It should not be allowed to update the role of a document access override
+    for a team with a role lower or equal to the inherited role.
+    """
+    user = factories.UserFactory()
+
+    client = APIClient()
+    client.force_login(user)
+
+    parent = factories.DocumentFactory(
+        users=[[user, "owner"]], teams=[["lasuite", parent_role]]
+    )
+    document = factories.DocumentFactory(parent=parent)
+
+    override_role = random.choice(choices.RoleChoices.values)
+    access = factories.TeamDocumentAccessFactory(
+        document=document, team="lasuite", role=override_role
+    )
+
+    get_priority = choices.RoleChoices.get_priority
+    if get_priority(new_override_role) > get_priority(parent_role):
+        with mock_reset_connections(document.id, str(access.user_id)):
+            response = client.put(
+                f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/",
+                data={"role": new_override_role},
+                format="json",
+            )
+
+        assert response.status_code == 200
+        access.refresh_from_db()
+        assert access.role == new_override_role
+    else:
+        response = client.put(
+            f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/",
+            data={"role": new_override_role},
+            format="json",
+        )
+        assert response.status_code == 400
+        access.refresh_from_db()
+        assert access.role == override_role
+        assert response.json() == {
+            "role": [
+                "Role overrides must be greater than the inherited role: "
+                f"{parent_role}/{new_override_role}"
+            ],
+        }
+
+
 @pytest.mark.parametrize("via", VIA)
 def test_api_document_accesses_update_owner_self_root(
     via,