Skip to content

Commit

Permalink
Merge pull request #4740 from open-formulieren/issue/4713-token-excha…
Browse files Browse the repository at this point in the history
…nge-brp-family-members

🐛 [#4713] Fix token exchange for BRP client
  • Loading branch information
sergei-maertens authored Oct 9, 2024
2 parents 961e607 + 7393002 commit d777da6
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 13 deletions.
7 changes: 7 additions & 0 deletions src/openforms/contrib/haal_centraal/clients/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
58 changes: 54 additions & 4 deletions src/openforms/contrib/haal_centraal/tests/test_brp_clients.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
11 changes: 2 additions & 9 deletions src/openforms/prefill/contrib/haalcentraal_brp/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {}

Expand All @@ -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 ({}, "")

Expand Down

0 comments on commit d777da6

Please sign in to comment.