From ebb6a92f719057923235ba36eedf7aa99e730b28 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Wed, 7 Jul 2021 15:48:28 +0200 Subject: [PATCH] saml: Don't raise AssertionError if no name is provided in SAMLResponse. This is an acceptable edge case for SAML and shouldn't raise any errors. --- zerver/tests/test_auth_backends.py | 33 +++++++++++++++++++++++++++--- zproject/backends.py | 13 ++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 6fda7795ab..6e41c805f8 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -1808,9 +1808,13 @@ class SAMLAuthBackendTest(SocialAuthBase): with {email}, {first_name}, {last_name} placeholders, that can be filled out with the data we want. """ - name_parts = name.split(" ") - first_name = name_parts[0] - last_name = name_parts[1] + if name: + name_parts = name.split(" ") + first_name = name_parts[0] + last_name = name_parts[1] + else: + first_name = "" + last_name = "" extra_attrs = "" for extra_attr_name, extra_attr_values in extra_attributes.items(): @@ -1840,6 +1844,29 @@ class SAMLAuthBackendTest(SocialAuthBase): def get_account_data_dict(self, email: str, name: str) -> Dict[str, Any]: return dict(email=email, name=name) + def test_auth_registration_with_no_name_provided(self) -> None: + """ + The SAMLResponse may not actually provide name values, which is considered + unexpected behavior for most social backends, but SAML is an exception. The + signup flow should proceed normally, without pre-filling the name in the + registration form. + """ + email = "newuser@zulip.com" + subdomain = "zulip" + realm = get_realm("zulip") + account_data_dict = self.get_account_data_dict(email=email, name="") + result = self.social_auth_test(account_data_dict, subdomain=subdomain, is_signup=True) + self.stage_two_of_registration( + result, + realm, + subdomain, + email, + "", + "Full Name", + skip_registration_form=False, + expect_full_name_prepopulated=False, + ) + def test_social_auth_no_key(self) -> None: """ Since in the case of SAML there isn't a direct equivalent of CLIENT_KEY_SETTING, diff --git a/zproject/backends.py b/zproject/backends.py index 650e65c523..50f763c68b 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -1397,11 +1397,16 @@ def social_associate_user_helper( full_name = kwargs["details"].get("fullname") first_name = kwargs["details"].get("first_name") last_name = kwargs["details"].get("last_name") - if all(name is None for name in [full_name, first_name, last_name]) and backend.name != "apple": - # Apple authentication provides the user's name only the very first time a user tries to log in. + if all(name is None for name in [full_name, first_name, last_name]) and backend.name not in [ + "apple", + "saml", + ]: + # (1) Apple authentication provides the user's name only the very first time a user tries to log in. # So if the user aborts login or otherwise is doing this the second time, - # we won't have any name data. So, this case is handled with the code below - # setting full name to empty string. + # we won't have any name data. + # (2) Some IdPs may not send any name value if the user doesn't have them set in the IdP's directory. + # + # The name will just default to the empty string in the code below. # We need custom code here for any social auth backends # that don't provide name details feature.