Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions api/migrations/0215_alter_generaldocument_document_and_more.py
Original file line number Diff line number Diff line change
@@ -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"
),
),
]
6 changes: 4 additions & 2 deletions api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,


Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
30 changes: 30 additions & 0 deletions api/test_views.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
3 changes: 2 additions & 1 deletion country_plan/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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"])],
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
),
),
]
9 changes: 5 additions & 4 deletions dref/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -648,7 +649,7 @@ def get_for(user):


class DrefFile(models.Model):
file = models.FileField(
file = SecureFileField(
verbose_name=_("file"),
upload_to="dref/images/",
)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
),
),
]
7 changes: 5 additions & 2 deletions flash_update/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# import os
# from uuid import uuid4
susilnem marked this conversation as resolved.
Show resolved Hide resolved
import reversion
from django.conf import settings
from django.contrib.auth.models import Group
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
13 changes: 13 additions & 0 deletions main/fields.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# 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 = [
("per", "0122_opslearningcacheresponse_and_more"),
]

operations = [
migrations.AlterField(
model_name="perdocumentupload",
name="file",
field=main.fields.SecureFileField(upload_to="per/documents/", verbose_name="file"),
),
migrations.AlterField(
model_name="perfile",
name="file",
field=main.fields.SecureFileField(upload_to="per/images/", verbose_name="file"),
),
]
5 changes: 3 additions & 2 deletions per/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from api.models import Appeal, Country
from deployments.models import SectorTag
from main.fields import SecureFileField


class ProcessPhase(models.IntegerChoices):
Expand Down Expand Up @@ -422,7 +423,7 @@ def save(self, *args, **kwargs):


class PerFile(models.Model):
file = models.FileField(
file = SecureFileField(
verbose_name=_("file"),
upload_to="per/images/",
)
Expand Down Expand Up @@ -738,7 +739,7 @@ def save(self, *args, **kwargs):


class PerDocumentUpload(models.Model):
file = models.FileField(
file = SecureFileField(
verbose_name=_("file"),
upload_to="per/documents/",
)
Expand Down