From 5b72a63a22cf71c392795edfee7c9a62c8697edf Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 29 Nov 2023 17:59:16 +0100 Subject: [PATCH] [#3607] Refactor validation plugin, add BRK validator --- src/openforms/conf/base.py | 1 + src/openforms/contrib/brk/__init__.py | 0 src/openforms/contrib/brk/admin.py | 10 +++ src/openforms/contrib/brk/apps.py | 12 +++ src/openforms/contrib/brk/client.py | 70 +++++++++++++++ .../contrib/brk/migrations/0001_initial.py | 45 ++++++++++ .../contrib/brk/migrations/__init__.py | 0 src/openforms/contrib/brk/models.py | 24 +++++ src/openforms/contrib/brk/validators.py | 90 +++++++++++++++++++ src/openforms/contrib/kvk/validators.py | 6 +- src/openforms/submissions/api/permissions.py | 3 +- src/openforms/validations/api/serializers.py | 4 + src/openforms/validations/api/views.py | 13 ++- src/openforms/validations/registry.py | 7 +- .../validations/validators/formats.py | 2 +- 15 files changed, 279 insertions(+), 8 deletions(-) create mode 100644 src/openforms/contrib/brk/__init__.py create mode 100644 src/openforms/contrib/brk/admin.py create mode 100644 src/openforms/contrib/brk/apps.py create mode 100644 src/openforms/contrib/brk/client.py create mode 100644 src/openforms/contrib/brk/migrations/0001_initial.py create mode 100644 src/openforms/contrib/brk/migrations/__init__.py create mode 100644 src/openforms/contrib/brk/models.py create mode 100644 src/openforms/contrib/brk/validators.py diff --git a/src/openforms/conf/base.py b/src/openforms/conf/base.py index 6ccc3984fe..a1edaa28e7 100644 --- a/src/openforms/conf/base.py +++ b/src/openforms/conf/base.py @@ -210,6 +210,7 @@ "openforms.submissions", "openforms.logging.apps.LoggingAppConfig", "openforms.contrib.bag.apps.BAGConfig", # TODO: remove once 2.4.0 is released + "openforms.contrib.brk", "openforms.contrib.brp", "openforms.contrib.digid_eherkenning", "openforms.contrib.haal_centraal", diff --git a/src/openforms/contrib/brk/__init__.py b/src/openforms/contrib/brk/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/openforms/contrib/brk/admin.py b/src/openforms/contrib/brk/admin.py new file mode 100644 index 0000000000..44acd25432 --- /dev/null +++ b/src/openforms/contrib/brk/admin.py @@ -0,0 +1,10 @@ +from django.contrib import admin + +from solo.admin import SingletonModelAdmin + +from .models import BRKConfig + + +@admin.register(BRKConfig) +class BRKConfigAdmin(SingletonModelAdmin): + pass diff --git a/src/openforms/contrib/brk/apps.py b/src/openforms/contrib/brk/apps.py new file mode 100644 index 0000000000..7b73f8fcdc --- /dev/null +++ b/src/openforms/contrib/brk/apps.py @@ -0,0 +1,12 @@ +from django.apps import AppConfig +from django.utils.translation import gettext_lazy as _ + + +class BRKApp(AppConfig): + name = "openforms.contrib.brk" + label = "brk" + verbose_name = _("BRK configuration") + + def ready(self): + # register the plugin + from . import validators # noqa diff --git a/src/openforms/contrib/brk/client.py b/src/openforms/contrib/brk/client.py new file mode 100644 index 0000000000..c3197619e5 --- /dev/null +++ b/src/openforms/contrib/brk/client.py @@ -0,0 +1,70 @@ +import logging +from typing import TypedDict + +import requests + +from openforms.contrib.hal_client import HALClient +from zgw_consumers_ext.api_client import ServiceClientFactory + +from .models import BRKConfig + +logger = logging.getLogger(__name__) + + +class NoServiceConfigured(RuntimeError): + pass + + +def get_client() -> "BRKClient": + config = BRKConfig.get_solo() + assert isinstance(config, BRKConfig) + if not (service := config.service): + raise NoServiceConfigured("No KVK service configured!") + service_client_factory = ServiceClientFactory(service) + return BRKClient.configure_from(service_client_factory) + + +class SearchParams(TypedDict, total=False): + postcode: str + huisnummer: str + huisletter: str + huisnummertoevoeging: str + + +class BRKClient(HALClient): + def get_cadastrals_by_address(self, query_params: SearchParams): + """ + Search for cadastrals by querying for a specifc address. + + API docs: https://vng-realisatie.github.io/Haal-Centraal-BRK-bevragen/swagger-ui-1.5#/Kadastraal%20Onroerende%20Zaken/GetKadastraalOnroerendeZaken + """ + assert query_params, "You must provide at least one query parameter" + + try: + response = self.get( + "kadastraalonroerendezaken", + params=query_params, + ) + response.raise_for_status() + except requests.RequestException as exc: + logger.exception("exception while making BRK request", exc_info=exc) + raise exc + + return response.json() + + def get_cadastral_titleholders_by_cadastral_id(self, cadastral_id: str): + """ + Search for commercial titleholders of a cadastral immovable property. + + API docs: https://vng-realisatie.github.io/Haal-Centraal-BRK-bevragen/swagger-ui-1.5#/Zakelijke%20Gerechtigden/GetZakelijkGerechtigden + """ + try: + response = self.get( + f"kadastraalonroerendezaken/{cadastral_id}/zakelijkgerechtigden", + ) + response.raise_for_status() + except requests.RequestException as exc: + logger.exception("exception while making BRK request", exc_info=exc) + raise exc + + return response.json() diff --git a/src/openforms/contrib/brk/migrations/0001_initial.py b/src/openforms/contrib/brk/migrations/0001_initial.py new file mode 100644 index 0000000000..0d67f0c3b6 --- /dev/null +++ b/src/openforms/contrib/brk/migrations/0001_initial.py @@ -0,0 +1,45 @@ +# Generated by Django 3.2.23 on 2023-11-29 16:05 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ("zgw_consumers", "0019_alter_service_uuid"), + ] + + operations = [ + migrations.CreateModel( + name="BRKConfig", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "service", + models.OneToOneField( + help_text="Service for API interaction with the BRK.", + limit_choices_to={"api_type": "orc"}, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="+", + to="zgw_consumers.service", + verbose_name="BRK API", + ), + ), + ], + options={ + "verbose_name": "BRK configuration", + }, + ), + ] diff --git a/src/openforms/contrib/brk/migrations/__init__.py b/src/openforms/contrib/brk/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/openforms/contrib/brk/models.py b/src/openforms/contrib/brk/models.py new file mode 100644 index 0000000000..9aee6b40a8 --- /dev/null +++ b/src/openforms/contrib/brk/models.py @@ -0,0 +1,24 @@ +from django.db import models +from django.utils.translation import gettext_lazy as _ + +from solo.models import SingletonModel +from zgw_consumers.constants import APITypes + + +class BRKConfig(SingletonModel): + """ + Global configuration and defaults. + """ + + service = models.OneToOneField( + "zgw_consumers.Service", + verbose_name=_("BRK API"), + help_text=_("Service for API interaction with the BRK."), + on_delete=models.PROTECT, + limit_choices_to={"api_type": APITypes.orc}, + related_name="+", + null=True, + ) + + class Meta: + verbose_name = _("BRK configuration") diff --git a/src/openforms/contrib/brk/validators.py b/src/openforms/contrib/brk/validators.py new file mode 100644 index 0000000000..cc456d3709 --- /dev/null +++ b/src/openforms/contrib/brk/validators.py @@ -0,0 +1,90 @@ +from typing import TypedDict + +from django.contrib.auth.hashers import check_password as check_salted_hash +from django.core.exceptions import ValidationError +from django.utils.deconstruct import deconstructible +from django.utils.translation import gettext_lazy as _ + +from requests import RequestException + +from openforms.authentication.constants import AuthAttribute +from openforms.submissions.models import Submission +from openforms.validations.registry import register + +from .client import NoServiceConfigured, SearchParams, get_client + + +class AddressValue(TypedDict, total=False): + postcode: str + housenumber: str + houseletter: str + housenumberaddition: str + + +@register( + "brk-Zaakgerechtigde", + verbose_name=_("BRK - Zaakgerechtigde"), + for_components=("address",), +) +@deconstructible +class BRKZaakgerechtigdeValidator: + + error_messages = { + "not_found": _("%(type)s does not exist."), + } + + def __call__(self, value: AddressValue, submission: Submission) -> bool: + + try: + client = get_client() + except NoServiceConfigured: + raise ValidationError( + self.error_messages["not_found"], + params={"type": _("Owner")}, + ) + + address_query: SearchParams = { + "postcode": value["postcode"], + "huisnummer": value["housenumber"], + "huisletter": value["houseletter"], + "huisnummertoevoeging": value["housenumberaddition"], + } + + # We assume submission has auth_info available, should we verify that this validator is used + # on a step that requires login? + if submission.auth_info.attribute != AuthAttribute.bsn: + raise ValidationError() + + try: + with client: + cadastrals_resp = client.get_cadastrals_by_address(address_query) + kadastraals = cadastrals_resp["_embedded"]["kadastraalOnroerendeZaken"] + if len(kadastraals) > 1: + # The query by address returned more than one cadastral, this shouldn't happen + raise ValidationError() + kadastraal_id = kadastraals[0]["identificatie"] + + titleholders_resp = client.get_cadastral_titleholders_by_cadastral_id( + kadastraal_id + ) + + bsns = [ + a["persoon"]["identificatie"] + for a in titleholders_resp["_embedded"]["zakelijkGerechtigden"] + ] + if submission.auth_info.attribute_hashed: + is_valid = any( + check_salted_hash(bsn, submission.auth_info.value) + for bsn in bsns + ) + else: + is_valid = submission.auth_info.value in bsns + + if not is_valid: + raise ValidationError() + + except (RequestException, KeyError): + # TODO raise not found? + pass + + return True diff --git a/src/openforms/contrib/kvk/validators.py b/src/openforms/contrib/kvk/validators.py index 84cb4eb616..dfbfe60e72 100644 --- a/src/openforms/contrib/kvk/validators.py +++ b/src/openforms/contrib/kvk/validators.py @@ -88,7 +88,7 @@ class KVKNumberRemoteValidator(KVKRemoteBaseValidator): query_param = "kvkNummer" value_label = _("KvK number") - def __call__(self, value): + def __call__(self, value, submission): validate_kvk(value) super().__call__(value) @@ -99,7 +99,7 @@ class KVKRSINRemoteValidator(KVKRemoteBaseValidator): query_param = "rsin" value_label = _("RSIN") - def __call__(self, value): + def __call__(self, value, submission): validate_rsin(value) super().__call__(value) @@ -114,6 +114,6 @@ class KVKBranchNumberRemoteValidator(KVKRemoteBaseValidator): query_param = "vestigingsnummer" value_label = _("Branch number") - def __call__(self, value): + def __call__(self, value, submission): validate_branchNumber(value) super().__call__(value) diff --git a/src/openforms/submissions/api/permissions.py b/src/openforms/submissions/api/permissions.py index 1c10cd4c7a..1d1a7ca5c5 100644 --- a/src/openforms/submissions/api/permissions.py +++ b/src/openforms/submissions/api/permissions.py @@ -62,7 +62,8 @@ def has_object_permission( class ActiveSubmissionPermission(AnyActiveSubmissionPermission): """ - Verify that there is at least one active submission for the user session. + Check that the submission matches one of the active submission set on the user session. + Additionally, filter the queryset against the active submissions. """ def has_object_permission(self, request: Request, view: APIView, obj) -> bool: diff --git a/src/openforms/validations/api/serializers.py b/src/openforms/validations/api/serializers.py index 0ea9cdb1c8..477019f55f 100644 --- a/src/openforms/validations/api/serializers.py +++ b/src/openforms/validations/api/serializers.py @@ -14,6 +14,10 @@ class ValidationInputSerializer(serializers.Serializer): value = serializers.CharField( label=_("value"), help_text=_("Value to be validated") ) + submission_uuid = serializers.UUIDField( + label=_("Submission UUID"), + help_text=_("UUID of the submission."), + ) class ValidatorsFilterSerializer(serializers.Serializer): diff --git a/src/openforms/validations/api/views.py b/src/openforms/validations/api/views.py index 2c2611fd27..5d4b2a8a89 100644 --- a/src/openforms/validations/api/views.py +++ b/src/openforms/validations/api/views.py @@ -5,11 +5,14 @@ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view from rest_framework import authentication, permissions +from rest_framework.exceptions import PermissionDenied from rest_framework.response import Response from rest_framework.views import APIView from openforms.api.authentication import AnonCSRFSessionAuthentication from openforms.api.views import ListMixin +from openforms.submissions.constants import SUBMISSIONS_SESSION_KEY +from openforms.submissions.models import Submission from openforms.validations.api.serializers import ( ValidationInputSerializer, ValidationPluginSerializer, @@ -77,5 +80,13 @@ def post(self, request, *args, **kwargs): serializer = ValidationInputSerializer(data=self.request.data) serializer.is_valid(raise_exception=True) - result = register.validate(self.kwargs["validator"], serializer.data["value"]) + if serializer.data["submission_uuid"] not in request.session.get( + SUBMISSIONS_SESSION_KEY, [] + ): + raise PermissionDenied() + + submission = Submission.objects.get(uuid=serializer.data["submission_uuid"]) + result = register.validate( + self.kwargs["validator"], serializer.data["value"], submission + ) return Response(ValidationResultSerializer(result).data) diff --git a/src/openforms/validations/registry.py b/src/openforms/validations/registry.py index 875693857c..d4ed485be8 100644 --- a/src/openforms/validations/registry.py +++ b/src/openforms/validations/registry.py @@ -10,6 +10,7 @@ from rest_framework.serializers import as_serializer_error from openforms.plugins.registry import BaseRegistry +from openforms.submissions.models import Submission logger = logging.getLogger(__name__) @@ -96,7 +97,9 @@ def decorator(validator: Union[Type, ValidatorType]): return decorator @elasticapm.capture_span("app.validations.validate") - def validate(self, plugin_id: str, value: str) -> ValidationResult: + def validate( + self, plugin_id: str, value: str, submission: Submission + ) -> ValidationResult: try: validator = self._registry[plugin_id] except KeyError: @@ -119,7 +122,7 @@ def validate(self, plugin_id: str, value: str) -> ValidationResult: ) try: - validator(value) + validator(value, submission) except (DJ_ValidationError, DRF_ValidationError) as e: errors = as_serializer_error(e) messages = flatten(errors.values()) diff --git a/src/openforms/validations/validators/formats.py b/src/openforms/validations/validators/formats.py index bdff1004cd..f574f5ffe7 100644 --- a/src/openforms/validations/validators/formats.py +++ b/src/openforms/validations/validators/formats.py @@ -23,7 +23,7 @@ class PhoneNumberBaseValidator: error_message = _("Not a valid %(country)s phone number") _parse_phonenumber: ParsePhoneNumber - def __call__(self, value): + def __call__(self, value, submission): z = self._parse_phonenumber(value) if not phonenumbers.is_possible_number(z) or not phonenumbers.is_valid_number(