diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index c218b64162..dbef1bf2c7 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -2717,6 +2717,39 @@ class SAMLAuthBackendTest(SocialAuthBase): ], ) + # Now test a slightly different case, where instead of an invalid issuer, + # the issuer is not included in the SAMLResponse at all. + # self.generate_saml_response doesn't support something as fiddly as a SAMLResponse + # with no issuers and it doesn't make sense to clutter it with such a "feature", so + # we manually load and tweak the fixture here. + unencoded_saml_response = self.fixture_data("samlresponse.txt", type="saml").format( + email=self.example_email("hamlet"), + first_name="King", + last_name="Hamlet", + extra_attrs="", + ) + # Simple regex to remove Issuer occurrences from the XML. + unencoded_saml_response_without_issuer = re.sub( + r"\", "", unencoded_saml_response + ) + + # SAMLResponse needs to be base64-encoded. + saml_response_without_issuer: str = base64.b64encode( + unencoded_saml_response_without_issuer.encode() + ).decode() + with self.assertLogs(self.logger_string, level="INFO") as m: + post_params = {"RelayState": relay_state, "SAMLResponse": saml_response_without_issuer} + result = self.client_post("/complete/saml/", post_params) + self.assertEqual(result.status_code, 302) + self.assertEqual("/login/", result["Location"]) + self.assertEqual( + m.output, + [ + "ERROR:zulip.auth.saml:Error parsing SAMLResponse: Issuer of the Assertion not found or multiple.", + "INFO:zulip.auth.saml:/complete/saml/: No valid IdP as issuer of the SAMLResponse.", + ], + ) + def test_social_auth_complete_valid_get_idp_bad_samlresponse(self) -> None: """ This tests for a hypothetical scenario where our basic parsing of the SAMLResponse diff --git a/zproject/backends.py b/zproject/backends.py index d5c3100a48..3b31c071e5 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -51,7 +51,7 @@ from django_auth_ldap.backend import LDAPBackend, _LDAPUser, ldap_error from lxml.etree import XMLSyntaxError from onelogin.saml2 import compat as onelogin_saml2_compat from onelogin.saml2.auth import OneLogin_Saml2_Auth -from onelogin.saml2.errors import OneLogin_Saml2_Error +from onelogin.saml2.errors import OneLogin_Saml2_Error, OneLogin_Saml2_ValidationError from onelogin.saml2.logout_request import OneLogin_Saml2_Logout_Request from onelogin.saml2.logout_response import OneLogin_Saml2_Logout_Response from onelogin.saml2.response import OneLogin_Saml2_Response @@ -2250,7 +2250,12 @@ class SAMLDocument: for wrapping the fiddly logic of handling these SAML XML documents. """ - SAML_PARSING_EXCEPTIONS = (OneLogin_Saml2_Error, binascii.Error, XMLSyntaxError) + SAML_PARSING_EXCEPTIONS = ( + OneLogin_Saml2_Error, + OneLogin_Saml2_ValidationError, + binascii.Error, + XMLSyntaxError, + ) def __init__(self, encoded_saml_message: str, backend: "SAMLAuthBackend") -> None: """