From 7d6f63590a6172c21205e6de7ec349e24eda9be8 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 27 Nov 2024 15:08:24 +0100 Subject: [PATCH] :bug: [#4862] Prevent submissions identifying attributes hashing if they still need cosigning A submission is only processed/registered when it's cosigned, so we may definitely not hash the identifying attributes when it's still pending cosigning. The logout endpoint can't tell if the cosigner or the original author is logged in to perform the cosigning, so we must look at the state of the submission itself. Backport-of: #4865 --- .../authentication/api/tests/test_logout.py | 48 +++++++++++++++++++ src/openforms/authentication/api/views.py | 13 +++-- .../submissions/models/submission.py | 30 ++++++++++++ 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/openforms/authentication/api/tests/test_logout.py b/src/openforms/authentication/api/tests/test_logout.py index d01735ac4f..833688f796 100644 --- a/src/openforms/authentication/api/tests/test_logout.py +++ b/src/openforms/authentication/api/tests/test_logout.py @@ -2,12 +2,14 @@ from unittest.mock import Mock, patch from django.contrib.auth import SESSION_KEY +from django.test import tag from django.urls import reverse from rest_framework import status from rest_framework.test import APITestCase from openforms.accounts.tests.factories import StaffUserFactory +from openforms.authentication.contrib.digid.constants import DIGID_DEFAULT_LOA from openforms.submissions.tests.factories import SubmissionFactory from openforms.submissions.tests.mixins import SubmissionsMixin @@ -146,3 +148,49 @@ def test_logout_non_existing_plugin(self): self.assertNotIn(FORM_AUTH_SESSION_KEY, session) self.assertNotIn(REGISTRATOR_SUBJECT_SESSION_KEY, session) + + @tag("gh-4862", "taiga-wdw-57") + def test_logout_as_cosigner(self): + """ + A cosigner logging out may not result in auth attributes being hashed. + + The BSN/KVK is still needed for the full registration phase. + """ + submission = SubmissionFactory.from_components( + form__slug="form-to-cosign", + form__authentication_backends=["digid"], + components_list=[{"type": "cosign", "key": "cosign"}], + submitted_data={"cosign": "test@example.com"}, + auth_info__value="111222333", + completed=True, + cosign_complete=False, + pre_registration_completed=True, + public_registration_reference="OF-9999", + ) + # simulate logging in as cosigner and starting the cosign flow + session = self.client.session + session[FORM_AUTH_SESSION_KEY] = { + "plugin": "digid", + "attribute": "bsn", + "value": "123456782", + "loa": DIGID_DEFAULT_LOA, + } + session.save() + view_url = reverse( + "submissions:find-submission-for-cosign", + kwargs={"form_slug": "form-to-cosign"}, + ) + response = self.client.post( + view_url, data={"code": "OF-9999"}, format="multipart" + ) + assert response.status_code == 302 + + url = reverse("api:submission-logout", kwargs={"uuid": submission.uuid}) + + response = self.client.delete(url) + + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + submission.refresh_from_db() + self.assertEqual(submission.auth_info.value, "111222333") + uuids = self._get_session_submission_uuids() + self.assertNotIn(str(submission.uuid), uuids) diff --git a/src/openforms/authentication/api/views.py b/src/openforms/authentication/api/views.py index 7e21661b2d..2e7fa5ae5b 100644 --- a/src/openforms/authentication/api/views.py +++ b/src/openforms/authentication/api/views.py @@ -68,11 +68,14 @@ def delete(self, request, *args, **kwargs): plugin = register[submission.auth_info.plugin] plugin.logout(request) - if not submission.auth_info.attribute_hashed: - submission.auth_info.hash_identifying_attributes() - - if not submission.registrator.attribute_hashed: - submission.registrator.hash_identifying_attributes() + if submission.is_ready_to_hash_identifying_attributes: + if not submission.auth_info.attribute_hashed: + submission.auth_info.hash_identifying_attributes() + + if ( + registrator := submission.registrator + ) and not registrator.attribute_hashed: + registrator.hash_identifying_attributes() if FORM_AUTH_SESSION_KEY in request.session: del request.session[FORM_AUTH_SESSION_KEY] diff --git a/src/openforms/submissions/models/submission.py b/src/openforms/submissions/models/submission.py index c6c63311b7..d02703e7b7 100644 --- a/src/openforms/submissions/models/submission.py +++ b/src/openforms/submissions/models/submission.py @@ -469,6 +469,36 @@ def has_registrator(self): def is_completed(self): return bool(self.completed_on) + @property + def is_ready_to_hash_identifying_attributes(self) -> bool: + """ + Check if the submission can safely hash the identifying auth attributes. + """ + # no authentication -> there's nothing to hash + if not self.is_authenticated: + return False + # completed, but not cosigned yet -> registration after cosigning requires + # unhashed attributes + if self.is_completed and self.waiting_on_cosign: + return False + + # the submission has not been submitted/completed/finalized, so it will + # definitely not be processed yet -> okay to hash + if not self.is_completed: + return True + + # fully registered, no more processing needed -> safe to hash + if self.is_registered: + return True + + # otherwise assume it's not safe yet + return False + + @property + def is_registered(self) -> bool: + # success is set if it succeeded or there was no registration backend configured + return self.registration_status == RegistrationStatuses.success + @transaction.atomic() def remove_sensitive_data(self): from .submission_files import SubmissionFileAttachment