From 125b3b86c542978357151d212feb102bd61371e1 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Fri, 20 Dec 2024 16:38:44 +0100 Subject: [PATCH] [#2808] Use KVK API to checkout legal status of compay at login --- src/eherkenning/tests/test_mock_views.py | 20 +++++-- src/open_inwoner/accounts/tests/test_auth.py | 35 +++++++++---- .../accounts/tests/test_oidc_views.py | 52 ++++++++++++++++--- src/open_inwoner/accounts/views/auth.py | 29 ++++++----- src/open_inwoner/kvk/client.py | 6 +++ 5 files changed, 111 insertions(+), 31 deletions(-) diff --git a/src/eherkenning/tests/test_mock_views.py b/src/eherkenning/tests/test_mock_views.py index 00778fc6dc..cc2f0718ac 100644 --- a/src/eherkenning/tests/test_mock_views.py +++ b/src/eherkenning/tests/test_mock_views.py @@ -108,6 +108,7 @@ def test_get_returns_valid_response(self): self.assertContains(response, reverse("login")) self.assertNoEHerkenningURLS(response) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk", return_value="123456789", @@ -115,12 +116,15 @@ def test_get_returns_valid_response(self): ) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches", autospec=True) def test_post_redirects_and_authenticates( - self, mock_kvk, mock_retrieve_rsin_with_kvk + self, mock_kvk, mock_retrieve_rsin_with_kvk, mock_get_basisprofiel ): mock_kvk.return_value = [ {"kvkNummer": "29664887", "vestigingsnummer": "1234"}, {"kvkNummer": "29664887", "vestigingsnummer": "5678"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } url = reverse("eherkenning-mock:password") params = { @@ -164,8 +168,9 @@ def test_post_redirects_and_authenticates( # check company branch number in session self.assertEqual(get_kvk_branch_number(self.client.session), "1234") + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches") - def test_redirect_flow_with_single_company(self, mock_kvk): + def test_redirect_flow_with_single_company(self, mock_kvk, mock_get_basisprofiel): """ Assert that if the KvK API returns only a single company: 1. the redirect flow passes automatically through `KvKLoginMiddleware` @@ -174,6 +179,9 @@ def test_redirect_flow_with_single_company(self, mock_kvk): mock_kvk.return_value = [ {"kvkNummer": "29664887", "vestigingsnummer": "1234"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } url = reverse("eherkenning-mock:password") params = { @@ -201,8 +209,11 @@ def test_redirect_flow_with_single_company(self, mock_kvk): # check company branch number in session self.assertEqual(get_kvk_branch_number(self.client.session), None) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches") - def test_redirect_flow_with_no_vestigingsnummer(self, mock_kvk): + def test_redirect_flow_with_no_vestigingsnummer( + self, mock_kvk, mock_get_basisprofiel + ): """ Assert that if the KvK API returns only a single company without vestigingsnummer: 1. the redirect flow passes automatically through `KvKLoginMiddleware` @@ -211,6 +222,9 @@ def test_redirect_flow_with_no_vestigingsnummer(self, mock_kvk): mock_kvk.return_value = [ {"kvkNummer": "29664887"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } url = reverse("eherkenning-mock:password") params = { diff --git a/src/open_inwoner/accounts/tests/test_auth.py b/src/open_inwoner/accounts/tests/test_auth.py index ff933c7ef7..d139fa9cd5 100644 --- a/src/open_inwoner/accounts/tests/test_auth.py +++ b/src/open_inwoner/accounts/tests/test_auth.py @@ -20,7 +20,6 @@ from open_inwoner.kvk.branches import get_kvk_branch_number from open_inwoner.kvk.tests.factories import CertificateFactory from open_inwoner.openklant.tests.data import MockAPIReadPatchData -from open_inwoner.openzaak.models import OpenZaakConfig from open_inwoner.utils.tests.helpers import AssertTimelineLogMixin from ...cms.collaborate.cms_apps import CollaborateApphook @@ -598,19 +597,20 @@ def test_eherkenning_fail_without_invite_redirects_to_login_page(self, m): self.assertRedirectsLogin(response, with_host=True) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk", return_value="", autospec=True, ) - @patch( - "open_inwoner.accounts.views.auth.OpenZaakConfig.get_solo", - return_value=OpenZaakConfig(fetch_eherkenning_zaken_with_rsin=True), - autospec=True, - ) def test_login_as_eenmanszaak_blocked( - self, mock_oz_config, mock_retrieve_rsin_with_kvk + self, + mock_retrieve_rsin_with_kvk, + mock_get_basisprofiel, ): + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Eenmanszaak"}} + } url = reverse("eherkenning-mock:password") params = { "acs": f"http://testserver{reverse('eherkenning:acs')}", @@ -693,6 +693,7 @@ def test_eherkenning_fail_with_invite_redirects_to_register_page(self, m): f"http://testserver{reverse('django_registration_register')}?invite={invite.key}", ) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk", return_value="123456789", @@ -711,10 +712,14 @@ def test_invite_url_not_in_session_after_successful_login( mock_solo, mock_kvk, mock_retrieve_rsin_with_kvk, + mock_get_basisprofiel, ): mock_kvk.return_value = [ {"kvkNummer": "12345678", "vestigingsnummer": "1234"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } mock_solo.return_value.api_key = "123" mock_solo.return_value.api_root = "http://foo.bar/api/v1/" @@ -758,11 +763,14 @@ def test_invite_url_not_in_session_after_successful_login( # check company branch number in session self.assertEqual(get_kvk_branch_number(self.client.session), None) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches") @patch( "open_inwoner.kvk.models.KvKConfig.get_solo", ) - def test_redirect_flow_with_no_vestigingsnummer(self, mock_solo, mock_kvk): + def test_redirect_flow_with_no_vestigingsnummer( + self, mock_solo, mock_kvk, mock_get_basisprofiel + ): """ Assert that if the KvK API returns only a single company without vestigingsnummer: 1. the redirect flow passes automatically through `KvKLoginMiddleware` @@ -771,6 +779,9 @@ def test_redirect_flow_with_no_vestigingsnummer(self, mock_solo, mock_kvk): mock_kvk.return_value = [ {"kvkNummer": "12345678"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } mock_solo.return_value.api_key = "123" mock_solo.return_value.api_root = "http://foo.bar/api/v1/" @@ -1139,6 +1150,7 @@ def test_digid_user_success(self): self.assertEqual(users.first().email, "test@example.com") self.assertEqual(users.last().email, "test@example.com") + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk", return_value="123456789", @@ -1148,7 +1160,9 @@ def test_digid_user_success(self): "open_inwoner.kvk.client.KvKClient.get_all_company_branches", autospec=True, ) - def test_eherkenning_user_success(self, mock_kvk, mock_retrieve_rsin_with_kvk): + def test_eherkenning_user_success( + self, mock_kvk, mock_retrieve_rsin_with_kvk, mock_get_basisprofiel + ): """Assert that eHerkenning users can register with duplicate emails""" mock_kvk.return_value = [ @@ -1163,6 +1177,9 @@ def test_eherkenning_user_success(self, mock_kvk, mock_retrieve_rsin_with_kvk): "naam": "Mijn bedrijf", }, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } test_user = eHerkenningUserFactory.create( email="test@localhost", diff --git a/src/open_inwoner/accounts/tests/test_oidc_views.py b/src/open_inwoner/accounts/tests/test_oidc_views.py index ba4adfae22..4c30bc8ff5 100644 --- a/src/open_inwoner/accounts/tests/test_oidc_views.py +++ b/src/open_inwoner/accounts/tests/test_oidc_views.py @@ -24,7 +24,6 @@ from open_inwoner.configurations.choices import OpenIDDisplayChoices from open_inwoner.configurations.models import SiteConfiguration from open_inwoner.kvk.branches import KVK_BRANCH_SESSION_VARIABLE -from open_inwoner.openzaak.models import OpenZaakConfig from ...cms.profile.cms_apps import ProfileApphook from ...cms.tests import cms_tools @@ -1061,6 +1060,7 @@ def setUpClass(cls): cms_tools.create_homepage() cms_tools.create_apphook_page(ProfileApphook) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches") @patch("open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") @@ -1082,10 +1082,14 @@ def test_existing_kvk_creates_no_new_user( mock_get_userinfo, mock_retrieve_rsin_with_kvk, mock_kvk, + mock_get_basisprofiel, ): mock_kvk.return_value = [ {"vestigingsnummer": "1234"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } mock_retrieve_rsin_with_kvk.return_value = "123456789" # set up a user with a colliding email address @@ -1135,6 +1139,7 @@ def test_existing_kvk_creates_no_new_user( self.assertEqual(db_user.first_name, "John") self.assertEqual(db_user.last_name, "Doe") + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens") @@ -1154,7 +1159,11 @@ def test_new_user_is_created_when_new_kvk( mock_store_tokens, mock_get_userinfo, mock_retrieve_rsin_with_kvk, + mock_get_basisprofiel, ): + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } mock_retrieve_rsin_with_kvk.return_value = "123456789" # set up a user with a non existing email address mock_get_userinfo.return_value = {"sub": "00000000"} @@ -1291,6 +1300,7 @@ def test_logout_without_sso_logout_configured(self, mock_get_solo): ] } ) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk", return_value="123456789", @@ -1329,12 +1339,16 @@ def test_error_first_cleared_after_succesful_login( mock_get_userinfo, mock_kvk, mock_retrieve_rsin_with_kvk, + mock_get_basisprofiel, ): mock_get_userinfo.return_value = { "sub": "some_username", "kvk": "12345678", } mock_kvk.return_value = [{"vestigingsnummber": "1234"}] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } session = self.client.session session["oidc-error"] = "some error" @@ -1488,6 +1502,7 @@ def test_login_error_message_not_mapped_in_config( self.assertEqual(error_msg, str(GENERIC_EHERKENNING_ERROR_MSG)) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.verify_token") @@ -1505,12 +1520,16 @@ def test_login_validation_error( mock_verify_token, mock_store_tokens, mock_get_userinfo, + mock_get_basisprofiel, ): mock_verify_token.side_effect = ValidationError("Something went wrong") mock_get_userinfo.return_value = { "sub": "some_username", "kvk": "12345678", } + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } session = self.client.session session["oidc_states"] = { @@ -1543,12 +1562,8 @@ def test_login_validation_error( self.assertEqual(error_msg, str(GENERIC_EHERKENNING_ERROR_MSG)) - @patch( - "open_inwoner.accounts.views.auth.OpenZaakConfig.get_solo", - return_value=OpenZaakConfig(fetch_eherkenning_zaken_with_rsin=True), - autospec=True, - ) @patch("open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk", autospec=True) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo", autospec=True, @@ -1579,8 +1594,8 @@ def test_login_as_eenmanszaak_blocked( mock_verify_token, mock_store_tokens, mock_get_userinfo, + mock_get_basisprofiel, mock_retrieve_rsin_with_kvk, - mock_oz_config, ): """ Eenmanszaken do not have an RSIN, which means that if we have a feature flag @@ -1588,6 +1603,9 @@ def test_login_as_eenmanszaak_blocked( let eenmanszaken log in using eHerkenning """ mock_retrieve_rsin_with_kvk.return_value = "" + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Eenmanszaak"}} + } # set up a user with a non existing email address mock_get_userinfo.return_value = {"sub": "00000000"} eHerkenningUserFactory.create(kvk="12345678", email="existing_user@example.com") @@ -1623,6 +1641,7 @@ def test_login_as_eenmanszaak_blocked( return_value="123456789", autospec=True, ) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "open_inwoner.kvk.client.KvKClient.get_all_company_branches", autospec=True, @@ -1666,6 +1685,7 @@ def test_redirect_after_login_with_registration_and_branch_selection( mock_get_userinfo, mock_siteconfig, mock_kvk, + mock_get_basisprofiel, mock_retrieve_rsin_with_kvk, ): """ @@ -1680,6 +1700,9 @@ def test_redirect_after_login_with_registration_and_branch_selection( {"kvkNummer": "12345678"}, {"kvkNummer": "12345678", "vestigingsnummer": "1234"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } self.assertEqual(User.objects.count(), 0) @@ -1741,6 +1764,7 @@ def test_redirect_after_login_with_registration_and_branch_selection( "open_inwoner.kvk.signals.KvKClient.retrieve_rsin_with_kvk", autospec=True, ) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch( "open_inwoner.kvk.client.KvKClient.get_all_company_branches", autospec=True, @@ -1784,6 +1808,7 @@ def test_redirect_after_login_no_registration_with_branch_selection( mock_get_userinfo, mock_siteconfig, mock_kvk, + mock_get_basisprofiel, mock_retrieve_rsin_with_kvk, ): """ @@ -1799,6 +1824,9 @@ def test_redirect_after_login_no_registration_with_branch_selection( {"kvkNummer": "12345678"}, {"kvkNummer": "12345678", "vestigingsnummer": "1234"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } self.assertEqual(User.objects.count(), 1) @@ -1842,6 +1870,7 @@ def test_redirect_after_login_no_registration_with_branch_selection( self.assertEqual(profile_response.status_code, 200) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches") @patch("open_inwoner.utils.context_processors.SiteConfiguration") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") @@ -1866,6 +1895,7 @@ def test_redirect_after_login_no_registration_and_no_branch_selection( mock_get_userinfo, mock_siteconfig, mock_kvk, + mock_get_basisprofiel, ): """ Full authentication flow with redirect after successful login @@ -1879,6 +1909,9 @@ def test_redirect_after_login_no_registration_and_no_branch_selection( mock_kvk.return_value = [ {"kvkNummer": "12345678"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } self.assertEqual(User.objects.count(), 1) @@ -1921,6 +1954,7 @@ def test_redirect_after_login_no_registration_and_no_branch_selection( self.assertEqual(profile_response.status_code, 200) + @patch("open_inwoner.kvk.signals.KvKClient.get_basisprofiel", autospec=True) @patch("open_inwoner.kvk.client.KvKClient.get_all_company_branches") @patch("open_inwoner.utils.context_processors.SiteConfiguration") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") @@ -1945,6 +1979,7 @@ def test_redirect_after_login_branch_already_selected( mock_get_userinfo, mock_siteconfig, mock_kvk, + mock_get_basisprofiel, ): """ KVK branch selection should be skipped if KVK_BRANCH_SESSION_VARIABLE is present in session @@ -1960,6 +1995,9 @@ def test_redirect_after_login_branch_already_selected( {"kvkNummer": "12345678"}, {"kvkNummer": "87654321"}, ] + mock_get_basisprofiel.return_value = { + "_embedded": {"eigenaar": {"rechtsvorm": "Stichting"}} + } self.assertEqual(User.objects.count(), 1) diff --git a/src/open_inwoner/accounts/views/auth.py b/src/open_inwoner/accounts/views/auth.py index c4c407046d..09f46506eb 100644 --- a/src/open_inwoner/accounts/views/auth.py +++ b/src/open_inwoner/accounts/views/auth.py @@ -23,8 +23,7 @@ from eherkenning.mock.views.eherkenning import ( eHerkenningAssertionConsumerServiceMockView, ) -from open_inwoner.openklant.models import OpenKlantConfig -from open_inwoner.openzaak.models import OpenZaakConfig +from open_inwoner.kvk.client import KvKClient from open_inwoner.utils.views import LogMixin from ..choices import LoginTypeChoices @@ -136,16 +135,22 @@ class BlockEenmanszaakLoginMixin: def get(self, request): response = super().get(request) - openzaak_config = OpenZaakConfig.get_solo() - openklant_config = OpenKlantConfig.get_solo() - if ( - hasattr(request.user, "rsin") - and not request.user.rsin - and ( - openzaak_config.fetch_eherkenning_zaken_with_rsin - or openklant_config.use_rsin_for_innNnpId_query_parameter - ) - ): + if not hasattr(request.user, "kvk"): + return response + if not (kvk := request.user.kvk): + return response + + client = KvKClient() + basisprofiel = client.get_basisprofiel(kvk=kvk) + + if basisprofiel.get("fout"): + auth.logout(request) + message = basisprofiel["fout"][0]["omschrijving"] + messages.error(request, message) + failure_url = self.get_failure_url() + return HttpResponseRedirect(failure_url) + + if basisprofiel["_embedded"]["eigenaar"]["rechtsvorm"] == "Eenmanszaak": auth.logout(request) message = _("Use DigiD to log in as a sole proprietor.") messages.error(request, message) diff --git a/src/open_inwoner/kvk/client.py b/src/open_inwoner/kvk/client.py index 23ca0e4773..d9b9f92337 100644 --- a/src/open_inwoner/kvk/client.py +++ b/src/open_inwoner/kvk/client.py @@ -5,6 +5,8 @@ import requests from requests.exceptions import JSONDecodeError +from open_inwoner.utils.decorators import cache as cache_result + from .constants import CompanyType from .models import KvKConfig @@ -122,6 +124,10 @@ def get_all_company_branches(self, kvk: str, **kwargs) -> list[dict | None]: return branches + @cache_result("kvk:{kvk}") + def get_basisprofiel(self, kvk: str, **kwargs) -> dict: + return self._request(f"{self.basisprofielen_endpoint}/{kvk}", params=kwargs) + def retrieve_rsin_with_kvk(self, kvk, **kwargs) -> str | None: basisprofiel = self._request( f"{self.basisprofielen_endpoint}/{kvk}", params=kwargs