From 28b81d1a200ab321d5a5eab014a5f18299dca461 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Tue, 8 Oct 2024 16:19:10 +0200 Subject: [PATCH 1/2] :white_check_mark: [#4713] Test for BRP client pre request hooks to ensure that token_exchange works correctly --- .../haal_centraal/tests/test_brp_clients.py | 58 +++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/src/openforms/contrib/haal_centraal/tests/test_brp_clients.py b/src/openforms/contrib/haal_centraal/tests/test_brp_clients.py index d56b2bbbe4..a629e29bf8 100644 --- a/src/openforms/contrib/haal_centraal/tests/test_brp_clients.py +++ b/src/openforms/contrib/haal_centraal/tests/test_brp_clients.py @@ -1,6 +1,6 @@ -from unittest.mock import patch +from unittest.mock import MagicMock, patch -from django.test import SimpleTestCase, TestCase +from django.test import SimpleTestCase, TestCase, tag import requests_mock from glom import glom @@ -9,6 +9,8 @@ from openforms.authentication.service import AuthAttribute from openforms.authentication.utils import store_registrator_details from openforms.config.models import GlobalConfiguration +from openforms.pre_requests.base import PreRequestHookBase +from openforms.pre_requests.registry import Registry from openforms.submissions.tests.factories import SubmissionFactory from ..clients import get_brp_client @@ -171,8 +173,40 @@ def test_default_client_context(self): self.assertIsNone(client.pre_request_context) # type: ignore + @tag("gh-4713") + def test_pre_request_hooks_called(self): + """ + Regression test for #4713, assert that the pre request hooks are called with the + expected context to make sure that token exchange works properly + """ + pre_req_register = Registry() + mock = MagicMock() -class HaalCentraalFindPersonV1Test(HaalCentraalFindPersonTests, SimpleTestCase): + @pre_req_register("test") + class PreRequestHook(PreRequestHookBase): + def __call__(self, *args, **kwargs): + mock(*args, **kwargs) + + submission_bsn = SubmissionFactory.create( + form__generate_minimal_setup=True, + form__authentication_backends=["demo"], + form__formstep__form_definition__login_required=False, + auth_info__attribute_hashed=False, + auth_info__attribute=AuthAttribute.bsn, + auth_info__value="71291440", + auth_info__plugin="demo", + ) + client = get_brp_client(submission_bsn) + + with patch("openforms.pre_requests.clients.registry", new=pre_req_register): + client.find_person("999990676", attributes=self.attributes_to_query) + + self.assertEqual(mock.call_count, 1) # 1 API calls expected + context = mock.call_args.kwargs["context"] + self.assertEqual(context, {"submission": submission_bsn}) # type: ignore + + +class HaalCentraalFindPersonV1Test(HaalCentraalFindPersonTests, TestCase): version = BRPVersions.v13 def test_find_person_succesfully(self): @@ -369,8 +403,16 @@ def test_get_family_members(self): ) super().test_get_family_members() + def test_pre_request_hooks_called(self): + self.requests_mock.get( + "https://personen/api/ingeschrevenpersonen/999990676", + status_code=200, + json=load_json_mock("ingeschrevenpersonen.v1.json"), + ) + super().test_pre_request_hooks_called() + -class HaalCentraalFindPersonV2Test(HaalCentraalFindPersonTests, SimpleTestCase): +class HaalCentraalFindPersonV2Test(HaalCentraalFindPersonTests, TestCase): version = BRPVersions.v20 def test_find_person_succesfully(self): @@ -546,6 +588,14 @@ def test_get_family_members(self): ) super().test_get_family_members() + def test_pre_request_hooks_called(self): + self.requests_mock.post( + "https://personen/api/personen", + status_code=200, + json=load_json_mock("ingeschrevenpersonen.v2-full.json"), + ) + super().test_pre_request_hooks_called() + class ClientFactoryInvalidVersionTests(SimpleTestCase): def test_invalid_version_raises_error(self): From 739300283863097867d9d1f36892954703dae4b1 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Tue, 8 Oct 2024 16:19:52 +0200 Subject: [PATCH 2/2] :bug: [#4713] Fix token exchange for BRP client previously the submission was not passed, causing token exchange to not work --- .../contrib/haal_centraal/clients/__init__.py | 7 +++++++ .../prefill/contrib/haalcentraal_brp/plugin.py | 11 ++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/openforms/contrib/haal_centraal/clients/__init__.py b/src/openforms/contrib/haal_centraal/clients/__init__.py index 24e656c274..5824ef084e 100644 --- a/src/openforms/contrib/haal_centraal/clients/__init__.py +++ b/src/openforms/contrib/haal_centraal/clients/__init__.py @@ -60,6 +60,13 @@ def get_brp_client(submission: Submission | None = None, **kwargs: Any) -> BRPCl f"No suitable client class configured for API version {version}" ) + if submission: + kwargs.setdefault("context", {}) + # pass submission to ensure token exchange works properly + # only do this if the submission is not None, to avoid unnecessary database queries + # done by token_exchange.auth.TokenAccessAuth + kwargs["context"]["submission"] = submission + return build_client( service, client_factory=ClientCls, diff --git a/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py b/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py index 2c7507bb6f..43ae3a9d7d 100644 --- a/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py +++ b/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py @@ -13,7 +13,6 @@ from openforms.contrib.haal_centraal.constants import BRPVersions from openforms.contrib.haal_centraal.models import HaalCentraalConfig from openforms.plugins.exceptions import InvalidPluginConfiguration -from openforms.pre_requests.clients import PreRequestClientContext from openforms.submissions.models import Submission from ...base import BasePlugin @@ -111,10 +110,7 @@ def get_prefill_values( identifier_role: IdentifierRoles = IdentifierRoles.main, ) -> dict[str, Any]: try: - client = get_brp_client( - submission=submission, - context=PreRequestClientContext(submission=submission), - ) + client = get_brp_client(submission=submission) except NoServiceConfigured: return {} @@ -141,10 +137,7 @@ def get_co_sign_values( the value is the prefill value to use for that attribute. """ try: - client = get_brp_client( - submission=submission, - context=PreRequestClientContext(submission=submission), - ) + client = get_brp_client(submission=submission) except NoServiceConfigured: return ({}, "")