Skip to content

Commit cee4da6

Browse files
mateuszmanderatimabbott
authored andcommitted
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.
1 parent 5a61c9b commit cee4da6

File tree

2 files changed

+39
-7
lines changed

2 files changed

+39
-7
lines changed

zerver/tests/test_auth_backends.py

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,9 +1845,13 @@ def generate_saml_response(
18451845
with {email}, {first_name}, {last_name} placeholders, that can
18461846
be filled out with the data we want.
18471847
"""
1848-
name_parts = name.split(" ")
1849-
first_name = name_parts[0]
1850-
last_name = name_parts[1]
1848+
if name:
1849+
name_parts = name.split(" ")
1850+
first_name = name_parts[0]
1851+
last_name = name_parts[1]
1852+
else:
1853+
first_name = ""
1854+
last_name = ""
18511855

18521856
extra_attrs = ""
18531857
for extra_attr_name, extra_attr_values in extra_attributes.items():
@@ -1877,6 +1881,29 @@ def generate_saml_response(
18771881
def get_account_data_dict(self, email: str, name: str) -> Dict[str, Any]:
18781882
return dict(email=email, name=name)
18791883

1884+
def test_auth_registration_with_no_name_provided(self) -> None:
1885+
"""
1886+
The SAMLResponse may not actually provide name values, which is considered
1887+
unexpected behavior for most social backends, but SAML is an exception. The
1888+
signup flow should proceed normally, without pre-filling the name in the
1889+
registration form.
1890+
"""
1891+
1892+
subdomain = "zulip"
1893+
realm = get_realm("zulip")
1894+
account_data_dict = self.get_account_data_dict(email=email, name="")
1895+
result = self.social_auth_test(account_data_dict, subdomain=subdomain, is_signup=True)
1896+
self.stage_two_of_registration(
1897+
result,
1898+
realm,
1899+
subdomain,
1900+
email,
1901+
"",
1902+
"Full Name",
1903+
skip_registration_form=False,
1904+
expect_full_name_prepopulated=False,
1905+
)
1906+
18801907
def test_social_auth_no_key(self) -> None:
18811908
"""
18821909
Since in the case of SAML there isn't a direct equivalent of CLIENT_KEY_SETTING,

zproject/backends.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,11 +1430,16 @@ def social_associate_user_helper(
14301430
full_name = kwargs["details"].get("fullname")
14311431
first_name = kwargs["details"].get("first_name")
14321432
last_name = kwargs["details"].get("last_name")
1433-
if all(name is None for name in [full_name, first_name, last_name]) and backend.name != "apple":
1434-
# Apple authentication provides the user's name only the very first time a user tries to log in.
1433+
if all(name is None for name in [full_name, first_name, last_name]) and backend.name not in [
1434+
"apple",
1435+
"saml",
1436+
]:
1437+
# (1) Apple authentication provides the user's name only the very first time a user tries to log in.
14351438
# So if the user aborts login or otherwise is doing this the second time,
1436-
# we won't have any name data. So, this case is handled with the code below
1437-
# setting full name to empty string.
1439+
# we won't have any name data.
1440+
# (2) Some IdPs may not send any name value if the user doesn't have them set in the IdP's directory.
1441+
#
1442+
# The name will just default to the empty string in the code below.
14381443

14391444
# We need custom code here for any social auth backends
14401445
# that don't provide name details feature.

0 commit comments

Comments
 (0)