🚚(back) serve legacy documents from Scaleway S3

Documents are now served from the `aws/` directory in Scaleway S3.
They are served using the django-storage already in place.
As content headers cannot be set to Scaleway Edge services URLs, legacy
documents need to be renamed to their filenames. Adding a management
command to do that.

Author: wilbrdt

CHANGELOG.md

@@ -11,6 +11,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 ### Changed
 
 - Serve legacy deposited files from Scaleway S3 after AWS migration
+- Serve legacy documents from Scaleway S3 after AWS migration
 
 ## [5.10.0] - 2025-07-09
 

src/backend/marsha/core/management/commands/rename_documents.py

@@ -0,0 +1,83 @@
+"""Rename documents in Scaleway S3 for serving them with the right name."""
+
+import logging
+
+from django.conf import settings
+from django.core.management.base import BaseCommand
+
+import boto3
+
+from marsha.core.defaults import AWS_S3, AWS_STORAGE_BASE_DIRECTORY, READY
+from marsha.core.models.file import Document
+from marsha.core.storage.storage_class import file_storage
+from marsha.core.utils import time_utils
+
+
+logger = logging.getLogger(__name__)
+
+scw_credentials = {
+    "aws_access_key_id": settings.STORAGE_S3_ACCESS_KEY,
+    "aws_secret_access_key": settings.STORAGE_S3_SECRET_KEY,
+    "region_name": settings.STORAGE_S3_REGION_NAME,
+    "endpoint_url": settings.STORAGE_S3_ENDPOINT_URL,
+}
+
+# Configure medialive client
+s3_client = boto3.client("s3", **scw_credentials)
+
+
+class Command(BaseCommand):
+    """Rename documents in Scaleway S3 to their filename."""
+
+    help = "Rename documents in Scaleway S3 to their filename."
+
+    def validate_filename(self, value):
+        """Transform filename to make it valid."""
+
+        value = value.replace("/", "_")
+        value = value.replace("\\", "_")
+        value = value.lstrip(".")
+
+        return value
+
+    def handle(self, *args, **options):
+        """Execute management command."""
+
+        documents = Document.objects.filter(storage_location=AWS_S3, upload_state=READY)
+
+        for document in documents:
+            # Get the file stored on Scaleway S3 under `aws/`
+            stamp = time_utils.to_timestamp(document.uploaded_on)
+            extension = "." + document.extension if document.extension else ""
+
+            file_key_src = document.get_storage_key(
+                filename=f"{stamp}{extension}", base_dir=AWS_STORAGE_BASE_DIRECTORY
+            )
+            copy_source = {
+                "Bucket": settings.STORAGE_S3_BUCKET_NAME,
+                "Key": file_key_src,
+            }
+
+            filename = self.validate_filename(document.filename)
+
+            # Override document filename with the validated S3-compatible filename
+            if filename != document.filename:
+                document.filename = filename
+                document.save()
+
+            # Compute file key destination which should be the document filename
+            file_key_dest = document.get_storage_key(
+                filename, base_dir=AWS_STORAGE_BASE_DIRECTORY
+            )
+            if file_storage.exists(file_key_dest):
+                logger.info("Object %s already exists", file_key_dest)
+                continue
+
+            logger.info("Copying %s to %s", file_key_src, file_key_dest)
+            s3_client.copy_object(
+                Bucket=settings.STORAGE_S3_BUCKET_NAME,
+                CopySource=copy_source,
+                Key=file_key_dest,
+            )
+
+        logger.info("Finished copying!")

src/backend/marsha/core/models/file.py

@@ -4,6 +4,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from marsha.core.defaults import (
+    AWS_STORAGE_BASE_DIRECTORY,
     DELETED_STORAGE_BASE_DIRECTORY,
     DOCUMENT_STORAGE_BASE_DIRECTORY,
     PENDING,
@@ -227,6 +228,9 @@ def get_storage_key(
             passed.
         """
         base = base_dir
+        if base == AWS_STORAGE_BASE_DIRECTORY:
+            return f"{base}/{self.pk}/document/{filename}"
+
         if base == DELETED_STORAGE_BASE_DIRECTORY:
             base = f"{base}/{DOCUMENT_STORAGE_BASE_DIRECTORY}"
 

src/backend/marsha/core/serializers/file.py

@@ -1,27 +1,22 @@
 """Structure of Document related models API responses with Django Rest Framework serializers."""
 
-from datetime import timedelta
 import mimetypes
 from os.path import splitext
-from urllib.parse import quote_plus
 
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.urls import reverse
-from django.utils import timezone
 
-from botocore.signers import CloudFrontSigner
 from rest_framework import serializers
 
-from marsha.core.defaults import SCW_S3
+from marsha.core.defaults import AWS_STORAGE_BASE_DIRECTORY, SCW_S3
 from marsha.core.models import Document
 from marsha.core.serializers.base import (
     TimestampField,
     UploadableFileWithExtensionSerializerMixin,
 )
 from marsha.core.serializers.playlist import PlaylistLiteSerializer
 from marsha.core.storage.storage_class import file_storage
-from marsha.core.utils import cloudfront_utils, time_utils
 
 
 class DocumentSerializer(
@@ -117,29 +112,12 @@ def get_url(self, obj):
 
         if obj.storage_location == SCW_S3:
             file_key = obj.get_storage_key(self.get_filename(obj))
-
-            return file_storage.url(file_key)
-
-        # Default AWS fallback:
-        url = (
-            f"{settings.AWS_S3_URL_PROTOCOL}://{settings.CLOUDFRONT_DOMAIN}/{obj.pk}/document/"
-            f"{time_utils.to_timestamp(obj.uploaded_on)}{self._get_extension_string(obj)}?response"
-            f"-content-disposition={quote_plus('attachment; filename=' + self.get_filename(obj))}"
-        )
-
-        # Sign the document urls only if the functionality is activated
-        if settings.CLOUDFRONT_SIGNED_URLS_ACTIVE:
-            date_less_than = timezone.now() + timedelta(
-                seconds=settings.CLOUDFRONT_SIGNED_URLS_VALIDITY
-            )
-            cloudfront_signer = CloudFrontSigner(
-                settings.CLOUDFRONT_SIGNED_PUBLIC_KEY_ID, cloudfront_utils.rsa_signer
-            )
-            url = cloudfront_signer.generate_presigned_url(
-                url, date_less_than=date_less_than
+        else:
+            file_key = obj.get_storage_key(
+                self.get_filename(obj), base_dir=AWS_STORAGE_BASE_DIRECTORY
             )
 
-        return url
+        return file_storage.url(file_key)
 
 
 class DocumentSelectLTISerializer(serializers.ModelSerializer):

src/backend/marsha/core/tests/management_commands/test_rename_documents.py

@@ -0,0 +1,110 @@
+"""Test the ``rename_documents`` management command."""
+
+from datetime import datetime, timezone
+from unittest import mock
+
+from django.core.management import call_command
+from django.test import TestCase
+
+from botocore.stub import Stubber
+
+from marsha import settings
+from marsha.core.defaults import AWS_S3, PENDING, READY, SCW_S3
+from marsha.core.factories import DocumentFactory
+from marsha.core.management.commands import rename_documents
+from marsha.core.models.file import Document
+from marsha.core.utils import time_utils
+
+
+class RenameDocumentsTestCase(TestCase):
+    """
+    Test the ``rename_documents`` command.
+    """
+
+    @mock.patch("marsha.core.storage.storage_class.file_storage.exists")
+    def test_rename_documents(self, mock_exists):
+        """Command should rename document S3 objects to their filename."""
+
+        mock_exists.return_value = False
+
+        now = datetime(2018, 8, 8, tzinfo=timezone.utc)
+
+        with Stubber(rename_documents.s3_client) as s3_client_stubber:
+            # Generate some documents
+            # (<original filename>, <expected and cleaned>)
+            filenames = [
+                ("normal_filename.pdf", "normal_filename.pdf"),
+                ("weird\\file/name.pdf", "weird_file_name.pdf"),
+                (".hidden_file", "hidden_file"),
+            ]
+
+            documents = []
+            for filename_src, _ in filenames:
+                document = DocumentFactory(
+                    filename=filename_src,
+                    uploaded_on=now,
+                    upload_state=READY,
+                    storage_location=AWS_S3,
+                )
+                documents.append(document)
+
+            # Create mocks for copy_objects with Stubber
+            # Note: Stubber requires that its mocks are called in the exact order they
+            # were created, so we must iterate over objects.all() in the same sequence
+            for document in Document.objects.all():
+                stamp = time_utils.to_timestamp(document.uploaded_on)
+                extension = "." + document.extension if document.extension else ""
+
+                file_key_src = f"aws/{document.id}/document/{stamp}{extension}"
+
+                sanitized_filename = rename_documents.Command().validate_filename(
+                    document.filename
+                )
+                file_key_dest = f"aws/{document.id}/document/{sanitized_filename}"
+
+                expected_params = {
+                    "Bucket": settings.STORAGE_S3_BUCKET_NAME,
+                    "CopySource": {
+                        "Bucket": settings.STORAGE_S3_BUCKET_NAME,
+                        "Key": file_key_src,
+                    },
+                    "Key": file_key_dest,
+                }
+                s3_client_stubber.add_response("copy_object", {}, expected_params)
+
+            # Create some documents that should not be concerned
+            DocumentFactory(
+                upload_state=READY,
+                storage_location=SCW_S3,
+            )
+            DocumentFactory(
+                upload_state=PENDING,
+                storage_location=AWS_S3,
+            )
+
+            call_command("rename_documents")
+
+            s3_client_stubber.assert_no_pending_responses()
+
+            # Check that each document.filename has been updated with the clean
+            # S3-compatible filename
+            for document, (_, expected_filename) in zip(documents, filenames):
+                document.refresh_from_db()
+                assert document.filename == expected_filename
+
+    @mock.patch("marsha.core.storage.storage_class.file_storage.exists")
+    def test_rename_documents_file_exists(self, mock_exists):
+        """Command should not copy document if file already exists."""
+
+        mock_exists.return_value = True
+
+        now = datetime(2018, 8, 8, tzinfo=timezone.utc)
+
+        DocumentFactory(
+            filename="filename.pdf",
+            uploaded_on=now,
+        )
+
+        with Stubber(rename_documents.s3_client) as s3_client_stubber:
+            call_command("rename_documents")
+            s3_client_stubber.assert_no_pending_responses()

src/backend/marsha/core/tests/test_api_document.py

@@ -56,7 +56,7 @@ def test_api_document_fetch_student(self):
             content, {"detail": "You do not have permission to perform this action."}
         )
 
-    @override_settings(CLOUDFRONT_SIGNED_URLS_ACTIVE=False)
+    @override_settings(MEDIA_URL="https://abc.svc.edge.scw.cloud/")
     def test_api_document_fetch_instructor_on_aws(self):
         """An instructor should be able to fetch a document."""
         document = DocumentFactory(
@@ -88,9 +88,8 @@ def test_api_document_fetch_instructor_on_aws(self):
                 "is_ready_to_show": True,
                 "title": document.title,
                 "upload_state": "ready",
-                "url": "https://abc.cloudfront.net/4c51f469-f91e-4998-b438-e31ee3bd3ea6/"
-                "document/1533686400.pdf"
-                "?response-content-disposition=attachment%3B+filename%3Dfoo_bar-baz.pdf",
+                "url": "https://abc.svc.edge.scw.cloud/aws/4c51f469-f91e-4998-b438-e31ee3bd3ea6/"
+                "document/foo_bar-baz.pdf",
                 "show_download": True,
                 "playlist": {
                     "id": str(document.playlist.id),

src/backend/marsha/core/tests/views/test_public_document.py

@@ -27,6 +27,7 @@ def tearDown(self):
         super().tearDown()
         cache.clear()
 
+    @override_settings(MEDIA_URL="https://abc.svc.edge.scw.cloud/")
     def test_document_publicly_accessible_on_aws(self):
         """Validate to access to a public document."""
         document = DocumentFactory(
@@ -85,9 +86,8 @@ def test_document_publicly_accessible_on_aws(self):
                     "lti_id": "course-v1:ufr+mathematics+00001",
                 },
                 "url": (
-                    "https://abc.cloudfront.net/301b5f4f-b9f1-4a5f-897d-f8f1bf22c396"
-                    "/document/1569309880?response-content-disposition=attachment%3B"
-                    "+filename%3Dplaylist-003_document-001"
+                    "https://abc.svc.edge.scw.cloud/aws/301b5f4f-b9f1-4a5f-897d-f8f1bf22c396"
+                    "/document/playlist-003_document-001"
                 ),
             },
         )
@@ -192,6 +192,7 @@ def test_document_publicly_accessible_on_scw(self):
             "frame-ancestors trusted_domain.com *.trusted_domain.com;",
         )
 
+    @override_settings(MEDIA_URL="https://abc.svc.edge.scw.cloud/")
     def test_public_document_without_consumer_site(self):
         """Public document without consumer site should have x-frame-options header"""
         organization = OrganizationFactory()
@@ -249,9 +250,8 @@ def test_public_document_without_consumer_site(self):
                     "lti_id": None,
                 },
                 "url": (
-                    f"https://abc.cloudfront.net/{document.pk}"
-                    "/document/1569309880?response-content-disposition=attachment%3B"
-                    "+filename%3Dplaylist-003_document-001"
+                    f"https://abc.svc.edge.scw.cloud/aws/{document.pk}"
+                    "/document/playlist-003_document-001"
                 ),
             },
         )