diff --git a/zerver/tests/test_email_change.py b/zerver/tests/test_email_change.py index 324429db18..e2b4d8f38c 100644 --- a/zerver/tests/test_email_change.py +++ b/zerver/tests/test_email_change.py @@ -241,6 +241,49 @@ class EmailChangeTestCase(ZulipTestCase): self.assertEqual(result.status_code, 400) self.assert_in_response("Already has an account", result) + def test_email_change_already_taken_later(self) -> None: + conflict_email = "conflict@zulip.com" + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + + self.login_user(hamlet) + hamlet_url = self.generate_email_change_link(conflict_email) + self.logout() + + self.login_user(cordelia) + cordelia_url = self.generate_email_change_link(conflict_email) + response = self.client_get(cordelia_url) + self.assertEqual(response.status_code, 200) + cordelia.refresh_from_db() + self.assertEqual(cordelia.delivery_email, conflict_email) + + self.logout() + self.login_user(hamlet) + response = self.client_get(hamlet_url) + self.assertEqual(response.status_code, 400) + self.assert_in_response("Already has an account", response) + + def test_change_email_to_disposable_email(self) -> None: + hamlet = self.example_user("hamlet") + + # Make the realm allow permissive email changes, create a + # link, and then lock the permissions down -- the change + # should not be allowed when the link is followed. + realm = hamlet.realm + realm.disallow_disposable_email_addresses = False + realm.emails_restricted_to_domains = False + realm.save() + + self.login_user(hamlet) + confirmation_url = self.generate_email_change_link("hamlet@mailnator.com") + + realm.disallow_disposable_email_addresses = True + realm.save() + + response = self.client_get(confirmation_url) + self.assertEqual(response.status_code, 400) + self.assert_in_response("Please use your real email address.", response) + def test_unauthorized_email_change_from_email_confirmation_link(self) -> None: new_email = "hamlet-new@zulip.com" user_profile = self.example_user("hamlet") diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 6f87c134cd..861b5eae0a 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -64,6 +64,31 @@ from zproject.backends import check_password_strength, email_belongs_to_ldap AVATAR_CHANGES_DISABLED_ERROR = gettext_lazy("Avatar changes are disabled in this organization.") +def validate_email_change_request(user_profile: UserProfile, new_email: str) -> None: + if not user_profile.is_active: + # TODO: Make this into a user-facing error, not JSON + raise UserDeactivatedError + + if user_profile.realm.email_changes_disabled and not user_profile.is_realm_admin: + raise JsonableError(_("Email address changes are disabled in this organization.")) + + error = validate_email_is_valid( + new_email, + get_realm_email_validator(user_profile.realm), + ) + if error: + raise JsonableError(error) + + try: + validate_email_not_already_in_realm( + user_profile.realm, + new_email, + verbose=False, + ) + except ValidationError as e: + raise JsonableError(e.message) + + def confirm_email_change(request: HttpRequest, confirmation_key: str) -> HttpResponse: try: email_change_object = get_object_from_key( @@ -91,13 +116,7 @@ def confirm_email_change(request: HttpRequest, confirmation_key: str) -> HttpRes if user_profile.realm.deactivated: return redirect_to_deactivation_notice() - if not user_profile.is_active: - # TODO: Make this into a user-facing error, not JSON - raise UserDeactivatedError - - if user_profile.realm.email_changes_disabled and not user_profile.is_realm_admin: - raise JsonableError(_("Email address changes are disabled in this organization.")) - + validate_email_change_request(user_profile, new_email) do_change_user_delivery_email(user_profile, new_email) user_profile = UserProfile.objects.get(id=email_change_object.user_profile_id) @@ -351,24 +370,7 @@ def json_change_settings( if email is not None: new_email = email.strip() if user_profile.delivery_email != new_email: - if user_profile.realm.email_changes_disabled and not user_profile.is_realm_admin: - raise JsonableError(_("Email address changes are disabled in this organization.")) - - error = validate_email_is_valid( - new_email, - get_realm_email_validator(user_profile.realm), - ) - if error: - raise JsonableError(error) - - try: - validate_email_not_already_in_realm( - user_profile.realm, - new_email, - verbose=False, - ) - except ValidationError as e: - raise JsonableError(e.message) + validate_email_change_request(user_profile, new_email) ratelimited, time_until_free = RateLimitedUser( user_profile, domain="email_change_by_user"