From 685faa559ef0049a3e49cf6e11dd54d5dd316e9a Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 23 May 2024 15:17:29 +0200 Subject: [PATCH 1/5] :sparkles: [#4199] Optional anonymous option for Submission viewset to allow the SDK to explicitly indicate that the submission is without authentication --- src/openapi.yaml | 5 +++++ src/openforms/authentication/signals.py | 5 ++++- src/openforms/submissions/api/serializers.py | 13 +++++++++++++ src/openforms/submissions/api/viewsets.py | 5 ++++- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/openapi.yaml b/src/openapi.yaml index eb42f419f9..cee680eda6 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -9600,6 +9600,11 @@ components: format: uri description: URL where the user initialized the submission. maxLength: 1000 + anonymous: + type: boolean + writeOnly: true + default: false + description: Whether the submission was started anonymously or not. required: - form - formUrl diff --git a/src/openforms/authentication/signals.py b/src/openforms/authentication/signals.py index 06a1570f1a..2b638dbebb 100644 --- a/src/openforms/authentication/signals.py +++ b/src/openforms/authentication/signals.py @@ -53,8 +53,11 @@ [submission_start, submission_resumed], dispatch_uid="auth.set_submission_form_auth" ) def set_auth_attribute_on_session( - sender, instance: Submission, request: Request, **kwargs + sender, instance: Submission, request: Request, anonymous=False, **kwargs ): + if anonymous: + return + # form_auth has information from an authentication backend, so could be a client or employee form_auth = request.session.get(FORM_AUTH_SESSION_KEY) diff --git a/src/openforms/submissions/api/serializers.py b/src/openforms/submissions/api/serializers.py index d67da03b4e..29737e89c4 100644 --- a/src/openforms/submissions/api/serializers.py +++ b/src/openforms/submissions/api/serializers.py @@ -148,6 +148,14 @@ class SubmissionSerializer(serializers.HyperlinkedModelSerializer): read_only=True, ) + anonymous = serializers.BooleanField( + label=_("Anonymous"), + help_text=_("Whether the submission was started anonymously or not."), + required=False, + write_only=True, + default=False, # type: ignore + ) + class Meta: model = Submission fields = ( @@ -159,6 +167,7 @@ class Meta: "is_authenticated", "payment", "form_url", + "anonymous", ) extra_kwargs = { "id": { @@ -183,6 +192,10 @@ class Meta: }, } + def create(self, validated_data): + validated_data.pop("anonymous", None) + return super().create(validated_data) + def to_representation(self, instance): check_submission_logic(instance, unsaved_data=self.context.get("unsaved_data")) return super().to_representation(instance) diff --git a/src/openforms/submissions/api/viewsets.py b/src/openforms/submissions/api/viewsets.py index 1e2f9ec2a0..687f1eb770 100644 --- a/src/openforms/submissions/api/viewsets.py +++ b/src/openforms/submissions/api/viewsets.py @@ -166,7 +166,10 @@ def perform_create(self, serializer): # dispatch signal for modules to tap into submission_start.send( - sender=self.__class__, instance=serializer.instance, request=self.request + sender=self.__class__, + instance=serializer.instance, + request=self.request, + anonymous=serializer.validated_data["anonymous"], ) # store the submission ID in the session, so that only the session owner can From a47459ff6231dfe90933b2f5f1eb386d1fd8b5cb Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 23 May 2024 15:21:41 +0200 Subject: [PATCH 2/5] :white_check_mark: [#4199] Add/fix tests for starting anonymous submission --- .../tests/test_authentication_requirements.py | 9 +++- .../tests/test_start_submission.py | 44 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/openforms/submissions/tests/test_authentication_requirements.py b/src/openforms/submissions/tests/test_authentication_requirements.py index 84aab67273..249adf9b6f 100644 --- a/src/openforms/submissions/tests/test_authentication_requirements.py +++ b/src/openforms/submissions/tests/test_authentication_requirements.py @@ -11,6 +11,8 @@ If authentication is optional, then this behaviour does not apply. """ +from unittest.mock import patch + from django.test import override_settings, tag from rest_framework import status @@ -51,16 +53,21 @@ def setUpTestData(cls) -> None: "api:form-detail", kwargs={"uuid_or_slug": cls.form.uuid} ) - def test_start_submission_is_allowed(self): + @patch("openforms.submissions.api.viewsets.submission_start.send", autospec=True) + def test_start_submission_is_allowed(self, mock_signal): body = { "form": f"http://testserver.com{self.form_url}", "formUrl": "http://testserver.com/my-form", + "anonymous": True, } response = self.client.post(self.endpoint, body, HTTP_HOST="testserver.com") self.assertEqual(response.status_code, status.HTTP_201_CREATED) + mock_signal.assert_called_once() + self.assertEqual(mock_signal.call_args_list[0].kwargs["anonymous"], True) + def test_submitting_step_data_is_allowed_anon_user(self): submission = SubmissionFactory.create(form=self.form) assert not submission.is_authenticated, "Submission must be anonymous" diff --git a/src/openforms/submissions/tests/test_start_submission.py b/src/openforms/submissions/tests/test_start_submission.py index 6160606a22..1965aa7de1 100644 --- a/src/openforms/submissions/tests/test_start_submission.py +++ b/src/openforms/submissions/tests/test_start_submission.py @@ -16,7 +16,7 @@ from unittest.mock import patch -from django.test import override_settings +from django.test import override_settings, tag from rest_framework import status from rest_framework.reverse import reverse, reverse_lazy @@ -125,6 +125,48 @@ def test_start_submission_bsn_in_session(self): self.assertEqual(submission.auth_info.value, "123456782") self.assertEqual(submission.auth_info.plugin, "digid") + # Auth info should be removed from session after the submission is started + self.assertNotIn(FORM_AUTH_SESSION_KEY, self.client.session) + + @tag("gh-4199") + def test_two_submissions_within_same_session(self): + session = self.client.session + session[FORM_AUTH_SESSION_KEY] = { + "plugin": "digid", + "attribute": AuthAttribute.bsn, + "value": "123456782", + } + session.save() + + body = { + "form": f"http://testserver.com{self.form_url}", + "formUrl": "http://testserver.com/my-form", + } + + response = self.client.post(self.endpoint, body) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + submission = Submission.objects.get() + self.assertEqual(submission.auth_info.value, "123456782") + self.assertEqual(submission.auth_info.plugin, "digid") + + # Auth info should be removed from session after the submission is started + self.assertNotIn(FORM_AUTH_SESSION_KEY, self.client.session) + + body = { + "form": f"http://testserver.com{self.form_url}", + "formUrl": "http://testserver.com/my-form", + "anonymous": True, + } + + response = self.client.post(self.endpoint, body) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + submission = Submission.objects.last() + with self.assertRaises(Submission.auth_info.RelatedObjectDoesNotExist): + submission.auth_info + def test_start_submission_on_deleted_form(self): form = FormFactory.create(deleted_=True) FormStepFactory.create(form=form) From cce44520a0b62fea295ccbba2c91e6ae60a90fa5 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 23 May 2024 15:18:56 +0200 Subject: [PATCH 3/5] :bug: [#4199] Delete authinfo from session after it's stored on submission or after the cosign data is set, previously an anonymous submission that is started after the user started an authenticated submission within the same session would incorrectly get the same authentication information attached to it as the authenticated submission --- src/openforms/authentication/service.py | 2 ++ src/openforms/authentication/signals.py | 13 ++++++++++++- src/openforms/authentication/utils.py | 6 ++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/openforms/authentication/service.py b/src/openforms/authentication/service.py index cea4f24ccd..43dce1ff4d 100644 --- a/src/openforms/authentication/service.py +++ b/src/openforms/authentication/service.py @@ -3,6 +3,7 @@ from .utils import ( is_authenticated_with_an_allowed_plugin, is_authenticated_with_plugin, + remove_auth_info_from_session, store_auth_details, ) @@ -13,4 +14,5 @@ "store_auth_details", "is_authenticated_with_plugin", "is_authenticated_with_an_allowed_plugin", + "remove_auth_info_from_session", ] diff --git a/src/openforms/authentication/signals.py b/src/openforms/authentication/signals.py index 2b638dbebb..941556b395 100644 --- a/src/openforms/authentication/signals.py +++ b/src/openforms/authentication/signals.py @@ -18,7 +18,11 @@ from .constants import FORM_AUTH_SESSION_KEY, REGISTRATOR_SUBJECT_SESSION_KEY from .registry import register -from .utils import store_auth_details, store_registrator_details +from .utils import ( + remove_auth_info_from_session, + store_auth_details, + store_registrator_details, +) logger = logging.getLogger(__name__) @@ -126,6 +130,11 @@ def set_auth_attribute_on_session( else: store_auth_details(instance, form_auth) + # After the authentication details have been attached to the submission, the session + # no longer has to keep track of this information, because the user could start another + # form with different / no authentication + remove_auth_info_from_session(request) + @receiver( [submission_complete, authentication_logout], @@ -148,3 +157,5 @@ def set_cosign_data_on_submission( instance.co_sign_data = form_auth instance.co_sign_data["cosign_date"] = timezone.now().isoformat() instance.save() + + remove_auth_info_from_session(request) diff --git a/src/openforms/authentication/utils.py b/src/openforms/authentication/utils.py index cac89658ad..ba169a8234 100644 --- a/src/openforms/authentication/utils.py +++ b/src/openforms/authentication/utils.py @@ -4,6 +4,7 @@ from openforms.forms.models import Form from openforms.submissions.models import Submission +from openforms.typing import AnyRequest from .constants import FORM_AUTH_SESSION_KEY, AuthAttribute from .models import AuthInfo, RegistratorInfo @@ -81,3 +82,8 @@ def get_cosign_login_url(request: Request, form: Form, plugin_id: str) -> str: auth_page = furl(auth_url) auth_page.args.set("next", next_url) return auth_page.url + + +def remove_auth_info_from_session(request: AnyRequest) -> None: + if FORM_AUTH_SESSION_KEY in request.session: + del request.session[FORM_AUTH_SESSION_KEY] From 40bb9fd19bd9e03fb2bc0e259fca09ac1612e731 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 23 May 2024 15:20:50 +0200 Subject: [PATCH 4/5] :white_check_mark: [#4199] Add/fix tests for removing authinfo from session --- .../digid/tests/test_signicat_integration.py | 2 +- .../tests/test_signicat_integration.py | 2 +- .../authentication/tests/test_utils.py | 29 ++++++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/openforms/authentication/contrib/digid/tests/test_signicat_integration.py b/src/openforms/authentication/contrib/digid/tests/test_signicat_integration.py index 4b205aaf54..f2238a3cd2 100644 --- a/src/openforms/authentication/contrib/digid/tests/test_signicat_integration.py +++ b/src/openforms/authentication/contrib/digid/tests/test_signicat_integration.py @@ -515,7 +515,7 @@ def test_raising_loa_requirements_fails_in_flight_authentications(self): acs_response_2 = self.client.get(acs_url_2.url, follow=True) # we are logged in - self.assertIn(FORM_AUTH_SESSION_KEY, self.client.session) + self.assertTrue(submission.is_authenticated) # and we end up starting the form self.assertEqual(acs_response_2.resolver_match.view_name, "core:form-detail") self.assertEqual(acs_response_2.resolver_match.kwargs["slug"], "slurm") diff --git a/src/openforms/authentication/contrib/eherkenning/tests/test_signicat_integration.py b/src/openforms/authentication/contrib/eherkenning/tests/test_signicat_integration.py index 304db7ebc7..2a184b33b9 100644 --- a/src/openforms/authentication/contrib/eherkenning/tests/test_signicat_integration.py +++ b/src/openforms/authentication/contrib/eherkenning/tests/test_signicat_integration.py @@ -525,7 +525,7 @@ def test_raising_loa_requirements_fails_in_flight_authentications(self): acs_response_2 = self.client.get(acs_url_2.url, follow=True) # we are logged in - self.assertIn(FORM_AUTH_SESSION_KEY, self.client.session) + self.assertTrue(submission.is_authenticated) # and we end up starting the form self.assertEqual(acs_response_2.resolver_match.view_name, "core:form-detail") self.assertEqual(acs_response_2.resolver_match.kwargs["slug"], "slurm") diff --git a/src/openforms/authentication/tests/test_utils.py b/src/openforms/authentication/tests/test_utils.py index 84fa881299..3f1e2e7e61 100644 --- a/src/openforms/authentication/tests/test_utils.py +++ b/src/openforms/authentication/tests/test_utils.py @@ -2,11 +2,19 @@ from django.urls import reverse from furl import furl +from rest_framework.test import APIRequestFactory +from openforms.authentication.contrib.digid.constants import DIGID_DEFAULT_LOA from openforms.forms.tests.factories import FormStepFactory from openforms.submissions.tests.factories import SubmissionFactory -from ..utils import get_cosign_login_url, store_auth_details, store_registrator_details +from ..constants import FORM_AUTH_SESSION_KEY, AuthAttribute +from ..utils import ( + get_cosign_login_url, + remove_auth_info_from_session, + store_auth_details, + store_registrator_details, +) class UtilsTests(TestCase): @@ -71,3 +79,22 @@ def test_get_cosign_info(self): kwargs={"form_slug": "form-with-cosign"}, ), ) + + def test_remove_auth_info_from_session(self): + factory = APIRequestFactory() + request = factory.get("/foo") + request.session = self.client.session + + request.session[FORM_AUTH_SESSION_KEY] = { + "plugin": "digid", + "attribute": AuthAttribute.bsn, + "value": "123456782", + "loa": DIGID_DEFAULT_LOA, + } + + remove_auth_info_from_session(request) + + self.assertNotIn(FORM_AUTH_SESSION_KEY, request.session) + + # Should not raise any errors if the key is not present in the session + remove_auth_info_from_session(request) From 49783deff87e94a1aff964e469b0e93bf01bc875 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 23 May 2024 15:21:10 +0200 Subject: [PATCH 5/5] :green_heart: Fix broken eToegang link in docs --- docs/configuration/authentication/eherkenning_eidas.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/authentication/eherkenning_eidas.rst b/docs/configuration/authentication/eherkenning_eidas.rst index 7a8412b48e..da4d74490d 100644 --- a/docs/configuration/authentication/eherkenning_eidas.rst +++ b/docs/configuration/authentication/eherkenning_eidas.rst @@ -34,7 +34,7 @@ that describes the authenticated entity (person or company). EH4 hoog/high urn:etoegang:core:assurance-class:loa4 =========== =============== =========================================== - Source: `Afsprakenstelsel eToegang `_ + Source: `Afsprakenstelsel eToegang `_ .. note::