From a9eeb040232f1d9aebb9abf8e62e8aa006eacd94 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 79dd4c8f69..9797d301ad 100644 --- a/src/openforms/submissions/models/submission.py +++ b/src/openforms/submissions/models/submission.py @@ -456,6 +456,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