🥅(backend) link role could be updated when restricted document

AntoLC · AntoLC · commit 1ed01fd64b28 · 2025-09-18T12:17:08.000+02:00
When a document was restricted, the link role could
be updated from "link-configuration" and gives a
200 response, but the change did not
have any effect because of a restriction in
LinkReachChoices.
We added a validation step to ensure that the
link role can only be updated if the document
is not restricted.
diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py
@@ -506,13 +506,69 @@ class LinkDocumentSerializer(serializers.ModelSerializer):
     We expose it separately from document in order to simplify and secure access control.
     """
 
+    link_reach = serializers.ChoiceField(
+        choices=models.LinkReachChoices.choices, required=True
+    )
+
     class Meta:
         model = models.Document
         fields = [
             "link_role",
             "link_reach",
         ]
 
+    def validate(self, attrs):
+        """Validate that link_role and link_reach are compatible using get_select_options."""
+        link_reach = attrs.get("link_reach")
+        link_role = attrs.get("link_role")
+
+        if not link_reach:
+            raise serializers.ValidationError(
+                {"link_reach": _("This field is required.")}
+            )
+
+        # Get available options based on ancestors' link definition
+        available_options = models.LinkReachChoices.get_select_options(
+            **self.instance.ancestors_link_definition
+        )
+
+        # Validate link_reach is allowed
+        if link_reach not in available_options:
+            msg = _(
+                "Link reach '%(link_reach)s' is not allowed based on parent document configuration."
+            )
+            raise serializers.ValidationError(
+                {"link_reach": msg % {"link_reach": link_reach}}
+            )
+
+        # Validate link_role is compatible with link_reach
+        allowed_roles = available_options[link_reach]
+
+        # Restricted reach: link_role must be None
+        if link_reach == models.LinkReachChoices.RESTRICTED:
+            if link_role is not None:
+                raise serializers.ValidationError(
+                    {
+                        "link_role": (
+                            "Cannot set link_role when link_reach is 'restricted'. "
+                            "Link role must be null for restricted reach."
+                        )
+                    }
+                )
+            return attrs
+        # Non-restricted: link_role must be in allowed roles
+        if link_role not in allowed_roles:
+            allowed_roles_str = ", ".join(allowed_roles) if allowed_roles else "none"
+            raise serializers.ValidationError(
+                {
+                    "link_role": (
+                        f"Link role '{link_role}' is not allowed for link reach '{link_reach}'. "
+                        f"Allowed roles: {allowed_roles_str}"
+                    )
+                }
+            )
+        return attrs
+
 
 class DocumentDuplicationSerializer(serializers.Serializer):
     """
diff --git a/src/backend/core/tests/documents/test_api_documents_link_configuration.py b/src/backend/core/tests/documents/test_api_documents_link_configuration.py
@@ -133,7 +133,10 @@ def test_api_documents_link_configuration_update_authenticated_related_success(
     client = APIClient()
     client.force_login(user)
 
-    document = factories.DocumentFactory()
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.AUTHENTICATED,
+        link_role=models.LinkRoleChoices.READER,
+    )
     if via == USER:
         factories.UserDocumentAccessFactory(document=document, user=user, role=role)
     elif via == TEAM:
@@ -143,7 +146,10 @@ def test_api_documents_link_configuration_update_authenticated_related_success(
         )
 
     new_document_values = serializers.LinkDocumentSerializer(
-        instance=factories.DocumentFactory()
+        instance=factories.DocumentFactory(
+            link_reach=models.LinkReachChoices.PUBLIC,
+            link_role=models.LinkRoleChoices.EDITOR,
+        )
     ).data
 
     with mock_reset_connections(document.id):
@@ -158,3 +164,240 @@ def test_api_documents_link_configuration_update_authenticated_related_success(
         document_values = serializers.LinkDocumentSerializer(instance=document).data
         for key, value in document_values.items():
             assert value == new_document_values[key]
+
+
+def test_api_documents_link_configuration_update_role_restricted_forbidden():
+    """
+    Test that trying to set link_role on a document with restricted link_reach
+    returns a validation error.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.RESTRICTED,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Try to set a meaningful role on a restricted document
+    new_data = {
+        "link_reach": models.LinkReachChoices.RESTRICTED,
+        "link_role": models.LinkRoleChoices.EDITOR,
+    }
+
+    response = client.put(
+        f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_role" in response.json()
+    assert (
+        "Cannot set link_role when link_reach is 'restricted'"
+        in response.json()["link_role"][0]
+    )
+
+
+def test_api_documents_link_configuration_update_link_reach_required():
+    """
+    Test that link_reach is required when updating link configuration.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Try to update without providing link_reach
+    new_data = {"link_role": models.LinkRoleChoices.EDITOR}
+
+    response = client.put(
+        f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_reach" in response.json()
+    assert "This field is required" in response.json()["link_reach"][0]
+
+
+def test_api_documents_link_configuration_update_restricted_without_role_success(
+    mock_reset_connections,  # pylint: disable=redefined-outer-name
+):
+    """
+    Test that setting link_reach to restricted without specifying link_role succeeds.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Only specify link_reach, not link_role
+    new_data = {
+        "link_reach": models.LinkReachChoices.RESTRICTED,
+    }
+
+    with mock_reset_connections(document.id):
+        response = client.put(
+            f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+            new_data,
+            format="json",
+        )
+
+        assert response.status_code == 200
+        document.refresh_from_db()
+        assert document.link_reach == models.LinkReachChoices.RESTRICTED
+
+
+@pytest.mark.parametrize(
+    "reach", [models.LinkReachChoices.PUBLIC, models.LinkReachChoices.AUTHENTICATED]
+)
+@pytest.mark.parametrize("role", models.LinkRoleChoices.values)
+def test_api_documents_link_configuration_update_non_restricted_with_valid_role_success(
+    reach,
+    role,
+    mock_reset_connections,  # pylint: disable=redefined-outer-name
+):
+    """
+    Test that setting non-restricted link_reach with valid link_role succeeds.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.RESTRICTED,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    new_data = {
+        "link_reach": reach,
+        "link_role": role,
+    }
+
+    with mock_reset_connections(document.id):
+        response = client.put(
+            f"/api/v1.0/documents/{document.id!s}/link-configuration/",
+            new_data,
+            format="json",
+        )
+
+        assert response.status_code == 200
+        document.refresh_from_db()
+        assert document.link_reach == reach
+        assert document.link_role == role
+
+
+def test_api_documents_link_configuration_update_with_ancestor_constraints():
+    """
+    Test that link configuration respects ancestor constraints using get_select_options.
+    This test may need adjustment based on the actual get_select_options implementation.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    parent_document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    child_document = factories.DocumentFactory(
+        parent=parent_document,
+        link_reach=models.LinkReachChoices.PUBLIC,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=child_document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    # Try to set child to PUBLIC when parent is RESTRICTED
+    new_data = {
+        "link_reach": models.LinkReachChoices.RESTRICTED,
+        "link_role": models.LinkRoleChoices.READER,
+    }
+
+    response = client.put(
+        f"/api/v1.0/documents/{child_document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_reach" in response.json()
+    assert (
+        "Link reach 'restricted' is not allowed based on parent"
+        in response.json()["link_reach"][0]
+    )
+
+
+def test_api_documents_link_configuration_update_invalid_role_for_reach_validation():
+    """
+    Test the specific validation logic that checks if link_role is allowed for link_reach.
+    This tests the code section that validates allowed_roles from get_select_options.
+    """
+    user = factories.UserFactory()
+    client = APIClient()
+    client.force_login(user)
+
+    parent_document = factories.DocumentFactory(
+        link_reach=models.LinkReachChoices.AUTHENTICATED,
+        link_role=models.LinkRoleChoices.EDITOR,
+    )
+
+    child_document = factories.DocumentFactory(
+        parent=parent_document,
+        link_reach=models.LinkReachChoices.RESTRICTED,
+        link_role=models.LinkRoleChoices.READER,
+    )
+
+    factories.UserDocumentAccessFactory(
+        document=child_document, user=user, role=models.RoleChoices.OWNER
+    )
+
+    new_data = {
+        "link_reach": models.LinkReachChoices.AUTHENTICATED,
+        "link_role": models.LinkRoleChoices.READER,  # This should be rejected
+    }
+
+    response = client.put(
+        f"/api/v1.0/documents/{child_document.id!s}/link-configuration/",
+        new_data,
+        format="json",
+    )
+
+    assert response.status_code == 400
+    assert "link_role" in response.json()
+    error_message = response.json()["link_role"][0]
+    assert (
+        "Link role 'reader' is not allowed for link reach 'authenticated'"
+        in error_message
+    )
+    assert "Allowed roles: editor" in error_message
