Merge pull request #247 from co-cddo/bug/DEV_231-remove-alpha-numeric…

…-characters-from-file-name

Remove non alphanumeric characters from uploaded file name.

request_a_govuk_domain/request/models/application.py

@@ -1,4 +1,5 @@
 import logging
+import re
 
 from django.contrib.auth.models import User
 from django.core.files.uploadedfile import InMemoryUploadedFile
@@ -105,6 +106,8 @@ def save(
         if S3_STORAGE_ENABLED:
             storage = select_storage()
             # Move any temporary files in to the application specific folder
+            # We have to do this custom moving because we store the initially uploaded
+            # files in the temp location.
             for file_field in [
                 self.policy_exemption_evidence,
                 self.ministerial_request_evidence,
@@ -126,7 +129,9 @@ def save(
                             Body=file_field.file.read(),
                         )
 
-                    to_path = f"applications/{self.reference}/" + file_field.name
+                    to_path = f"applications/{self.reference}/" + re.sub(
+                        r"[^A-Za-z0-9.]+", "_", file_field.name
+                    )
                     logger.info(
                         "Copying temporary file %s to application folder %s",
                         from_path,

tests/test_model_admins.py

@@ -6,6 +6,7 @@
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase, Client
 from django.urls import reverse
+from parameterized import parameterized
 
 from request_a_govuk_domain.request import db
 from request_a_govuk_domain.request.admin.model_admins import convert_to_local_time
@@ -56,15 +57,25 @@ def test_gmt_time_is_converted_correctly(self):
         )
         self.assertEqual("01 Nov 2024 00:00:00 AM", convert_to_local_time(gmt_date))
 
-    def test_file_is_uploaded_from_admin_screen(self):
+    @parameterized.expand(
+        [
+            ["new_document.pdf", "new_document.pdf"],
+            ["new document.pdf", "new_document.pdf"],
+            [
+                "new -document _with more ' spaces.pdf",
+                "new_document_with_more_spaces.pdf",
+            ],
+        ]
+    )
+    def test_file_is_uploaded_from_admin_screen(self, file_name, expected_name):
         request = Mock()
         request.session = SessionDict({"registration_data": self.registration_data})
         app = db.save_data_in_database("ABCDEFGHIJK", request)
         c = Client()
         c.login(username="superuser", password="secret")  # pragma: allowlist secret
 
         written_permission_evidence = SimpleUploadedFile(
-            "new_document.pdf", b"file_content", content_type="application/pdf"
+            file_name, b"file_content", content_type="application/pdf"
         )
         with patch(
             "request_a_govuk_domain.request.models.application.S3_STORAGE_ENABLED", True
@@ -98,15 +109,15 @@ def test_file_is_uploaded_from_admin_screen(self):
             [
                 call.connection.meta.client.put_object(
                     Bucket="mock-data-bucket",
-                    Key="temp_files/new_document.pdf",
+                    Key=f"temp_files/{file_name}",
                     Body=b"file_content",
                 ),
                 call.connection.meta.client.copy_object(
                     Bucket="mock-data-bucket",
-                    CopySource="mock-data-bucket/temp_files/new_document.pdf",
-                    Key="applications/ABCDEFGHIJK/new_document.pdf",
+                    CopySource=f"mock-data-bucket/temp_files/{file_name}",
+                    Key=f"applications/ABCDEFGHIJK/{expected_name}",
                 ),
-                call.delete("temp_files/new_document.pdf"),
+                call.delete(f"temp_files/{file_name}"),
             ]
         )
         self.assertEqual(response.status_code, 200)