From 92e27afdba8b7f911774411551b97a804f0f147a Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 3 May 2024 18:28:56 +0200 Subject: [PATCH] :green_heart: [#4246] Update existing tests The tests were testing the implementation details way too much, so they've been updated by removing the fluff and asserting the functional aspects instead. --- .../tests/digid/test_auth_procedure.py | 392 ++++++------------ src/openforms/authentication/tests/utils.py | 26 ++ 2 files changed, 154 insertions(+), 264 deletions(-) create mode 100644 src/openforms/authentication/tests/utils.py diff --git a/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/digid/test_auth_procedure.py b/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/digid/test_auth_procedure.py index 8a57272131..0bc1593627 100644 --- a/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/digid/test_auth_procedure.py +++ b/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/digid/test_auth_procedure.py @@ -9,10 +9,11 @@ from digid_eherkenning_oidc_generics.models import OpenIDConnectPublicConfig from openforms.accounts.tests.factories import StaffUserFactory +from openforms.authentication.tests.utils import get_start_form_url, get_start_url from openforms.authentication.views import BACKEND_OUTAGE_RESPONSE_PARAMETER from openforms.forms.tests.factories import FormFactory -default_config = dict( +default_config = OpenIDConnectPublicConfig( enabled=True, oidc_rp_client_id="testclient", oidc_rp_client_secret="secret", @@ -29,132 +30,103 @@ class DigiDOIDCTests(TestCase): @classmethod def setUpTestData(cls): + super().setUpTestData() cls.form = FormFactory.create( - generate_minimal_setup=True, authentication_backends=["digid_oidc"] + generate_minimal_setup=True, + authentication_backends=["digid_oidc"], ) - @patch( - "digid_eherkenning_oidc_generics.models.OpenIDConnectPublicConfig.get_solo", - return_value=OpenIDConnectPublicConfig(**default_config), - ) - def test_redirect_to_digid_oidc(self, *m): - login_url = reverse( - "authentication:start", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ) - - form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug}) - form_url = str(furl(f"http://testserver{form_path}").set({"_start": "1"})) - start_url = furl(login_url).set({"next": form_url}) - response = self.client.get(start_url) - - self.assertEqual(status.HTTP_302_FOUND, response.status_code) - - parsed = furl(response.url) - query_params = parsed.query.params + def setUp(self): + super().setUp() - self.assertEqual(parsed.host, "testserver") - self.assertEqual(parsed.path, reverse("digid_oidc:oidc_authentication_init")) + config_patcher = patch( + "digid_eherkenning_oidc_generics.models.OpenIDConnectPublicConfig.get_solo", + return_value=default_config, + ) + self.mock_config = config_patcher.start() + self.addCleanup(config_patcher.stop) - parsed = furl(query_params["next"]) - query_params = parsed.query.params + self.requests_mocker = requests_mock.Mocker() + self.addCleanup(self.requests_mocker.stop) + self.requests_mocker.start() - self.assertEqual( - parsed.path, - reverse( - "authentication:return", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ), + def test_redirect_to_digid_oidc(self): + self.requests_mocker.get( + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + status_code=200, ) - self.assertEqual(query_params["next"], form_url) + form_url = get_start_form_url(self.form) + start_url = get_start_url(self.form, plugin_id="digid_oidc") - with requests_mock.Mocker() as m: - m.get( - "http://provider.com/auth/realms/master/protocol/openid-connect/auth", - status_code=200, - ) - response = self.client.get(response.url) + response = self.client.get(start_url) - self.assertEqual(status.HTTP_302_FOUND, response.status_code) + with self.subTest("Sends user to IdP"): + self.assertEqual(status.HTTP_302_FOUND, response.status_code) - parsed = furl(response.url) - query_params = parsed.query.params + redirect_target = furl(response.url) # type: ignore + query_params = redirect_target.query.params - self.assertEqual(parsed.host, "provider.com") - self.assertEqual( - parsed.path, "/auth/realms/master/protocol/openid-connect/auth" - ) - self.assertEqual(query_params["scope"], "openid bsn") - self.assertEqual(query_params["client_id"], "testclient") - self.assertEqual( - query_params["redirect_uri"], - f"http://testserver{reverse('digid_oidc:oidc_authentication_callback')}", - ) + self.assertEqual(redirect_target.host, "provider.com") + self.assertEqual( + redirect_target.path, + "/auth/realms/master/protocol/openid-connect/auth", + ) + self.assertEqual(query_params["scope"], "openid bsn") + self.assertEqual(query_params["client_id"], "testclient") + self.assertEqual( + query_params["redirect_uri"], + f"http://testserver{reverse('digid_oidc:oidc_authentication_callback')}", + ) - parsed = furl(self.client.session["oidc_login_next"]) - query_params = parsed.query.params + with self.subTest("Return state setup"): + oidc_login_next = furl(self.client.session["oidc_login_next"]) + query_params = oidc_login_next.query.params - self.assertEqual( - parsed.path, - reverse( - "authentication:return", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ), - ) - self.assertEqual(query_params["next"], form_url) + self.assertEqual( + oidc_login_next.path, + reverse( + "authentication:return", + kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, + ), + ) + self.assertEqual(query_params["next"], form_url) - @patch( - "digid_eherkenning_oidc_generics.models.OpenIDConnectPublicConfig.get_solo", - return_value=OpenIDConnectPublicConfig(**default_config), - ) - def test_redirect_to_digid_oidc_internal_server_error(self, *m): - login_url = reverse( - "authentication:start", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, + def test_redirect_to_digid_oidc_internal_server_error(self): + self.requests_mocker.get( + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + status_code=500, ) + start_url = get_start_url(self.form, plugin_id="digid_oidc") - form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug}) - form_url = str(furl(f"http://testserver{form_path}").set({"_start": "1"})) - start_url = furl(login_url).set({"next": form_url}) response = self.client.get(start_url) self.assertEqual(status.HTTP_302_FOUND, response.status_code) - - with requests_mock.Mocker() as m: - m.get( - "http://provider.com/auth/realms/master/protocol/openid-connect/auth", - status_code=500, - ) - response = self.client.get(response.url) - - parsed = furl(response.url) - query_params = parsed.query.params + assert self.requests_mocker.last_request is not None + self.assertEqual( + self.requests_mocker.last_request.url, + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + ) + parsed = furl(response.url) # type: ignore self.assertEqual(parsed.host, "testserver") - self.assertEqual(parsed.path, form_path) - self.assertEqual(query_params["of-auth-problem"], "digid_oidc") - - @patch( - "digid_eherkenning_oidc_generics.models.OpenIDConnectPublicConfig.get_solo", - return_value=OpenIDConnectPublicConfig(**default_config), - ) - def test_redirect_to_digid_oidc_callback_error(self, *m): - form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug}) - form_url = f"http://testserver{form_path}" - redirect_form_url = furl(form_url).set({"_start": "1"}) - redirect_url = furl( - reverse( - "authentication:return", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ) - ).set({"next": redirect_form_url}) + self.assertEqual(parsed.path, f"/{self.form.slug}/") + query_params = parsed.query.params + self.assertEqual(query_params[BACKEND_OUTAGE_RESPONSE_PARAMETER], "digid_oidc") - session = self.client.session - session["of_redirect_next"] = redirect_url.url - session.save() + def test_redirect_to_digid_oidc_callback_error(self): + # set up session/state + self.requests_mocker.get( + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + status_code=200, + ) + start_url = get_start_url(self.form, plugin_id="digid_oidc") + response = self.client.get(start_url) + assert response.status_code == 302 + assert response.url.startswith("http://provider.com") # type: ignore with patch( - "openforms.authentication.contrib.digid_eherkenning_oidc.backends.OIDCAuthenticationDigiDBackend.verify_claims", + "openforms.authentication.contrib.digid_eherkenning_oidc.backends" + ".OIDCAuthenticationDigiDBackend.verify_claims", return_value=False, ): response = self.client.get(reverse("digid_oidc:callback")) @@ -164,164 +136,62 @@ def test_redirect_to_digid_oidc_callback_error(self, *m): parsed = furl(response.url) query_params = parsed.query.params - self.assertEqual(parsed.path, form_path) + self.assertEqual(parsed.path, f"/{self.form.slug}/") self.assertEqual(query_params["_start"], "1") self.assertEqual(query_params[BACKEND_OUTAGE_RESPONSE_PARAMETER], "digid_oidc") @override_settings(CORS_ALLOW_ALL_ORIGINS=False, CORS_ALLOWED_ORIGINS=[]) - @patch( - "digid_eherkenning_oidc_generics.models.OpenIDConnectPublicConfig.get_solo", - return_value=OpenIDConnectPublicConfig(**default_config), - ) - def test_redirect_to_disallowed_domain(self, *m): - login_url = reverse( - "digid_oidc:oidc_authentication_init", + def test_redirect_to_disallowed_domain(self): + start_url = get_start_url( + self.form, plugin_id="digid_oidc", host="http://example.com" ) - form_url = "http://example.com" - start_url = furl(login_url).set({"next": form_url}) response = self.client.get(start_url) - self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @override_settings( CORS_ALLOW_ALL_ORIGINS=False, CORS_ALLOWED_ORIGINS=["http://example.com"] ) - @patch( - "digid_eherkenning_oidc_generics.models.OpenIDConnectPublicConfig.get_solo", - return_value=OpenIDConnectPublicConfig( - enabled=True, - oidc_rp_client_id="testclient", - oidc_rp_client_secret="secret", - oidc_rp_sign_algo="RS256", - oidc_rp_scopes_list=["openid", "bsn"], - oidc_op_jwks_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/certs", - oidc_op_authorization_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/auth", - oidc_op_token_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/token", - oidc_op_user_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/userinfo", - ), - ) - def test_redirect_to_allowed_domain(self, *m): - login_url = reverse( - "authentication:start", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, + def test_redirect_to_allowed_domain(self): + self.requests_mocker.get( + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + status_code=200, + ) + form_url = get_start_form_url(self.form, host="http://example.com") + start_url = get_start_url( + self.form, plugin_id="digid_oidc", host="http://example.com" ) - form_url = "http://example.com" - start_url = furl(login_url).set({"next": form_url}) response = self.client.get(start_url) - self.assertEqual(status.HTTP_302_FOUND, response.status_code) - - parsed = furl(response.url) - query_params = parsed.query.params - - self.assertEqual(parsed.host, "testserver") - self.assertEqual(parsed.path, reverse("digid_oidc:oidc_authentication_init")) - - parsed = furl(query_params["next"]) - query_params = parsed.query.params + with self.subTest("Sends user to IdP"): + self.assertEqual(status.HTTP_302_FOUND, response.status_code) + self.assertTrue(response.url.startswith("http://provider.com/")) # type: ignore - self.assertEqual( - parsed.path, - reverse( + with self.subTest("Return state setup"): + oidc_login_next = furl(self.client.session["oidc_login_next"]) + expected_next = reverse( "authentication:return", kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ), - ) - self.assertEqual(query_params["next"], form_url) - - with requests_mock.Mocker() as m: - m.get( - "http://provider.com/auth/realms/master/protocol/openid-connect/auth", - status_code=200, ) - response = self.client.get(response.url) - - self.assertEqual(status.HTTP_302_FOUND, response.status_code) - - parsed = furl(response.url) - query_params = parsed.query.params - - self.assertEqual(parsed.host, "provider.com") - self.assertEqual( - parsed.path, "/auth/realms/master/protocol/openid-connect/auth" - ) - self.assertEqual(query_params["scope"], "openid bsn") - self.assertEqual(query_params["client_id"], "testclient") - self.assertEqual( - query_params["redirect_uri"], - f"http://testserver{reverse('digid_oidc:oidc_authentication_callback')}", - ) + self.assertEqual(oidc_login_next.path, expected_next) + query_params = oidc_login_next.query.params + self.assertEqual(query_params["next"], form_url) - @patch( - "digid_eherkenning_oidc_generics.models.OpenIDConnectPublicConfig.get_solo", - return_value=OpenIDConnectPublicConfig( - enabled=True, - oidc_rp_client_id="testclient", - oidc_rp_client_secret="secret", - oidc_rp_sign_algo="RS256", - oidc_rp_scopes_list=["openid", "bsn"], - oidc_op_jwks_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/certs", - oidc_op_authorization_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/auth", - oidc_op_token_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/token", - oidc_op_user_endpoint="http://provider.com/auth/realms/master/protocol/openid-connect/userinfo", - oidc_keycloak_idp_hint="oidc-digid", - ), - ) def test_redirect_with_keycloak_identity_provider_hint(self, *m): - login_url = reverse( - "authentication:start", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, + self.mock_config.return_value.oidc_keycloak_idp_hint = "oidc-digid" + self.requests_mocker.get( + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + status_code=200, ) + start_url = get_start_url(self.form, plugin_id="digid_oidc") - form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug}) - form_url = str(furl(f"http://testserver{form_path}").set({"_start": "1"})) - start_url = furl(login_url).set({"next": form_url}) response = self.client.get(start_url) self.assertEqual(status.HTTP_302_FOUND, response.status_code) - - parsed = furl(response.url) - query_params = parsed.query.params - - self.assertEqual(parsed.host, "testserver") - self.assertEqual(parsed.path, reverse("digid_oidc:oidc_authentication_init")) - - parsed = furl(query_params["next"]) - query_params = parsed.query.params - - self.assertEqual( - parsed.path, - reverse( - "authentication:return", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ), - ) - self.assertEqual(query_params["next"], form_url) - - with requests_mock.Mocker() as m: - m.get( - "http://provider.com/auth/realms/master/protocol/openid-connect/auth", - status_code=200, - ) - response = self.client.get(response.url) - - self.assertEqual(status.HTTP_302_FOUND, response.status_code) - - parsed = furl(response.url) + parsed = furl(response.url) # type: ignore query_params = parsed.query.params - - self.assertEqual(parsed.host, "provider.com") - self.assertEqual( - parsed.path, "/auth/realms/master/protocol/openid-connect/auth" - ) - self.assertEqual(query_params["scope"], "openid bsn") - self.assertEqual(query_params["client_id"], "testclient") - self.assertEqual( - query_params["redirect_uri"], - f"http://testserver{reverse('digid_oidc:oidc_authentication_callback')}", - ) self.assertEqual(query_params["kc_idp_hint"], "oidc-digid") @tag("gh-3656", "gh-3692") @@ -329,51 +199,45 @@ def test_redirect_with_keycloak_identity_provider_hint(self, *m): # According to https://openid.net/specs/openid-connect-core-1_0.html#AuthError and # https://www.rfc-editor.org/rfc/rfc6749.html#section-4.1.2.1 , this is the error we expect from OIDC def test_redirect_to_form_when_login_cancelled_by_anonymous_user(self): - form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug}) - form_url = f"http://testserver{form_path}" - redirect_form_url = furl(form_url).set({"_start": "1"}) - redirect_url = furl( - reverse( - "authentication:return", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ) - ).set({"next": redirect_form_url}) - - session = self.client.session - session["of_redirect_next"] = redirect_url.url - session.save() + # set up session/state + self.requests_mocker.get( + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + status_code=200, + ) + start_url = get_start_url(self.form, plugin_id="digid_oidc") + response = self.client.get(start_url) + assert response.status_code == 302 + assert response.url.startswith("http://provider.com") # type: ignore response = self.client.get( reverse("digid_oidc:callback"), - {"error": "access_denied", "error_description": "The user cancelled"}, + { + "error": "access_denied", + "error_description": "The user cancelled", + }, ) self.assertEqual(response.status_code, status.HTTP_302_FOUND) - parsed = furl(response.url) + parsed = furl(response.url) # type: ignore query_params = parsed.query.params self.assertEqual(query_params["_digid-message"], "login-cancelled") - self.assertIsNone(query_params.get("of-auth-problem")) + self.assertIsNone(query_params.get(BACKEND_OUTAGE_RESPONSE_PARAMETER)) @tag("gh-3656", "gh-3692") def test_redirect_to_form_when_login_cancelled_by_authenticated_user(self): + # set up session/state + self.requests_mocker.get( + "http://provider.com/auth/realms/master/protocol/openid-connect/auth", + status_code=200, + ) user = StaffUserFactory.create() + start_url = get_start_url(self.form, plugin_id="digid_oidc") self.client.force_login(user=user) - - form_path = reverse("core:form-detail", kwargs={"slug": self.form.slug}) - form_url = f"http://testserver{form_path}" - redirect_form_url = furl(form_url).set({"_start": "1"}) - redirect_url = furl( - reverse( - "authentication:return", - kwargs={"slug": self.form.slug, "plugin_id": "digid_oidc"}, - ) - ).set({"next": redirect_form_url}) - - session = self.client.session - session["of_redirect_next"] = redirect_url.url - session.save() + response = self.client.get(start_url) + assert response.status_code == 302 + assert response.url.startswith("http://provider.com") # type: ignore response = self.client.get( reverse("digid_oidc:callback"), @@ -386,4 +250,4 @@ def test_redirect_to_form_when_login_cancelled_by_authenticated_user(self): query_params = parsed.query.params self.assertEqual(query_params["_digid-message"], "login-cancelled") - self.assertIsNone(query_params.get("of-auth-problem")) + self.assertIsNone(query_params.get(BACKEND_OUTAGE_RESPONSE_PARAMETER)) diff --git a/src/openforms/authentication/tests/utils.py b/src/openforms/authentication/tests/utils.py new file mode 100644 index 0000000000..bc3ac56c60 --- /dev/null +++ b/src/openforms/authentication/tests/utils.py @@ -0,0 +1,26 @@ +from django.urls import reverse + +from furl import furl + +from openforms.forms.models import Form + + +def get_start_form_url(form: Form, host: str = "") -> str: + form_path = reverse("core:form-detail", kwargs={"slug": form.slug}) + if not host: + host = "http://testserver" + form_url = furl(f"{host}{form_path}").set({"_start": "1"}) + return str(form_url) + + +def get_start_url(form: Form, plugin_id: str, host: str = "") -> str: + """ + Build the authentication start URL as constructed by the SDK. + """ + login_url = reverse( + "authentication:start", + kwargs={"slug": form.slug, "plugin_id": plugin_id}, + ) + form_url = get_start_form_url(form, host=host) + start_url = furl(login_url).set({"next": form_url}) + return str(start_url)