From d98aff26c9f67597359ec6a9cbd129465b99a16f Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 6 Oct 2023 16:16:22 +0000 Subject: [PATCH] config_error: Return status code 500. --- zerver/tests/test_auth_backends.py | 209 ++++++++++++++++++++--------- zerver/tests/test_docs.py | 11 +- zerver/tests/test_urls.py | 10 +- zerver/views/errors.py | 2 +- 4 files changed, 162 insertions(+), 70 deletions(-) diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 0ff232eeaf..75b0bac162 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -1086,38 +1086,59 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase, ABC): def test_social_auth_no_key(self) -> None: account_data_dict = self.get_account_data_dict(email=self.email, name=self.name) - with self.settings(**{self.CLIENT_KEY_SETTING: None}): + with self.settings(**{self.CLIENT_KEY_SETTING: None}), self.assertLogs( + "django.request", level="ERROR" + ) as m: result = self.social_auth_test( account_data_dict, subdomain="zulip", next="/user_uploads/image" ) - self.assert_in_success_response(["Configuration error"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assertEqual( + m.output, + [f"ERROR:django.request:Internal Server Error: {self.LOGIN_URL}"], + ) def test_config_error_development(self) -> None: if hasattr(self, "CLIENT_KEY_SETTING") and hasattr(self, "CLIENT_SECRET_SETTING"): - with self.settings(**{self.CLIENT_KEY_SETTING: None}): + with self.settings(**{self.CLIENT_KEY_SETTING: None}), self.assertLogs( + "django.request", level="ERROR" + ) as m: result = self.client_get(self.LOGIN_URL) - self.assert_in_success_response(["Configuration error"], result) - self.assert_in_success_response([self.CLIENT_KEY_SETTING.lower()], result) - self.assert_in_success_response([self.CLIENT_SECRET_SETTING.lower()], result) - self.assert_in_success_response(["zproject/dev-secrets.conf"], result) - self.assert_not_in_success_response([self.CLIENT_KEY_SETTING], result) - self.assert_not_in_success_response(["zproject/dev_settings.py"], result) - self.assert_not_in_success_response(["/etc/zulip/settings.py"], result) - self.assert_not_in_success_response(["/etc/zulip/zulip-secrets.conf"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response(self.CLIENT_KEY_SETTING.lower(), result) + self.assert_in_response(self.CLIENT_SECRET_SETTING.lower(), result) + self.assert_in_response("zproject/dev-secrets.conf", result) + self.assertNotIn(self.CLIENT_KEY_SETTING, result.content.decode()) + self.assertNotIn("zproject/dev_settings.py", result.content.decode()) + self.assertNotIn("/etc/zulip/settings.py", result.content.decode()) + self.assertNotIn("/etc/zulip/zulip-secrets.conf", result.content.decode()) + self.assertEqual( + m.output, + [f"ERROR:django.request:Internal Server Error: {self.LOGIN_URL}"], + ) @override_settings(DEVELOPMENT=False) def test_config_error_production(self) -> None: if hasattr(self, "CLIENT_KEY_SETTING") and hasattr(self, "CLIENT_SECRET_SETTING"): - with self.settings(**{self.CLIENT_KEY_SETTING: None}): + with self.settings(**{self.CLIENT_KEY_SETTING: None}), self.assertLogs( + "django.request", level="ERROR" + ) as m: result = self.client_get(self.LOGIN_URL) - self.assert_in_success_response(["Configuration error"], result) - self.assert_in_success_response([self.CLIENT_KEY_SETTING], result) - self.assert_in_success_response(["/etc/zulip/settings.py"], result) - self.assert_in_success_response([self.CLIENT_SECRET_SETTING.lower()], result) - self.assert_in_success_response(["/etc/zulip/zulip-secrets.conf"], result) - self.assert_not_in_success_response([self.CLIENT_KEY_SETTING.lower()], result) - self.assert_not_in_success_response(["zproject/dev_settings.py"], result) - self.assert_not_in_success_response(["zproject/dev-secrets.conf"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response(self.CLIENT_KEY_SETTING, result) + self.assert_in_response("/etc/zulip/settings.py", result) + self.assert_in_response(self.CLIENT_SECRET_SETTING.lower(), result) + self.assert_in_response("/etc/zulip/zulip-secrets.conf", result) + self.assertNotIn(self.CLIENT_KEY_SETTING.lower(), result.content.decode()) + self.assertNotIn("zproject/dev_settings.py", result.content.decode()) + self.assertNotIn("zproject/dev-secrets.conf", result.content.decode()) + self.assertEqual( + m.output, + [f"ERROR:django.request:Internal Server Error: {self.LOGIN_URL}"], + ) def test_social_auth_success(self) -> None: account_data_dict = self.get_account_data_dict(email=self.email, name=self.name) @@ -2448,25 +2469,39 @@ class SAMLAuthBackendTest(SocialAuthBase): """ account_data_dict = self.get_account_data_dict(email=self.email, name=self.name) with self.settings(SOCIAL_AUTH_SAML_ENABLED_IDPS=None): - result = self.social_auth_test( - account_data_dict, subdomain="zulip", next="/user_uploads/image" - ) - self.assert_in_success_response(["Configuration error", "SAML authentication"], result) + with self.assertLogs("django.request", level="ERROR") as m: + result = self.social_auth_test( + account_data_dict, subdomain="zulip", next="/user_uploads/image" + ) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("SAML authentication", result) + self.assertEqual( + m.output, [f"ERROR:django.request:Internal Server Error: {self.LOGIN_URL}"] + ) - # Test the signup path too: - result = self.social_auth_test( - account_data_dict, is_signup=True, subdomain="zulip", next="/user_uploads/image" - ) - self.assert_in_success_response(["Configuration error", "SAML authentication"], result) + with self.assertLogs("django.request", level="ERROR") as m: + # Test the signup path too: + result = self.social_auth_test( + account_data_dict, is_signup=True, subdomain="zulip", next="/user_uploads/image" + ) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("SAML authentication", result) + self.assertEqual( + m.output, [f"ERROR:django.request:Internal Server Error: {self.SIGNUP_URL}"] + ) def test_config_error_page(self) -> None: with self.assertLogs(level="INFO") as info_log: result = self.client_get("/accounts/login/social/saml") self.assertEqual( - info_log.output, - ["INFO:root:Attempted to initiate SAML authentication with wrong idp argument: None"], + info_log.output[0], + "INFO:root:Attempted to initiate SAML authentication with wrong idp argument: None", ) - self.assert_in_success_response(["Configuration error", "SAML authentication"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("SAML authentication", result) def test_saml_auth_works_without_private_public_keys(self) -> None: with self.settings(SOCIAL_AUTH_SAML_SP_PUBLIC_CERT="", SOCIAL_AUTH_SAML_SP_PRIVATE_KEY=""): @@ -2905,24 +2940,24 @@ class SAMLAuthBackendTest(SocialAuthBase): with self.assertLogs(level="INFO") as info_log: result = self.client_get(f"/accounts/{action}/social/saml") # Missing idp argument. - self.assert_in_success_response(["Configuration error", "SAML authentication"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("SAML authentication", result) self.assertEqual( - info_log.output, - [ - "INFO:root:Attempted to initiate SAML authentication with wrong idp argument: None" - ], + info_log.output[0], + "INFO:root:Attempted to initiate SAML authentication with wrong idp argument: None", ) with self.assertLogs(level="INFO") as info_log: result = self.client_get(f"/accounts/{action}/social/saml/nonexistent_idp") # No such IdP is configured. self.assertEqual( - info_log.output, - [ - "INFO:root:Attempted to initiate SAML authentication with wrong idp argument: nonexistent_idp" - ], + info_log.output[0], + "INFO:root:Attempted to initiate SAML authentication with wrong idp argument: nonexistent_idp", ) - self.assert_in_success_response(["Configuration error", "SAML authentication"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("SAML authentication", result) result = self.client_get(f"/accounts/{action}/social/saml/") # No matching URL pattern. @@ -3788,11 +3823,19 @@ class GenericOpenIdConnectTest(SocialAuthBase): mock_oidc_setting_dict = copy.deepcopy(settings.SOCIAL_AUTH_OIDC_ENABLED_IDPS) [idp_config_dict] = mock_oidc_setting_dict.values() del idp_config_dict["client_id"] - with self.settings(SOCIAL_AUTH_OIDC_ENABLED_IDPS=mock_oidc_setting_dict): + with self.settings(SOCIAL_AUTH_OIDC_ENABLED_IDPS=mock_oidc_setting_dict), self.assertLogs( + "django.request", level="ERROR" + ) as m: result = self.social_auth_test( account_data_dict, subdomain="zulip", next="/user_uploads/image" ) - self.assert_in_success_response(["Configuration error", "OpenID Connect"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("OpenID Connect", result) + self.assertEqual( + m.output, + [f"ERROR:django.request:Internal Server Error: {self.LOGIN_URL}"], + ) def test_too_many_idps(self) -> None: """ @@ -3803,11 +3846,19 @@ class GenericOpenIdConnectTest(SocialAuthBase): mock_oidc_setting_dict = copy.deepcopy(settings.SOCIAL_AUTH_OIDC_ENABLED_IDPS) [idp_config_dict] = mock_oidc_setting_dict.values() mock_oidc_setting_dict["secondprovider"] = idp_config_dict - with self.settings(SOCIAL_AUTH_OIDC_ENABLED_IDPS=mock_oidc_setting_dict): + with self.settings(SOCIAL_AUTH_OIDC_ENABLED_IDPS=mock_oidc_setting_dict), self.assertLogs( + "django.request", level="ERROR" + ) as m: result = self.social_auth_test( account_data_dict, subdomain="zulip", next="/user_uploads/image" ) - self.assert_in_success_response(["Configuration error", "OpenID Connect"], result) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("OpenID Connect", result) + self.assertEqual( + m.output, + [f"ERROR:django.request:Internal Server Error: {self.LOGIN_URL}"], + ) def test_config_error_development(self) -> None: """ @@ -5402,21 +5453,39 @@ class TestDevAuthBackend(ZulipTestCase): def test_login_failure(self) -> None: email = self.example_email("hamlet") data = {"direct_email": email} - with self.settings(AUTHENTICATION_BACKENDS=("zproject.backends.EmailAuthBackend",)): + with self.settings( + AUTHENTICATION_BACKENDS=("zproject.backends.EmailAuthBackend",) + ), self.assertLogs("django.request", level="ERROR") as m: response = self.client_post("/accounts/login/local/", data) - self.assert_in_success_response(["Configuration error", "DevAuthBackend"], response) + self.assertEqual(response.status_code, 500) + self.assert_in_response("Configuration error", response) + self.assert_in_response("DevAuthBackend", response) + self.assertEqual( + m.output, ["ERROR:django.request:Internal Server Error: /accounts/login/local/"] + ) def test_dev_direct_production_config_error(self) -> None: - result = self.client_get("/config-error/dev_not_supported") - self.assertEqual(result.status_code, 200) - self.assert_in_success_response(["DevAuthBackend"], result) + with self.assertLogs("django.request", level="ERROR") as m: + result = self.client_get("/config-error/dev_not_supported") + self.assertEqual(result.status_code, 500) + self.assert_in_response("DevAuthBackend", result) + self.assertEqual( + m.output, + ["ERROR:django.request:Internal Server Error: /config-error/dev_not_supported"], + ) def test_login_failure_due_to_nonexistent_user(self) -> None: email = "nonexisting@zulip.com" data = {"direct_email": email} - response = self.client_post("/accounts/login/local/", data) - self.assert_in_success_response(["Configuration error", "DevAuthBackend"], response) + with self.assertLogs("django.request", level="ERROR") as m: + response = self.client_post("/accounts/login/local/", data) + self.assertEqual(response.status_code, 500) + self.assert_in_response("Configuration error", response) + self.assert_in_response("DevAuthBackend", response) + self.assertEqual( + m.output, ["ERROR:django.request:Internal Server Error: /accounts/login/local/"] + ) class TestZulipRemoteUserBackend(DesktopFlowTestingLib, ZulipTestCase): @@ -5466,11 +5535,15 @@ class TestZulipRemoteUserBackend(DesktopFlowTestingLib, ZulipTestCase): def test_login_failure(self) -> None: email = self.example_email("hamlet") - result = self.client_get("/accounts/login/sso/", REMOTE_USER=email) - self.assert_in_success_response( - ["Configuration error", "Authentication via the REMOTE_USER header is"], result - ) + with self.assertLogs("django.request", level="ERROR") as m: + result = self.client_get("/accounts/login/sso/", REMOTE_USER=email) + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("Authentication via the REMOTE_USER header is", result) self.assert_logged_in_user_id(None) + self.assertEqual( + m.output, ["ERROR:django.request:Internal Server Error: /accounts/login/sso/"] + ) def test_login_failure_due_to_nonexisting_user(self) -> None: email = "nonexisting@zulip.com" @@ -5487,10 +5560,15 @@ class TestZulipRemoteUserBackend(DesktopFlowTestingLib, ZulipTestCase): self.assert_json_error_contains(result, "Enter a valid email address.", 400) def test_login_failure_due_to_missing_field(self) -> None: - with self.settings(AUTHENTICATION_BACKENDS=("zproject.backends.ZulipRemoteUserBackend",)): + with self.settings( + AUTHENTICATION_BACKENDS=("zproject.backends.ZulipRemoteUserBackend",) + ), self.assertLogs("django.request", level="ERROR") as m: result = self.client_get("/accounts/login/sso/") - self.assert_in_success_response( - ["Configuration error", "The REMOTE_USER header is not set."], result + self.assertEqual(result.status_code, 500) + self.assert_in_response("Configuration error", result) + self.assert_in_response("The REMOTE_USER header is not set.", result) + self.assertEqual( + m.output, ["ERROR:django.request:Internal Server Error: /accounts/login/sso/"] ) def test_login_failure_due_to_wrong_subdomain(self) -> None: @@ -7083,11 +7161,14 @@ class LDAPBackendTest(ZulipTestCase): "zproject.backends.ZulipLDAPAuthBackend.get_or_build_user", side_effect=error ), mock.patch("django_auth_ldap.backend._LDAPUser._authenticate_user_dn"), self.assertLogs( "django_auth_ldap", "WARNING" - ) as warn_log: + ) as warn_log, self.assertLogs( + "django.request", level="ERROR" + ): response = self.client_post("/login/", data) - self.assert_in_success_response( - ["Configuration error", "You are trying to log in using LDAP without creating an"], - response, + self.assertEqual(response.status_code, 500) + self.assert_in_response("Configuration error", response) + self.assert_in_response( + "You are trying to log in using LDAP without creating an", response ) self.assertEqual( warn_log.output, diff --git a/zerver/tests/test_docs.py b/zerver/tests/test_docs.py index 28cf71a321..1a16c112e1 100644 --- a/zerver/tests/test_docs.py +++ b/zerver/tests/test_docs.py @@ -468,9 +468,14 @@ class AboutPageTest(ZulipTestCase): class SmtpConfigErrorTest(ZulipTestCase): def test_smtp_error(self) -> None: - result = self.client_get("/config-error/smtp") - self.assertEqual(result.status_code, 200) - self.assert_in_success_response(["email configuration"], result) + with self.assertLogs("django.request", level="ERROR") as m: + result = self.client_get("/config-error/smtp") + self.assertEqual(result.status_code, 500) + self.assert_in_response("email configuration", result) + self.assertEqual( + m.output, + ["ERROR:django.request:Internal Server Error: /config-error/smtp"], + ) class PlansPageTest(ZulipTestCase): diff --git a/zerver/tests/test_urls.py b/zerver/tests/test_urls.py index 35796464bf..a36fd26b06 100644 --- a/zerver/tests/test_urls.py +++ b/zerver/tests/test_urls.py @@ -150,8 +150,14 @@ class PublicURLTest(ZulipTestCase): urls = [f"/config-error/{err_page_name}" for err_page_name in auth_error_pages] with self.settings(DEVELOPMENT=True): for url in urls: - response = self.client_get(url) - self.assert_in_success_response(["Configuration error"], response) + with self.assertLogs("django.request", level="ERROR") as m: + response = self.client_get(url) + self.assertEqual(response.status_code, 500) + self.assert_in_response("Configuration error", response) + self.assertEqual( + m.output, + [f"ERROR:django.request:Internal Server Error: {url}"], + ) class URLResolutionTest(ZulipTestCase): diff --git a/zerver/views/errors.py b/zerver/views/errors.py index 9042d1c937..6293811680 100644 --- a/zerver/views/errors.py +++ b/zerver/views/errors.py @@ -15,4 +15,4 @@ def config_error(request: HttpRequest, error_name: str) -> HttpResponse: context["auth_settings_path"] = "/etc/zulip/settings.py" context["client_id_key_name"] = f"SOCIAL_AUTH_{error_name.upper()}_KEY" - return render(request, f"zerver/config_error/{error_name}.html", context) + return render(request, f"zerver/config_error/{error_name}.html", context=context, status=500)