mirror of
https://github.com/zulip/zulip.git
synced 2025-11-01 20:44:04 +00:00
registration: Move "already in realm" check outside of validation.
Checking for `validate_email_not_already_in_realm` again (after the form already did so), but only in the case that the form fails to validate, means that we may be spending time pushing totally invalid emails to the DB to check. In the case of emails containing nulls, this can even trigger a 500 error from PostgreSQL. Stop calling `validate_email_not_already_in_realm` in the form validation. The form is currently only used in two places -- in `accounts_home` and in `maybe_send_to_registration`. The latter is only called if the address is known to not currently have an account, so checking in there is unnecessary; and in the former case, we wish different behaviour (the redirect) than just validation failure, which is all the validator can do. Fixes #17015. Co-authored-by: Alex Vandiver <alexmv@zulip.com>
This commit is contained in:
@@ -19,7 +19,7 @@ from two_factor.forms import AuthenticationTokenForm as TwoFactorAuthenticationT
|
||||
from two_factor.utils import totp_digits
|
||||
|
||||
from zerver.lib.actions import do_change_password, email_not_system_bot
|
||||
from zerver.lib.email_validation import email_allowed_for_realm, validate_email_not_already_in_realm
|
||||
from zerver.lib.email_validation import email_allowed_for_realm
|
||||
from zerver.lib.name_restrictions import is_disposable_domain, is_reserved_subdomain
|
||||
from zerver.lib.rate_limiter import RateLimited, RateLimitedObject
|
||||
from zerver.lib.request import JsonableError
|
||||
@@ -177,8 +177,6 @@ class HomepageForm(forms.Form):
|
||||
except EmailContainsPlusError:
|
||||
raise ValidationError(_("Email addresses containing + are not allowed in this organization."))
|
||||
|
||||
validate_email_not_already_in_realm(realm, email)
|
||||
|
||||
if realm.is_zephyr_mirror_realm:
|
||||
email_is_not_mit_mailing_list(email)
|
||||
|
||||
|
||||
@@ -710,6 +710,18 @@ class LoginTest(ZulipTestCase):
|
||||
with self.assertRaises(UserProfile.DoesNotExist):
|
||||
self.nonreg_user('test')
|
||||
|
||||
def test_register_with_invalid_email(self) -> None:
|
||||
"""
|
||||
If you try to register with invalid email, you get an invalid email
|
||||
page
|
||||
"""
|
||||
invalid_email = "foo\x00bar"
|
||||
result = self.client_post('/accounts/home/', {'email': invalid_email},
|
||||
subdomain="zulip")
|
||||
|
||||
self.assertEqual(result.status_code, 200)
|
||||
self.assertContains(result, "Enter a valid email address")
|
||||
|
||||
def test_register_deactivated_partway_through(self) -> None:
|
||||
"""
|
||||
If you try to register for a deactivated realm, you get a clear error
|
||||
|
||||
@@ -555,6 +555,12 @@ def accounts_home(request: HttpRequest, multiuse_object_key: str="",
|
||||
form = HomepageForm(request.POST, realm=realm, from_multiuse_invite=from_multiuse_invite)
|
||||
if form.is_valid():
|
||||
email = form.cleaned_data['email']
|
||||
|
||||
try:
|
||||
validate_email_not_already_in_realm(realm, email)
|
||||
except ValidationError:
|
||||
return redirect_to_email_login_url(email)
|
||||
|
||||
activation_url = prepare_activation_url(email, request, streams=streams_to_subscribe,
|
||||
invited_as=invited_as)
|
||||
try:
|
||||
@@ -565,11 +571,6 @@ def accounts_home(request: HttpRequest, multiuse_object_key: str="",
|
||||
|
||||
return HttpResponseRedirect(reverse('signup_send_confirm', kwargs={'email': email}))
|
||||
|
||||
email = request.POST['email']
|
||||
try:
|
||||
validate_email_not_already_in_realm(realm, email)
|
||||
except ValidationError:
|
||||
return redirect_to_email_login_url(email)
|
||||
else:
|
||||
form = HomepageForm(realm=realm)
|
||||
context = login_context(request)
|
||||
|
||||
Reference in New Issue
Block a user