From bab481efc27d21987068ff711084f758111e166a Mon Sep 17 00:00:00 2001 From: Rishi Gupta Date: Sat, 5 Nov 2016 16:29:55 -0700 Subject: [PATCH] forms.py: Refactor MIT mailing list check into a modern style. No change to behavior. non_mit_mailing_list never returned False, so it was never possible to reach the line "Otherwise, the user is an MIT mailing list, and .." --- zerver/forms.py | 24 +++++++----------------- zerver/tests/test_external.py | 8 ++++---- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/zerver/forms.py b/zerver/forms.py index 88c729ebbe..adfcc8da5e 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -58,21 +58,19 @@ def get_valid_realm(email): return None return realm -def not_mit_mailing_list(value): - # type: (str) -> bool +def email_is_not_mit_mailing_list(email): + # type: (text_type) -> None """Prevent MIT mailing lists from signing up for Zulip""" - if "@mit.edu" in value: - username = value.rsplit("@", 1)[0] + if "@mit.edu" in email: + username = email.rsplit("@", 1)[0] # Check whether the user exists and can get mail. try: DNS.dnslookup("%s.pobox.ns.athena.mit.edu" % username, DNS.Type.TXT) - return True except DNS.Base.ServerError as e: if e.rcode == DNS.Status.NXDOMAIN: raise ValidationError(mark_safe(MIT_VALIDATION_ERROR)) else: raise - return True class RegistrationForm(forms.Form): full_name = forms.CharField(max_length=100) @@ -160,18 +158,10 @@ class HomepageForm(forms.Form): if realm is None: raise ValidationError(mark_safe(SIGNUP_STRING)) - # If it's a clear realm not used for Zephyr mirroring, pass - if not realm.is_zephyr_mirror_realm: - return email + if realm.is_zephyr_mirror_realm: + email_is_not_mit_mailing_list(email) - # At this point, the user is trying to join a Zephyr mirroring - # realm. We confirm that they are a real account (not a - # mailing list), and if so, let them in. - if not_mit_mailing_list(email): - return email - - # Otherwise, the user is an MIT mailing list, and we return failure - raise ValidationError(mark_safe(SIGNUP_STRING)) + return email def email_is_not_disposable(email): # type: (text_type) -> None diff --git a/zerver/tests/test_external.py b/zerver/tests/test_external.py index 419a2997dd..3337be4148 100644 --- a/zerver/tests/test_external.py +++ b/zerver/tests/test_external.py @@ -5,7 +5,7 @@ from django.core.exceptions import ValidationError from django.http import HttpResponse from django.test import TestCase -from zerver.forms import not_mit_mailing_list +from zerver.forms import email_is_not_mit_mailing_list from zerver.lib.rate_limiter import ( add_ratelimit_rule, @@ -45,13 +45,13 @@ class MITNameTest(TestCase): def test_mailinglist(self): # type: () -> None with mock.patch('DNS.dnslookup', side_effect=DNS.Base.ServerError('DNS query status: NXDOMAIN', 3)): - self.assertRaises(ValidationError, not_mit_mailing_list, "1234567890@mit.edu") + self.assertRaises(ValidationError, email_is_not_mit_mailing_list, "1234567890@mit.edu") with mock.patch('DNS.dnslookup', side_effect=DNS.Base.ServerError('DNS query status: NXDOMAIN', 3)): - self.assertRaises(ValidationError, not_mit_mailing_list, "ec-discuss@mit.edu") + self.assertRaises(ValidationError, email_is_not_mit_mailing_list, "ec-discuss@mit.edu") def test_notmailinglist(self): # type: () -> None with mock.patch('DNS.dnslookup', return_value=[['POP IMAP.EXCHANGE.MIT.EDU starnine']]): - self.assertTrue(not_mit_mailing_list("sipbexch@mit.edu")) + email_is_not_mit_mailing_list("sipbexch@mit.edu") class RateLimitTests(ZulipTestCase):