password_reset: Send email unconditionally.

This was basically rewritten by tabbott, because the code is a lot
cleaner after just rewriting the ZulipPasswordResetForm code to no
longer copy the model of the original Django version.

Fixes #4733.

templates/zerver/emails/password_reset.source.html

@@ -2,8 +2,15 @@
 
 {% block content %}
 <p>
-    {% if attempted_realm %}
+    {% if no_account_in_realm %}
-    Someone (possibly you) requested a password reset email for {{ email }} on {{ attempted_realm.uri }}, but {{ email }} does not have an active account in {{ attempted_realm.uri }}.  However, {{ email }} does have an active account in {{ user.realm.uri }} organization; you can try logging in or resetting your password there.
+        Someone (possibly you) requested a password reset email for {{ email }}
+        on {{ realm_uri }}, but {{ email }} does not have an
+        active account in {{ realm_uri }}.
+
+        {% if account_exists_another_realm %}
+        However, {{ email }} does have an active account in the {{ user.realm.uri }}
+        organization; you can try logging in or resetting your password there.
+        {% endif %}
     {% else %}
         Psst. Word on the street is that you need a new password, {{ email }}.<br />
         It's all good. Follow the link below and we'll take care of the rest:<br />

templates/zerver/emails/password_reset.txt

@@ -1,9 +1,13 @@
-{% if attempted_realm %}
+{% if no_account_in_realm %}
-Someone (possibly you) requested a password reset email for {{ email }} on
+Someone (possibly you) requested a password reset email for
-{{ attempted_realm.uri }}, but {{ email }} does not
+{{ email }} on {{ realm_uri }}, but
-have an active account in {{ attempted_realm.uri }}.  However, {{ email }} does
+{{ email }} does not have an active account in
-have an active account in {{ user.realm.uri }} organization; you
+{{ realm_uri }}.
-can try logging in or resetting your password there.
+{% if account_exists_another_realm %}
+However, {{ email }} does have an active account in
+{{ user.realm.uri }} organization; you can try
+logging in or resetting your password there.
+{% endif %}
 {% else %}
 Psst. Word on the street is that you need a new password, {{ email }}.
 

zerver/forms.py

@@ -183,49 +183,8 @@ class LoggingSetPasswordForm(SetPasswordForm):
         return self.user
 
 class ZulipPasswordResetForm(PasswordResetForm):
-    def get_users(self, email):
-        # type: (str) -> QuerySet
-        """Given an email, return matching user(s) who should receive a reset.
-
-        This is modified from the original in that it allows non-bot
-        users who don't have a usable password to reset their
-        passwords.
-        """
-        if not email_auth_enabled():
-            logging.info("Password reset attempted for %s even though password auth is disabled." % (email,))
-            return []
-        result = UserProfile.objects.filter(email__iexact=email, is_active=True,
-                                            is_bot=False)
-        if len(result) == 0:
-            logging.info("Password reset attempted for %s; no active account." % (email,))
-        return result
-
-    def send_mail(self, context, from_email, to_email):
-        # type: (Dict[str, Any], str, str) -> None
-        """
-        Currently we don't support accounts in multiple subdomains using
-        a single email address. We override this function so that we do
-        not send a reset link to an email address if the reset attempt is
-        done on the subdomain which does not match user.realm.subdomain.
-
-        Once we start supporting accounts with the same email in
-        multiple subdomains, we may be able to refactor this function.
-
-        A second reason we override this function is so that we can send
-        the mail through the functions in zerver.lib.send_email, to match
-        how we send all other mail in the codebase.
-        """
-        user = get_user_profile_by_email(to_email)
-        attempted_subdomain = get_subdomain(self.request)
-        context['attempted_realm'] = False
-        if not user_matches_subdomain(attempted_subdomain, user):
-            context['attempted_realm'] = get_realm(attempted_subdomain)
-
-        send_email('zerver/emails/password_reset', to_user_id=user.id,
-                   from_name="Zulip Account Security",
-                   from_address=FromAddress.NOREPLY, context=context)
-
     def save(self,
+             domain_override=None,  # type: Optional[bool]
              subject_template_name='registration/password_reset_subject.txt',  # type: Text
              email_template_name='registration/password_reset_email.html',  # type: Text
              use_https=False,  # type: bool
@@ -237,33 +196,56 @@ class ZulipPasswordResetForm(PasswordResetForm):
              ):
         # type: (...) -> None
         """
-        Currently we don't support accounts in multiple subdomains using a
+        If the email address has an account in the target realm,
-        single email addresss. Once we start supporting accounts with the same
+        generates a one-use only link for resetting password and sends
-        email in multiple subdomains, we may be able to delete or refactor this
+        to the user.
-        function.
 
-        Generates a one-use only link for resetting password and sends to the
+        We send a different email if an associated account does not exist in the
-        user.
+        database, or an account does exist, but not in the realm.
 
         Note: We ignore the various email template arguments (those
         are an artifact of using Django's password reset framework)
-
         """
-        setattr(self, 'request', request)
         email = self.cleaned_data["email"]
-        users = list(self.get_users(email))
 
-        for user in users:
+        subdomain = get_subdomain(request)
+        realm = get_realm(subdomain)
+        if realm is None:
+            raise ValidationError("Invalid realm")
+
+        if not email_auth_enabled(realm):
+            logging.info("Password reset attempted for %s even though password auth is disabled." % (email,))
+            return
+
+        try:
+            user = get_user_profile_by_email(email)
+        except UserProfile.DoesNotExist:
+            user = None
+
         context = {
             'email': email,
-                'uid': urlsafe_base64_encode(force_bytes(user.id)),
+            'realm_uri': realm.uri,
             'user': user,
-                'token': token_generator.make_token(user),
             'protocol': 'https' if use_https else 'http',
         }
-            if extra_email_context is not None:
+
-                context.update(extra_email_context)
+        if user is not None and user_matches_subdomain(subdomain, user):
-            self.send_mail(context, from_email, email)
+            context['no_account_in_realm'] = False
+            context['token'] = token_generator.make_token(user)
+            context['uid'] = urlsafe_base64_encode(force_bytes(user.id))
+
+            send_email('zerver/emails/password_reset', to_user_id=user.id,
+                       from_name="Zulip Account Security",
+                       from_address=FromAddress.NOREPLY, context=context)
+        else:
+            context['no_account_in_realm'] = True
+            if user is not None:
+                context['account_exists_another_realm'] = True
+            else:
+                context['account_exists_another_realm'] = False
+            send_email('zerver/emails/password_reset', to_email=email,
+                       from_name="Zulip Account Security",
+                       from_address=FromAddress.NOREPLY, context=context)
 
 class CreateUserForm(forms.Form):
     full_name = forms.CharField(max_length=100)

zerver/tests/test_signup.py

@@ -173,6 +173,7 @@ class PasswordResetTest(ZulipTestCase):
         from_email = outbox[0].from_email
         self.assertIn("Zulip Account Security", from_email)
         self.assertIn(FromAddress.NOREPLY, from_email)
+        self.assertIn("Psst. Word on the street is that you", outbox[0].body)
 
         # Visit the password reset link.
         password_reset_url = self.get_confirmation_url_from_outbox(
@@ -197,7 +198,34 @@ class PasswordResetTest(ZulipTestCase):
         # make sure old password no longer works
         self.login(email, password=old_password, fails=True)
 
-    def test_invalid_subdomain(self):
+    def test_password_reset_for_non_existent_user(self):
+        # type: () -> None
+        email = 'nonexisting@mars.com'
+
+        with patch('logging.info'):
+            # start the password reset process by supplying an email address
+            result = self.client_post('/accounts/password/reset/', {'email': email})
+
+        # check the redirect link telling you to check mail for password reset link
+        self.assertEqual(result.status_code, 302)
+        self.assertTrue(result["Location"].endswith(
+            "/accounts/password/reset/done/"))
+        result = self.client_get(result["Location"])
+
+        self.assert_in_response("Check your email to finish the process.", result)
+
+        # Check that the password reset email is from a noreply address.
+        from django.core.mail import outbox
+        from_email = outbox[0].from_email
+        self.assertIn("Zulip Account Security", from_email)
+        self.assertIn(FromAddress.NOREPLY, from_email)
+
+        self.assertIn('Someone (possibly you) requested a password',
+                      outbox[0].body)
+        self.assertNotIn('does have an active account in the zulip.testserver',
+                         outbox[0].body)
+
+    def test_wrong_subdomain(self):
         # type: () -> None
         email = self.example_email("hamlet")
         string_id = 'hamlet'
@@ -226,7 +254,9 @@ class PasswordResetTest(ZulipTestCase):
         self.assertEqual(len(outbox), 1)
         message = outbox.pop()
         self.assertIn(FromAddress.NOREPLY, message.from_email)
-        self.assertIn("hamlet@zulip.com does not\nhave an active account in http://",
+        self.assertIn('Someone (possibly you) requested a password',
+                      message.body)
+        self.assertIn("hamlet@zulip.com does not have an active account in\nhttp://zephyr.testserver",
                       message.body)
 
     def test_correct_subdomain(self):