From 752ac7d1a164dabcd92963c4662cfd4c87701d33 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Thu, 2 Nov 2023 17:05:14 +0100 Subject: [PATCH 1/6] =?UTF-8?q?=E2=9C=A8=20[#3362]=20Handle=20hash=20based?= =?UTF-8?q?=20frontend=20routing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../appointments/tests/test_views.py | 21 ++++++---- src/openforms/appointments/views.py | 26 +++++++------ src/openforms/frontend.py | 28 ++++++++++++++ .../submissions/models/submission.py | 4 +- .../tests/test_resume_form_view.py | 38 ++++++++++++------- src/openforms/submissions/tests/test_views.py | 2 +- src/openforms/submissions/views.py | 28 ++++++++------ 7 files changed, 100 insertions(+), 47 deletions(-) create mode 100644 src/openforms/frontend.py diff --git a/src/openforms/appointments/tests/test_views.py b/src/openforms/appointments/tests/test_views.py index 3024c3272f..a4bc89abc4 100644 --- a/src/openforms/appointments/tests/test_views.py +++ b/src/openforms/appointments/tests/test_views.py @@ -50,9 +50,10 @@ def test_good_token_and_submission_redirect_and_add_submission_to_session(self): response = self.client.get(endpoint) expected_redirect_url = ( - furl("http://maykinmedia.nl/myform/afspraak-annuleren") + furl("http://maykinmedia.nl/myform") .add( { + "_action": "afspraak-annuleren", "time": "2021-07-21T12:00:00+00:00", "submission_uuid": str(submission.uuid), } @@ -198,9 +199,9 @@ def test_after_successful_auth_redirects_to_form(self): }, ) expected_redirect_url = furl(submission.form_url) - expected_redirect_url /= "afspraak-annuleren" expected_redirect_url.add( { + "_action": "afspraak-annuleren", "time": start_time.isoformat(), "submission_uuid": str(submission.uuid), } @@ -398,8 +399,8 @@ def test_good_token_and_submission_redirect_and_add_submission_to_session(self): # after initiating change, we expect the bsn to be stored in plain text (again) self.assertEqual(new_submission.auth_info.value, "000000000") expected_redirect_url = ( - f"http://maykinmedia.nl/myform/stap/{form_definition.slug}" - f"?submission_uuid={new_submission.uuid}" + f"http://maykinmedia.nl/myform/?_action=stap&_action_args={form_definition.slug}" + f"&submission_uuid={new_submission.uuid}" ) self.assertRedirects( @@ -580,7 +581,7 @@ def test_redirect_to_first_step_when_appointment_form_definition_can_not_be_foun response = self.client.get(endpoint) new_submission = Submission.objects.exclude(id=submission.id).get() - expected_redirect_url = f"http://maykinmedia.nl/myform/stap/step-1?submission_uuid={new_submission.uuid}" + expected_redirect_url = f"http://maykinmedia.nl/myform/?_action=stap&_action_args=step-1&submission_uuid={new_submission.uuid}" self.assertRedirects( response, expected_redirect_url, fetch_redirect_response=False ) @@ -659,9 +660,13 @@ def test_after_successful_auth_redirects_to_form(self): self.assertIsNotNone(new_submission) expected_redirect_url = furl(submission.form_url) - expected_redirect_url /= "stap" - expected_redirect_url /= "test-step" - expected_redirect_url.args["submission_uuid"] = str(new_submission.uuid) + expected_redirect_url.add( + { + "_action": "stap", + "_action_args": "test-step", + "submission_uuid": new_submission.uuid, + } + ) self.assertRedirects( response, expected_redirect_url.url, fetch_redirect_response=False diff --git a/src/openforms/appointments/views.py b/src/openforms/appointments/views.py index 7c3ecd960b..8499358054 100644 --- a/src/openforms/appointments/views.py +++ b/src/openforms/appointments/views.py @@ -3,6 +3,7 @@ from django.views.generic import RedirectView from openforms.authentication.service import FORM_AUTH_SESSION_KEY, store_auth_details +from openforms.frontend import get_frontend_redirect_url from openforms.submissions.models import Submission from openforms.submissions.views import ResumeFormMixin @@ -16,15 +17,14 @@ class VerifyCancelAppointmentLinkView(ResumeFormMixin, RedirectView): token_generator = submission_appointment_token_generator def get_form_resume_url(self, submission: Submission) -> str: - f = submission.cleaned_form_url - f /= "afspraak-annuleren" - f.add( - { + return get_frontend_redirect_url( + submission, + action="afspraak-annuleren", + query={ "time": submission.appointment_info.start_time.isoformat(), "submission_uuid": str(submission.uuid), - } + }, ) - return f.url class VerifyChangeAppointmentLinkView(ResumeFormMixin, RedirectView): @@ -58,9 +58,11 @@ def get_form_resume_url(self, submission: Submission) -> str: assert next_step is not None, "Form has no steps to redirect to!" - f = submission.cleaned_form_url - f /= "stap" - f /= next_step.slug - f.add({"submission_uuid": submission.uuid}) - - return f.url + return get_frontend_redirect_url( + submission, + action="stap", + action_args=[next_step.slug], + query={ + "submission_uuid": str(submission.uuid), + }, + ) diff --git a/src/openforms/frontend.py b/src/openforms/frontend.py new file mode 100644 index 0000000000..aa543490bf --- /dev/null +++ b/src/openforms/frontend.py @@ -0,0 +1,28 @@ +from typing import Iterable, Literal, TypeAlias + +from openforms.submissions.models import Submission + +SDKAction: TypeAlias = Literal["stap", "afspraak-annuleren", "cosign"] + + +def get_frontend_redirect_url( + submission: Submission, + action: SDKAction, + action_args: Iterable[str] | None = None, + query: dict[str, str] | None = None, +) -> str: + """Get the frontend redirect URL depending on the action. + + Some actions require arguments to be specified. The frontend will take care of building the right redirection + based on the action and action arguments. Extra query parameters can be specified with ``query``. + """ + f = submission.cleaned_form_url + _query = { + "_action": action, + } + if action_args: + _query["_action_args"] = ",".join(action_args) + if query: + _query.update(query) + + return f.add(_query).url diff --git a/src/openforms/submissions/models/submission.py b/src/openforms/submissions/models/submission.py index f0360e64a4..ca85117248 100644 --- a/src/openforms/submissions/models/submission.py +++ b/src/openforms/submissions/models/submission.py @@ -349,7 +349,9 @@ def cleaned_form_url(self) -> furl: # if the url path ends with 'startpagina', strip it off if furl_instance.path.segments[-1:] == ["startpagina"]: furl_instance.path.segments.remove("startpagina") - return furl_instance + return furl_instance.remove( + fragment=True + ) # Fragments are present in hash based routing @property def total_configuration_wrapper(self) -> FormioConfigurationWrapper: diff --git a/src/openforms/submissions/tests/test_resume_form_view.py b/src/openforms/submissions/tests/test_resume_form_view.py index 508df90613..af084e0bb3 100644 --- a/src/openforms/submissions/tests/test_resume_form_view.py +++ b/src/openforms/submissions/tests/test_resume_form_view.py @@ -50,11 +50,14 @@ def test_good_token_and_submission_redirect_and_add_submission_to_session(self): response = self.client.get(endpoint) f = furl(submission.form_url) - # furl adds paths with the /= operator - f /= "stap" - f /= submission.get_last_completed_step().form_step.slug - # Add the submission uuid to the query param - f.add({"submission_uuid": submission.uuid}) + # Add the redirect path and submission uuid to the query param + f.add( + { + "_action": "stap", + "_action_args": submission.get_last_completed_step().form_step.slug, + "submission_uuid": submission.uuid, + } + ) expected_redirect_url = f.url self.assertRedirects( @@ -210,9 +213,13 @@ def test_after_successful_auth_redirects_to_form(self): }, ) expected_redirect_url = furl(submission.form_url) - expected_redirect_url /= "stap" - expected_redirect_url /= form_step.slug - expected_redirect_url.args["submission_uuid"] = submission.uuid + expected_redirect_url.add( + { + "_action": "stap", + "_action_args": form_step.slug, + "submission_uuid": submission.uuid, + } + ) # Add form_auth to session, as the authentication plugin would do it session = self.client.session @@ -405,12 +412,15 @@ def test_resume_creates_valid_url(self): response = self.client.get(endpoint) - f = furl("http://maykinmedia.nl/some-form/") - # furl adds paths with the /= operator - f /= "stap" - f /= submission.get_last_completed_step().form_step.slug - # Add the submission uuid to the query param - f.add({"submission_uuid": submission.uuid}) + f = furl("http://maykinmedia.nl/some-form") + # Add the redirect path and submission uuid to the query param + f.add( + { + "_action": "stap", + "_action_args": submission.get_last_completed_step().form_step.slug, + "submission_uuid": submission.uuid, + } + ) expected_redirect_url = f.url self.assertRedirects( diff --git a/src/openforms/submissions/tests/test_views.py b/src/openforms/submissions/tests/test_views.py index 25440b957c..8861fef153 100644 --- a/src/openforms/submissions/tests/test_views.py +++ b/src/openforms/submissions/tests/test_views.py @@ -56,7 +56,7 @@ def test_successfully_submit_form(self): self.assertRedirects( submission_response, - f"http://url-to-form.nl/cosign/check?submission_uuid={submission.uuid}", + f"http://url-to-form.nl/?_action=cosign&_action_args=check&submission_uuid={submission.uuid}", fetch_redirect_response=False, ) diff --git a/src/openforms/submissions/views.py b/src/openforms/submissions/views.py index 36f4a61a6a..501d92126d 100644 --- a/src/openforms/submissions/views.py +++ b/src/openforms/submissions/views.py @@ -21,6 +21,7 @@ meets_plugin_requirements, ) from openforms.forms.models import Form +from openforms.frontend import get_frontend_redirect_url from openforms.tokens import BaseTokenGenerator from .constants import RegistrationStatuses @@ -175,18 +176,18 @@ class ResumeSubmissionView(ResumeFormMixin, RedirectView): token_generator = submission_resume_token_generator def get_form_resume_url(self, submission: Submission) -> str: - form_resume_url = submission.cleaned_form_url - state = submission.load_execution_state() last_completed_step = state.get_last_completed_step() target_step = last_completed_step or state.submission_steps[0] - # furl adds paths with the /= operator - form_resume_url /= "stap" - form_resume_url /= target_step.form_step.slug - # Add the submission uuid to the query param - form_resume_url.add({"submission_uuid": submission.uuid}) - return form_resume_url.url + return get_frontend_redirect_url( + submission, + action="stap", + action_args=[target_step.form_step.slug], + query={ + "submission_uuid": submission.uuid, + }, + ) class SubmissionAttachmentDownloadView(LoginRequiredMixin, PrivateMediaView): @@ -279,6 +280,11 @@ def form_valid(self, form): return super().form_valid(form) def get_success_url(self): - cosign_page = self.submission.cleaned_form_url / "cosign" / "check" - cosign_page.args["submission_uuid"] = self.submission.uuid - return cosign_page.url + return get_frontend_redirect_url( + self.submission, + action="cosign", + action_args=["check"], + query={ + "submission_uuid": str(self.submission.uuid), + }, + ) From feaf9e965c8fa28a1fb0c9f695fa986d5624a5cc Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Thu, 9 Nov 2023 14:09:08 +0100 Subject: [PATCH 2/6] =?UTF-8?q?=E2=9C=85=20[#3362]=20Add=20some=20tests=20?= =?UTF-8?q?for=20the=20utility=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/openforms/tests/test_frontend_utils.py | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 src/openforms/tests/test_frontend_utils.py diff --git a/src/openforms/tests/test_frontend_utils.py b/src/openforms/tests/test_frontend_utils.py new file mode 100644 index 0000000000..31e26120b2 --- /dev/null +++ b/src/openforms/tests/test_frontend_utils.py @@ -0,0 +1,34 @@ +from django.test import TestCase + +from openforms.frontend import get_frontend_redirect_url +from openforms.submissions.tests.factories import SubmissionFactory + + +class FrontendRedirectTests(TestCase): + def test_frontend_redirect_no_hash(self): + submission = SubmissionFactory.create( + form_url="https://example.com/basepath", + ) + + redirect_url = get_frontend_redirect_url( + submission, "stap", ("action_1", "action_2"), {"q": "dummy"} + ) + + self.assertURLEqual( + redirect_url, + "https://example.com/basepath?_action=stap&_action_args=action_1,action_2&q=dummy", + ) + + def test_frontend_redirect_hash(self): + submission = SubmissionFactory.create( + form_url="https://example.com/basepath#after_hash", + ) + + redirect_url = get_frontend_redirect_url( + submission, "stap", ("action_1", "action_2"), {"q": "dummy"} + ) + + self.assertURLEqual( + redirect_url, + "https://example.com/basepath?_action=stap&_action_args=action_1,action_2&q=dummy", + ) From 3116dddd851fcd6078c19b21d52a4d2ddc4b9050 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Mon, 13 Nov 2023 14:18:32 +0100 Subject: [PATCH 3/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20[#3362]=20Refactor=20a?= =?UTF-8?q?ction=20args?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../appointments/tests/test_views.py | 95 ++++++++++--------- src/openforms/appointments/views.py | 8 +- src/openforms/frontend.py | 20 ++-- .../tests/test_resume_form_view.py | 66 ++++++------- src/openforms/submissions/tests/test_views.py | 11 ++- src/openforms/submissions/views.py | 11 +-- src/openforms/tests/mixins.py | 48 ++++++++++ src/openforms/tests/test_frontend_utils.py | 89 ++++++++++++++++- 8 files changed, 241 insertions(+), 107 deletions(-) create mode 100644 src/openforms/tests/mixins.py diff --git a/src/openforms/appointments/tests/test_views.py b/src/openforms/appointments/tests/test_views.py index a4bc89abc4..80085c91ae 100644 --- a/src/openforms/appointments/tests/test_views.py +++ b/src/openforms/appointments/tests/test_views.py @@ -20,13 +20,14 @@ SubmissionFactory, SubmissionStepFactory, ) +from openforms.tests.mixins import FrontendRedirectMixin from ..tokens import submission_appointment_token_generator from .factories import AppointmentFactory, AppointmentInfoFactory @freeze_time("2021-07-15T21:15:00Z") -class VerifyCancelAppointmentLinkViewTests(TestCase): +class VerifyCancelAppointmentLinkViewTests(TestCase, FrontendRedirectMixin): def test_good_token_and_submission_redirect_and_add_submission_to_session(self): submission = SubmissionFactory.create( completed=True, form_url="http://maykinmedia.nl/myform" @@ -49,19 +50,15 @@ def test_good_token_and_submission_redirect_and_add_submission_to_session(self): with freeze_time("2021-07-16T21:15:00Z"): response = self.client.get(endpoint) - expected_redirect_url = ( - furl("http://maykinmedia.nl/myform") - .add( - { - "_action": "afspraak-annuleren", - "time": "2021-07-21T12:00:00+00:00", - "submission_uuid": str(submission.uuid), - } - ) - .url - ) - self.assertRedirects( - response, expected_redirect_url, fetch_redirect_response=False + self.assertRedirectsToFrontend( + response, + frontend_base_url="http://maykinmedia.nl/myform", + action="afspraak-annuleren", + action_params={ + "time": "2021-07-21T12:00:00+00:00", + "submission_uuid": str(submission.uuid), + }, + fetch_redirect_response=False, ) # Assert submission is stored in session self.assertIn( @@ -198,14 +195,6 @@ def test_after_successful_auth_redirects_to_form(self): "submission_uuid": submission.uuid, }, ) - expected_redirect_url = furl(submission.form_url) - expected_redirect_url.add( - { - "_action": "afspraak-annuleren", - "time": start_time.isoformat(), - "submission_uuid": str(submission.uuid), - } - ) # Add form_auth to session, as the authentication plugin would do it session = self.client.session @@ -219,9 +208,17 @@ def test_after_successful_auth_redirects_to_form(self): response = self.client.get(endpoint) - self.assertRedirects( - response, expected_redirect_url.url, fetch_redirect_response=False + self.assertRedirectsToFrontend( + response, + frontend_base_url=submission.form_url, + action="afspraak-annuleren", + action_params={ + "time": start_time.isoformat(), + "submission_uuid": str(submission.uuid), + }, + fetch_redirect_response=False, ) + self.assertIn(SUBMISSIONS_SESSION_KEY, self.client.session) self.assertIn( str(submission.uuid), self.client.session[SUBMISSIONS_SESSION_KEY] @@ -344,7 +341,7 @@ def test_invalid_auth_value_raises_exception(self): @freeze_time("2021-07-15T21:15:00Z") -class VerifyChangeAppointmentLinkViewTests(TestCase): +class VerifyChangeAppointmentLinkViewTests(TestCase, FrontendRedirectMixin): def test_good_token_and_submission_redirect_and_add_submission_to_session(self): submission = SubmissionFactory.from_components( completed=True, @@ -398,14 +395,18 @@ def test_good_token_and_submission_redirect_and_add_submission_to_session(self): new_submission = Submission.objects.exclude(id=submission.id).get() # after initiating change, we expect the bsn to be stored in plain text (again) self.assertEqual(new_submission.auth_info.value, "000000000") - expected_redirect_url = ( - f"http://maykinmedia.nl/myform/?_action=stap&_action_args={form_definition.slug}" - f"&submission_uuid={new_submission.uuid}" - ) - self.assertRedirects( - response, expected_redirect_url, fetch_redirect_response=False + self.assertRedirectsToFrontend( + response, + frontend_base_url=submission.form_url, + action="resume", + action_params={ + "step_slug": form_definition.slug, + "submission_uuid": str(new_submission.uuid), + }, + fetch_redirect_response=False, ) + # Assert new submission was created self.assertEqual(Submission.objects.count(), 2) # Assert old submission not stored in session @@ -581,9 +582,16 @@ def test_redirect_to_first_step_when_appointment_form_definition_can_not_be_foun response = self.client.get(endpoint) new_submission = Submission.objects.exclude(id=submission.id).get() - expected_redirect_url = f"http://maykinmedia.nl/myform/?_action=stap&_action_args=step-1&submission_uuid={new_submission.uuid}" - self.assertRedirects( - response, expected_redirect_url, fetch_redirect_response=False + + self.assertRedirectsToFrontend( + response, + frontend_base_url=submission.form_url, + action="resume", + action_params={ + "step_slug": "step-1", + "submission_uuid": str(new_submission.uuid), + }, + fetch_redirect_response=False, ) def test_redirects_to_auth_if_form_requires_login(self): @@ -659,18 +667,17 @@ def test_after_successful_auth_redirects_to_form(self): self.assertIsNotNone(new_submission) - expected_redirect_url = furl(submission.form_url) - expected_redirect_url.add( - { - "_action": "stap", - "_action_args": "test-step", - "submission_uuid": new_submission.uuid, - } + self.assertRedirectsToFrontend( + response, + frontend_base_url=submission.form_url, + action="resume", + action_params={ + "step_slug": "test-step", + "submission_uuid": str(new_submission.uuid), + }, + fetch_redirect_response=False, ) - self.assertRedirects( - response, expected_redirect_url.url, fetch_redirect_response=False - ) self.assertIn(SUBMISSIONS_SESSION_KEY, self.client.session) self.assertIn( str(new_submission.uuid), self.client.session[SUBMISSIONS_SESSION_KEY] diff --git a/src/openforms/appointments/views.py b/src/openforms/appointments/views.py index 8499358054..5361d73711 100644 --- a/src/openforms/appointments/views.py +++ b/src/openforms/appointments/views.py @@ -20,7 +20,7 @@ def get_form_resume_url(self, submission: Submission) -> str: return get_frontend_redirect_url( submission, action="afspraak-annuleren", - query={ + action_params={ "time": submission.appointment_info.start_time.isoformat(), "submission_uuid": str(submission.uuid), }, @@ -60,9 +60,9 @@ def get_form_resume_url(self, submission: Submission) -> str: return get_frontend_redirect_url( submission, - action="stap", - action_args=[next_step.slug], - query={ + action="resume", + action_params={ + "step_slug": next_step.slug, "submission_uuid": str(submission.uuid), }, ) diff --git a/src/openforms/frontend.py b/src/openforms/frontend.py index aa543490bf..b09d56d7fa 100644 --- a/src/openforms/frontend.py +++ b/src/openforms/frontend.py @@ -1,28 +1,28 @@ -from typing import Iterable, Literal, TypeAlias +import json +from typing import Literal, TypeAlias from openforms.submissions.models import Submission -SDKAction: TypeAlias = Literal["stap", "afspraak-annuleren", "cosign"] +SDKAction: TypeAlias = Literal["resume", "afspraak-annuleren", "cosign"] def get_frontend_redirect_url( submission: Submission, action: SDKAction, - action_args: Iterable[str] | None = None, - query: dict[str, str] | None = None, + action_params: dict[str, str] | None = None, ) -> str: """Get the frontend redirect URL depending on the action. Some actions require arguments to be specified. The frontend will take care of building the right redirection - based on the action and action arguments. Extra query parameters can be specified with ``query``. + based on the action and action arguments. """ f = submission.cleaned_form_url + f.query.remove("_of_action") + f.query.remove("_of_action_params") _query = { - "_action": action, + "_of_action": action, } - if action_args: - _query["_action_args"] = ",".join(action_args) - if query: - _query.update(query) + if action_params: + _query["_of_action_params"] = json.dumps(action_params) return f.add(_query).url diff --git a/src/openforms/submissions/tests/test_resume_form_view.py b/src/openforms/submissions/tests/test_resume_form_view.py index af084e0bb3..2d2f7265d1 100644 --- a/src/openforms/submissions/tests/test_resume_form_view.py +++ b/src/openforms/submissions/tests/test_resume_form_view.py @@ -10,13 +10,14 @@ from openforms.authentication.constants import FORM_AUTH_SESSION_KEY, AuthAttribute from openforms.authentication.contrib.digid.constants import DIGID_DEFAULT_LOA from openforms.config.models import GlobalConfiguration +from openforms.tests.mixins import FrontendRedirectMixin from ..constants import SUBMISSIONS_SESSION_KEY from ..tokens import submission_resume_token_generator from .factories import SubmissionFactory, SubmissionStepFactory -class SubmissionResumeViewTests(TestCase): +class SubmissionResumeViewTests(TestCase, FrontendRedirectMixin): def test_good_token_and_submission_redirect_and_add_submission_to_session(self): submission = SubmissionFactory.from_components( completed=True, @@ -49,20 +50,17 @@ def test_good_token_and_submission_redirect_and_add_submission_to_session(self): response = self.client.get(endpoint) - f = furl(submission.form_url) - # Add the redirect path and submission uuid to the query param - f.add( - { - "_action": "stap", - "_action_args": submission.get_last_completed_step().form_step.slug, - "submission_uuid": submission.uuid, - } + self.assertRedirectsToFrontend( + response, + frontend_base_url=submission.form_url, + action="resume", + action_params={ + "step_slug": submission.get_last_completed_step().form_step.slug, + "submission_uuid": str(submission.uuid), + }, + fetch_redirect_response=False, ) - expected_redirect_url = f.url - self.assertRedirects( - response, expected_redirect_url, fetch_redirect_response=False - ) # Assert submission is stored in session self.assertIn( str(submission.uuid), self.client.session[SUBMISSIONS_SESSION_KEY] @@ -212,14 +210,6 @@ def test_after_successful_auth_redirects_to_form(self): "submission_uuid": submission.uuid, }, ) - expected_redirect_url = furl(submission.form_url) - expected_redirect_url.add( - { - "_action": "stap", - "_action_args": form_step.slug, - "submission_uuid": submission.uuid, - } - ) # Add form_auth to session, as the authentication plugin would do it session = self.client.session @@ -233,9 +223,17 @@ def test_after_successful_auth_redirects_to_form(self): response = self.client.get(endpoint) - self.assertRedirects( - response, expected_redirect_url.url, fetch_redirect_response=False + self.assertRedirectsToFrontend( + response, + frontend_base_url=submission.form_url, + action="resume", + action_params={ + "step_slug": form_step.slug, + "submission_uuid": str(submission.uuid), + }, + fetch_redirect_response=False, ) + self.assertIn(SUBMISSIONS_SESSION_KEY, self.client.session) self.assertIn( str(submission.uuid), self.client.session[SUBMISSIONS_SESSION_KEY] @@ -412,17 +410,13 @@ def test_resume_creates_valid_url(self): response = self.client.get(endpoint) - f = furl("http://maykinmedia.nl/some-form") - # Add the redirect path and submission uuid to the query param - f.add( - { - "_action": "stap", - "_action_args": submission.get_last_completed_step().form_step.slug, - "submission_uuid": submission.uuid, - } - ) - - expected_redirect_url = f.url - self.assertRedirects( - response, expected_redirect_url, fetch_redirect_response=False + self.assertRedirectsToFrontend( + response, + frontend_base_url="http://maykinmedia.nl/some-form", + action="resume", + action_params={ + "step_slug": submission.get_last_completed_step().form_step.slug, + "submission_uuid": str(submission.uuid), + }, + fetch_redirect_response=False, ) diff --git a/src/openforms/submissions/tests/test_views.py b/src/openforms/submissions/tests/test_views.py index 8861fef153..e98a2df4a2 100644 --- a/src/openforms/submissions/tests/test_views.py +++ b/src/openforms/submissions/tests/test_views.py @@ -7,9 +7,10 @@ from openforms.authentication.constants import FORM_AUTH_SESSION_KEY from openforms.authentication.contrib.digid.constants import DIGID_DEFAULT_LOA from openforms.submissions.tests.factories import SubmissionFactory +from openforms.tests.mixins import FrontendRedirectMixin -class SearchSubmissionForCosignView(WebTest): +class SearchSubmissionForCosignView(WebTest, FrontendRedirectMixin): def setUp(self) -> None: super().setUp() @@ -54,9 +55,13 @@ def test_successfully_submit_form(self): form["code"] = submission.public_registration_reference submission_response = form.submit() - self.assertRedirects( + self.assertRedirectsToFrontend( submission_response, - f"http://url-to-form.nl/?_action=cosign&_action_args=check&submission_uuid={submission.uuid}", + frontend_base_url="http://url-to-form.nl/", + action="cosign", + action_params={ + "submission_uuid": str(submission.uuid), + }, fetch_redirect_response=False, ) diff --git a/src/openforms/submissions/views.py b/src/openforms/submissions/views.py index 501d92126d..632ce24ec3 100644 --- a/src/openforms/submissions/views.py +++ b/src/openforms/submissions/views.py @@ -182,10 +182,10 @@ def get_form_resume_url(self, submission: Submission) -> str: return get_frontend_redirect_url( submission, - action="stap", - action_args=[target_step.form_step.slug], - query={ - "submission_uuid": submission.uuid, + action="resume", + action_params={ + "step_slug": target_step.form_step.slug, + "submission_uuid": str(submission.uuid), }, ) @@ -283,8 +283,7 @@ def get_success_url(self): return get_frontend_redirect_url( self.submission, action="cosign", - action_args=["check"], - query={ + action_params={ "submission_uuid": str(self.submission.uuid), }, ) diff --git a/src/openforms/tests/mixins.py b/src/openforms/tests/mixins.py new file mode 100644 index 0000000000..cbe125f489 --- /dev/null +++ b/src/openforms/tests/mixins.py @@ -0,0 +1,48 @@ +import json +from typing import Any + +from django.http.response import HttpResponse +from django.test import SimpleTestCase + +from furl import furl + +from openforms.frontend import SDKAction + + +class FrontendRedirectMixin: + """A mixin providing a helper method checking frontend redirects.""" + + def assertRedirectsToFrontend( + self: SimpleTestCase, + response: HttpResponse, + frontend_base_url: str, + action: SDKAction, + action_params: dict[str, str] | None = None, + **kwargs: Any + ) -> None: + """Assert that a response redirected to a specific frontend URL. + + :param response: The response to test the redirection on. + :param frontend_base_url: The base URL of the frontend. + :param action: The SDK action performed. + :param action_params: Optional parameters for the action. + :param **kwargs: Additional kwargs to be passed to ``assertRedirects``. + """ + + expected_redirect_url = furl(frontend_base_url) + expected_redirect_url.remove("_of_action") + expected_redirect_url.remove("_of_action_params") + expected_redirect_url.add( + { + "_of_action": action, + } + ) + + if action_params: + expected_redirect_url.add({"_of_action_params": json.dumps(action_params)}) + + return self.assertRedirects( + response, + expected_redirect_url.url, + **kwargs, + ) diff --git a/src/openforms/tests/test_frontend_utils.py b/src/openforms/tests/test_frontend_utils.py index 31e26120b2..c82e8e8333 100644 --- a/src/openforms/tests/test_frontend_utils.py +++ b/src/openforms/tests/test_frontend_utils.py @@ -1,5 +1,9 @@ +import json + from django.test import TestCase +from furl import furl + from openforms.frontend import get_frontend_redirect_url from openforms.submissions.tests.factories import SubmissionFactory @@ -10,13 +14,25 @@ def test_frontend_redirect_no_hash(self): form_url="https://example.com/basepath", ) + action_params = {"action_1": "arg_1", "action_2": "arg_2"} + redirect_url = get_frontend_redirect_url( - submission, "stap", ("action_1", "action_2"), {"q": "dummy"} + submission, + "resume", + action_params, + ) + + excpected_redirect_url = furl("https://example.com/basepath") + excpected_redirect_url.add( + { + "_of_action": "resume", + "_of_action_params": (json.dumps(action_params)), + } ) self.assertURLEqual( redirect_url, - "https://example.com/basepath?_action=stap&_action_args=action_1,action_2&q=dummy", + excpected_redirect_url.url, ) def test_frontend_redirect_hash(self): @@ -24,11 +40,76 @@ def test_frontend_redirect_hash(self): form_url="https://example.com/basepath#after_hash", ) + action_params = {"action_1": "arg_1", "action_2": "arg_2"} + redirect_url = get_frontend_redirect_url( - submission, "stap", ("action_1", "action_2"), {"q": "dummy"} + submission, + "resume", + action_params, + ) + + excpected_redirect_url = furl("https://example.com/basepath") + excpected_redirect_url.add( + { + "_of_action": "resume", + "_of_action_params": (json.dumps(action_params)), + } + ) + + self.assertURLEqual( + redirect_url, + excpected_redirect_url.url, + ) + + def test_frontend_redirect_special_chars(self): + submission = SubmissionFactory.create( + form_url="https://example.com/basepath", + ) + + action_params = {"action&=": " arg_1 +"} + + redirect_url = get_frontend_redirect_url( + submission, + "resume", + action_params, + ) + + excpected_redirect_url = furl("https://example.com/basepath") + excpected_redirect_url.add( + { + "_of_action": "resume", + "_of_action_params": (json.dumps(action_params)), + } + ) + + self.assertURLEqual( + redirect_url, + excpected_redirect_url.url, + ) + + def test_frontend_redirect_query_param(self): + submission = SubmissionFactory.create( + form_url="https://example.com/basepath?unrelated_query=1&_of_action=unrelated", + ) + + action_params = {"action": "arg"} + + redirect_url = get_frontend_redirect_url( + submission, + "resume", + action_params, + ) + + excpected_redirect_url = furl("https://example.com/basepath") + excpected_redirect_url.add( + { + "_of_action": "resume", + "_of_action_params": (json.dumps(action_params)), + "unrelated_query": "1", + } ) self.assertURLEqual( redirect_url, - "https://example.com/basepath?_action=stap&_action_args=action_1,action_2&q=dummy", + excpected_redirect_url.url, ) From f2369d353c78c657a3071925e40c9e93bc190b71 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Thu, 16 Nov 2023 16:13:30 +0100 Subject: [PATCH 4/6] [#3362] Add a migration to clean up urls --- .../migrations/0079_cleanup_urls.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 src/openforms/submissions/migrations/0079_cleanup_urls.py diff --git a/src/openforms/submissions/migrations/0079_cleanup_urls.py b/src/openforms/submissions/migrations/0079_cleanup_urls.py new file mode 100644 index 0000000000..06892645b7 --- /dev/null +++ b/src/openforms/submissions/migrations/0079_cleanup_urls.py @@ -0,0 +1,29 @@ +# Generated by Django 3.2.21 on 2023-11-16 14:57 +from django.db import migrations +from django.db.migrations.state import StateApps +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +from furl import furl + + +def cleanup_submission_urls( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + + Submission = apps.get_model("submissions", "Submission") + + for submission in Submission.objects.all(): + f = furl(submission.form_url) + f.remove(fragment=True) + submission.form_url = f.url + + submission.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("submissions", "0078_submission_finalised_registration_backend_key"), + ] + + operations = [migrations.RunPython(cleanup_submission_urls)] From 031e3f12bd0dfc9d67564304d9bab8b47dd26a0a Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Thu, 16 Nov 2023 16:30:36 +0100 Subject: [PATCH 5/6] [#3362] Update and clarify docs --- docs/developers/sdk/embedding.rst | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/developers/sdk/embedding.rst b/docs/developers/sdk/embedding.rst index 86c912d153..dca652ec06 100644 --- a/docs/developers/sdk/embedding.rst +++ b/docs/developers/sdk/embedding.rst @@ -57,7 +57,19 @@ Available options ``basePath``: Optional, but highly recommended. The SDK considers this as the base URL and builds all - URLs relatively to this URL. If not provided, ``window.location.pathname`` is used. + URLs relatively to this URL. + + If not provided, ``window.location.pathname`` is used. + + .. note:: + ``basePath`` only applies when using the default browser routing. If hash based routing + is used (see ``useHashRouting`` below), the option will be silently ignored. + +``useHashRouting``: + Whether hash based routing should be used. Defaults to ``false``. This option is useful when embedding + Open Forms with a CMS. If the SDK is hosted at ``https://example.com/basepath?cms_query=1``, the resulting URL + would be ``https://example.com/basepath?cms_query=1#/startpagina`` (SDK specific query parameters would come + at the end of the URL). ``CSPNonce``: Recommended. The page's CSP Nonce value if inline styles are blocked by your From 2dbe60804fd8845a297eac4d0060380c66526d2f Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Fri, 17 Nov 2023 11:41:45 +0100 Subject: [PATCH 6/6] [#3362] PR feedback --- docs/developers/backend/core/testing-tools.rst | 6 ++++++ docs/developers/sdk/embedding.rst | 4 ++++ src/openforms/appointments/tests/test_views.py | 6 +++--- src/openforms/frontend/__init__.py | 3 +++ src/openforms/{ => frontend}/frontend.py | 0 .../{tests/mixins.py => frontend/tests.py} | 4 ++-- .../{0079_cleanup_urls.py => 0003_cleanup_urls.py} | 13 +++++++++---- .../submissions/tests/test_resume_form_view.py | 4 ++-- src/openforms/submissions/tests/test_views.py | 4 ++-- src/openforms/tests/test_frontend_utils.py | 8 ++++---- 10 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 src/openforms/frontend/__init__.py rename src/openforms/{ => frontend}/frontend.py (100%) rename src/openforms/{tests/mixins.py => frontend/tests.py} (90%) rename src/openforms/submissions/migrations/{0079_cleanup_urls.py => 0003_cleanup_urls.py} (59%) diff --git a/docs/developers/backend/core/testing-tools.rst b/docs/developers/backend/core/testing-tools.rst index 84f98f6ee5..58e785f1ca 100644 --- a/docs/developers/backend/core/testing-tools.rst +++ b/docs/developers/backend/core/testing-tools.rst @@ -13,6 +13,12 @@ HTML assertions .. automodule:: openforms.utils.tests.webtest_base :members: +Frontend redirects +================== + +.. automodule:: openforms.frontend.tests + :members: + Migrations ========== diff --git a/docs/developers/sdk/embedding.rst b/docs/developers/sdk/embedding.rst index dca652ec06..664f454b09 100644 --- a/docs/developers/sdk/embedding.rst +++ b/docs/developers/sdk/embedding.rst @@ -71,6 +71,10 @@ Available options would be ``https://example.com/basepath?cms_query=1#/startpagina`` (SDK specific query parameters would come at the end of the URL). + .. warning:: + This is a last resort solution - preferably the backend where you embed the form would set up "wildcard" routes to + ensure that refreshing the page works, e.g. ``/some-path//*`` should just load the CMS page for a specific form. + ``CSPNonce``: Recommended. The page's CSP Nonce value if inline styles are blocked by your `Content Security Policy `_. The Open diff --git a/src/openforms/appointments/tests/test_views.py b/src/openforms/appointments/tests/test_views.py index 80085c91ae..5c497d38c3 100644 --- a/src/openforms/appointments/tests/test_views.py +++ b/src/openforms/appointments/tests/test_views.py @@ -11,6 +11,7 @@ from openforms.authentication.constants import FORM_AUTH_SESSION_KEY, AuthAttribute from openforms.authentication.contrib.digid.constants import DIGID_DEFAULT_LOA from openforms.forms.tests.factories import FormFactory +from openforms.frontend.tests import FrontendRedirectMixin from openforms.logging.models import TimelineLogProxy from openforms.payments.constants import PaymentStatus from openforms.payments.tests.factories import SubmissionPaymentFactory @@ -20,14 +21,13 @@ SubmissionFactory, SubmissionStepFactory, ) -from openforms.tests.mixins import FrontendRedirectMixin from ..tokens import submission_appointment_token_generator from .factories import AppointmentFactory, AppointmentInfoFactory @freeze_time("2021-07-15T21:15:00Z") -class VerifyCancelAppointmentLinkViewTests(TestCase, FrontendRedirectMixin): +class VerifyCancelAppointmentLinkViewTests(FrontendRedirectMixin, TestCase): def test_good_token_and_submission_redirect_and_add_submission_to_session(self): submission = SubmissionFactory.create( completed=True, form_url="http://maykinmedia.nl/myform" @@ -341,7 +341,7 @@ def test_invalid_auth_value_raises_exception(self): @freeze_time("2021-07-15T21:15:00Z") -class VerifyChangeAppointmentLinkViewTests(TestCase, FrontendRedirectMixin): +class VerifyChangeAppointmentLinkViewTests(FrontendRedirectMixin, TestCase): def test_good_token_and_submission_redirect_and_add_submission_to_session(self): submission = SubmissionFactory.from_components( completed=True, diff --git a/src/openforms/frontend/__init__.py b/src/openforms/frontend/__init__.py new file mode 100644 index 0000000000..2c624b19a0 --- /dev/null +++ b/src/openforms/frontend/__init__.py @@ -0,0 +1,3 @@ +from .frontend import get_frontend_redirect_url + +__all__ = ["get_frontend_redirect_url"] diff --git a/src/openforms/frontend.py b/src/openforms/frontend/frontend.py similarity index 100% rename from src/openforms/frontend.py rename to src/openforms/frontend/frontend.py diff --git a/src/openforms/tests/mixins.py b/src/openforms/frontend/tests.py similarity index 90% rename from src/openforms/tests/mixins.py rename to src/openforms/frontend/tests.py index cbe125f489..82d61cda9d 100644 --- a/src/openforms/tests/mixins.py +++ b/src/openforms/frontend/tests.py @@ -6,7 +6,7 @@ from furl import furl -from openforms.frontend import SDKAction +from .frontend import SDKAction class FrontendRedirectMixin: @@ -26,7 +26,7 @@ def assertRedirectsToFrontend( :param frontend_base_url: The base URL of the frontend. :param action: The SDK action performed. :param action_params: Optional parameters for the action. - :param **kwargs: Additional kwargs to be passed to ``assertRedirects``. + :param `**kwargs`: Additional kwargs to be passed to :meth:`django.test.SimpleTestCase.assertRedirects`. """ expected_redirect_url = furl(frontend_base_url) diff --git a/src/openforms/submissions/migrations/0079_cleanup_urls.py b/src/openforms/submissions/migrations/0003_cleanup_urls.py similarity index 59% rename from src/openforms/submissions/migrations/0079_cleanup_urls.py rename to src/openforms/submissions/migrations/0003_cleanup_urls.py index 06892645b7..e3b63d7235 100644 --- a/src/openforms/submissions/migrations/0079_cleanup_urls.py +++ b/src/openforms/submissions/migrations/0003_cleanup_urls.py @@ -1,4 +1,5 @@ -# Generated by Django 3.2.21 on 2023-11-16 14:57 +# Generated by Django 3.2.21 on 2023-11-17 10:40 + from django.db import migrations from django.db.migrations.state import StateApps from django.db.backends.base.schema import BaseDatabaseSchemaEditor @@ -12,9 +13,11 @@ def cleanup_submission_urls( Submission = apps.get_model("submissions", "Submission") - for submission in Submission.objects.all(): + for submission in Submission.objects.iterator(): f = furl(submission.form_url) f.remove(fragment=True) + if f.path.segments[-1:] == ["startpagina"]: + f.path.segments.remove("startpagina") submission.form_url = f.url submission.save() @@ -23,7 +26,9 @@ def cleanup_submission_urls( class Migration(migrations.Migration): dependencies = [ - ("submissions", "0078_submission_finalised_registration_backend_key"), + ("submissions", "0002_change_json_encoder"), ] - operations = [migrations.RunPython(cleanup_submission_urls)] + operations = [ + migrations.RunPython(cleanup_submission_urls, migrations.RunPython.noop) + ] diff --git a/src/openforms/submissions/tests/test_resume_form_view.py b/src/openforms/submissions/tests/test_resume_form_view.py index 2d2f7265d1..60f93ddb90 100644 --- a/src/openforms/submissions/tests/test_resume_form_view.py +++ b/src/openforms/submissions/tests/test_resume_form_view.py @@ -10,14 +10,14 @@ from openforms.authentication.constants import FORM_AUTH_SESSION_KEY, AuthAttribute from openforms.authentication.contrib.digid.constants import DIGID_DEFAULT_LOA from openforms.config.models import GlobalConfiguration -from openforms.tests.mixins import FrontendRedirectMixin +from openforms.frontend.tests import FrontendRedirectMixin from ..constants import SUBMISSIONS_SESSION_KEY from ..tokens import submission_resume_token_generator from .factories import SubmissionFactory, SubmissionStepFactory -class SubmissionResumeViewTests(TestCase, FrontendRedirectMixin): +class SubmissionResumeViewTests(FrontendRedirectMixin, TestCase): def test_good_token_and_submission_redirect_and_add_submission_to_session(self): submission = SubmissionFactory.from_components( completed=True, diff --git a/src/openforms/submissions/tests/test_views.py b/src/openforms/submissions/tests/test_views.py index e98a2df4a2..bcde6e6e67 100644 --- a/src/openforms/submissions/tests/test_views.py +++ b/src/openforms/submissions/tests/test_views.py @@ -6,11 +6,11 @@ from openforms.authentication.constants import FORM_AUTH_SESSION_KEY from openforms.authentication.contrib.digid.constants import DIGID_DEFAULT_LOA +from openforms.frontend.tests import FrontendRedirectMixin from openforms.submissions.tests.factories import SubmissionFactory -from openforms.tests.mixins import FrontendRedirectMixin -class SearchSubmissionForCosignView(WebTest, FrontendRedirectMixin): +class SearchSubmissionForCosignView(FrontendRedirectMixin, WebTest): def setUp(self) -> None: super().setUp() diff --git a/src/openforms/tests/test_frontend_utils.py b/src/openforms/tests/test_frontend_utils.py index c82e8e8333..4de7b3561e 100644 --- a/src/openforms/tests/test_frontend_utils.py +++ b/src/openforms/tests/test_frontend_utils.py @@ -26,7 +26,7 @@ def test_frontend_redirect_no_hash(self): excpected_redirect_url.add( { "_of_action": "resume", - "_of_action_params": (json.dumps(action_params)), + "_of_action_params": json.dumps(action_params), } ) @@ -52,7 +52,7 @@ def test_frontend_redirect_hash(self): excpected_redirect_url.add( { "_of_action": "resume", - "_of_action_params": (json.dumps(action_params)), + "_of_action_params": json.dumps(action_params), } ) @@ -78,7 +78,7 @@ def test_frontend_redirect_special_chars(self): excpected_redirect_url.add( { "_of_action": "resume", - "_of_action_params": (json.dumps(action_params)), + "_of_action_params": json.dumps(action_params), } ) @@ -104,7 +104,7 @@ def test_frontend_redirect_query_param(self): excpected_redirect_url.add( { "_of_action": "resume", - "_of_action_params": (json.dumps(action_params)), + "_of_action_params": json.dumps(action_params), "unrelated_query": "1", } )