From 9863eb9ee9b21fb8a21c3b742197cc8cd3e1b169 Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Mon, 27 Jan 2025 10:58:05 +0200 Subject: [PATCH 01/12] attachment public serializers: remove unnecessary and sensitive data from output --- forms/serializers/form.py | 9 +++++++++ forms/viewsets/form.py | 3 ++- plotsearch/serializers/plot_search.py | 9 +++++++++ plotsearch/views/plot_search.py | 2 ++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/forms/serializers/form.py b/forms/serializers/form.py index 3c903e21..ddf18454 100755 --- a/forms/serializers/form.py +++ b/forms/serializers/form.py @@ -923,5 +923,14 @@ def save(self, **kwargs): return super().save(**kwargs) +class AttachmentPublicSerializer(AttachmentSerializer): + def to_representation(self, instance): + representation = super().to_representation(instance) + for key in ["attachment"]: + if key in representation: + representation.pop(key) + return representation + + class ReadAttachmentSerializer(AttachmentSerializer): field = serializers.CharField(source="field.identifier") diff --git a/forms/viewsets/form.py b/forms/viewsets/form.py index 6d94dda6..ddf9ee73 100755 --- a/forms/viewsets/form.py +++ b/forms/viewsets/form.py @@ -20,6 +20,7 @@ AnswerOpeningRecordSerializer, AnswerPublicSerializer, AnswerSerializer, + AttachmentPublicSerializer, AttachmentSerializer, FormSerializer, MeetingMemoSerializer, @@ -178,7 +179,7 @@ def download(self, request, pk=None): class AttachmentPublicViewSet(FileExtensionFileMixin, AttachmentViewSet): """Includes FileExtensionFileMixin to validate file extensions.""" - pass + serializer_class = AttachmentPublicSerializer class TargetStatusViewset( diff --git a/plotsearch/serializers/plot_search.py b/plotsearch/serializers/plot_search.py index 2cbf063b..5f2c81e8 100755 --- a/plotsearch/serializers/plot_search.py +++ b/plotsearch/serializers/plot_search.py @@ -840,6 +840,15 @@ def create(self, validated_data): return attachment +class AreaSearchAttachmentPublicSerializer(AreaSearchAttachmentSerializer): + def to_representation(self, instance): + representation = super().to_representation(instance) + for key in ["attachment", "user", "area_search", "created_at"]: + if key in representation: + representation.pop(key) + return representation + + class AreaSearchStatusNoteSerializer(serializers.ModelSerializer): preparer = UserSerializer(read_only=True) time_stamp = serializers.DateTimeField(read_only=True) diff --git a/plotsearch/views/plot_search.py b/plotsearch/views/plot_search.py index 4ed805d1..fc3e3af7 100755 --- a/plotsearch/views/plot_search.py +++ b/plotsearch/views/plot_search.py @@ -64,6 +64,7 @@ PlotSearchOpeningRecordPermissions, ) from plotsearch.serializers.plot_search import ( + AreaSearchAttachmentPublicSerializer, AreaSearchAttachmentSerializer, AreaSearchDetailSerializer, AreaSearchListSerializer, @@ -411,6 +412,7 @@ class AreaSearchAttachmentPublicViewset( ): """Includes FileExtensionFileMixin to validate file extensions.""" + serializer_class = AreaSearchAttachmentPublicSerializer permission_classes = (AreaSearchAttachmentPublicPermissions,) def destroy(self, request, *args, **kwargs): From 2f58847d65c38d22cfe37fb91a07067b9b98372d Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Mon, 27 Jan 2025 09:38:33 +0000 Subject: [PATCH 02/12] exclude created_at --- forms/serializers/form.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forms/serializers/form.py b/forms/serializers/form.py index ddf18454..8be3dffb 100755 --- a/forms/serializers/form.py +++ b/forms/serializers/form.py @@ -926,7 +926,7 @@ def save(self, **kwargs): class AttachmentPublicSerializer(AttachmentSerializer): def to_representation(self, instance): representation = super().to_representation(instance) - for key in ["attachment"]: + for key in ["attachment", "created_at"]: if key in representation: representation.pop(key) return representation From 068279281fb421e57eee3243b439e92e4b70a628 Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Tue, 28 Jan 2025 14:23:04 +0000 Subject: [PATCH 03/12] exclude sensitive public attachment endpoints: unit tests --- forms/serializers/form.py | 4 +- forms/tests/test_api.py | 93 +++++++++++++++++++++++++++ plotsearch/serializers/plot_search.py | 9 ++- 3 files changed, 104 insertions(+), 2 deletions(-) diff --git a/forms/serializers/form.py b/forms/serializers/form.py index 8be3dffb..cdc8ca90 100755 --- a/forms/serializers/form.py +++ b/forms/serializers/form.py @@ -33,6 +33,8 @@ RequiredFormFieldValidator, ) +SENSITIVE_ATTACHMENT_FIELDS = ["attachment", "created_at"] + class RecursiveSerializer(serializers.Serializer): def to_representation(self, value): @@ -926,7 +928,7 @@ def save(self, **kwargs): class AttachmentPublicSerializer(AttachmentSerializer): def to_representation(self, instance): representation = super().to_representation(instance) - for key in ["attachment", "created_at"]: + for key in SENSITIVE_ATTACHMENT_FIELDS: if key in representation: representation.pop(key) return representation diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index 57692df5..19f29648 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -13,8 +13,10 @@ from forms.enums import FormState from forms.models import Entry, Field from forms.models.form import AnswerOpeningRecord, Attachment +from forms.serializers.form import SENSITIVE_ATTACHMENT_FIELDS from plotsearch.enums import DeclineReason from plotsearch.models import TargetStatus +from plotsearch.serializers.plot_search import SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS fake = Faker("fi_FI") @@ -430,6 +432,97 @@ def test_attachment_delete( assert os.path.isfile(file_path) is False +@pytest.mark.django_db +def test_attachment_post_public( + django_db_setup, admin_client, admin_user, plot_search_target, basic_form +): + example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") + payload = { + "field": Field.objects.all().first().id, + "name": fake.name(), + "attachment": example_file, + } + url = reverse("v1:pub_attachment-list") + response = admin_client.post(url, data=payload) + assert response.status_code == 201 + + attachment_keys = response.json().keys() + assert len(set(SENSITIVE_ATTACHMENT_FIELDS).intersection(set(attachment_keys))) == 0 + + attachment_id = response.json()["id"] + url = reverse("v1:pub_answer-list") + payload = { + "form": basic_form.id, + "user": admin_user.pk, + "targets": [ + plot_search_target.pk, + ], # noqa: E231 + "entries": { + "sections": { + "company-information": [ + { + "sections": {}, + "fields": {"company-name": {"value": "", "extraValue": ""}}, + }, + { + "sections": {}, + "fields": { + "business-id": {"value": "", "extraValue": ""}, + "hallintaosuus": {"value": "1/1", "extraValue": ""}, + }, + }, + ], + "contact-person": { + "sections": {}, + "fields": { + "first-name": {"value": "False", "extraValue": ""}, + "last-name": { + "value": "99", + "extraValue": "developers developers developers", + }, + }, + }, + }, + "fields": {}, + }, + "attachments": [ + attachment_id, + ], # noqa: E231 + "ready": True, + } + response = admin_client.post(url, data=payload, content_type="application/json") + + assert response.status_code == 201 + assert Attachment.objects.filter(answer=response.json()["id"]).exists() + + +@pytest.mark.django_db +def test_area_search_attachment_post_public( + django_db_setup, admin_client, admin_user, area_search_test_data +): + example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") + payload = { + "name": example_file.name, + "area_search": area_search_test_data.id, + # "user": admin_user, + "attachment": example_file, + } + url = reverse("v1:pub_area_search_attachment-list") + response = admin_client.post(url, data=payload) + assert response.status_code == 201 + + # Ensure that the attachment does not contain any sensitive fields + attachment_keys = response.json().keys() + assert ( + len( + set(SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS).intersection( + set(attachment_keys) + ) + ) + == 0 + ) + + @pytest.mark.django_db def test_opening_record_create( django_db_setup, diff --git a/plotsearch/serializers/plot_search.py b/plotsearch/serializers/plot_search.py index 5f2c81e8..d0f0ca66 100755 --- a/plotsearch/serializers/plot_search.py +++ b/plotsearch/serializers/plot_search.py @@ -65,6 +65,13 @@ from users.models import User from users.serializers import UserSerializer +SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS = [ + "attachment", + "user", + "area_search", + "created_at", +] + class PlotSearchSubTypeLinkedSerializer(NameModelSerializer): class Meta: @@ -843,7 +850,7 @@ def create(self, validated_data): class AreaSearchAttachmentPublicSerializer(AreaSearchAttachmentSerializer): def to_representation(self, instance): representation = super().to_representation(instance) - for key in ["attachment", "user", "area_search", "created_at"]: + for key in SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS: if key in representation: representation.pop(key) return representation From 1448a15b0da1953f9fec2917ccac7bcfd2bc49fe Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Tue, 28 Jan 2025 14:32:40 +0000 Subject: [PATCH 04/12] rename and document excluded attachment fields for public api --- forms/serializers/form.py | 5 +++-- forms/tests/test_api.py | 8 ++++---- plotsearch/serializers/plot_search.py | 5 +++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/forms/serializers/form.py b/forms/serializers/form.py index cdc8ca90..8c72e305 100755 --- a/forms/serializers/form.py +++ b/forms/serializers/form.py @@ -33,7 +33,8 @@ RequiredFormFieldValidator, ) -SENSITIVE_ATTACHMENT_FIELDS = ["attachment", "created_at"] +# These fields include sensitive or unnecessary data for the public API +EXCLUDED_ATTACHMENT_FIELDS = ["attachment", "created_at"] class RecursiveSerializer(serializers.Serializer): @@ -928,7 +929,7 @@ def save(self, **kwargs): class AttachmentPublicSerializer(AttachmentSerializer): def to_representation(self, instance): representation = super().to_representation(instance) - for key in SENSITIVE_ATTACHMENT_FIELDS: + for key in EXCLUDED_ATTACHMENT_FIELDS: if key in representation: representation.pop(key) return representation diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index 19f29648..19d9a1a9 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -13,10 +13,10 @@ from forms.enums import FormState from forms.models import Entry, Field from forms.models.form import AnswerOpeningRecord, Attachment -from forms.serializers.form import SENSITIVE_ATTACHMENT_FIELDS +from forms.serializers.form import EXCLUDED_ATTACHMENT_FIELDS from plotsearch.enums import DeclineReason from plotsearch.models import TargetStatus -from plotsearch.serializers.plot_search import SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS +from plotsearch.serializers.plot_search import EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS fake = Faker("fi_FI") @@ -447,7 +447,7 @@ def test_attachment_post_public( assert response.status_code == 201 attachment_keys = response.json().keys() - assert len(set(SENSITIVE_ATTACHMENT_FIELDS).intersection(set(attachment_keys))) == 0 + assert len(set(EXCLUDED_ATTACHMENT_FIELDS).intersection(set(attachment_keys))) == 0 attachment_id = response.json()["id"] url = reverse("v1:pub_answer-list") @@ -515,7 +515,7 @@ def test_area_search_attachment_post_public( attachment_keys = response.json().keys() assert ( len( - set(SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS).intersection( + set(EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS).intersection( set(attachment_keys) ) ) diff --git a/plotsearch/serializers/plot_search.py b/plotsearch/serializers/plot_search.py index d0f0ca66..b7fcd5b0 100755 --- a/plotsearch/serializers/plot_search.py +++ b/plotsearch/serializers/plot_search.py @@ -65,7 +65,8 @@ from users.models import User from users.serializers import UserSerializer -SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS = [ +# These fields include sensitive or unnecessary data for the public API +EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS = [ "attachment", "user", "area_search", @@ -850,7 +851,7 @@ def create(self, validated_data): class AreaSearchAttachmentPublicSerializer(AreaSearchAttachmentSerializer): def to_representation(self, instance): representation = super().to_representation(instance) - for key in SENSITIVE_AREA_SEARCH_ATTACHMENT_FIELDS: + for key in EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS: if key in representation: representation.pop(key) return representation From 29503ce2acc8700ccee122932c2d97f4d76e3c2f Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Wed, 29 Jan 2025 16:46:54 +0200 Subject: [PATCH 05/12] attachment upload unit tests: rename, add docstrings and specify which field is being used --- forms/tests/test_api.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index 19d9a1a9..6252c467 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -345,12 +345,18 @@ def test_meeting_memo_create( @pytest.mark.django_db -def test_attachment_post( +def test_attachment_upload( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): + """ + should upload an attachment, create a form answer, and the attachment should be linked to the form answer + """ example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") + field = basic_form.sections.get(identifier="application-target").fields.get( + identifier="reference-attachments" + ) payload = { - "field": Field.objects.all().first().id, + "field": field.id, "name": fake.name(), "attachment": example_file, } @@ -420,7 +426,7 @@ def test_attachment_post( def test_attachment_delete( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): - test_attachment_post( + test_attachment_upload( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ) attachment = Attachment.objects.all().first() @@ -433,12 +439,19 @@ def test_attachment_delete( @pytest.mark.django_db -def test_attachment_post_public( +def test_attachment_upload_public( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): + """ + should upload an attachment with the public API and not return any sensitive or unnecessary fields in the HTTP, + create a form answer, and the attachment should be linked to a form answer + """ example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") + field = basic_form.sections.get(identifier="application-target").fields.get( + identifier="reference-attachments" + ) payload = { - "field": Field.objects.all().first().id, + "field": field.id, "name": fake.name(), "attachment": example_file, } From ad022faf7ed25ae2a74e65f4b8a9e5ea0453b947 Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Thu, 30 Jan 2025 07:50:22 +0000 Subject: [PATCH 06/12] remove unnecessary import --- forms/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index 6252c467..6a53de17 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -11,7 +11,7 @@ from faker import Faker from forms.enums import FormState -from forms.models import Entry, Field +from forms.models import Entry from forms.models.form import AnswerOpeningRecord, Attachment from forms.serializers.form import EXCLUDED_ATTACHMENT_FIELDS from plotsearch.enums import DeclineReason From 3ce56b984b6c327b7d6f35793c5954f2e8c680a1 Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Thu, 30 Jan 2025 08:30:08 +0000 Subject: [PATCH 07/12] area search attachment unit test: add docstring, rename function and add assert to test if attachment is linked to an area search --- forms/tests/test_api.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index 6a53de17..7aa15615 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -16,6 +16,7 @@ from forms.serializers.form import EXCLUDED_ATTACHMENT_FIELDS from plotsearch.enums import DeclineReason from plotsearch.models import TargetStatus +from plotsearch.models.plot_search import AreaSearchAttachment from plotsearch.serializers.plot_search import EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS fake = Faker("fi_FI") @@ -510,14 +511,18 @@ def test_attachment_upload_public( @pytest.mark.django_db -def test_area_search_attachment_post_public( +def test_area_search_attachment_upload_public( django_db_setup, admin_client, admin_user, area_search_test_data ): + """ + should upload an area search attachment with the public API + and not return any sensitive or unnecessary fields in the HTTP response, + and the area search attachment should be linked to an area search + """ example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") payload = { "name": example_file.name, "area_search": area_search_test_data.id, - # "user": admin_user, "attachment": example_file, } url = reverse("v1:pub_area_search_attachment-list") @@ -534,6 +539,9 @@ def test_area_search_attachment_post_public( ) == 0 ) + assert AreaSearchAttachment.objects.filter( + area_search=area_search_test_data + ).exists() @pytest.mark.django_db From fbc44f7f6190df605ac6d8dc0e5d4eebccb3df74 Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Thu, 30 Jan 2025 10:26:28 +0000 Subject: [PATCH 08/12] test area search attachment create public: move test to plotsearch app --- forms/tests/test_api.py | 36 ------------------------ plotsearch/tests/api/test_plot_search.py | 34 +++++++++++++++++++++- 2 files changed, 33 insertions(+), 37 deletions(-) diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index 7aa15615..a7f2bfcc 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -16,8 +16,6 @@ from forms.serializers.form import EXCLUDED_ATTACHMENT_FIELDS from plotsearch.enums import DeclineReason from plotsearch.models import TargetStatus -from plotsearch.models.plot_search import AreaSearchAttachment -from plotsearch.serializers.plot_search import EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS fake = Faker("fi_FI") @@ -510,40 +508,6 @@ def test_attachment_upload_public( assert Attachment.objects.filter(answer=response.json()["id"]).exists() -@pytest.mark.django_db -def test_area_search_attachment_upload_public( - django_db_setup, admin_client, admin_user, area_search_test_data -): - """ - should upload an area search attachment with the public API - and not return any sensitive or unnecessary fields in the HTTP response, - and the area search attachment should be linked to an area search - """ - example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") - payload = { - "name": example_file.name, - "area_search": area_search_test_data.id, - "attachment": example_file, - } - url = reverse("v1:pub_area_search_attachment-list") - response = admin_client.post(url, data=payload) - assert response.status_code == 201 - - # Ensure that the attachment does not contain any sensitive fields - attachment_keys = response.json().keys() - assert ( - len( - set(EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS).intersection( - set(attachment_keys) - ) - ) - == 0 - ) - assert AreaSearchAttachment.objects.filter( - area_search=area_search_test_data - ).exists() - - @pytest.mark.django_db def test_opening_record_create( django_db_setup, diff --git a/plotsearch/tests/api/test_plot_search.py b/plotsearch/tests/api/test_plot_search.py index 805df78b..96bb5f94 100755 --- a/plotsearch/tests/api/test_plot_search.py +++ b/plotsearch/tests/api/test_plot_search.py @@ -16,7 +16,8 @@ from leasing.models import PlanUnit from plotsearch.enums import SearchClass, SearchStage from plotsearch.models import AreaSearch, PlotSearch, PlotSearchTarget -from plotsearch.models.plot_search import PlotSearchStage +from plotsearch.models.plot_search import AreaSearchAttachment, PlotSearchStage +from plotsearch.serializers.plot_search import EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS fake = Faker("fi_FI") @@ -824,6 +825,37 @@ def test_area_search_attachment_create( assert len(response.data["area_search_attachments"]) == 1 +@pytest.mark.django_db +def test_area_search_attachment_create_public( + django_db_setup, admin_client, area_search_test_data +): + attachment = { + "area_search": area_search_test_data.id, + "attachment": SimpleUploadedFile(content=b"Lorem Impsum", name="test.txt"), + "name": fake.name(), + } + + url = reverse( + "v1:pub_area_search_attachment-list", + ) + response = admin_client.post(url, data=attachment) + + assert response.status_code == 201 + + attachment_keys = response.json().keys() + assert ( + len( + set(EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS).intersection( + set(attachment_keys) + ) + ) + == 0 + ) + assert AreaSearchAttachment.objects.filter( + area_search=area_search_test_data + ).exists() + + @pytest.mark.django_db def test_opening_record_create( django_db_setup, From c0e09e570dcbbdce65a33c50855608d7b9cf61b4 Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Thu, 30 Jan 2025 14:06:03 +0000 Subject: [PATCH 09/12] area search public serializer --- plotsearch/serializers/plot_search.py | 11 +++++++ plotsearch/tests/api/test_plot_search.py | 42 ++++++++++++++++++++++++ plotsearch/views/plot_search.py | 3 +- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/plotsearch/serializers/plot_search.py b/plotsearch/serializers/plot_search.py index b7fcd5b0..f08929dd 100755 --- a/plotsearch/serializers/plot_search.py +++ b/plotsearch/serializers/plot_search.py @@ -1092,6 +1092,17 @@ def update(self, instance, validated_data): return instance +class AreaSearchPublicSerializer(AreaSearchSerializer): + area_search_attachments = InstanceDictPrimaryKeyRelatedField( + instance_class=AreaSearchAttachment, + queryset=AreaSearchAttachment.objects.all(), + related_serializer=AreaSearchAttachmentPublicSerializer, + required=False, + allow_null=True, + many=True, + ) + + class AreaSearchDetailSerializer(AreaSearchSerializer): answer = AnswerSerializer(read_only=True, required=False) plot = serializers.ListField(child=serializers.CharField(), read_only=True) diff --git a/plotsearch/tests/api/test_plot_search.py b/plotsearch/tests/api/test_plot_search.py index 96bb5f94..0da8ede0 100755 --- a/plotsearch/tests/api/test_plot_search.py +++ b/plotsearch/tests/api/test_plot_search.py @@ -799,6 +799,48 @@ def test_area_search_create_simple( ) +@pytest.mark.django_db +def test_area_search_create_simple_public( + django_db_setup, admin_client, area_search_test_data, area_search_attachment_factory +): + url = reverse("v1:pub_area_search-list") # list == create + + attachment = area_search_attachment_factory( + area_search=area_search_test_data, + attachment=SimpleUploadedFile(content=b"Lorem Impsum", name="test.txt"), + name="test.txt", + ) + + data = { + "description_area": get_random_string(length=12), + "description_intended_use": get_random_string(length=12), + "intended_use": area_search_test_data.intended_use.pk, + "geometry": area_search_test_data.geometry.geojson, + "attachments": 1, + "area_search_attachments": [attachment.id], + } + + response = admin_client.post( + url, json.dumps(data, cls=DjangoJSONEncoder), content_type="application/json" + ) + attachments = response.json().get("area_search_attachments") + assert ( + len( + set(attachments[0].keys()).intersection( + set(EXCLUDED_AREA_SEARCH_ATTACHMENT_FIELDS) + ) + ) + == 0 + ) + + assert response.status_code == 201, "%s %s" % (response.status_code, response.data) + assert AreaSearch.objects.filter(id=response.data["id"]).exists() + assert ( + AreaSearch.objects.get(id=response.data["id"]).intended_use.id + == area_search_test_data.intended_use.pk + ) + + @pytest.mark.django_db def test_area_search_attachment_create( django_db_setup, admin_client, area_search_test_data diff --git a/plotsearch/views/plot_search.py b/plotsearch/views/plot_search.py index fc3e3af7..1aabd568 100755 --- a/plotsearch/views/plot_search.py +++ b/plotsearch/views/plot_search.py @@ -68,6 +68,7 @@ AreaSearchAttachmentSerializer, AreaSearchDetailSerializer, AreaSearchListSerializer, + AreaSearchPublicSerializer, AreaSearchSerializer, DirectReservationLinkSerializer, FAQSerializer, @@ -364,7 +365,7 @@ class AreaSearchPublicViewSet( mixins.CreateModelMixin, mixins.UpdateModelMixin, viewsets.GenericViewSet ): queryset = AreaSearch.objects.all() - serializer_class = AreaSearchSerializer + serializer_class = AreaSearchPublicSerializer permission_classes = ( AreaSearchPublicPermissions, IsAuthenticated, From c6b01ff10e95bdef077e0674db7bc5b921c72fcf Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Fri, 31 Jan 2025 12:25:52 +0000 Subject: [PATCH 10/12] disable downloading in public attachment viewsets --- forms/tests/test_api.py | 24 ++++++++++++++++++++---- forms/viewsets/form.py | 10 ++++++++-- plotsearch/views/plot_search.py | 8 ++++++-- utils/viewsets/mixins.py | 8 +++++++- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index a7f2bfcc..723b38bf 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -441,10 +441,6 @@ def test_attachment_delete( def test_attachment_upload_public( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): - """ - should upload an attachment with the public API and not return any sensitive or unnecessary fields in the HTTP, - create a form answer, and the attachment should be linked to a form answer - """ example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") field = basic_form.sections.get(identifier="application-target").fields.get( identifier="reference-attachments" @@ -458,6 +454,7 @@ def test_attachment_upload_public( response = admin_client.post(url, data=payload) assert response.status_code == 201 + # When attachments are created, the HTTP response should not return any sensitive or unnecessary data attachment_keys = response.json().keys() assert len(set(EXCLUDED_ATTACHMENT_FIELDS).intersection(set(attachment_keys))) == 0 @@ -504,9 +501,28 @@ def test_attachment_upload_public( } response = admin_client.post(url, data=payload, content_type="application/json") + # When an answer is created, the HTTP response should not contain attachments + assert response.json().get("attachments") is None + assert response.status_code == 201 assert Attachment.objects.filter(answer=response.json()["id"]).exists() + # Public endpoint should not allow getting attachments + error = None + try: + url = reverse("v1:pub_answer-attachments", kwargs={"pk": response.json()["id"]}) + except Exception as e: + error = type(e).__name__ + assert error == "NoReverseMatch" + + # Public endpoint should not allow downloading attachments + url = reverse("v1:pub_attachment-download", kwargs={"pk": attachment_id}) + + file_response: FileResponse = admin_client.get(url) + assert response.status_code == 201 + assert len(response.json()) != 0 + assert file_response.status_code == 405 # Method not allowed + @pytest.mark.django_db def test_opening_record_create( diff --git a/forms/viewsets/form.py b/forms/viewsets/form.py index ddf9ee73..92458905 100755 --- a/forms/viewsets/form.py +++ b/forms/viewsets/form.py @@ -32,7 +32,11 @@ from leasing.permissions import MvjDjangoModelPermissions from plotsearch.models import TargetStatus from plotsearch.models.plot_search import MeetingMemo -from utils.viewsets.mixins import FileDownloadMixin, FileExtensionFileMixin +from utils.viewsets.mixins import ( + DisablePublicDownloadMixin, + FileDownloadMixin, + FileExtensionFileMixin, +) class FormViewSet( @@ -176,7 +180,9 @@ def download(self, request, pk=None): return super().download(request, pk, file_field="attachment") -class AttachmentPublicViewSet(FileExtensionFileMixin, AttachmentViewSet): +class AttachmentPublicViewSet( + DisablePublicDownloadMixin, FileExtensionFileMixin, AttachmentViewSet +): """Includes FileExtensionFileMixin to validate file extensions.""" serializer_class = AttachmentPublicSerializer diff --git a/plotsearch/views/plot_search.py b/plotsearch/views/plot_search.py index 1aabd568..c1db1c60 100755 --- a/plotsearch/views/plot_search.py +++ b/plotsearch/views/plot_search.py @@ -88,7 +88,11 @@ RelatedPlotApplicationCreateDeleteSerializer, ) from plotsearch.utils import build_pdf_context -from utils.viewsets.mixins import FileExtensionFileMixin, FileMixin +from utils.viewsets.mixins import ( + DisablePublicDownloadMixin, + FileExtensionFileMixin, + FileMixin, +) class PlotSearchSubtypeViewSet( @@ -409,7 +413,7 @@ def download(self, request, pk=None): class AreaSearchAttachmentPublicViewset( - FileExtensionFileMixin, AreaSearchAttachmentViewset + DisablePublicDownloadMixin, FileExtensionFileMixin, AreaSearchAttachmentViewset ): """Includes FileExtensionFileMixin to validate file extensions.""" diff --git a/utils/viewsets/mixins.py b/utils/viewsets/mixins.py index 1a3018dd..d2891efc 100644 --- a/utils/viewsets/mixins.py +++ b/utils/viewsets/mixins.py @@ -6,7 +6,7 @@ from django.utils.translation import gettext_lazy as _ from rest_framework import status from rest_framework.decorators import action -from rest_framework.exceptions import ValidationError +from rest_framework.exceptions import MethodNotAllowed, ValidationError from rest_framework.response import Response MAX_FILE_SIZE_MB = 20 @@ -165,3 +165,9 @@ def update(self, request, *args, **kwargs): @action(methods=["get"], detail=True) def download(self, request, pk=None, file_field: str | None = None): return FileDownloadMixin.download(self, request, pk, file_field=file_field) + + +class DisablePublicDownloadMixin: + @action(methods=["get"], detail=True) + def download(self, *args, **kwargs): + raise MethodNotAllowed("GET") From 83f7ff4f47141ab44d79fec46f9557e25ee1845d Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Fri, 31 Jan 2025 12:26:11 +0000 Subject: [PATCH 11/12] test_attachment_upload: remove docstring --- forms/tests/test_api.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index 723b38bf..bb6d0695 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -347,9 +347,6 @@ def test_meeting_memo_create( def test_attachment_upload( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): - """ - should upload an attachment, create a form answer, and the attachment should be linked to the form answer - """ example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") field = basic_form.sections.get(identifier="application-target").fields.get( identifier="reference-attachments" From 00a15e891679bdececa265ec7cd2448bfb85383f Mon Sep 17 00:00:00 2001 From: Jukka Ahonen Date: Fri, 31 Jan 2025 12:44:35 +0000 Subject: [PATCH 12/12] test forms attachment upload and download: rename functions and improve comments --- forms/tests/test_api.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/forms/tests/test_api.py b/forms/tests/test_api.py index bb6d0695..5de5eba5 100644 --- a/forms/tests/test_api.py +++ b/forms/tests/test_api.py @@ -344,7 +344,7 @@ def test_meeting_memo_create( @pytest.mark.django_db -def test_attachment_upload( +def test_attachment_upload_and_download( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") @@ -403,15 +403,19 @@ def test_attachment_upload( } response = admin_client.post(url, data=payload, content_type="application/json") id = response.json()["id"] - assert response.status_code == 201 + + # Attachment should exist assert Attachment.objects.filter(answer=response.json()["id"]).exists() + # Should get attachments url = reverse("v1:answer-attachments", kwargs={"pk": id}) response = admin_client.get(url) + assert response.status_code == 200 + + # Should download attachment url = reverse("v1:attachment-download", kwargs={"pk": attachment_id}) file_response: FileResponse = admin_client.get(url) - assert response.status_code == 200 assert len(response.json()) != 0 assert file_response.status_code == 200 # Get the content from the generator @@ -422,7 +426,7 @@ def test_attachment_upload( def test_attachment_delete( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): - test_attachment_upload( + test_attachment_upload_and_download( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ) attachment = Attachment.objects.all().first() @@ -435,7 +439,7 @@ def test_attachment_delete( @pytest.mark.django_db -def test_attachment_upload_public( +def test_attachment_upload_and_download_public( django_db_setup, admin_client, admin_user, plot_search_target, basic_form ): example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") @@ -501,6 +505,7 @@ def test_attachment_upload_public( # When an answer is created, the HTTP response should not contain attachments assert response.json().get("attachments") is None + # Attachment should exist assert response.status_code == 201 assert Attachment.objects.filter(answer=response.json()["id"]).exists() @@ -514,9 +519,7 @@ def test_attachment_upload_public( # Public endpoint should not allow downloading attachments url = reverse("v1:pub_attachment-download", kwargs={"pk": attachment_id}) - file_response: FileResponse = admin_client.get(url) - assert response.status_code == 201 assert len(response.json()) != 0 assert file_response.status_code == 405 # Method not allowed