diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 63a9f44b..793fa886 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -1,5 +1,8 @@ """Client serializers for the impress core app.""" +import mimetypes + +from django.conf import settings from django.db.models import Q from django.utils.translation import gettext_lazy as _ @@ -153,6 +156,34 @@ class DocumentSerializer(BaseResourceSerializer): read_only_fields = ["id", "accesses", "abilities", "created_at", "updated_at"] +# 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 +class FileUploadSerializer(serializers.Serializer): + """Receive file upload requests.""" + + file = serializers.FileField() + + def validate_file(self, file): + """Add file size and type constraints as defined in settings.""" + # Validate file size + if file.size > settings.DOCUMENT_IMAGE_MAX_SIZE: + max_size = settings.DOCUMENT_IMAGE_MAX_SIZE // (1024 * 1024) + raise serializers.ValidationError( + f"File size exceeds the maximum limit of {max_size:d} MB." + ) + + # Validate file type + mime_type, _ = mimetypes.guess_type(file.name) + if mime_type not in settings.DOCUMENT_IMAGE_ALLOWED_MIME_TYPES: + mime_types = ", ".join(settings.DOCUMENT_IMAGE_ALLOWED_MIME_TYPES) + raise serializers.ValidationError( + f"File type '{mime_type:s}' is not allowed. Allowed types are: {mime_types:s}" + ) + + return file + + class TemplateSerializer(BaseResourceSerializer): """Serialize templates.""" diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 04d860b3..9248c270 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1,6 +1,11 @@ """API endpoints""" +import os +import uuid + +from django.conf import settings from django.contrib.postgres.aggregates import ArrayAgg +from django.core.files.storage import default_storage from django.db.models import ( OuterRef, Q, @@ -29,6 +34,8 @@ from . import permissions, serializers # pylint: disable=too-many-ancestors +ATTACHMENTS_FOLDER = "attachments" + class NestedGenericViewSet(viewsets.GenericViewSet): """ @@ -265,7 +272,7 @@ class ResourceAccessViewsetMixin: ): return drf_response.Response( {"detail": "Cannot delete the last owner access for the resource."}, - status=403, + status=status.HTTP_403_FORBIDDEN, ) return super().destroy(request, *args, **kwargs) @@ -390,6 +397,31 @@ class DocumentViewSet( } ) + @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""" + # Check permissions first + document = self.get_object() + + # Validate metadata in payload + serializer = serializers.FileUploadSerializer(data=request.data) + if not serializer.is_valid(): + return drf_response.Response( + serializer.errors, status=status.HTTP_400_BAD_REQUEST + ) + # Extract the file extension from the original filename + file = serializer.validated_data["file"] + extension = os.path.splitext(file.name)[1] + + # Generate a generic yet unique filename to store the image in object storage + file_id = uuid.uuid4() + key = f"{document.key_base}/{ATTACHMENTS_FOLDER:s}/{file_id!s}{extension:s}" + + default_storage.save(key, file) + return drf_response.Response( + {"file": f"{settings.MEDIA_URL:s}{key:s}"}, status=status.HTTP_201_CREATED + ) + class DocumentAccessViewSet( ResourceAccessViewsetMixin, diff --git a/src/backend/core/models.py b/src/backend/core/models.py index d00b66de..a818b21b 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -36,18 +36,19 @@ logger = getLogger(__name__) def get_resource_roles(resource, user): """Compute the roles a user has on a resource.""" - roles = [] - if user.is_authenticated: + if not user.is_authenticated: + return [] + + try: + roles = resource.user_roles or [] + except AttributeError: + teams = user.get_teams() try: - roles = resource.user_roles or [] - except AttributeError: - teams = user.get_teams() - try: - roles = resource.accesses.filter( - models.Q(user=user) | models.Q(team__in=teams), - ).values_list("role", flat=True) - except (models.ObjectDoesNotExist, IndexError): - roles = [] + roles = resource.accesses.filter( + models.Q(user=user) | models.Q(team__in=teams), + ).values_list("role", flat=True) + except (models.ObjectDoesNotExist, IndexError): + roles = [] return roles @@ -403,7 +404,7 @@ class Document(BaseModel): response = default_storage.connection.meta.client.list_object_versions( Bucket=default_storage.bucket_name, Prefix=self.file_key, - MaxKeys=settings.S3_VERSIONS_PAGE_SIZE, + MaxKeys=settings.DOCUMENT_VERSIONS_PAGE_SIZE, **token, ) @@ -421,7 +422,7 @@ class Document(BaseModel): if response["NextVersionIdMarker"]: return self.get_versions_slice( from_version_id=response["NextVersionIdMarker"], - page_size=settings.S3_VERSIONS_PAGE_SIZE, + page_size=settings.DOCUMENT_VERSIONS_PAGE_SIZE, from_datetime=from_datetime, ) return { @@ -433,9 +434,9 @@ class Document(BaseModel): response = default_storage.connection.meta.client.list_object_versions( Bucket=default_storage.bucket_name, Prefix=self.file_key, - MaxKeys=min(page_size, settings.S3_VERSIONS_PAGE_SIZE) + MaxKeys=min(page_size, settings.DOCUMENT_VERSIONS_PAGE_SIZE) if page_size - else settings.S3_VERSIONS_PAGE_SIZE, + else settings.DOCUMENT_VERSIONS_PAGE_SIZE, **token, ) return { @@ -475,13 +476,14 @@ class Document(BaseModel): return { "destroy": RoleChoices.OWNER in roles, + "attachment_upload": is_owner_or_admin or is_editor, + "manage_accesses": is_owner_or_admin, + "partial_update": is_owner_or_admin or is_editor, + "retrieve": can_get, + "update": is_owner_or_admin or is_editor, "versions_destroy": is_owner_or_admin, "versions_list": can_get_versions, "versions_retrieve": can_get_versions, - "manage_accesses": is_owner_or_admin, - "update": is_owner_or_admin or is_editor, - "partial_update": is_owner_or_admin or is_editor, - "retrieve": can_get, } diff --git a/src/backend/core/tests/documents/test_api_documents_attachment_upload.py b/src/backend/core/tests/documents/test_api_documents_attachment_upload.py new file mode 100644 index 00000000..b2bb9200 --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_attachment_upload.py @@ -0,0 +1,199 @@ +""" +Test file uploads API endpoint for users in impress's core app. +""" + +import re +import uuid + +from django.core.files.base import ContentFile +from django.core.files.uploadedfile import SimpleUploadedFile + +import pytest +from rest_framework.test import APIClient + +from core import factories +from core.tests.conftest import TEAM, USER, VIA + +pytestmark = pytest.mark.django_db + + +def test_api_documents_attachment_upload_anonymous(): + """Anonymous users can't upload attachments to a document.""" + document = factories.DocumentFactory() + file = SimpleUploadedFile("test_file.jpg", b"Dummy content") + + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + response = APIClient().post(url, {"file": file}, format="multipart") + + assert response.status_code == 401 + assert response.json() == { + "detail": "Authentication credentials were not provided." + } + + +def test_api_documents_attachment_upload_authenticated_public(): + """ + Users who are not related to a public document should not be allowed to upload an attachment. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(is_public=True) + file = SimpleUploadedFile("test_file.jpg", b"Dummy content") + + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + response = client.post(url, {"file": file}, format="multipart") + + assert response.status_code == 403 + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + + +def test_api_documents_attachment_upload_authenticated_private(): + """ + Users who are not related to a private document should not be able to upload an attachment. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(is_public=False) + file = SimpleUploadedFile("test_file.jpg", b"Dummy content") + + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + response = client.post(url, {"file": file}, format="multipart") + + assert response.status_code == 404 + assert response.json() == {"detail": "No Document matches the given query."} + + +@pytest.mark.parametrize("via", VIA) +def test_api_documents_attachment_upload_reader(via, mock_user_get_teams): + """ + Users who are simple readers on a document should not be allowed to upload an attachment. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory() + if via == USER: + factories.UserDocumentAccessFactory(document=document, user=user, role="reader") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role="reader" + ) + + file = SimpleUploadedFile("test_file.jpg", b"Dummy content") + + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + response = client.post(url, {"file": file}, format="multipart") + + assert response.status_code == 403 + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + + +@pytest.mark.parametrize("role", ["editor", "administrator", "owner"]) +@pytest.mark.parametrize("via", VIA) +def test_api_documents_attachment_upload_success( + via, role, mock_user_get_teams, settings +): + """ + Editors, administrators and owners of a document should be able to upload an attachment. + """ + 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_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=role + ) + + file = SimpleUploadedFile("test_file.jpg", b"Dummy content") + + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + response = client.post(url, {"file": file}, format="multipart") + + assert response.status_code == 201 + + pattern = re.compile(rf"^{settings.MEDIA_URL}{document.id!s}/attachments/(.*)\.jpg") + match = pattern.search(response.json()["file"]) + file_id = match.group(1) + + # Validate that file_id is a valid UUID + uuid.UUID(file_id) + + +def test_api_documents_attachment_upload_invalid(client): + """Attempt to upload without a file should return an explicit error.""" + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(users=[(user, "owner")]) + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + + response = client.post(url, {}, format="multipart") + + assert response.status_code == 400 + assert response.json() == {"file": ["No file was submitted."]} + + +def test_api_documents_attachment_upload_size_limit_exceeded(settings): + """The uploaded file should not exceeed the maximum size in settings.""" + settings.DOCUMENT_IMAGE_MAX_SIZE = 1048576 # 1 MB for test + + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(users=[(user, "owner")]) + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + + # Create a temporary file larger than the allowed size + content = b"a" * (1048576 + 1) + file = ContentFile(content, name="test.jpg") + + response = client.post(url, {"file": file}, format="multipart") + + assert response.status_code == 400 + assert response.json() == {"file": ["File size exceeds the maximum limit of 1 MB."]} + + +def test_api_documents_attachment_upload_type_not_allowed(settings): + """The uploaded file should be of a whitelisted type.""" + settings.DOCUMENT_IMAGE_ALLOWED_MIME_TYPES = ["image/jpeg", "image/png"] + + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(users=[(user, "owner")]) + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + + # Create a temporary file with a not allowed type (e.g., text file) + file = ContentFile(b"a" * 1048576, name="test.txt") + + response = client.post(url, {"file": file}, format="multipart") + + assert response.status_code == 400 + assert response.json() == { + "file": [ + "File type 'text/plain' is not allowed. Allowed types are: image/jpeg, image/png" + ] + } 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 2ec46605..7cea1af9 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -22,6 +22,7 @@ def test_api_documents_retrieve_anonymous_public(): "id": str(document.id), "abilities": { "destroy": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, "retrieve": True, @@ -69,6 +70,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public(): "id": str(document.id), "abilities": { "destroy": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, "retrieve": True, diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index e47ba1a2..194f3e59 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -63,10 +63,11 @@ def test_models_documents_get_abilities_anonymous_public(): abilities = document.get_abilities(AnonymousUser()) assert abilities == { "destroy": False, - "retrieve": True, - "update": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, + "retrieve": True, + "update": False, "versions_destroy": False, "versions_list": False, "versions_retrieve": False, @@ -79,10 +80,11 @@ def test_models_documents_get_abilities_anonymous_not_public(): abilities = document.get_abilities(AnonymousUser()) assert abilities == { "destroy": False, - "retrieve": False, - "update": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, + "retrieve": False, + "update": False, "versions_destroy": False, "versions_list": False, "versions_retrieve": False, @@ -95,10 +97,11 @@ def test_models_documents_get_abilities_authenticated_unrelated_public(): abilities = document.get_abilities(factories.UserFactory()) assert abilities == { "destroy": False, - "retrieve": True, - "update": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, + "retrieve": True, + "update": False, "versions_destroy": False, "versions_list": False, "versions_retrieve": False, @@ -111,10 +114,11 @@ def test_models_documents_get_abilities_authenticated_unrelated_not_public(): abilities = document.get_abilities(factories.UserFactory()) assert abilities == { "destroy": False, - "retrieve": False, - "update": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, + "retrieve": False, + "update": False, "versions_destroy": False, "versions_list": False, "versions_retrieve": False, @@ -128,10 +132,11 @@ def test_models_documents_get_abilities_owner(): abilities = access.document.get_abilities(access.user) assert abilities == { "destroy": True, - "retrieve": True, - "update": True, + "attachment_upload": True, "manage_accesses": True, "partial_update": True, + "retrieve": True, + "update": True, "versions_destroy": True, "versions_list": True, "versions_retrieve": True, @@ -144,10 +149,11 @@ def test_models_documents_get_abilities_administrator(): abilities = access.document.get_abilities(access.user) assert abilities == { "destroy": False, - "retrieve": True, - "update": True, + "attachment_upload": True, "manage_accesses": True, "partial_update": True, + "retrieve": True, + "update": True, "versions_destroy": True, "versions_list": True, "versions_retrieve": True, @@ -163,10 +169,11 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): assert abilities == { "destroy": False, - "retrieve": True, - "update": True, + "attachment_upload": True, "manage_accesses": False, "partial_update": True, + "retrieve": True, + "update": True, "versions_destroy": False, "versions_list": True, "versions_retrieve": True, @@ -182,10 +189,11 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries): assert abilities == { "destroy": False, - "retrieve": True, - "update": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, + "retrieve": True, + "update": False, "versions_destroy": False, "versions_list": True, "versions_retrieve": True, @@ -202,10 +210,11 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): assert abilities == { "destroy": False, - "retrieve": True, - "update": False, + "attachment_upload": False, "manage_accesses": False, "partial_update": False, + "retrieve": True, + "update": False, "versions_destroy": False, "versions_list": True, "versions_retrieve": True, @@ -217,7 +226,7 @@ def test_models_documents_get_versions_slice(settings): The "get_versions_slice" method should allow navigating all versions of the document with pagination. """ - settings.S3_VERSIONS_PAGE_SIZE = 4 + settings.DOCUMENT_VERSIONS_PAGE_SIZE = 4 # Create a document with 7 versions document = factories.DocumentFactory() diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index 4f5ec723..ab9c0b4d 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -138,7 +138,24 @@ class Base(Configuration): environ_prefix=None, ) - S3_VERSIONS_PAGE_SIZE = 50 + # Document images + DOCUMENT_IMAGE_MAX_SIZE = values.Value( + 10 * (2**20), # 10MB + environ_name="DOCUMENT_IMAGE_MAX_SIZE", + environ_prefix=None, + ) + DOCUMENT_IMAGE_ALLOWED_MIME_TYPES = [ + "image/bmp", + "image/gif", + "image/jpeg", + "image/png", + "image/svg+xml", + "image/tiff", + "image/webp", + ] + + # Document versions + DOCUMENT_VERSIONS_PAGE_SIZE = 50 # Internationalization # https://docs.djangoproject.com/en/3.1/topics/i18n/