From 748a857ad6240935fac8e4745666ee5f67df3c2e Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 16 Jul 2024 16:37:39 +0200 Subject: [PATCH 1/2] :sparkles: [#4488] Initiate download of submission report PDF * Refactored to use the PrivateMediaView generic CBV * URL moved to non-API url * Deleted the API-related code that is now obsolete --- src/openforms/submissions/api/permissions.py | 9 +--- src/openforms/submissions/api/renderers.py | 13 ----- src/openforms/submissions/api/urls.py | 7 +-- src/openforms/submissions/api/views.py | 47 ++----------------- .../tests/test_submission_co_sign.py | 2 +- .../tests/test_submission_report.py | 28 +++++++++-- src/openforms/submissions/urls.py | 6 +++ src/openforms/submissions/utils.py | 2 +- src/openforms/submissions/views.py | 46 +++++++++++++++++- 9 files changed, 81 insertions(+), 79 deletions(-) diff --git a/src/openforms/submissions/api/permissions.py b/src/openforms/submissions/api/permissions.py index 0c1002d76d..e6636ac531 100644 --- a/src/openforms/submissions/api/permissions.py +++ b/src/openforms/submissions/api/permissions.py @@ -9,10 +9,7 @@ from ..constants import SUBMISSIONS_SESSION_KEY from ..form_logic import check_submission_logic from ..models import SubmissionStep, TemporaryFileUpload -from ..tokens import ( - submission_report_token_generator, - submission_status_token_generator, -) +from ..tokens import submission_status_token_generator from .validation import is_step_unexpectedly_incomplete @@ -93,10 +90,6 @@ def has_object_permission( return owns_submission(request, submission_uuid) -class DownloadSubmissionReportPermission(TimestampedTokenPermission): - token_generator = submission_report_token_generator - - class SubmissionStatusPermission(TimestampedTokenPermission): token_generator = submission_status_token_generator diff --git a/src/openforms/submissions/api/renderers.py b/src/openforms/submissions/api/renderers.py index 413c6775d7..b7ce317303 100644 --- a/src/openforms/submissions/api/renderers.py +++ b/src/openforms/submissions/api/renderers.py @@ -1,19 +1,6 @@ from rest_framework import renderers -class PDFRenderer(renderers.BaseRenderer): - # this is a bit of a lie, since we're using it for a send-file response that returns - # generated PDFs. So it's actually nginx doing the actual response. - # Example in DRF docs: https://www.django-rest-framework.org/api-guide/renderers/#setting-the-character-set - media_type = "application/pdf" - format = "pdf" - charset = None - render_style = "binary" - - def render(self, data, media_type=None, renderer_context=None): - return data - - class FileRenderer(renderers.BaseRenderer): media_type = "application/octet-stream" format = "octet" diff --git a/src/openforms/submissions/api/urls.py b/src/openforms/submissions/api/urls.py index bc2efadcf1..983e20ed5b 100644 --- a/src/openforms/submissions/api/urls.py +++ b/src/openforms/submissions/api/urls.py @@ -1,15 +1,10 @@ from django.urls import path -from .views import DownloadSubmissionReportView, TemporaryFileView +from .views import TemporaryFileView app_name = "submissions" urlpatterns = [ - path( - "//download", - DownloadSubmissionReportView.as_view(), - name="download-submission", - ), path( "files/", TemporaryFileView.as_view(), diff --git a/src/openforms/submissions/api/views.py b/src/openforms/submissions/api/views.py index 4dd957951c..345f2a9915 100644 --- a/src/openforms/submissions/api/views.py +++ b/src/openforms/submissions/api/views.py @@ -1,56 +1,17 @@ from django.conf import settings -from django.utils import timezone from django.utils.translation import gettext_lazy as _ from django_sendfile import sendfile from djangorestframework_camel_case.render import CamelCaseJSONRenderer from drf_spectacular.utils import extend_schema, extend_schema_view -from rest_framework.generics import DestroyAPIView, GenericAPIView +from rest_framework.generics import DestroyAPIView from openforms.api.authentication import AnonCSRFSessionAuthentication from openforms.api.serializers import ExceptionSerializer -from ..models import SubmissionReport, TemporaryFileUpload -from .permissions import ( - DownloadSubmissionReportPermission, - OwnsTemporaryUploadPermission, -) -from .renderers import FileRenderer, PDFRenderer - - -@extend_schema( - summary=_("Download the PDF report"), - description=_( - "Download the PDF report containing the submission data. The endpoint requires " - "a token which is tied to the submission from the session. The token automatically expires " - "after {expire_days} day(s)." - ).format(expire_days=settings.SUBMISSION_REPORT_URL_TOKEN_TIMEOUT_DAYS), - responses={200: bytes}, -) -class DownloadSubmissionReportView(GenericAPIView): - queryset = SubmissionReport.objects.all() - lookup_url_kwarg = "report_id" - authentication_classes = () - permission_classes = (DownloadSubmissionReportPermission,) - # FIXME: 404s etc. are now also rendered with this, which breaks. - renderer_classes = (PDFRenderer,) - serializer_class = None - - # see :func:`sendfile.sendfile` for available parameters - sendfile_options = None - - def get_sendfile_opts(self) -> dict: - return self.sendfile_options or {} - - def get(self, request, report_id: int, token: str, *args, **kwargs): - submission_report = self.get_object() - - submission_report.last_accessed = timezone.now() - submission_report.save() - - filename = submission_report.content.path - sendfile_options = self.get_sendfile_opts() - return sendfile(request, filename, **sendfile_options) +from ..models import TemporaryFileUpload +from .permissions import OwnsTemporaryUploadPermission +from .renderers import FileRenderer @extend_schema( diff --git a/src/openforms/submissions/tests/test_submission_co_sign.py b/src/openforms/submissions/tests/test_submission_co_sign.py index 0db0459718..2059eadbe8 100644 --- a/src/openforms/submissions/tests/test_submission_co_sign.py +++ b/src/openforms/submissions/tests/test_submission_co_sign.py @@ -185,7 +185,7 @@ def test_cosign_happy_flow_calls_on_cosign_task(self): match = resolve(furl(data["reportDownloadUrl"]).path) - self.assertEqual(match.view_name, "api:submissions:download-submission") + self.assertEqual(match.view_name, "submissions:download-submission") submission.refresh_from_db() diff --git a/src/openforms/submissions/tests/test_submission_report.py b/src/openforms/submissions/tests/test_submission_report.py index b657263c7b..dc145a2e00 100644 --- a/src/openforms/submissions/tests/test_submission_report.py +++ b/src/openforms/submissions/tests/test_submission_report.py @@ -25,7 +25,7 @@ def test_valid_token(self): report = SubmissionReportFactory.create(submission__completed=True) token = submission_report_token_generator.make_token(report) download_report_url = reverse( - "api:submissions:download-submission", + "submissions:download-submission", kwargs={"report_id": report.id, "token": token}, ) @@ -34,11 +34,29 @@ def test_valid_token(self): self.assertEqual(status.HTTP_200_OK, response.status_code) + def test_download_response(self): + report = SubmissionReportFactory.create( + submission__completed=True, content__filename="report.pdf" + ) + token = submission_report_token_generator.make_token(report) + download_report_url = reverse( + "submissions:download-submission", + kwargs={"report_id": report.id, "token": token}, + ) + + response = self.client.get(download_report_url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response["Content-Type"], "application/pdf") + self.assertEqual( + response["Content-Disposition"], 'attachment; filename="report.pdf"' + ) + def test_expired_token(self): report = SubmissionReportFactory.create(submission__completed=True) token = submission_report_token_generator.make_token(report) download_report_url = reverse( - "api:submissions:download-submission", + "submissions:download-submission", kwargs={"report_id": report.id, "token": token}, ) @@ -51,7 +69,7 @@ def test_token_not_invalidated_by_earlier_download(self): report = SubmissionReportFactory.create(submission__completed=True) token = submission_report_token_generator.make_token(report) download_report_url = reverse( - "api:submissions:download-submission", + "submissions:download-submission", kwargs={"report_id": report.id, "token": token}, ) @@ -68,7 +86,7 @@ def test_token_not_invalidated_by_earlier_download(self): def test_wrongly_formatted_token(self): report = SubmissionReportFactory.create(submission__completed=True) download_report_url = reverse( - "api:submissions:download-submission", + "submissions:download-submission", kwargs={"report_id": report.id, "token": "dummy"}, ) @@ -79,7 +97,7 @@ def test_wrongly_formatted_token(self): def test_invalid_token_timestamp(self): report = SubmissionReportFactory.create(submission__completed=True) download_report_url = reverse( - "api:submissions:download-submission", + "submissions:download-submission", kwargs={"report_id": report.id, "token": "$$$-blegh"}, ) diff --git a/src/openforms/submissions/urls.py b/src/openforms/submissions/urls.py index 8c97caadbd..750837c709 100644 --- a/src/openforms/submissions/urls.py +++ b/src/openforms/submissions/urls.py @@ -1,6 +1,7 @@ from django.urls import path from .views import ( + DownloadSubmissionReportView, ResumeSubmissionView, SearchSubmissionForCosignFormView, SubmissionAttachmentDownloadView, @@ -9,6 +10,11 @@ app_name = "submissions" urlpatterns = [ + path( + "pdf///download", + DownloadSubmissionReportView.as_view(), + name="download-submission", + ), path( "//resume", ResumeSubmissionView.as_view(), diff --git a/src/openforms/submissions/utils.py b/src/openforms/submissions/utils.py index 9e317d324a..6d8d1e8b52 100644 --- a/src/openforms/submissions/utils.py +++ b/src/openforms/submissions/utils.py @@ -305,7 +305,7 @@ def check_form_status( def get_report_download_url(request: Request, report: SubmissionReport) -> str: token = submission_report_token_generator.make_token(report) download_url = reverse( - "api:submissions:download-submission", + "submissions:download-submission", kwargs={"report_id": report.id, "token": token}, ) return request.build_absolute_uri(download_url) diff --git a/src/openforms/submissions/views.py b/src/openforms/submissions/views.py index c34164c289..e8820b97cc 100644 --- a/src/openforms/submissions/views.py +++ b/src/openforms/submissions/views.py @@ -1,11 +1,13 @@ import logging import uuid +from pathlib import Path from django.contrib.auth.hashers import check_password as check_salted_hash from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.core.exceptions import PermissionDenied from django.http import HttpRequest from django.shortcuts import get_object_or_404 +from django.utils import timezone from django.utils.crypto import constant_time_compare from django.utils.translation import gettext_lazy as _ from django.views.generic import FormView, RedirectView @@ -28,9 +30,9 @@ from .constants import RegistrationStatuses from .exceptions import FormDeactivated, FormMaintenance from .forms import SearchSubmissionForCosignForm -from .models import Submission, SubmissionFileAttachment +from .models import Submission, SubmissionFileAttachment, SubmissionReport from .signals import submission_resumed -from .tokens import submission_resume_token_generator +from .tokens import submission_report_token_generator, submission_resume_token_generator from .utils import add_submmission_to_session, check_form_status logger = logging.getLogger(__name__) @@ -192,6 +194,46 @@ def get_form_resume_url(self, submission: Submission) -> str: ) +class DownloadSubmissionReportView(PrivateMediaView): + """ + Download the PDF report containing the submission data. + + This URL requires a token which is tied to the submission from the session. The + token automatically expires after + ``settings.SUBMISSION_REPORT_URL_TOKEN_TIMEOUT_DAYS`` days. + """ + + queryset = SubmissionReport.objects.all() + pk_url_kwarg = "report_id" + file_field = "content" + raise_exception = True + + object: SubmissionReport + + def has_permission(self): + report: SubmissionReport = self.get_object() + token = self.kwargs["token"] + return submission_report_token_generator.check_token(report, token) + + def get_sendfile_opts(self) -> dict: + path = Path(self.object.content.name) + opts = { + "attachment": True, + "attachment_filename": path.name, + "mimetype": "application/pdf", + } + return opts + + def get(self, request: HttpRequest, *args, **kwargs): + self.object = self.get_object() + + response = super().get(request, *args, **kwargs) + + self.object.last_accessed = timezone.now() + self.object.save() + return response + + class SubmissionAttachmentDownloadView(LoginRequiredMixin, PrivateMediaView): # only consider finished submissions (those are eligible for further processing) # and submissions that have been successfully registered with the registration From 92903eed06d8a2719e59b9a804728183ee22be1d Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 16 Jul 2024 16:46:59 +0200 Subject: [PATCH 2/2] :bento: [#4488] Regenerate API spec --- src/openapi.yaml | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/src/openapi.yaml b/src/openapi.yaml index b04b6c5f27..599027283a 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -4302,43 +4302,6 @@ paths: schema: type: string required: true - /api/v2/submissions/{report_id}/{token}/download: - get: - operationId: submissions_download_retrieve - description: Download het document met de ingezonden gegevens, in PDF vorm. - Dit document kan alleen worden opgevraagd indien het juiste token is meegegeven, - die hoort bij de formulier sessie. Het token vervalt automatisch na 1 dag(en). - summary: Download the PDF report - parameters: - - in: path - name: report_id - schema: - type: integer - required: true - - in: path - name: token - schema: - type: string - required: true - tags: - - submissions - responses: - '200': - content: - application/pdf: - schema: - type: string - format: binary - description: '' - headers: - X-Session-Expires-In: - $ref: '#/components/headers/X-Session-Expires-In' - X-CSRFToken: - $ref: '#/components/headers/X-CSRFToken' - X-Is-Form-Designer: - $ref: '#/components/headers/X-Is-Form-Designer' - Content-Language: - $ref: '#/components/headers/Content-Language' /api/v2/submissions/{submission_uuid}/steps/{step_uuid}: get: operationId: submissions_steps_retrieve