Merge pull request #2256 from IFRCGo/feature/secure-files

Secure file fields by adding random uuid in the file name.

Author: szabozoltan69

api/migrations/0215_alter_generaldocument_document_and_more.py

@@ -0,0 +1,30 @@
+# Generated by Django 4.2.16 on 2024-10-29 08:51
+
+from django.db import migrations
+
+import api.models
+import main.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("api", "0214_alter_profile_limit_access_to_guest"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="generaldocument",
+            name="document",
+            field=main.fields.SecureFileField(
+                blank=True, null=True, upload_to=api.models.general_document_path, verbose_name="document"
+            ),
+        ),
+        migrations.AlterField(
+            model_name="situationreport",
+            name="document",
+            field=main.fields.SecureFileField(
+                blank=True, null=True, upload_to=api.models.sitrep_document_path, verbose_name="document"
+            ),
+        ),
+    ]

api/models.py

@@ -22,6 +22,8 @@
 from django.utils.translation import gettext_lazy as _
 from tinymce.models import HTMLField
 
+from main.fields import SecureFileField
+
 from .utils import validate_slug_number  # is_user_ifrc,
 
 
@@ -984,7 +986,7 @@ def sitrep_document_path(instance, filename):
 class SituationReport(models.Model):
     created_at = models.DateTimeField(verbose_name=_("created at"), auto_now_add=True)
     name = models.CharField(verbose_name=_("name"), max_length=100)
-    document = models.FileField(verbose_name=_("document"), null=True, blank=True, upload_to=sitrep_document_path)
+    document = SecureFileField(verbose_name=_("document"), null=True, blank=True, upload_to=sitrep_document_path)
     document_url = models.URLField(verbose_name=_("document url"), blank=True)
 
     event = models.ForeignKey(Event, verbose_name=_("event"), on_delete=models.CASCADE)
@@ -1287,7 +1289,7 @@ class GeneralDocument(models.Model):
     name = models.CharField(verbose_name=_("name"), max_length=100)
     # Don't set `auto_now_add` so we can modify it on save
     created_at = models.DateTimeField(verbose_name=_("created at"), blank=True)
-    document = models.FileField(verbose_name=_("document"), null=True, blank=True, upload_to=general_document_path)
+    document = SecureFileField(verbose_name=_("document"), null=True, blank=True, upload_to=general_document_path)
     document_url = models.URLField(verbose_name=_("document url"), blank=True)
 
     class Meta:

api/test_views.py

@@ -1,6 +1,8 @@
 import json
+import uuid
 
 from django.contrib.auth.models import User
+from django.core.files.uploadedfile import SimpleUploadedFile
 
 import api.models as models
 from api.factories.event import (
@@ -13,9 +15,37 @@
 from api.factories.field_report import FieldReportFactory
 from api.models import Profile, VisibilityChoices
 from deployments.factories.user import UserFactory
+from dref.models import DrefFile
 from main.test_case import APITestCase, SnapshotTestCase
 
 
+class SecureFileFieldTest(APITestCase):
+    def is_valid_uuid(self, uuid_to_test):
+        try:
+            # Validate if the directory name is a valid UUID (hex)
+            uuid_obj = uuid.UUID(uuid_to_test, version=4)
+        except ValueError:
+            return False
+        return uuid_obj.hex == uuid_to_test
+
+    def test_filefield_uuid_directory_and_filename(self):
+        # Mocking a file upload with SimpleUploadedFile
+        original_filename = "test_file.txt"
+        mock_file = SimpleUploadedFile(original_filename, b"file_content", content_type="text/plain")
+        # Create an instance of MyModel and save it with the mocked file
+        instance = DrefFile.objects.create(file=mock_file)
+        # Check the uploaded file name
+        uploaded_file_name = instance.file.name
+        # Example output: uploads/5f9d54c8b5a34d3e8fdc4e4f43e2f82a/test_file.txt
+
+        # Extract UUID directory part and filename part
+        directory_name, file_name = uploaded_file_name.split("/")[-2:]
+        # Check that the directory name is a valid UUID (hexadecimal)
+        self.assertTrue(self.is_valid_uuid(directory_name))
+        # Check that the file name retains the original name
+        self.assertEqual(file_name, original_filename)
+
+
 class GuestUserPermissionTest(APITestCase):
     def setUp(self):
         # Create guest user

country_plan/migrations/0008_alter_countryplan_internal_plan_file.py

@@ -0,0 +1,28 @@
+# Generated by Django 4.2.16 on 2024-10-29 08:51
+
+import django.core.validators
+from django.db import migrations
+
+import country_plan.models
+import main.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("country_plan", "0007_alter_membershipcoordination_sector_and_more"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="countryplan",
+            name="internal_plan_file",
+            field=main.fields.SecureFileField(
+                blank=True,
+                null=True,
+                upload_to=country_plan.models.pdf_upload_to,
+                validators=[django.core.validators.FileExtensionValidator(["pdf"])],
+                verbose_name="Internal Plan",
+            ),
+        ),
+    ]

country_plan/models.py

@@ -5,6 +5,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from api.models import Country
+from main.fields import SecureFileField
 
 
 def file_upload_to(instance, filename):
@@ -63,7 +64,7 @@ def save(self, *args, **kwargs):
 
 class CountryPlan(CountryPlanAbstract):
     country = models.OneToOneField(Country, on_delete=models.CASCADE, related_name="country_plan", primary_key=True)
-    internal_plan_file = models.FileField(
+    internal_plan_file = SecureFileField(
         verbose_name=_("Internal Plan"),
         upload_to=pdf_upload_to,
         validators=[FileExtensionValidator(["pdf"])],

dref/migrations/0075_alter_dref_budget_file_preview_alter_dreffile_file_and_more.py

@@ -0,0 +1,39 @@
+# Generated by Django 4.2.16 on 2024-10-29 08:51
+
+from django.db import migrations
+
+import main.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("dref", "0074_auto_20240129_0909"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="dref",
+            name="budget_file_preview",
+            field=main.fields.SecureFileField(
+                blank=True, null=True, upload_to="dref/images/", verbose_name="budget file preview"
+            ),
+        ),
+        migrations.AlterField(
+            model_name="dreffile",
+            name="file",
+            field=main.fields.SecureFileField(upload_to="dref/images/", verbose_name="file"),
+        ),
+        migrations.AlterField(
+            model_name="dreffinalreport",
+            name="financial_report_preview",
+            field=main.fields.SecureFileField(blank=True, null=True, upload_to="dref/images/", verbose_name="financial preview"),
+        ),
+        migrations.AlterField(
+            model_name="drefoperationalupdate",
+            name="budget_file_preview",
+            field=main.fields.SecureFileField(
+                blank=True, null=True, upload_to="dref-op-update/images/", verbose_name="budget file preview"
+            ),
+        ),
+    ]

dref/models.py

@@ -12,6 +12,7 @@
 from pdf2image import convert_from_bytes
 
 from api.models import Country, DisasterType, District, FieldReport
+from main.fields import SecureFileField
 
 
 @reversion.register()
@@ -536,7 +537,7 @@ class Status(models.IntegerChoices):
         verbose_name=_("budget file"),
         related_name="budget_file_dref",
     )
-    budget_file_preview = models.FileField(verbose_name=_("budget file preview"), null=True, blank=True, upload_to="dref/images/")
+    budget_file_preview = SecureFileField(verbose_name=_("budget file preview"), null=True, blank=True, upload_to="dref/images/")
     assessment_report = models.ForeignKey(
         "DrefFile",
         on_delete=models.SET_NULL,
@@ -648,7 +649,7 @@ def get_for(user):
 
 
 class DrefFile(models.Model):
-    file = models.FileField(
+    file = SecureFileField(
         verbose_name=_("file"),
         upload_to="dref/images/",
     )
@@ -750,7 +751,7 @@ class DrefOperationalUpdate(models.Model):
         verbose_name=_("budget file"),
         related_name="budget_file_dref_operational_update",
     )
-    budget_file_preview = models.FileField(
+    budget_file_preview = SecureFileField(
         verbose_name=_("budget file preview"), null=True, blank=True, upload_to="dref-op-update/images/"
     )
     assessment_report = models.ForeignKey(
@@ -1316,7 +1317,7 @@ class DrefFinalReport(models.Model):
         verbose_name=_("financial report"),
         related_name="financial_report_dref_final_report",
     )
-    financial_report_preview = models.FileField(
+    financial_report_preview = SecureFileField(
         verbose_name=_("financial preview"), null=True, blank=True, upload_to="dref/images/"
     )
     num_assisted = models.IntegerField(verbose_name=_("number of assisted"), blank=True, null=True)

flash_update/migrations/0013_alter_flashgraphicmap_file_and_more.py

@@ -0,0 +1,27 @@
+# Generated by Django 4.2.16 on 2024-10-29 08:51
+
+from django.db import migrations
+
+import main.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("flash_update", "0012_auto_20230410_0720"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="flashgraphicmap",
+            name="file",
+            field=main.fields.SecureFileField(upload_to="flash_update/images", verbose_name="file"),
+        ),
+        migrations.AlterField(
+            model_name="flashupdate",
+            name="extracted_file",
+            field=main.fields.SecureFileField(
+                blank=True, null=True, upload_to="flash_update/pdf/", verbose_name="extracted file"
+            ),
+        ),
+    ]

flash_update/models.py

@@ -1,3 +1,5 @@
+# import os
+# from uuid import uuid4
 import reversion
 from django.conf import settings
 from django.contrib.auth.models import Group
@@ -14,11 +16,12 @@
     DisasterType,
     District,
 )
+from main.fields import SecureFileField
 
 
 @reversion.register()
 class FlashGraphicMap(models.Model):
-    file = models.FileField(verbose_name=_("file"), upload_to="flash_update/images/")
+    file = SecureFileField(verbose_name=_("file"), upload_to="flash_update/images")
     caption = models.CharField(max_length=225, blank=True, null=True)
     created_by = models.ForeignKey(
         settings.AUTH_USER_MODEL,
@@ -116,7 +119,7 @@ class FlashShareWith(models.TextChoices):
         verbose_name=_("share with"),
     )
     references = models.ManyToManyField(FlashReferences, blank=True, verbose_name=_("references"))
-    extracted_file = models.FileField(verbose_name=_("extracted file"), upload_to="flash_update/pdf/", blank=True, null=True)
+    extracted_file = SecureFileField(verbose_name=_("extracted file"), upload_to="flash_update/pdf/", blank=True, null=True)
     extracted_at = models.DateTimeField(verbose_name=_("extracted at"), blank=True, null=True)
 
     class Meta:

main/fields.py

@@ -0,0 +1,13 @@
+from uuid import uuid4
+
+from django.db.models.fields.files import FileField
+
+
+class SecureFileField(FileField):
+    def generate_filename(self, instance, filename):
+        """
+        Overwrites https://github.com/django/django/blob/main/django/db/models/fields/files.py#L345
+        """
+        # Append uuid4 path to the filename
+        filename = f"{uuid4().hex}/{filename}"
+        return super().generate_filename(instance, filename)  # return self.storage.generate_filename(filename)