diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index d3485f75..2abcc91e 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -164,6 +164,20 @@ class DocumentSerializer(BaseResourceSerializer): ] +class LinkDocumentSerializer(BaseResourceSerializer): + """ + Serialize link configuration for documents. + We expose it separately from document in order to simplify and secure access control. + """ + + class Meta: + model = models.Document + fields = [ + "link_role", + "link_reach", + ] + + # Suppress the warning about not implementing `create` and `update` methods # since we don't use a model and only rely on the serializer for validation # pylint: disable=abstract-method diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 367e9f16..c895278a 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -451,6 +451,24 @@ class DocumentViewSet( } ) + @decorators.action(detail=True, methods=["put"], url_path="link-configuration") + def link_configuration(self, request, *args, **kwargs): + """Update link configuration with specific rights (cf get_abilities).""" + # Check permissions first + document = self.get_object() + + # Deserialize and validate the data + serializer = serializers.LinkDocumentSerializer( + document, data=request.data, partial=True + ) + if not serializer.is_valid(): + return drf_response.Response( + serializer.errors, status=status.HTTP_400_BAD_REQUEST + ) + + serializer.save() + return drf_response.Response(serializer.data, status=status.HTTP_200_OK) + @decorators.action(detail=True, methods=["post"], url_path="attachment-upload") def attachment_upload(self, request, *args, **kwargs): """Upload a file related to a given document""" diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 1ce5dd3a..f5e1ba85 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -512,6 +512,7 @@ class Document(BaseModel): return { "attachment_upload": is_owner_or_admin or is_editor, "destroy": RoleChoices.OWNER in roles, + "link_configuration": is_owner_or_admin, "manage_accesses": is_owner_or_admin, "partial_update": is_owner_or_admin or is_editor, "retrieve": can_get, 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 new file mode 100644 index 00000000..4f8340d3 --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_link_configuration.py @@ -0,0 +1,152 @@ +"""Tests for link configuration of documents on API endpoint""" + +import pytest +from rest_framework.test import APIClient + +from core import factories, models +from core.api import serializers +from core.tests.conftest import TEAM, USER, VIA + +pytestmark = pytest.mark.django_db + + +@pytest.mark.parametrize("role", models.LinkRoleChoices.values) +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +def test_api_documents_link_configuration_update_anonymous(reach, role): + """Anonymous users should not be allowed to update a link configuration.""" + document = factories.DocumentFactory(link_reach=reach, link_role=role) + old_document_values = serializers.LinkDocumentSerializer(instance=document).data + + new_document_values = serializers.LinkDocumentSerializer( + instance=factories.DocumentFactory() + ).data + response = APIClient().put( + f"/api/v1.0/documents/{document.id!s}/link-configuration/", + new_document_values, + format="json", + ) + assert response.status_code == 401 + assert response.json() == { + "detail": "Authentication credentials were not provided." + } + + document.refresh_from_db() + document_values = serializers.LinkDocumentSerializer(instance=document).data + assert document_values == old_document_values + + +@pytest.mark.parametrize("role", models.LinkRoleChoices.values) +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +def test_api_documents_link_configuration_update_authenticated_unrelated(reach, role): + """ + Authenticated users should not be allowed to update the link configuration for + a document to which they are not related. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(link_reach=reach, link_role=role) + old_document_values = serializers.LinkDocumentSerializer(instance=document).data + + new_document_values = serializers.LinkDocumentSerializer( + instance=factories.DocumentFactory() + ).data + response = client.put( + f"/api/v1.0/documents/{document.id!s}/link-configuration/", + new_document_values, + format="json", + ) + + assert response.status_code == 403 + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + + document.refresh_from_db() + document_values = serializers.LinkDocumentSerializer(instance=document).data + assert document_values == old_document_values + + +@pytest.mark.parametrize("role", ["editor", "reader"]) +@pytest.mark.parametrize("via", VIA) +def test_api_documents_link_configuration_update_authenticated_related_forbidden( + via, role, mock_user_teams +): + """ + Users who are readers or editors of a document should not be allowed to update + the link configuration. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory() + if via == USER: + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=role + ) + + old_document_values = serializers.LinkDocumentSerializer(instance=document).data + + new_document_values = serializers.LinkDocumentSerializer( + instance=factories.DocumentFactory() + ).data + response = client.put( + f"/api/v1.0/documents/{document.id!s}/link-configuration/", + new_document_values, + format="json", + ) + + assert response.status_code == 403 + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + + document.refresh_from_db() + document_values = serializers.LinkDocumentSerializer(instance=document).data + assert document_values == old_document_values + + +@pytest.mark.parametrize("role", ["administrator", "owner"]) +@pytest.mark.parametrize("via", VIA) +def test_api_documents_link_configuration_update_authenticated_related_success( + via, role, mock_user_teams +): + """ + A user who is administrator or owner of a document should be allowed to update + the link configuration. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory() + if via == USER: + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=role + ) + + new_document_values = serializers.LinkDocumentSerializer( + instance=factories.DocumentFactory() + ).data + response = client.put( + f"/api/v1.0/documents/{document.id!s}/link-configuration/", + new_document_values, + format="json", + ) + assert response.status_code == 200 + + document = models.Document.objects.get(pk=document.pk) + document_values = serializers.LinkDocumentSerializer(instance=document).data + for key, value in document_values.items(): + assert value == new_document_values[key] diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index 85d22fff..f37339f6 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -23,6 +23,7 @@ def test_api_documents_retrieve_anonymous_public(): "abilities": { "attachment_upload": document.link_role == "editor", "destroy": False, + "link_configuration": False, "manage_accesses": False, "partial_update": document.link_role == "editor", "retrieve": True, @@ -75,6 +76,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "id": str(document.id), "abilities": { "attachment_upload": document.link_role == "editor", + "link_configuration": False, "destroy": False, "manage_accesses": False, "partial_update": document.link_role == "editor", diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 0f536e57..fe81036f 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -78,6 +78,7 @@ def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role) abilities = document.get_abilities(user) assert abilities == { "attachment_upload": False, + "link_configuration": False, "destroy": False, "manage_accesses": False, "partial_update": False, @@ -108,6 +109,7 @@ def test_models_documents_get_abilities_reader(is_authenticated, reach): assert abilities == { "attachment_upload": False, "destroy": False, + "link_configuration": False, "manage_accesses": False, "partial_update": False, "retrieve": True, @@ -137,6 +139,7 @@ def test_models_documents_get_abilities_editor(is_authenticated, reach): assert abilities == { "attachment_upload": True, "destroy": False, + "link_configuration": False, "manage_accesses": False, "partial_update": True, "retrieve": True, @@ -155,6 +158,7 @@ def test_models_documents_get_abilities_owner(): assert abilities == { "attachment_upload": True, "destroy": True, + "link_configuration": True, "manage_accesses": True, "partial_update": True, "retrieve": True, @@ -172,6 +176,7 @@ def test_models_documents_get_abilities_administrator(): assert abilities == { "attachment_upload": True, "destroy": False, + "link_configuration": True, "manage_accesses": True, "partial_update": True, "retrieve": True, @@ -192,6 +197,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): assert abilities == { "attachment_upload": True, "destroy": False, + "link_configuration": False, "manage_accesses": False, "partial_update": True, "retrieve": True, @@ -214,6 +220,7 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries): assert abilities == { "attachment_upload": False, "destroy": False, + "link_configuration": False, "manage_accesses": False, "partial_update": False, "retrieve": True, @@ -237,6 +244,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): assert abilities == { "attachment_upload": False, "destroy": False, + "link_configuration": False, "manage_accesses": False, "partial_update": False, "retrieve": True,