From f1b398e1ae64179598b97e6c0ae69f5bd8c6bdec Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Wed, 21 May 2025 14:22:31 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(back)=20add=20endpoint=20checking=20m?= =?UTF-8?q?edia=20status?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the usage of a malware detection system, we need a way to know the file status. The front will use it to display a loader while the analyse is not ended. --- CHANGELOG.md | 5 +- src/backend/core/api/viewsets.py | 49 +++- src/backend/core/models.py | 1 + .../test_api_documents_media_check.py | 244 ++++++++++++++++++ .../documents/test_api_documents_retrieve.py | 7 +- .../documents/test_api_documents_trashbin.py | 1 + .../core/tests/test_models_documents.py | 8 + 7 files changed, 311 insertions(+), 4 deletions(-) create mode 100644 src/backend/core/tests/documents/test_api_documents_media_check.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b0a03a3..660deabd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,9 @@ and this project adheres to ## [Unreleased] -## Added +### Added +- ✨(back) add endpoint checking media status - ✨(backend) allow setting session cookie age via env var #977 - ✨(backend) allow theme customnization using a configuration file #948 - ✨ Add a custom callout block to the editor #892 @@ -18,7 +19,7 @@ and this project adheres to - 🩺(CI) add lint spell mistakes #954 - 🛂(frontend) block edition to not connected users #945 -## Changed +### Changed - 📝(frontend) Update documentation - ✅(frontend) Improve tests coverage diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 578c49d6..f8b6a56e 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1279,7 +1279,10 @@ class DocumentViewSet( # Check if the attachment is ready s3_client = default_storage.connection.meta.client bucket_name = default_storage.bucket_name - head_resp = s3_client.head_object(Bucket=bucket_name, Key=key) + try: + head_resp = s3_client.head_object(Bucket=bucket_name, Key=key) + except ClientError as err: + raise drf.exceptions.PermissionDenied() from err metadata = head_resp.get("Metadata", {}) # In order to be compatible with existing upload without `status` metadata, # we consider them as ready. @@ -1294,6 +1297,50 @@ class DocumentViewSet( return drf.response.Response("authorized", headers=request.headers, status=200) + @drf.decorators.action(detail=True, methods=["get"], url_path="media-check") + def media_check(self, request, *args, **kwargs): + """ + Check if the media is ready to be served. + """ + document = self.get_object() + + key = request.query_params.get("key") + if not key: + return drf.response.Response( + {"detail": "Missing 'key' query parameter"}, + status=drf.status.HTTP_400_BAD_REQUEST, + ) + + if key not in document.attachments: + return drf.response.Response( + {"detail": "Attachment missing"}, + status=drf.status.HTTP_404_NOT_FOUND, + ) + + # Check if the attachment is ready + s3_client = default_storage.connection.meta.client + bucket_name = default_storage.bucket_name + try: + head_resp = s3_client.head_object(Bucket=bucket_name, Key=key) + except ClientError as err: + logger.error("Client Error fetching file %s metadata: %s", key, err) + return drf.response.Response( + {"detail": "Media not found"}, + status=drf.status.HTTP_404_NOT_FOUND, + ) + metadata = head_resp.get("Metadata", {}) + + body = { + "status": metadata.get("status", enums.DocumentAttachmentStatus.PROCESSING), + } + if metadata.get("status") == enums.DocumentAttachmentStatus.READY: + body = { + "status": enums.DocumentAttachmentStatus.READY, + "file": f"{settings.MEDIA_URL:s}{key:s}", + } + + return drf.response.Response(body, status=drf.status.HTTP_200_OK) + @drf.decorators.action( detail=True, methods=["post"], diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 2c5239ea..e9880f52 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -835,6 +835,7 @@ class Document(MP_Node, BaseModel): "ai_transform": ai_access, "ai_translate": ai_access, "attachment_upload": can_update, + "media_check": can_get, "children_list": can_get, "children_create": can_update and user.is_authenticated, "collaboration_auth": can_get, diff --git a/src/backend/core/tests/documents/test_api_documents_media_check.py b/src/backend/core/tests/documents/test_api_documents_media_check.py new file mode 100644 index 00000000..81cec061 --- /dev/null +++ b/src/backend/core/tests/documents/test_api_documents_media_check.py @@ -0,0 +1,244 @@ +"""Test the "media_check" endpoint.""" + +from io import BytesIO +from uuid import uuid4 + +from django.core.files.storage import default_storage + +import pytest +from rest_framework.test import APIClient + +from core import factories +from core.enums import DocumentAttachmentStatus +from core.tests.conftest import TEAM, USER, VIA + +pytestmark = pytest.mark.django_db + + +def test_api_documents_media_check_unknown_document(): + """ + The "media_check" endpoint should return a 404 error if the document does not exist. + """ + client = APIClient() + response = client.get(f"/api/v1.0/documents/{uuid4()!s}media-check/") + assert response.status_code == 404 + + +def test_api_documents_media_check_missing_key(): + """ + The "media_check" endpoint should return a 404 error if the key is missing. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user=user) + + document = factories.DocumentFactory(users=[user]) + + response = client.get(f"/api/v1.0/documents/{document.id!s}/media-check/") + assert response.status_code == 400 + assert response.json() == {"detail": "Missing 'key' query parameter"} + + +def test_api_documents_media_check_key_parameter_not_related_to_document(): + """ + The "media_check" endpoint should return a 404 error if the key is not related to the document. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user=user) + + document = factories.DocumentFactory(users=[user]) + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/media-check/", + {"key": f"{document.id!s}/attachments/unknown.jpg"}, + ) + assert response.status_code == 404 + assert response.json() == {"detail": "Attachment missing"} + + +def test_api_documents_media_check_anonymous_public_document(): + """ + The "media_check" endpoint should return a 200 status code if the document is public. + """ + document = factories.DocumentFactory(link_reach="public") + + filename = f"{uuid4()!s}.jpg" + key = f"{document.id!s}/attachments/{filename:s}" + default_storage.connection.meta.client.put_object( + Bucket=default_storage.bucket_name, + Key=key, + Body=BytesIO(b"my prose"), + ContentType="text/plain", + Metadata={"status": DocumentAttachmentStatus.PROCESSING}, + ) + document.attachments = [key] + document.save(update_fields=["attachments"]) + + client = APIClient() + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key} + ) + assert response.status_code == 200 + assert response.json() == {"status": DocumentAttachmentStatus.PROCESSING} + + +def test_api_documents_media_check_anonymous_public_document_ready(): + """ + The "media_check" endpoint should return a 200 status code if the document is public. + """ + document = factories.DocumentFactory(link_reach="public") + + filename = f"{uuid4()!s}.jpg" + key = f"{document.id!s}/attachments/{filename:s}" + default_storage.connection.meta.client.put_object( + Bucket=default_storage.bucket_name, + Key=key, + Body=BytesIO(b"my prose"), + ContentType="text/plain", + Metadata={"status": DocumentAttachmentStatus.READY}, + ) + document.attachments = [key] + document.save(update_fields=["attachments"]) + + client = APIClient() + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key} + ) + assert response.status_code == 200 + assert response.json() == { + "status": DocumentAttachmentStatus.READY, + "file": f"/media/{key:s}", + } + + +@pytest.mark.parametrize("link_reach", ["restricted", "authenticated"]) +def test_api_documents_media_check_anonymous_non_public_document(link_reach): + """ + The "media_check" endpoint should return a 403 error if the document is not public. + """ + document = factories.DocumentFactory(link_reach=link_reach) + + client = APIClient() + + response = client.get(f"/api/v1.0/documents/{document.id!s}/media-check/") + assert response.status_code == 401 + + +def test_api_documents_media_check_connected_document(): + """ + The "media_check" endpoint should return a 200 status code for a user connected + checking for a document with link_reach authenticated. + """ + document = factories.DocumentFactory(link_reach="authenticated") + + filename = f"{uuid4()!s}.jpg" + key = f"{document.id!s}/attachments/{filename:s}" + default_storage.connection.meta.client.put_object( + Bucket=default_storage.bucket_name, + Key=key, + Body=BytesIO(b"my prose"), + ContentType="text/plain", + Metadata={"status": DocumentAttachmentStatus.READY}, + ) + document.attachments = [key] + document.save(update_fields=["attachments"]) + + user = factories.UserFactory() + client = APIClient() + client.force_login(user=user) + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key} + ) + assert response.status_code == 200 + assert response.json() == { + "status": DocumentAttachmentStatus.READY, + "file": f"/media/{key:s}", + } + + +def test_api_documents_media_check_connected_document_media_not_related(): + """ + The "media_check" endpoint should return a 404 error if the key is not related to the document. + """ + document = factories.DocumentFactory(link_reach="authenticated") + + filename = f"{uuid4()!s}.jpg" + key = f"{document.id!s}/attachments/{filename:s}" + + user = factories.UserFactory() + client = APIClient() + client.force_login(user=user) + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key} + ) + assert response.status_code == 404 + assert response.json() == {"detail": "Attachment missing"} + + +def test_api_documents_media_check_media_missing_on_storage(): + """ + The "media_check" endpoint should return a 404 error if the media is missing on storage. + """ + document = factories.DocumentFactory(link_reach="authenticated") + + filename = f"{uuid4()!s}.jpg" + key = f"{document.id!s}/attachments/{filename:s}" + + document.attachments = [key] + document.save(update_fields=["attachments"]) + + user = factories.UserFactory() + client = APIClient() + client.force_login(user=user) + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key} + ) + assert response.status_code == 404 + assert response.json() == {"detail": "Media not found"} + + +@pytest.mark.parametrize("via", VIA) +def test_api_documents_media_check_restricted_document(via, mock_user_teams): + """ + The "media_check" endpoint should return a 200 status code if the document is restricted and + the user has access to it. + """ + document = factories.DocumentFactory(link_reach="restricted") + filename = f"{uuid4()!s}.jpg" + key = f"{document.id!s}/attachments/{filename:s}" + default_storage.connection.meta.client.put_object( + Bucket=default_storage.bucket_name, + Key=key, + Body=BytesIO(b"my prose"), + ContentType="text/plain", + Metadata={"status": DocumentAttachmentStatus.READY}, + ) + document.attachments = [key] + document.save(update_fields=["attachments"]) + + user = factories.UserFactory() + client = APIClient() + client.force_login(user=user) + + if via == USER: + factories.UserDocumentAccessFactory(document=document, user=user) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory(document=document, team="lasuite") + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/media-check/", {"key": key} + ) + assert response.status_code == 200 + assert response.json() == { + "status": DocumentAttachmentStatus.READY, + "file": f"/media/{key:s}", + } 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 38d66cd4..91e6ca0e 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -48,6 +48,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, "partial_update": document.link_role == "editor", "restore": False, @@ -111,6 +112,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "link_configuration": False, "link_select_options": models.LinkReachChoices.get_select_options(links), "media_auth": True, + "media_check": True, "move": False, "partial_update": grand_parent.link_role == "editor", "restore": False, @@ -210,6 +212,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, "partial_update": document.link_role == "editor", "restore": False, @@ -279,8 +282,9 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "invite_owner": False, "link_configuration": False, "link_select_options": models.LinkReachChoices.get_select_options(links), - "move": False, "media_auth": True, + "media_check": True, + "move": False, "partial_update": grand_parent.link_role == "editor", "restore": False, "retrieve": True, @@ -460,6 +464,7 @@ def test_api_documents_retrieve_authenticated_related_parent(): "link_configuration": access.role in ["administrator", "owner"], "link_select_options": models.LinkReachChoices.get_select_options(links), "media_auth": True, + "media_check": True, "move": access.role in ["administrator", "owner"], "partial_update": access.role != "reader", "restore": access.role == "owner", diff --git a/src/backend/core/tests/documents/test_api_documents_trashbin.py b/src/backend/core/tests/documents/test_api_documents_trashbin.py index 6db898ea..4e4eb276 100644 --- a/src/backend/core/tests/documents/test_api_documents_trashbin.py +++ b/src/backend/core/tests/documents/test_api_documents_trashbin.py @@ -91,6 +91,7 @@ def test_api_documents_trashbin_format(): "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, # Can't move a deleted document "partial_update": True, "restore": True, diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 6599b737..01d5181e 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -165,6 +165,7 @@ def test_models_documents_get_abilities_forbidden( "favorite": False, "invite_owner": False, "media_auth": False, + "media_check": False, "move": False, "link_configuration": False, "link_select_options": { @@ -231,6 +232,7 @@ def test_models_documents_get_abilities_reader( "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, "partial_update": False, "restore": False, @@ -293,6 +295,7 @@ def test_models_documents_get_abilities_editor( "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, "partial_update": True, "restore": False, @@ -344,6 +347,7 @@ def test_models_documents_get_abilities_owner(django_assert_num_queries): "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": True, "partial_update": True, "restore": True, @@ -392,6 +396,7 @@ def test_models_documents_get_abilities_administrator(django_assert_num_queries) "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": True, "partial_update": True, "restore": False, @@ -443,6 +448,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, "partial_update": True, "restore": False, @@ -501,6 +507,7 @@ def test_models_documents_get_abilities_reader_user( "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, "partial_update": access_from_link, "restore": False, @@ -557,6 +564,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries): "restricted": ["reader", "editor"], }, "media_auth": True, + "media_check": True, "move": False, "partial_update": False, "restore": False,