Skip to content

Commit

Permalink
🐛 [#4862] Prevent submissions identifying attributes hashing if they …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
sergei-maertens committed Nov 27, 2024
1 parent 91a04cb commit a9eeb04
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 5 deletions.
48 changes: 48 additions & 0 deletions src/openforms/authentication/api/tests/test_logout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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": "[email protected]"},
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)
13 changes: 8 additions & 5 deletions src/openforms/authentication/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
30 changes: 30 additions & 0 deletions src/openforms/submissions/models/submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a9eeb04

Please sign in to comment.