🚚(backend) store classroom documents in S3 using filenames as keys

Refactored S3 key generation for classroom documents: instead of using
timestamp-based keys, files are now stored with their original filenames
to support proper download behavior via Scaleway Edge Service URLs
(since `Response-Content-Disposition` headers cannot be set).

Author: wilbrdt

env.d/development.dist

@@ -15,6 +15,9 @@ DJANGO_AWS_ACCESS_KEY_ID=yourAwsAccesKey
 DJANGO_AWS_SECRET_ACCESS_KEY=YourAwsSecretAccessKeyId
 DJANGO_AWS_S3_REGION_NAME=eu-west-1
 
+# Scaleway
+DJANGO_SCW_EDGE_SERVICE_DOMAIN=yourEdgeServiceDomain
+
 # STORAGE_S3
 DJANGO_STORAGE_S3_ACCESS_KEY=yourS3AccesKey
 DJANGO_STORAGE_S3_SECRET_KEY=yourS3AccesKey

src/backend/marsha/bbb/api.py

@@ -622,10 +622,17 @@ def initiate_upload(self, request, pk=None, classroom_id=None):
         if serializer.is_valid() is not True:
             return Response(serializer.errors, status=400)
 
+        filename = serializer.validated_data["filename"]
+        extension = serializer.validated_data["extension"]
+
+        if not filename.endswith(extension):
+            filename = f"{filename}{extension}"
+
         presigned_post = (
             storage.get_initiate_backend().initiate_classroom_document_storage_upload(
                 request,
                 classroom_document,
+                filename,
                 [
                     ["eq", "$Content-Type", serializer.validated_data["mimetype"]],
                     [
@@ -674,10 +681,8 @@ def upload_ended(self, request, pk=None, classroom_id=None):
         )
         serializer.is_valid(raise_exception=True)
 
-        file_key = serializer.validated_data["file_key"]
-        # The file_key have the "classroom/{classroom_pk}/classroomdocument/{stamp}"
-        # format
-        stamp = file_key.split("/")[-1]
+        now = timezone.now()
+        stamp = to_timestamp(now)
 
         classroom_document.update_upload_state(READY, to_datetime(stamp))
 

src/backend/marsha/bbb/models.py

@@ -352,68 +352,33 @@ class Meta:
         verbose_name = _("Classroom document")
         verbose_name_plural = _("Classroom documents")
 
-    def get_source_s3_key(self, stamp=None, extension=None):
-        """Compute the S3 key in the source bucket.
-        It is built from the classroom ID + ID of the classroom document + version stamp.
-        Parameters
-        ----------
-        stamp: Type[string]
-            Passing a value for this argument will return the source S3 key for the classroom
-            document assuming its active stamp is set to this value. This is useful to create an
-            upload policy for this prospective version of the track, so that the client can
-            upload the file to S3 and the confirmation lambda can set the `uploaded_on` field
-            to this value only after the file upload and processing is successful.
-        extension: Type[string]
-            The extension used by the uploaded media. This extension is added at the end of the key
-            to keep a record of the extension. We will use it in the update-state endpoint to
-            record it in the database.
-        Returns
-        -------
-        string
-            The S3 key for the classroom document in the source bucket, where uploaded files are
-            stored before they are converted and copied to the destination bucket.
-        """
-        # We don't want to deal with None value, so we set it with an empty string
-        extension = extension or ""
-
-        # We check if the extension starts with a leading dot or not. If it's not the case we add
-        # it at the beginning of the string
-        if extension and not extension.startswith("."):
-            extension = "." + extension
-
-        stamp = stamp or to_timestamp(self.uploaded_on)
-        return f"{self.classroom.pk}/classroomdocument/{self.pk}/{stamp}{extension}"
-
-    def get_storage_prefix(
+    def get_storage_key(
         self,
-        stamp=None,
+        filename,
         base_dir: STORAGE_BASE_DIRECTORY = CLASSROOM_STORAGE_BASE_DIRECTORY,
     ):
-        """Compute the storage prefix for the classroom document.
+        """Compute the storage key for the classroom document.
 
         Parameters
         ----------
-        stamp: Type[string]
-            Passing a value for this argument will return the storage prefix for the
-            classroom document assuming its active stamp is set to this value. This is
-            useful to create an upload policy for this prospective version of the
-            classroom, so that the client can upload the file to S3.
-
+        filename: Type[string]
+            The filename of the uploaded media. For classroom documents, the filename is
+            directly set into the key.
         base: Type[STORAGE_BASE_DIRECTORY]
             The storage base directory. Defaults to Classroom. It will be used to
-            compute the storage prefix.
+            compute the storage key.
 
         Returns
         -------
         string
-            The storage prefix for the classroom document, depending on the base directory passed.
+            The storage key for the classroom document, depending on the base directory
+            passed.
         """
-        stamp = stamp or self.uploaded_on_stamp()
         base = base_dir
         if base == DELETED_STORAGE_BASE_DIRECTORY:
             base = f"{base}/{CLASSROOM_STORAGE_BASE_DIRECTORY}"
 
-        return f"{base}/{self.classroom.pk}/classroomdocument/{self.pk}/{stamp}"
+        return f"{base}/{self.classroom.pk}/classroomdocument/{self.pk}/{filename}"
 
 
 class ClassroomRecording(BaseModel):

src/backend/marsha/bbb/serializers.py

@@ -22,19 +22,15 @@
     ClassroomSession,
 )
 from marsha.bbb.utils.bbb_utils import get_recording_url, get_url as get_document_url
-from marsha.core.defaults import (
-    CLASSROOM_RECORDINGS_KEY_CACHE,
-    CLASSROOM_STORAGE_BASE_DIRECTORY,
-    VOD_CONVERT,
-)
+from marsha.core.defaults import CLASSROOM_RECORDINGS_KEY_CACHE, SCW_S3, VOD_CONVERT
 from marsha.core.serializers import (
     BaseInitiateUploadSerializer,
     PlaylistLiteSerializer,
     ReadOnlyModelSerializer,
     UploadableFileWithExtensionSerializerMixin,
     VideoFromRecordingSerializer,
 )
-from marsha.core.utils import time_utils
+from marsha.core.storage.storage_class import file_storage
 
 
 class ClassroomRecordingSerializer(ReadOnlyModelSerializer):
@@ -409,16 +405,25 @@ def get_url(self, obj):
         Returns
         -------
         String or None
-            the url to fetch the classroom document on CloudFront
+            the url to fetch the classroom document on CloudFront/Storage
             None if the classroom document is still not uploaded to S3 with success
 
         """
-        if url := get_document_url(obj):
-            return (
-                f"{url}?response-content-disposition="
-                f"{quote_plus('attachment; filename=' + obj.filename)}"
-            )
-        return None
+        if not obj.uploaded_on:
+            return None
+
+        if obj.storage_location == SCW_S3:
+            file_key = obj.get_storage_key(obj.filename)
+
+            return file_storage.url(file_key)
+
+        # Default AWS fallback
+        url = get_document_url(obj)
+
+        return (
+            f"{url}?response-content-disposition="
+            f"{quote_plus('attachment; filename=' + obj.filename)}"
+        )
 
 
 class ClassroomDocumentInitiateUploadSerializer(BaseInitiateUploadSerializer):
@@ -434,6 +439,19 @@ def max_upload_file_size(self):
         """
         return settings.CLASSROOM_DOCUMENT_SOURCE_MAX_SIZE
 
+    def validate_filename(self, value):
+        """Check if the filename is valid."""
+        if "/" in value or "\\" in value:
+            raise serializers.ValidationError("Filename must not contain slashes.")
+
+        if value.startswith("."):
+            raise serializers.ValidationError("Filename must not start with a dot.")
+
+        if len(value) > 255:
+            raise serializers.ValidationError("Filename is too long.")
+
+        return value
+
     def validate(self, attrs):
         """Validate if the mimetype is allowed or not."""
         # mimetype is provided, we directly check it
@@ -467,19 +485,13 @@ class ClassroomDocumentUploadEndedSerializer(serializers.Serializer):
 
     def validate_file_key(self, value):
         """Check if the file_key is valid."""
+        filename = value.split("/")[-1]
+        _, extension = splitext(filename)
 
-        stamp = value.split("/")[-1]
+        if not extension:
+            raise serializers.ValidationError("Filename must include an extension.")
 
-        try:
-            time_utils.to_datetime(stamp)
-        except serializers.ValidationError as error:
-            raise serializers.ValidationError("file_key is not valid") from error
-
-        if (
-            self.context["obj"].get_storage_prefix(
-                stamp, CLASSROOM_STORAGE_BASE_DIRECTORY
-            )
-            != value
-        ):
+        if self.context["obj"].get_storage_key(filename=filename) != value:
             raise serializers.ValidationError("file_key is not valid")
+
         return value