diff --git a/zerver/actions/invites.py b/zerver/actions/invites.py index bb6b14b067..af3205caf7 100644 --- a/zerver/actions/invites.py +++ b/zerver/actions/invites.py @@ -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: diff --git a/zerver/lib/email_validation.py b/zerver/lib/email_validation.py index 2b0f7f533c..36e0620ebf 100644 --- a/zerver/lib/email_validation.py +++ b/zerver/lib/email_validation.py @@ -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(): diff --git a/zerver/lib/scim.py b/zerver/lib/scim.py index 4a44a7e461..d5e81e1064 100644 --- a/zerver/lib/scim.py +++ b/zerver/lib/scim.py @@ -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)) diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 29cb3e0c10..cb08aa668e 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -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",)) diff --git a/zerver/tests/test_email_change.py b/zerver/tests/test_email_change.py index a075327a1c..6734870657 100644 --- a/zerver/tests/test_email_change.py +++ b/zerver/tests/test_email_change.py @@ -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") diff --git a/zerver/views/registration.py b/zerver/views/registration.py index e69943d44d..653359807d 100644 --- a/zerver/views/registration.py +++ b/zerver/views/registration.py @@ -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) diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index ea9b446992..8220814864 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -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: diff --git a/zerver/views/users.py b/zerver/views/users.py index 1542173c17..9103b977e4 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -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)) diff --git a/zproject/backends.py b/zproject/backends.py index b581df0f1a..bc1f0f889b 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -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):