paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-05 06:23:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

validate_email_not_already_in_realm: Add kwarg for mirror dummies.

Browse Source 
In user signup context, we are okay with there being an existing mirror
dummy user with the matching email - at the end of the signup, that
mirror dummy account will be activated and control of it given to the
user doing this signup.

However, in email change contexts (SCIM API and regular email change
flow), we can't change an account's email address to the address that
already belongs to an existing mirror dummy user.

To avoid subtle bugs like this, we make callers have to explicitly
specify whether existance of mirror dummies with the matching email
address is okay or not.
This commit is contained in:
Mateusz Mandera
2025-06-25 03:57:35 +08:00
committed by Tim Abbott
parent 9abdb17d1f
commit fe993032a6
9 changed files with 61 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
zerver/actions/invites.py
View File

@@ -251,7 +251,7 @@ def do_invite_users(
but we still need to make sure they're not
gonna conflict with existing users
"""
error_dict = get_existing_user_errors(realm, good_emails)
error_dict = get_existing_user_errors(realm, good_emails, allow_inactive_mirror_dummies=True)
skipped: list[tuple[str, str, bool]] = []
for email in error_dict:

13
zerver/lib/email_validation.py
View File

@@ -122,6 +122,8 @@ def email_reserved_for_system_bots_error(email: str) -> str:
def get_existing_user_errors(
target_realm: Realm,
emails: set[str],
*,
allow_inactive_mirror_dummies: bool,
verbose: bool = False,
) -> dict[str, tuple[str, bool]]:
"""
@@ -166,7 +168,7 @@ def get_existing_user_errors(
# HAPPY PATH! Most people invite users that don't exist yet.
return
if existing_user_profile.is_mirror_dummy:
if existing_user_profile.is_mirror_dummy and allow_inactive_mirror_dummies:
if existing_user_profile.is_active:
raise AssertionError("Mirror dummy user is already active!")
return
@@ -193,7 +195,7 @@ def get_existing_user_errors(
def validate_email_not_already_in_realm(
target_realm: Realm, email: str, verbose: bool = True
target_realm: Realm, email: str, *, allow_inactive_mirror_dummies: bool, verbose: bool = True
) -> None:
"""
NOTE:
@@ -204,7 +206,12 @@ def validate_email_not_already_in_realm(
for any endpoint that takes multiple emails,
such as the "invite" interface.
"""
error_dict = get_existing_user_errors(target_realm, {email}, verbose)
error_dict = get_existing_user_errors(
target_realm,
{email},
allow_inactive_mirror_dummies=allow_inactive_mirror_dummies,
verbose=verbose,
)
# Loop through errors, the only key should be our email.
for key, error_info in error_dict.items():

4
zerver/lib/scim.py
View File

@@ -282,7 +282,9 @@ class ZulipSCIMUser(SCIMUser):
raise scim_exceptions.BadRequestError("Email address can't contain + characters.")
try:
validate_email_not_already_in_realm(realm, email_new_value)
validate_email_not_already_in_realm(
realm, email_new_value, allow_inactive_mirror_dummies=False
)
except ValidationError as e:
raise ConflictError("Email address already in use: " + str(e))

25
zerver/tests/test_auth_backends.py
View File

@@ -7693,21 +7693,40 @@ class EmailValidatorTestCase(ZulipTestCase):
self.assertIn("containing + are not allowed", error)
cordelia_email = cordelia.delivery_email
errors = get_existing_user_errors(realm, {cordelia_email})
errors = get_existing_user_errors(
realm, {cordelia_email}, allow_inactive_mirror_dummies=True
)
error, is_deactivated = errors[cordelia_email]
self.assertEqual(False, is_deactivated)
self.assertEqual(error, "Already has an account.")
change_user_is_active(cordelia, False)
errors = get_existing_user_errors(realm, {cordelia_email})
errors = get_existing_user_errors(
realm, {cordelia_email}, allow_inactive_mirror_dummies=True
)
error, is_deactivated = errors[cordelia_email]
self.assertEqual(True, is_deactivated)
self.assertEqual(error, "Account has been deactivated.")
errors = get_existing_user_errors(realm, {"fred-is-fine@zulip.com"})
errors = get_existing_user_errors(
realm, {"fred-is-fine@zulip.com"}, allow_inactive_mirror_dummies=True
)
self.assertEqual(errors, {})
cordelia.is_mirror_dummy = True
cordelia.save()
errors = get_existing_user_errors(
realm, {cordelia_email}, allow_inactive_mirror_dummies=True
)
self.assertEqual(errors, {})
errors = get_existing_user_errors(
realm, {cordelia_email}, allow_inactive_mirror_dummies=False
)
error, is_deactivated = errors[cordelia_email]
self.assertEqual(True, is_deactivated)
self.assertEqual(error, "Account has been deactivated.")
class LDAPBackendTest(ZulipTestCase):
@override_settings(AUTHENTICATION_BACKENDS=("zproject.backends.ZulipLDAPAuthBackend",))

16
zerver/tests/test_email_change.py
View File

@@ -269,6 +269,22 @@ class EmailChangeTestCase(ZulipTestCase):
self.assertEqual(result.status_code, 400)
self.assert_in_response("Already has an account", result)
def test_email_change_conflict_with_existing_mirror_dummy(self) -> None:
cordelia = self.example_user("cordelia")
do_deactivate_user(cordelia, acting_user=None)
cordelia.is_mirror_dummy = True
cordelia.save()
data = {"email": "cordelia@zulip.com"}
user_profile = self.example_user("hamlet")
self.login_user(user_profile)
url = "/json/settings"
result = self.client_patch(url, data)
self.assert_length(mail.outbox, 0)
self.assertEqual(result.status_code, 400)
self.assert_in_response("Account has been deactivated.", result)
def test_email_change_already_taken_later(self) -> None:
conflict_email = "conflict@zulip.com"
hamlet = self.example_user("hamlet")

6
zerver/views/registration.py
View File

@@ -476,7 +476,7 @@ def registration_helper(
return redirect_to_deactivation_notice()
try:
validate_email_not_already_in_realm(realm, email)
validate_email_not_already_in_realm(realm, email, allow_inactive_mirror_dummies=True)
except ValidationError:
return redirect_to_email_login_url(email)
@@ -1463,7 +1463,9 @@ def accounts_home(
email = form.cleaned_data["email"]
try:
validate_email_not_already_in_realm(realm, email)
validate_email_not_already_in_realm(
realm, email, allow_inactive_mirror_dummies=True
)
except ValidationError:
return redirect_to_email_login_url(email)

1
zerver/views/user_settings.py
View File

@@ -82,6 +82,7 @@ def validate_email_change_request(user_profile: UserProfile, new_email: str) ->
validate_email_not_already_in_realm(
user_profile.realm,
new_email,
allow_inactive_mirror_dummies=False,
verbose=False,
)
except ValidationError as e:

1
zerver/views/users.py
View File

@@ -328,6 +328,7 @@ def update_user_backend(
user_profile.realm,
new_email,
verbose=False,
allow_inactive_mirror_dummies=False,
)
except ValidationError as e:
raise JsonableError(_("New email value error: {message}").format(message=e.message))

4
zproject/backends.py
View File

@@ -1092,7 +1092,9 @@ class ZulipLDAPAuthBackend(ZulipLDAPAuthBackendBase):
# even though its checks were already done above.
try:
email_allowed_for_realm(username, self._realm)
validate_email_not_already_in_realm(self._realm, username)
validate_email_not_already_in_realm(
self._realm, username, allow_inactive_mirror_dummies=True
)
except DomainNotAllowedForRealmError:
raise ZulipLDAPError("This email domain isn't allowed in this organization.")
except (DisposableEmailError, EmailContainsPlusError):