Merge pull request #4865 from open-formulieren/issue/4862-hashed-bsn-…

…pause-and-cosign

Fix unintended hashing of identifying attributes when the cosigner logs out

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)

src/openforms/authentication/api/views.py

@@ -39,7 +39,7 @@ def get_objects(self):
         return list(register.iter_enabled_plugins())
 
 
-class SubmissionLogoutView(GenericAPIView):
+class SubmissionLogoutView(GenericAPIView[Submission]):
     authentication_classes = (AnonCSRFSessionAuthentication,)
     permission_classes = (ActiveSubmissionPermission,)
     serializer_class = (
@@ -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 submission.is_ready_to_hash_identifying_attributes:
+                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 (
+                    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]

src/openforms/submissions/models/submission.py

@@ -451,7 +451,7 @@ def form_login_required(self):
         return self.form.login_required
 
     @property
-    def is_authenticated(self):
+    def is_authenticated(self) -> bool:
         return hasattr(self, "auth_info")
 
     @property
@@ -469,8 +469,34 @@ 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()

src/openforms/submissions/tasks/cleanup.py

@@ -8,7 +8,6 @@
 
 from openforms.celery import app
 
-from ..constants import RegistrationStatuses
 from ..models import Submission
 from ..status import SubmissionProcessingStatus
 from ..tokens import submission_status_token_generator
@@ -74,11 +73,7 @@ def maybe_hash_identifying_attributes(submission_id: int) -> None:
     submission = Submission.objects.get(id=submission_id)
     assert submission.completed_on is not None, "Submission must be completed first"
 
-    # success is set if it succeeded or there was no registration backend configured
-    if submission.registration_status != RegistrationStatuses.success:
-        return
-
-    if not submission.is_authenticated:
+    if not submission.is_ready_to_hash_identifying_attributes:
         return
 
     if not submission.auth_info.attribute_hashed: