diff --git a/src/openforms/contrib/objects_api/ownership_validation.py b/src/openforms/contrib/objects_api/ownership_validation.py index 3a692014d8..ce7269b618 100644 --- a/src/openforms/contrib/objects_api/ownership_validation.py +++ b/src/openforms/contrib/objects_api/ownership_validation.py @@ -1,7 +1,6 @@ from __future__ import annotations import logging -from typing import TYPE_CHECKING from django.core.exceptions import PermissionDenied @@ -10,14 +9,12 @@ from openforms.contrib.objects_api.clients import ObjectsClient from openforms.logging import logevent +from openforms.prefill.base import BasePlugin as BasePrefillPlugin +from openforms.registrations.base import BasePlugin as BaseRegistrationPlugin +from openforms.submissions.models import Submission logger = logging.getLogger(__name__) -if TYPE_CHECKING: - from openforms.prefill.base import BasePlugin as BasePrefillPlugin - from openforms.registrations.base import BasePlugin as BaseRegistrationPlugin - from openforms.submissions.models import Submission - def validate_object_ownership( submission: Submission, @@ -63,7 +60,7 @@ def validate_object_ownership( ) try: - auth_value = glom(object["record"]["data"], Path(*(object_attribute or []))) + auth_value = glom(object["record"]["data"], Path(*object_attribute)) except PathAccessError as e: logger.exception( "Could not retrieve auth value for path %s, it could be incorrectly configured", diff --git a/src/openforms/logging/models.py b/src/openforms/logging/models.py index 54c9e33e50..41b6c1a149 100644 --- a/src/openforms/logging/models.py +++ b/src/openforms/logging/models.py @@ -1,3 +1,4 @@ +from django.contrib.contenttypes.models import ContentType from django.db import models from django.template.defaultfilters import capfirst from django.urls import reverse @@ -13,6 +14,11 @@ class TimelineLogProxyQueryset(models.QuerySet): + # vendored from https://github.com/maykinmedia/django-timeline-logger/pull/32/files + def for_object(self, obj: models.Model): + content_type = ContentType.objects.get_for_model(obj) + return self.filter(content_type=content_type, object_id=obj.pk) + def filter_event(self, event: str): return self.filter(extra_data__log_event=event) diff --git a/src/openforms/prefill/base.py b/src/openforms/prefill/base.py index 80ac16e049..81b6b61292 100644 --- a/src/openforms/prefill/base.py +++ b/src/openforms/prefill/base.py @@ -66,7 +66,7 @@ def get_prefill_values( raise NotImplementedError("You must implement the 'get_prefill_values' method.") def verify_initial_data_ownership( - self, submission: Submission, prefill_options: dict + self, submission: Submission, prefill_options: OptionsT ) -> None: """ Hook to check if the authenticated user is the owner of the object @@ -75,7 +75,8 @@ def verify_initial_data_ownership( If any error occurs in this check, it should raise a `PermissionDenied` :param submission: an active :class:`Submission` instance - :param prefill_options: a dictionary containing the configuration options + :param prefill_options: the configuration options, after validation and + deserialization through the :attr:`options` serializer class. """ raise NotImplementedError( "You must implement the 'verify_initial_data_ownership' method." diff --git a/src/openforms/prefill/contrib/objects_api/api/serializers.py b/src/openforms/prefill/contrib/objects_api/api/serializers.py index 91dd04a5c3..5470fdb307 100644 --- a/src/openforms/prefill/contrib/objects_api/api/serializers.py +++ b/src/openforms/prefill/contrib/objects_api/api/serializers.py @@ -24,12 +24,15 @@ class PrefillTargetPathsSerializer(serializers.Serializer): class ObjecttypeVariableMappingSerializer(serializers.Serializer): - """A mapping between a form variable key and the corresponding Objecttype attribute.""" + """ + A mapping between a form variable key and the corresponding Objecttype attribute. + """ variable_key = FormioVariableKeyField( label=_("variable key"), help_text=_( - "The 'dotted' path to a form variable key. The format should comply to how Formio handles nested component keys." + "The 'dotted' path to a form variable key. The format should comply to how " + "Formio handles nested component keys." ), ) target_path = serializers.ListField( @@ -47,24 +50,25 @@ class ObjectsAPIOptionsSerializer(JsonSchemaSerializerMixin, serializers.Seriali Q(objects_service=None) | Q(objecttypes_service=None) ), label=("Objects API group"), - required=False, + required=True, help_text=_("Which Objects API group to use."), ) objecttype_uuid = serializers.UUIDField( label=_("objecttype"), - required=False, + required=True, help_text=_("UUID of the objecttype in the Objecttypes API. "), ) objecttype_version = serializers.IntegerField( label=_("objecttype version"), - required=False, + required=True, help_text=_("Version of the objecttype in the Objecttypes API."), ) auth_attribute_path = serializers.ListField( child=serializers.CharField(label=_("Segment of a JSON path")), label=_("Path to auth attribute (e.g. BSN/KVK) in objects"), help_text=_( - "This is used to perform validation to verify that the authenticated user is the owner of the object." + "This is used to perform validation to verify that the authenticated " + "user is the owner of the object." ), allow_empty=False, required=True, @@ -72,5 +76,5 @@ class ObjectsAPIOptionsSerializer(JsonSchemaSerializerMixin, serializers.Seriali variables_mapping = ObjecttypeVariableMappingSerializer( label=_("variables mapping"), many=True, - required=False, + required=True, ) diff --git a/src/openforms/prefill/contrib/objects_api/plugin.py b/src/openforms/prefill/contrib/objects_api/plugin.py index f0467796ad..fbab2f032f 100644 --- a/src/openforms/prefill/contrib/objects_api/plugin.py +++ b/src/openforms/prefill/contrib/objects_api/plugin.py @@ -32,32 +32,26 @@ class ObjectsAPIPrefill(BasePlugin[ObjectsAPIOptions]): options = ObjectsAPIOptionsSerializer def verify_initial_data_ownership( - self, submission: Submission, prefill_options: dict + self, submission: Submission, prefill_options: ObjectsAPIOptions ) -> None: assert submission.initial_data_reference - api_group = ObjectsAPIGroupConfig.objects.filter( - pk=prefill_options.get("objects_api_group") - ).first() + api_group = prefill_options["objects_api_group"] + assert api_group, "Can't do anything useful without an API group" - if not api_group: - logger.info( - "No api group found to perform initial_data_reference ownership check for submission %s with options %s", - submission, - prefill_options, - ) - return - - auth_attribute_path = prefill_options.get("auth_attribute_path") + auth_attribute_path = prefill_options["auth_attribute_path"] if not auth_attribute_path: logger.info( - "Cannot perform initial data ownership check, because `auth_attribute_path` is missing from %s", + "Cannot perform initial data ownership check, because " + "`auth_attribute_path` is unset in the options %r", prefill_options, + extra={"options": prefill_options}, ) logevent.object_ownership_check_improperly_configured( submission, plugin=self ) raise PermissionDenied( - f"`auth_attribute_path` missing from options {prefill_options}, cannot perform initial data ownership check" + "Can't run ownership check because the 'auth_attribute_path' option is " + "not set." ) with get_objects_client(api_group) as client: diff --git a/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/ObjectsAPIPrefillDataOwnershipCheckTests.test_verify_initial_data_ownership_called_if_initial_data_reference_specified.yaml b/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/ObjectsAPIPrefillDataOwnershipCheckTests.test_verify_initial_data_ownership_called_if_initial_data_reference_specified.yaml index 614f90c9f6..b7e744516c 100644 --- a/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/ObjectsAPIPrefillDataOwnershipCheckTests.test_verify_initial_data_ownership_called_if_initial_data_reference_specified.yaml +++ b/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/ObjectsAPIPrefillDataOwnershipCheckTests.test_verify_initial_data_ownership_called_if_initial_data_reference_specified.yaml @@ -15,10 +15,10 @@ interactions: User-Agent: - python-requests/2.32.2 method: GET - uri: http://localhost:8002/api/v2/objects/bd6a9316-f721-40e4-9d4d-73ada5590952 + uri: http://localhost:8002/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea response: body: - string: '{"url":"http://objects-web:8000/api/v2/objects/bd6a9316-f721-40e4-9d4d-73ada5590952","uuid":"bd6a9316-f721-40e4-9d4d-73ada5590952","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","some":{"path":"foo"}},"geometry":null,"startAt":"2024-11-26","endAt":null,"registrationAt":"2024-11-26","correctionFor":null,"correctedBy":null}}' + string: '{"url":"http://objects-web:8000/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea","uuid":"c7ec7188-c8f5-4c13-bd74-942468077eea","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","some":{"path":"foo"}},"geometry":null,"startAt":"2024-12-03","endAt":null,"registrationAt":"2024-12-03","correctionFor":null,"correctedBy":null}}' headers: Allow: - GET, PUT, PATCH, DELETE, HEAD, OPTIONS @@ -33,11 +33,59 @@ interactions: Cross-Origin-Opener-Policy: - same-origin Date: - - Tue, 26 Nov 2024 13:21:31 GMT + - Tue, 03 Dec 2024 16:13:43 GMT Referrer-Policy: - same-origin Server: - - nginx/1.27.0 + - nginx/1.27.3 + Vary: + - origin + X-Content-Type-Options: + - nosniff + X-Frame-Options: + - DENY + status: + code: 200 + message: OK +- request: + body: null + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate, br + Authorization: + - Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9 + Connection: + - keep-alive + Content-Crs: + - EPSG:4326 + User-Agent: + - python-requests/2.32.2 + method: GET + uri: http://localhost:8002/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea + response: + body: + string: '{"url":"http://objects-web:8000/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea","uuid":"c7ec7188-c8f5-4c13-bd74-942468077eea","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","some":{"path":"foo"}},"geometry":null,"startAt":"2024-12-03","endAt":null,"registrationAt":"2024-12-03","correctionFor":null,"correctedBy":null}}' + headers: + Allow: + - GET, PUT, PATCH, DELETE, HEAD, OPTIONS + Connection: + - keep-alive + Content-Crs: + - EPSG:4326 + Content-Length: + - '432' + Content-Type: + - application/json + Cross-Origin-Opener-Policy: + - same-origin + Date: + - Tue, 03 Dec 2024 16:13:43 GMT + Referrer-Policy: + - same-origin + Server: + - nginx/1.27.3 Vary: - origin X-Content-Type-Options: diff --git a/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/ObjectsAPIPrefillDataOwnershipCheckTests.test_verify_initial_data_ownership_raising_errors_causes_prefill_to_fail.yaml b/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/ObjectsAPIPrefillDataOwnershipCheckTests.test_verify_initial_data_ownership_raising_errors_causes_prefill_to_fail.yaml new file mode 100644 index 0000000000..9abedde96b --- /dev/null +++ b/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/ObjectsAPIPrefillDataOwnershipCheckTests.test_verify_initial_data_ownership_raising_errors_causes_prefill_to_fail.yaml @@ -0,0 +1,50 @@ +interactions: +- request: + body: null + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate, br + Authorization: + - Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9 + Connection: + - keep-alive + Content-Crs: + - EPSG:4326 + User-Agent: + - python-requests/2.32.2 + method: GET + uri: http://localhost:8002/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea + response: + body: + string: '{"url":"http://objects-web:8000/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea","uuid":"c7ec7188-c8f5-4c13-bd74-942468077eea","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","some":{"path":"foo"}},"geometry":null,"startAt":"2024-12-03","endAt":null,"registrationAt":"2024-12-03","correctionFor":null,"correctedBy":null}}' + headers: + Allow: + - GET, PUT, PATCH, DELETE, HEAD, OPTIONS + Connection: + - keep-alive + Content-Crs: + - EPSG:4326 + Content-Length: + - '432' + Content-Type: + - application/json + Cross-Origin-Opener-Policy: + - same-origin + Date: + - Tue, 03 Dec 2024 16:33:22 GMT + Referrer-Policy: + - same-origin + Server: + - nginx/1.27.3 + Vary: + - origin + X-Content-Type-Options: + - nosniff + X-Frame-Options: + - DENY + status: + code: 200 + message: OK +version: 1 diff --git a/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/setUpTestData.yaml b/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/setUpTestData.yaml index 2189fc88bf..0a8e7a51bd 100644 --- a/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/setUpTestData.yaml +++ b/src/openforms/prefill/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIPrefillDataOwnershipCheckTests/setUpTestData.yaml @@ -2,7 +2,7 @@ interactions: - request: body: '{"type": "http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e", "record": {"typeVersion": 1, "data": {"bsn": "111222333", "some": {"path": "foo"}}, - "startAt": "2024-11-26"}}' + "startAt": "2024-12-03"}}' headers: Accept: - '*/*' @@ -24,7 +24,7 @@ interactions: uri: http://localhost:8002/api/v2/objects response: body: - string: '{"url":"http://objects-web:8000/api/v2/objects/bd6a9316-f721-40e4-9d4d-73ada5590952","uuid":"bd6a9316-f721-40e4-9d4d-73ada5590952","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","some":{"path":"foo"}},"geometry":null,"startAt":"2024-11-26","endAt":null,"registrationAt":"2024-11-26","correctionFor":null,"correctedBy":null}}' + string: '{"url":"http://objects-web:8000/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea","uuid":"c7ec7188-c8f5-4c13-bd74-942468077eea","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","some":{"path":"foo"}},"geometry":null,"startAt":"2024-12-03","endAt":null,"registrationAt":"2024-12-03","correctionFor":null,"correctedBy":null}}' headers: Allow: - GET, POST, HEAD, OPTIONS @@ -39,13 +39,13 @@ interactions: Cross-Origin-Opener-Policy: - same-origin Date: - - Tue, 26 Nov 2024 13:21:30 GMT + - Tue, 03 Dec 2024 16:13:43 GMT Location: - - http://localhost:8002/api/v2/objects/bd6a9316-f721-40e4-9d4d-73ada5590952 + - http://localhost:8002/api/v2/objects/c7ec7188-c8f5-4c13-bd74-942468077eea Referrer-Policy: - same-origin Server: - - nginx/1.27.0 + - nginx/1.27.3 Vary: - origin X-Content-Type-Options: diff --git a/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py b/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py index 0ca437c79e..371a47c7e1 100644 --- a/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py +++ b/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py @@ -1,5 +1,4 @@ from pathlib import Path -from unittest.mock import patch from django.core.exceptions import PermissionDenied from django.test import TestCase, tag @@ -7,17 +6,15 @@ from openforms.authentication.service import AuthAttribute from openforms.contrib.objects_api.clients import get_objects_client from openforms.contrib.objects_api.helpers import prepare_data_for_registration -from openforms.contrib.objects_api.models import ObjectsAPIGroupConfig from openforms.contrib.objects_api.tests.factories import ObjectsAPIGroupConfigFactory from openforms.forms.tests.factories import ( FormFactory, FormRegistrationBackendFactory, - FormStepFactory, FormVariableFactory, ) from openforms.logging.models import TimelineLogProxy from openforms.prefill.service import prefill_variables -from openforms.submissions.tests.factories import SubmissionStepFactory +from openforms.submissions.tests.factories import SubmissionFactory from openforms.utils.tests.vcr import OFVCRMixin, with_setup_test_data_vcr TEST_FILES = (Path(__file__).parent / "files").resolve() @@ -29,11 +26,23 @@ class ObjectsAPIPrefillDataOwnershipCheckTests(OFVCRMixin, TestCase): @classmethod def setUpTestData(cls): + """ + Set up a form with basic configuration: + + * 1 formstep, with a component 'postcode' + * 3 registration backends (email: irrelevant, objects API: different group, + objects API: relevant) + * 2 user defined form variables: + + * one to to the objects API prefill + * one to get the mapped variable assigned to + """ super().setUpTestData() cls.objects_api_group_used = ObjectsAPIGroupConfigFactory.create( for_test_docker_compose=True ) + cls.objects_api_group_unused = ObjectsAPIGroupConfigFactory.create() with with_setup_test_data_vcr(cls.VCR_TEST_FILES, cls.__qualname__): with get_objects_client(cls.objects_api_group_used) as client: @@ -46,12 +55,18 @@ def setUpTestData(cls): ) cls.object_ref = object["uuid"] - cls.objects_api_group_used = ObjectsAPIGroupConfigFactory.create( - for_test_docker_compose=True + cls.form = FormFactory.create( + generate_minimal_setup=True, + formstep__form_definition__configuration={ + "components": [ + { + "type": "postcode", + "key": "postcode", + "inputMask": "9999 AA", + } + ], + }, ) - cls.objects_api_group_unused = ObjectsAPIGroupConfigFactory.create() - - cls.form = FormFactory.create() # An objects API backend with a different API group FormRegistrationBackendFactory.create( form=cls.form, @@ -77,28 +92,19 @@ def setUpTestData(cls): }, ) - cls.form_step = FormStepFactory.create( - form_definition__configuration={ - "components": [ - { - "type": "postcode", - "key": "postcode", - "inputMask": "9999 AA", - } - ] - } - ) + FormVariableFactory.create(form=cls.form, key="voornamen", user_defined=True) cls.variable = FormVariableFactory.create( - key="voornamen", - form=cls.form_step.form, + form=cls.form, + key="prefillData", + user_defined=True, prefill_plugin="objects_api", prefill_attribute="", prefill_options={ "version": 2, - "objecttype": "3edfdaf7-f469-470b-a391-bb7ea015bd6f", "objects_api_group": cls.objects_api_group_used.pk, + "objecttype_uuid": "3edfdaf7-f469-470b-a391-bb7ea015bd6f", "objecttype_version": 1, - "auth_attribute_path": ["nested", "bsn"], + "auth_attribute_path": ["bsn"], "variables_mapping": [ {"variable_key": "voornamen", "target_path": ["some", "path"]}, ], @@ -108,137 +114,77 @@ def setUpTestData(cls): def test_verify_initial_data_ownership_called_if_initial_data_reference_specified( self, ): - submission_step = SubmissionStepFactory.create( - submission__form=self.form_step.form, - form_step=self.form_step, - submission__auth_info__value="999990676", - submission__auth_info__attribute=AuthAttribute.bsn, - submission__initial_data_reference=self.object_ref, + submission = SubmissionFactory.create( + form=self.form, + auth_info__value="111222333", + auth_info__attribute=AuthAttribute.bsn, + initial_data_reference=self.object_ref, ) - with patch( - "openforms.prefill.contrib.objects_api.plugin.validate_object_ownership" - ) as mock_validate_object_ownership: - prefill_variables(submission=submission_step.submission) - - self.assertEqual(mock_validate_object_ownership.call_count, 1) - - # Cannot compare with `.assert_has_calls`, because the client objects - # won't match - call = mock_validate_object_ownership.mock_calls[0] - - self.assertEqual(call.args[0], submission_step.submission) - self.assertEqual( - call.args[1].base_url, - self.objects_api_group_used.objects_service.api_root, - ) - self.assertEqual(call.args[2], ["nested", "bsn"]) - - logs = TimelineLogProxy.objects.filter(object_id=submission_step.submission.id) + prefill_variables(submission=submission) - self.assertEqual( - logs.filter(extra_data__log_event="prefill_retrieve_success").count(), 1 - ) + logs = TimelineLogProxy.objects.for_object(submission) + self.assertEqual(logs.filter_event("object_ownership_check_success").count(), 1) + self.assertEqual(logs.filter_event("prefill_retrieve_success").count(), 1) def test_verify_initial_data_ownership_raising_errors_causes_prefill_to_fail(self): - submission_step = SubmissionStepFactory.create( - submission__form=self.form_step.form, - form_step=self.form_step, - submission__auth_info__value="999990676", - submission__auth_info__attribute=AuthAttribute.bsn, - submission__initial_data_reference=self.object_ref, + # configure an invalid path, which causes errors during validation + self.variable.prefill_options["auth_attribute_path"] = ["nested", "bsn"] + self.variable.save() + submission = SubmissionFactory.create( + form=self.form, + # valid BSN, invalid config + auth_info__value="111222333", + auth_info__attribute=AuthAttribute.bsn, + initial_data_reference=self.object_ref, ) - with patch( - "openforms.prefill.contrib.objects_api.plugin.validate_object_ownership", - side_effect=PermissionDenied, - ) as mock_validate_object_ownership: - with self.assertRaises(PermissionDenied): - prefill_variables(submission=submission_step.submission) - - self.assertEqual(mock_validate_object_ownership.call_count, 1) - - # Cannot compare with `.assert_has_calls`, because the client objects - # won't match - call = mock_validate_object_ownership.mock_calls[0] - - self.assertEqual(call.args[0], submission_step.submission) - self.assertEqual( - call.args[1].base_url, - self.objects_api_group_used.objects_service.api_root, - ) - self.assertEqual(call.args[2], ["nested", "bsn"]) + with self.assertRaises(PermissionDenied): + prefill_variables(submission=submission) - logs = TimelineLogProxy.objects.filter(object_id=submission_step.submission.id) - 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 - ) + logs = TimelineLogProxy.objects.for_object(submission) + self.assertEqual(logs.filter_event("prefill_retrieve_failure").count(), 1) + self.assertFalse(logs.filter_event("object_ownership_check_success").exists()) + self.assertFalse(logs.filter_event("prefill_retrieve_success").exists()) def test_verify_initial_data_ownership_missing_auth_attribute_path_causes_failing_prefill( self, ): - del self.variable.prefill_options["auth_attribute_path"] - self.variable.save() - submission_step = SubmissionStepFactory.create( - submission__form=self.form_step.form, - form_step=self.form_step, - submission__auth_info__value="999990676", - submission__auth_info__attribute=AuthAttribute.bsn, - submission__initial_data_reference=self.object_ref, - ) - - with patch( - "openforms.prefill.contrib.objects_api.plugin.validate_object_ownership", - ) as mock_validate_object_ownership: - with self.assertRaises(PermissionDenied): - prefill_variables(submission=submission_step.submission) - - self.assertEqual(mock_validate_object_ownership.call_count, 0) - - logs = TimelineLogProxy.objects.filter(object_id=submission_step.submission.id) - 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 - ) - self.assertEqual( - logs.filter( - extra_data__log_event="object_ownership_check_improperly_configured" - ).count(), - 1, - ) + with self.subTest("missing auth_attribute_path config option"): + # doesn't pass serializer validation, so prefill fails + del self.variable.prefill_options["auth_attribute_path"] + self.variable.save() + submission = SubmissionFactory.create( + form=self.form, + auth_info__value="111222333", + auth_info__attribute=AuthAttribute.bsn, + initial_data_reference=self.object_ref, + ) - def test_verify_initial_data_ownership_does_not_raise_errors_without_api_group( - self, - ): - self.variable.prefill_options["objects_api_group"] = ( - ObjectsAPIGroupConfig.objects.last().pk + 1 - ) - self.variable.save() - submission_step = SubmissionStepFactory.create( - submission__form=self.form_step.form, - form_step=self.form_step, - submission__auth_info__value="999990676", - submission__auth_info__attribute=AuthAttribute.bsn, - submission__initial_data_reference=self.object_ref, - ) + prefill_variables(submission=submission) - with patch( - "openforms.prefill.contrib.objects_api.plugin.validate_object_ownership", - ) as mock_validate_object_ownership: - prefill_variables(submission=submission_step.submission) + logs = TimelineLogProxy.objects.for_object(submission) + self.assertEqual(logs.filter_event("prefill_retrieve_failure").count(), 1) + self.assertFalse( + logs.filter_event("object_ownership_check_success").exists() + ) + self.assertFalse(logs.filter_event("prefill_retrieve_success").exists()) + + with self.subTest("empty auth_attribute_path config option value"): + self.variable.prefill_options["auth_attribute_path"] = [] + self.variable.save() + submission = SubmissionFactory.create( + form=self.form, + auth_info__value="111222333", + auth_info__attribute=AuthAttribute.bsn, + initial_data_reference=self.object_ref, + ) - self.assertEqual(mock_validate_object_ownership.call_count, 0) + prefill_variables(submission=submission) - logs = TimelineLogProxy.objects.filter(object_id=submission_step.submission.id) - self.assertEqual( - logs.filter(extra_data__log_event="prefill_retrieve_success").count(), 0 - ) - # Prefilling fails, because the API group does not exist - self.assertEqual( - logs.filter(extra_data__log_event="prefill_retrieve_failure").count(), 1 - ) + logs = TimelineLogProxy.objects.for_object(submission) + self.assertEqual(logs.filter_event("prefill_retrieve_failure").count(), 1) + self.assertFalse( + logs.filter_event("object_ownership_check_success").exists() + ) + self.assertFalse(logs.filter_event("prefill_retrieve_success").exists()) diff --git a/src/openforms/prefill/contrib/objects_api/typing.py b/src/openforms/prefill/contrib/objects_api/typing.py index 2f380f7033..36fb5e7e0e 100644 --- a/src/openforms/prefill/contrib/objects_api/typing.py +++ b/src/openforms/prefill/contrib/objects_api/typing.py @@ -11,6 +11,7 @@ class VariableMapping(TypedDict): class ObjectsAPIOptions(TypedDict): objects_api_group: ObjectsAPIGroupConfig - object_type_uuid: UUID + objecttype_uuid: UUID objecttype_version: int + auth_attribute_path: list[str] variables_mapping: list[VariableMapping] diff --git a/src/openforms/prefill/sources.py b/src/openforms/prefill/sources.py index ec3b4d1763..fd947c7780 100644 --- a/src/openforms/prefill/sources.py +++ b/src/openforms/prefill/sources.py @@ -1,7 +1,7 @@ import logging from collections import defaultdict -from django.core.exceptions import ImproperlyConfigured, PermissionDenied +from django.core.exceptions import PermissionDenied import elasticapm from rest_framework.exceptions import ValidationError @@ -14,6 +14,7 @@ ) from openforms.typing import JSONEncodable +from .constants import IdentifierRoles from .registry import Registry logger = logging.getLogger(__name__) @@ -37,7 +38,9 @@ def fetch_prefill_values_from_attribute( for variable in submission_variables: plugin_id = variable.form_variable.prefill_plugin - identifier_role = variable.form_variable.prefill_identifier_role + identifier_role = IdentifierRoles( + variable.form_variable.prefill_identifier_role + ) attribute_name = variable.form_variable.prefill_attribute grouped_fields[plugin_id][identifier_role].append( @@ -48,7 +51,7 @@ def fetch_prefill_values_from_attribute( @elasticapm.capture_span(span_type="app.prefill") def invoke_plugin( - item: tuple[str, str, list[dict[str, str]]] + item: tuple[str, IdentifierRoles, list[dict[str, str]]] ) -> tuple[list[dict[str, str]], dict[str, JSONEncodable]]: plugin_id, identifier_role, fields = item plugin = register[plugin_id] @@ -98,28 +101,45 @@ def fetch_prefill_values_from_options( values: dict[str, JSONEncodable] = {} for variable in variables: plugin = register[variable.form_variable.prefill_plugin] + raw_options = variable.form_variable.prefill_options + + # validate the options before processing them + options_serializer = plugin.options(data=raw_options) + try: + options_serializer.is_valid(raise_exception=True) + except ValidationError as exc: + logevent.prefill_retrieve_failure(submission, plugin, exc) + continue + + plugin_options = options_serializer.validated_data # 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: try: - plugin.verify_initial_data_ownership( - submission, variable.form_variable.prefill_options + plugin.verify_initial_data_ownership(submission, plugin_options) + except PermissionDenied as exc: + # XXX: these log records will typically not be created in the DB because + # the transaction is rolled back as part of DRFs exception handler + logger.warning( + "Submission %s attempted to prefill the initial_data_reference %s " + "using the %r plugin, but the ownership check failed.", + submission.uuid, + submission.initial_data_reference, + plugin, + exc_info=exc, + extra={ + "submission": submission.uuid, + "plugin_cls": type(plugin), + "initial_data_reference": submission.initial_data_reference, + }, ) - except (PermissionDenied, ImproperlyConfigured) as exc: logevent.prefill_retrieve_failure(submission, plugin, exc) raise exc - options_serializer = plugin.options(data=variable.form_variable.prefill_options) - - try: - options_serializer.is_valid(raise_exception=True) - except ValidationError as exc: - logevent.prefill_retrieve_failure(submission, plugin, exc) - continue try: new_values = plugin.get_prefill_values_from_options( - submission, options_serializer.validated_data + submission, plugin_options ) except Exception as exc: logger.exception(f"exception in prefill plugin '{plugin.identifier}'")