From bcf2262691b54c62469778521fd836fbcf5ba407 Mon Sep 17 00:00:00 2001 From: vasileios Date: Tue, 14 Jan 2025 11:45:49 +0100 Subject: [PATCH] [#4825] Fixed prefill when authentication is needed In case a form needs authentication and uses prefill, we need to make sure that we run the plugin only when the authentication type matches the plugin's requirements. --- src/openapi.yaml | 11 ++--- src/openforms/js/compiled-lang/en.json | 44 +++++++++---------- src/openforms/js/compiled-lang/nl.json | 44 +++++++++---------- .../admin/form_design/PluginWarning.js | 8 ++-- src/openforms/js/lang/en.json | 10 ++--- src/openforms/js/lang/nl.json | 10 ++--- src/openforms/prefill/api/serializers.py | 11 ++--- .../prefill/api/tests/test_endpoints.py | 16 +++---- src/openforms/prefill/base.py | 6 ++- .../contrib/haalcentraal_brp/plugin.py | 5 ++- .../haalcentraal_brp/tests/test_co_sign.py | 6 +-- src/openforms/prefill/contrib/kvk/plugin.py | 5 ++- .../prefill/contrib/stufbg/plugin.py | 5 ++- .../contrib/stufbg/tests/test_co_sign.py | 3 +- .../prefill/contrib/suwinet/plugin.py | 2 +- src/openforms/prefill/fields.py | 2 +- src/openforms/prefill/sources.py | 21 +++++---- .../prefill/tests/test_prefill_hook.py | 4 +- 18 files changed, 111 insertions(+), 102 deletions(-) diff --git a/src/openapi.yaml b/src/openapi.yaml index 135bee6329..7b0b14566c 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -9541,11 +9541,12 @@ components: type: string description: The human-readable name for a plugin. requiresAuth: - type: string - nullable: true - title: Required authentication attribute - description: The authentication attribute required for this plugin to lookup - remote data. + type: array + items: + type: string + title: Required authentication attribute + description: The authentication attribute required for this plugin to + lookup remote data. configurationContext: nullable: true title: Extra configuration context diff --git a/src/openforms/js/compiled-lang/en.json b/src/openforms/js/compiled-lang/en.json index 5fd1d83f4e..65ff58eb40 100644 --- a/src/openforms/js/compiled-lang/en.json +++ b/src/openforms/js/compiled-lang/en.json @@ -2649,6 +2649,28 @@ "value": "Component" } ], + "MIrPon": [ + { + "type": 0, + "value": "Component \"" + }, + { + "type": 1, + "value": "label" + }, + { + "type": 0, + "value": "\" uses a prefill that requires one of the \"" + }, + { + "type": 1, + "value": "requiredAuthAttribute" + }, + { + "type": 0, + "value": "\" attributes. Please select one or more authentication plugins that provide such an attribute." + } + ], "MTdKuN": [ { "type": 0, @@ -3529,28 +3551,6 @@ "value": "Maximum selected checkboxes (e.g. 1)" } ], - "VQYmOD": [ - { - "type": 0, - "value": "Component \"" - }, - { - "type": 1, - "value": "label" - }, - { - "type": 0, - "value": "\" uses a prefill that requires the \"" - }, - { - "type": 1, - "value": "requiredAuthAttribute" - }, - { - "type": 0, - "value": "\" attribute. Please select an authentication plugin that provides this attribute." - } - ], "VUOOSy": [ { "type": 0, diff --git a/src/openforms/js/compiled-lang/nl.json b/src/openforms/js/compiled-lang/nl.json index 2d9af4180a..f524fc6fd6 100644 --- a/src/openforms/js/compiled-lang/nl.json +++ b/src/openforms/js/compiled-lang/nl.json @@ -2666,6 +2666,28 @@ "value": "Veld" } ], + "MIrPon": [ + { + "type": 0, + "value": "Component \"" + }, + { + "type": 1, + "value": "label" + }, + { + "type": 0, + "value": "\" uses a prefill that requires one of the \"" + }, + { + "type": 1, + "value": "requiredAuthAttribute" + }, + { + "type": 0, + "value": "\" attributes. Please select one or more authentication plugins that provide such an attribute." + } + ], "MTdKuN": [ { "type": 0, @@ -3542,28 +3564,6 @@ "value": "Maximaal aantal aangevinkte opties (bijv. 1)" } ], - "VQYmOD": [ - { - "type": 0, - "value": "De component \"" - }, - { - "type": 1, - "value": "label" - }, - { - "type": 0, - "value": "\" gebruikt een prefill die het \"" - }, - { - "type": 1, - "value": "requiredAuthAttribute" - }, - { - "type": 0, - "value": "\"-attribuut nodig heeft. Gebruik een authenticatiemethode die dit attribuut aanbiedt." - } - ], "VUOOSy": [ { "type": 0, diff --git a/src/openforms/js/components/admin/form_design/PluginWarning.js b/src/openforms/js/components/admin/form_design/PluginWarning.js index 3d750162af..3279d1ed44 100644 --- a/src/openforms/js/components/admin/form_design/PluginWarning.js +++ b/src/openforms/js/components/admin/form_design/PluginWarning.js @@ -35,7 +35,7 @@ const PluginWarning = ({loginRequired, configuration}) => { const authPlugin = availableAuthPlugins.find(plugin => plugin.id === pluginName); if (!authPlugin) break; - if (authPlugin.providesAuth.includes(requiredAuthAttribute)) { + if (requiredAuthAttribute.includes(authPlugin.providesAuth)) { pluginProvidesAttribute = true; break; } @@ -46,12 +46,12 @@ const PluginWarning = ({loginRequired, configuration}) => { ); diff --git a/src/openforms/js/lang/en.json b/src/openforms/js/lang/en.json index 5320769b35..90c5f34416 100644 --- a/src/openforms/js/lang/en.json +++ b/src/openforms/js/lang/en.json @@ -1279,6 +1279,11 @@ "description": "Formio component selection label", "originalDefault": "Component" }, + "MIrPon": { + "defaultMessage": "Component \"{label}\" uses a prefill that requires one of the \"{requiredAuthAttribute}\" attributes. Please select one or more authentication plugins that provide such an attribute.", + "description": "Prefill plugin requires unavailable auth attribute warning", + "originalDefault": "Component \"{label}\" uses a prefill that requires one of the \"{requiredAuthAttribute}\" attributes. Please select one or more authentication plugins that provide such an attribute." + }, "MUfBmP": { "defaultMessage": "The value of the selected field will be the process variable value.", "description": "Column help text for variable to map to property", @@ -1694,11 +1699,6 @@ "description": "Confirmation page content label", "originalDefault": "Page content" }, - "VQYmOD": { - "defaultMessage": "Component \"{label}\" uses a prefill that requires the \"{requiredAuthAttribute}\" attribute. Please select an authentication plugin that provides this attribute.", - "description": "Prefill plugin requires unavailable auth attribute warning", - "originalDefault": "Component \"{label}\" uses a prefill that requires the \"{requiredAuthAttribute}\" attribute. Please select an authentication plugin that provides this attribute." - }, "VUOOSy": { "defaultMessage": "Name", "description": "Camunda complex process var 'name' label", diff --git a/src/openforms/js/lang/nl.json b/src/openforms/js/lang/nl.json index 0eb58fb0ff..a68bd3721d 100644 --- a/src/openforms/js/lang/nl.json +++ b/src/openforms/js/lang/nl.json @@ -1288,6 +1288,11 @@ "description": "Formio component selection label", "originalDefault": "Component" }, + "MIrPon": { + "defaultMessage": "Component \"{label}\" uses a prefill that requires one of the \"{requiredAuthAttribute}\" attributes. Please select one or more authentication plugins that provide such an attribute.", + "description": "Prefill plugin requires unavailable auth attribute warning", + "originalDefault": "Component \"{label}\" uses a prefill that requires one of the \"{requiredAuthAttribute}\" attributes. Please select one or more authentication plugins that provide such an attribute." + }, "MUfBmP": { "defaultMessage": "De waarde van de geselecteerde variabele wordt de waarde van de zaakeigenschap.", "description": "Column help text for variable to map to property", @@ -1710,11 +1715,6 @@ "description": "Confirmation page content label", "originalDefault": "Page content" }, - "VQYmOD": { - "defaultMessage": "De component \"{label}\" gebruikt een prefill die het \"{requiredAuthAttribute}\"-attribuut nodig heeft. Gebruik een authenticatiemethode die dit attribuut aanbiedt.", - "description": "Prefill plugin requires unavailable auth attribute warning", - "originalDefault": "Component \"{label}\" uses a prefill that requires the \"{requiredAuthAttribute}\" attribute. Please select an authentication plugin that provides this attribute." - }, "VUOOSy": { "defaultMessage": "Naam", "description": "Camunda complex process var 'name' label", diff --git a/src/openforms/prefill/api/serializers.py b/src/openforms/prefill/api/serializers.py index 00b3d10edc..a7a58f56bd 100644 --- a/src/openforms/prefill/api/serializers.py +++ b/src/openforms/prefill/api/serializers.py @@ -12,12 +12,13 @@ class PrefillPluginSerializer(PluginBaseSerializer): - requires_auth = serializers.CharField( - label=_("Required authentication attribute"), - help_text=_( - "The authentication attribute required for this plugin to lookup remote data." + requires_auth = serializers.ListField( + child=serializers.CharField( + label=_("Required authentication attribute"), + help_text=_( + "The authentication attribute required for this plugin to lookup remote data." + ), ), - allow_null=True, ) configuration_context = serializers.JSONField( label=_("Extra configuration context"), diff --git a/src/openforms/prefill/api/tests/test_endpoints.py b/src/openforms/prefill/api/tests/test_endpoints.py index 68eab0062d..833b585097 100644 --- a/src/openforms/prefill/api/tests/test_endpoints.py +++ b/src/openforms/prefill/api/tests/test_endpoints.py @@ -12,7 +12,7 @@ class TestPrefill(BasePlugin): - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) verbose_name = "Test" def get_available_attributes(self): @@ -26,7 +26,7 @@ def get_available_attributes(self): @register("onlyvars") class OnlyVarsPrefill(BasePlugin): - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) verbose_name = "Only Vars" for_components = () @@ -36,7 +36,7 @@ def get_available_attributes(self): @register("vanityplates") class VanityPlatePrefill(BasePlugin): - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) verbose_name = "Vanity Plates" for_components = {"licenseplate"} @@ -111,19 +111,19 @@ def test_prefill_list(self): { "id": "test", "label": "Test", - "requiresAuth": AuthAttribute.bsn, + "requiresAuth": [AuthAttribute.bsn], "configurationContext": None, }, { "id": "onlyvars", "label": "Only Vars", - "requiresAuth": AuthAttribute.bsn, + "requiresAuth": [AuthAttribute.bsn], "configurationContext": None, }, { "id": "vanityplates", "label": "Vanity Plates", - "requiresAuth": AuthAttribute.bsn, + "requiresAuth": [AuthAttribute.bsn], "configurationContext": None, }, ] @@ -141,14 +141,14 @@ def test_prefill_list_for_component_type(self): { "id": "test", "label": "Test", - "requiresAuth": AuthAttribute.bsn, + "requiresAuth": [AuthAttribute.bsn], "configurationContext": None, }, # spec'd for licenseplate { "id": "vanityplates", "label": "Vanity Plates", - "requiresAuth": AuthAttribute.bsn, + "requiresAuth": [AuthAttribute.bsn], "configurationContext": None, }, ] diff --git a/src/openforms/prefill/base.py b/src/openforms/prefill/base.py index 81b6b61292..1e3cf4bab2 100644 --- a/src/openforms/prefill/base.py +++ b/src/openforms/prefill/base.py @@ -1,3 +1,4 @@ +from collections.abc import Collection from typing import Any, Container, Iterable, TypedDict from rest_framework import serializers @@ -29,7 +30,7 @@ class Options(TypedDict): class BasePlugin[OptionsT: Options](AbstractBasePlugin): - requires_auth: AuthAttribute | None = None + requires_auth: Collection[AuthAttribute] = () for_components: Container[str] = AllComponentTypes() options: SerializerCls = EmptyOptions @@ -139,7 +140,8 @@ def get_identifier_value( if ( identifier_role == IdentifierRoles.main - and submission.auth_info.attribute == cls.requires_auth + and cls.requires_auth + and submission.auth_info.attribute in cls.requires_auth ): return submission.auth_info.value diff --git a/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py b/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py index 5492cba531..ff59a12fd2 100644 --- a/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py +++ b/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py @@ -45,7 +45,7 @@ def get_attributes_cls(): @register(PLUGIN_IDENTIFIER) class HaalCentraalPrefill(BasePlugin): verbose_name = _("Haal Centraal: BRP Personen Bevragen") - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) @staticmethod def get_available_attributes() -> list[tuple[str, str]]: @@ -84,7 +84,8 @@ def get_identifier_value( if ( identifier_role == IdentifierRoles.main - and submission.auth_info.attribute == cls.requires_auth + and cls.requires_auth + and submission.auth_info.attribute in cls.requires_auth ): return submission.auth_info.value diff --git a/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_co_sign.py b/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_co_sign.py index 275dc53352..9b0e6f9ad0 100644 --- a/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_co_sign.py +++ b/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_co_sign.py @@ -71,7 +71,7 @@ def test_store_names_on_co_sign_auth(self): } ) - add_co_sign_representation(submission, plugin.requires_auth) + add_co_sign_representation(submission, plugin.requires_auth[0]) submission.refresh_from_db() expected = { @@ -96,7 +96,7 @@ def test_incomplete_data_returned(self): } ) - add_co_sign_representation(submission, plugin.requires_auth) + add_co_sign_representation(submission, plugin.requires_auth[0]) submission.refresh_from_db() expected = { @@ -178,7 +178,7 @@ def test_store_names_on_co_sign_auth(self): } ) - add_co_sign_representation(submission, plugin.requires_auth) + add_co_sign_representation(submission, plugin.requires_auth[0]) submission.refresh_from_db() expected = { diff --git a/src/openforms/prefill/contrib/kvk/plugin.py b/src/openforms/prefill/contrib/kvk/plugin.py index a2bdf1b994..3853527664 100644 --- a/src/openforms/prefill/contrib/kvk/plugin.py +++ b/src/openforms/prefill/contrib/kvk/plugin.py @@ -35,7 +35,7 @@ def _select_address(items, type_): class KVK_KVKNumberPrefill(BasePlugin): verbose_name = _("KvK Company by KvK number") - requires_auth = AuthAttribute.kvk + requires_auth = (AuthAttribute.kvk,) @staticmethod def get_available_attributes() -> list[tuple[str, str]]: @@ -50,7 +50,8 @@ def get_identifier_value( if ( identifier_role == IdentifierRoles.main - and submission.auth_info.attribute == cls.requires_auth + and cls.requires_auth + and submission.auth_info.attribute in cls.requires_auth ): return submission.auth_info.value diff --git a/src/openforms/prefill/contrib/stufbg/plugin.py b/src/openforms/prefill/contrib/stufbg/plugin.py index 68c7cfb5dd..c7bef58acf 100644 --- a/src/openforms/prefill/contrib/stufbg/plugin.py +++ b/src/openforms/prefill/contrib/stufbg/plugin.py @@ -95,7 +95,7 @@ @register("stufbg") class StufBgPrefill(BasePlugin): verbose_name = _("StUF-BG") - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) @staticmethod def get_available_attributes() -> list[tuple[str, str]]: @@ -138,7 +138,8 @@ def get_identifier_value( if ( identifier_role == IdentifierRoles.main - and submission.auth_info.attribute == cls.requires_auth + and cls.requires_auth + and submission.auth_info.attribute in cls.requires_auth ): return submission.auth_info.value diff --git a/src/openforms/prefill/contrib/stufbg/tests/test_co_sign.py b/src/openforms/prefill/contrib/stufbg/tests/test_co_sign.py index 0bb16209d7..5ee61444d5 100644 --- a/src/openforms/prefill/contrib/stufbg/tests/test_co_sign.py +++ b/src/openforms/prefill/contrib/stufbg/tests/test_co_sign.py @@ -49,8 +49,7 @@ def test_store_names_on_co_sign_auth(self): "fields": {}, } ) - - add_co_sign_representation(submission, plugin.requires_auth) + add_co_sign_representation(submission, plugin.requires_auth[0]) submission.refresh_from_db() self.assertEqual( diff --git a/src/openforms/prefill/contrib/suwinet/plugin.py b/src/openforms/prefill/contrib/suwinet/plugin.py index c3d1782b05..f71897f8ab 100644 --- a/src/openforms/prefill/contrib/suwinet/plugin.py +++ b/src/openforms/prefill/contrib/suwinet/plugin.py @@ -28,7 +28,7 @@ def _get_client() -> SuwinetClient | None: @register("suwinet") class SuwinetPrefill(BasePlugin): - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) verbose_name = _("Suwinet") for_components = () diff --git a/src/openforms/prefill/fields.py b/src/openforms/prefill/fields.py index c3a2a01e90..5c000fdfe2 100644 --- a/src/openforms/prefill/fields.py +++ b/src/openforms/prefill/fields.py @@ -35,7 +35,7 @@ def formfield(self, **kwargs): def _get_plugin_choices(self): choices = [] for plugin in register.iter_enabled_plugins(): - if plugin.requires_auth != self.auth_attribute: + if self.auth_attribute not in plugin.requires_auth: continue choices.append((plugin.identifier, plugin.get_label())) return choices diff --git a/src/openforms/prefill/sources.py b/src/openforms/prefill/sources.py index ca1dc68ec9..ad6e00dbf3 100644 --- a/src/openforms/prefill/sources.py +++ b/src/openforms/prefill/sources.py @@ -14,6 +14,7 @@ ) from openforms.typing import JSONEncodable +from .base import BasePlugin from .constants import IdentifierRoles from .registry import Registry @@ -51,10 +52,9 @@ def fetch_prefill_values_from_attribute( @elasticapm.capture_span(span_type="app.prefill") def invoke_plugin( - item: tuple[str, IdentifierRoles, list[dict[str, str]]] + item: tuple[BasePlugin, IdentifierRoles, list[dict[str, str]]] ) -> tuple[list[dict[str, str]], dict[str, JSONEncodable]]: - plugin_id, identifier_role, fields = item - plugin = register[plugin_id] + plugin, identifier_role, fields = item if not plugin.is_enabled: raise PluginNotEnabled() @@ -70,17 +70,20 @@ def invoke_plugin( if values: logevent.prefill_retrieve_success(submission, plugin, fields) else: - if (required_attribute := plugin.requires_auth) is None or ( - (auth_info := getattr(submission, "auth_info", None)) - and auth_info.attribute == required_attribute - ): - logevent.prefill_retrieve_empty(submission, plugin, fields) + logevent.prefill_retrieve_empty(submission, plugin, fields) return fields, values invoke_plugin_args = [] for plugin_id, field_groups in grouped_fields.items(): + plugin = register[plugin_id] + + # check if we need to run the plugin when the user is authenticated + auth_info = getattr(submission, "auth_info", None) + if auth_info and auth_info.attribute not in plugin.requires_auth: + continue + for identifier_role, fields in field_groups.items(): - invoke_plugin_args.append((plugin_id, identifier_role, fields)) + invoke_plugin_args.append((plugin, identifier_role, fields)) with parallel() as executor: results = executor.map(invoke_plugin, invoke_plugin_args) diff --git a/src/openforms/prefill/tests/test_prefill_hook.py b/src/openforms/prefill/tests/test_prefill_hook.py index 76ff099849..3030944cf3 100644 --- a/src/openforms/prefill/tests/test_prefill_hook.py +++ b/src/openforms/prefill/tests/test_prefill_hook.py @@ -614,7 +614,7 @@ def test_demo_prefill_get_identifier_not_authenticated(self): def test_demo_prefill_get_identifier_authenticated(self): class TestPlugin(BasePlugin): - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) plugin = TestPlugin(identifier="test") @@ -646,7 +646,7 @@ def test_prefill_logging_with_mismatching_login_method(self): @register("demo") class MismatchPlugin(DemoPrefill): - requires_auth = AuthAttribute.bsn + requires_auth = (AuthAttribute.bsn,) @staticmethod def get_prefill_values(submission, attributes, identifier_role):