diff --git a/src/openforms/prefill/sources.py b/src/openforms/prefill/sources.py index ef68e0be0f..3b52e44c95 100644 --- a/src/openforms/prefill/sources.py +++ b/src/openforms/prefill/sources.py @@ -1,6 +1,8 @@ import logging from collections import defaultdict +from django.core.exceptions import PermissionDenied + import elasticapm from rest_framework.exceptions import ValidationError from zgw_consumers.concurrent import parallel @@ -54,11 +56,6 @@ def invoke_plugin( if not plugin.is_enabled: raise PluginNotEnabled() - # If an `initial_data_reference` was passed, we must verify that the - # authenticated user is the owner of the referenced object - if submission.initial_data_reference: - plugin.verify_initial_data_ownership(submission) - attributes = [attribute for field in fields for attribute in field] try: values = plugin.get_prefill_values(submission, attributes, identifier_role) @@ -101,6 +98,16 @@ def fetch_prefill_values_from_options( values: dict[str, JSONEncodable] = {} for variable in variables: plugin = register[variable.form_variable.prefill_plugin] + + # If an `initial_data_reference` was passed, we must verify that the + # authenticated user is the owner of the referenced object + try: + if submission.initial_data_reference: + plugin.verify_initial_data_ownership(submission) + except PermissionDenied as exc: + logevent.prefill_retrieve_failure(submission, plugin, exc) + continue + options_serializer = plugin.options(data=variable.form_variable.prefill_options) try: diff --git a/src/openforms/prefill/tests/test_prefill_variables.py b/src/openforms/prefill/tests/test_prefill_variables.py index 4944c14b7a..8810547817 100644 --- a/src/openforms/prefill/tests/test_prefill_variables.py +++ b/src/openforms/prefill/tests/test_prefill_variables.py @@ -1,7 +1,7 @@ from unittest.mock import patch from django.core.exceptions import PermissionDenied -from django.test import RequestFactory, TestCase, TransactionTestCase +from django.test import RequestFactory, TestCase, TransactionTestCase, tag import requests_mock from zgw_consumers.test.factories import ServiceFactory @@ -19,7 +19,6 @@ FormVariableFactory, ) from openforms.logging.models import TimelineLogProxy -from openforms.prefill.contrib.demo.constants import Attributes from openforms.submissions.constants import SubmissionValueVariableSources from openforms.submissions.tests.factories import ( SubmissionFactory, @@ -321,6 +320,7 @@ def test_no_success_message_on_failure(self, m, m_solo): for log in logs: self.assertNotEqual(log.event, "prefill_retrieve_success") + @tag("gh-4398") def test_verify_initial_data_ownership(self): form_step = FormStepFactory.create( form_definition__configuration={ @@ -329,15 +329,34 @@ def test_verify_initial_data_ownership(self): "type": "postcode", "key": "postcode", "inputMask": "9999 AA", - "prefill": { - "plugin": "demo", - "attribute": Attributes.random_string, - }, - "defaultValue": "", } ] } ) + # FormVariableFactory.create( + # key="voornamen", + # form=form_step.form, + # prefill_plugin="objects_api", + # prefill_attribute="", + # prefill_options={ + # "version": 2, + # "objecttype": "3edfdaf7-f469-470b-a391-bb7ea015bd6f", + # "objects_api_group": 1, + # "objecttype_version": 1, + # }, + # ) + FormVariableFactory.create( + key="voornamen", + form=form_step.form, + prefill_plugin="demo", + prefill_attribute="", + prefill_options={ + "version": 2, + "objecttype": "3edfdaf7-f469-470b-a391-bb7ea015bd6f", + "objects_api_group": 1, + "objecttype_version": 1, + }, + ) with self.subTest( "verify_initial_data_ownership is not called if initial_data_reference is not specified" @@ -352,9 +371,13 @@ def test_verify_initial_data_ownership(self): with patch( "openforms.prefill.contrib.demo.plugin.DemoPrefill.verify_initial_data_ownership" ) as mock_verify_ownership: - prefill_variables(submission=submission_step.submission) + with patch( + "openforms.prefill.contrib.demo.plugin.DemoPrefill.get_prefill_values_from_options", + return_value={"postcode": "1234AB"}, + ): + prefill_variables(submission=submission_step.submission) - mock_verify_ownership.assert_not_called() + mock_verify_ownership.assert_not_called() logs = TimelineLogProxy.objects.filter( object_id=submission_step.submission.id @@ -377,11 +400,15 @@ def test_verify_initial_data_ownership(self): with patch( "openforms.prefill.contrib.demo.plugin.DemoPrefill.verify_initial_data_ownership" ) as mock_verify_ownership: - prefill_variables(submission=submission_step.submission) + with patch( + "openforms.prefill.contrib.demo.plugin.DemoPrefill.get_prefill_values_from_options", + return_value={"postcode": "1234AB"}, + ): + prefill_variables(submission=submission_step.submission) - mock_verify_ownership.assert_called_once_with( - submission_step.submission - ) + mock_verify_ownership.assert_called_once_with( + submission_step.submission + ) logs = TimelineLogProxy.objects.filter( object_id=submission_step.submission.id @@ -405,8 +432,7 @@ def test_verify_initial_data_ownership(self): "openforms.prefill.contrib.demo.plugin.DemoPrefill.verify_initial_data_ownership", side_effect=PermissionDenied, ) as mock_verify_ownership: - with self.assertRaises(PermissionDenied): - prefill_variables(submission=submission_step.submission) + prefill_variables(submission=submission_step.submission) mock_verify_ownership.assert_called_once_with( submission_step.submission @@ -418,3 +444,6 @@ def test_verify_initial_data_ownership(self): self.assertEqual( logs.filter(extra_data__log_event="prefill_retrieve_success").count(), 0 ) + self.assertEqual( + logs.filter(extra_data__log_event="prefill_retrieve_failure").count(), 1 + )