Fix login and signup views to make email enumeration harder

CHANGELOG.md

@@ -17,6 +17,7 @@ All notable changes to this project will be documented in this file.
 - Fix special character encoding in project invite emails
 - Fix check transfer between same account's projects when at check limit
 - Fix wording in the invite email when inviting read-only users
+- Fix login and signup views to make email enumeration harder
 
 ## v2.5 - 2022-12-14
 

hc/accounts/forms.py

@@ -30,11 +30,6 @@ class SignupForm(forms.Form):
         if len(v) > 254:
             raise forms.ValidationError("Address is too long.")
 
-        if User.objects.filter(email=v).exists():
-            raise forms.ValidationError(
-                "An account with this email address already exists."
-            )
-
         return v
 
     def clean_tz(self):
@@ -60,7 +55,7 @@ class EmailLoginForm(forms.Form):
         try:
             self.user = User.objects.get(email=v)
         except User.DoesNotExist:
-            raise forms.ValidationError("Unknown email address.")
+            self.user = None
 
         return v
 

hc/accounts/tests/test_login.py

@@ -57,6 +57,16 @@ class LoginTestCase(BaseTestCase):
         body = mail.outbox[0].body
         self.assertTrue("/?next=" + self.channels_url in body)
 
+    def test_it_handles_unknown_email(self):
+        form = {"identity": "surprise@example.org"}
+
+        r = self.client.post("/accounts/login/", form)
+        self.assertRedirects(r, "/accounts/login_link_sent/")
+        self.assertIn("auto-login", r.cookies)
+
+        # There should be no sent emails.
+        self.assertEqual(len(mail.outbox), 0)
+
     @override_settings(SECRET_KEY="test-secret")
     def test_it_rate_limits_emails(self):
         # "d60d..." is sha1("alice@example.orgtest-secret")

hc/accounts/tests/test_signup.py

@@ -16,7 +16,7 @@ class SignupTestCase(TestCase):
         form = {"identity": "alice@example.org", "tz": "Europe/Riga"}
 
         r = self.client.post("/accounts/signup/", form)
-        self.assertContains(r, "Account created")
+        self.assertContains(r, "check your email")
         self.assertIn("auto-login", r.cookies)
 
         # An user should have been created
@@ -75,13 +75,17 @@ class SignupTestCase(TestCase):
         q = User.objects.filter(email="alice@example.org")
         self.assertTrue(q.exists)
 
-    def test_it_checks_for_existing_users(self):
+    def test_it_handles_existing_users(self):
         alice = User(username="alice", email="alice@example.org")
         alice.save()
 
         form = {"identity": "alice@example.org", "tz": ""}
         r = self.client.post("/accounts/signup/", form)
-        self.assertContains(r, "already exists")
+        self.assertContains(r, "check your email")
+        self.assertIn("auto-login", r.cookies)
+
+        # It should not send an email
+        self.assertEqual(len(mail.outbox), 0)
 
     def test_it_checks_syntax(self):
         form = {"identity": "alice at example org", "tz": ""}
@@ -101,7 +105,7 @@ class SignupTestCase(TestCase):
         form = {"identity": "alice@example.org", "tz": "Foo/Bar"}
 
         r = self.client.post("/accounts/signup/", form)
-        self.assertContains(r, "Account created")
+        self.assertContains(r, "check your email")
         self.assertIn("auto-login", r.cookies)
 
         profile = Profile.objects.get()

hc/accounts/views.py

@@ -161,10 +161,11 @@ def login(request):
                 if not _allow_redirect(redirect_url):
                     redirect_url = None
 
-                profile = Profile.objects.for_user(magic_form.user)
-                profile.send_instant_login_link(redirect_url=redirect_url)
-                response = redirect("hc-login-link-sent")
+                if magic_form.user:
+                    profile = Profile.objects.for_user(magic_form.user)
+                    profile.send_instant_login_link(redirect_url=redirect_url)
 
+                response = redirect("hc-login-link-sent")
                 # check_token looks for this cookie to decide if
                 # it needs to do the extra POST step.
                 response.set_cookie("auto-login", "1", max_age=300, httponly=True)
@@ -201,16 +202,16 @@ def signup(request):
     form = forms.SignupForm(request.POST)
     if form.is_valid():
         email = form.cleaned_data["identity"]
-        tz = form.cleaned_data["tz"]
-        user = _make_user(email, tz)
-        profile = Profile.objects.for_user(user)
-        profile.send_instant_login_link()
-        ctx["created"] = True
+        if not User.objects.filter(email=email).exists():
+            tz = form.cleaned_data["tz"]
+            user = _make_user(email, tz)
+            profile = Profile.objects.for_user(user)
+            profile.send_instant_login_link()
     else:
         ctx = {"form": form}
 
     response = render(request, "accounts/signup_result.html", ctx)
-    if ctx.get("created"):
+    if "form" not in ctx:
         response.set_cookie("auto-login", "1", max_age=300, httponly=True)
 
     return response

templates/accounts/login_link_sent.html

@@ -1,16 +1,16 @@
 {% extends "base.html" %}
+{% load hc_extras %}
 
 {% block content %}
 <div class="row">
     <div class="col-sm-6 col-sm-offset-3">
         <div class="hc-dialog">
-            <h1>Login Link Sent!</h1>
+            <h1>Check Your Email!</h1>
             <br />
             <p>
-                We've sent you an email with login instructions.
-                Please check your inbox!
+                If a {% site_name %} account exists for this email address,
+                you will receive a login link in your email shortly.
             </p>
-
         </div>
     </div>
 </div>

templates/accounts/signup_result.html

@@ -1,7 +1,5 @@
 {% for error in form.identity.errors %}
 <p class="text-danger">{{ error }}</p>
-{% endfor %}
-
-{% if created %}
-<p class="text-success">Account created, please check your email!</p>
-{% endif %}
+{% empty %}
+<p class="text-success">Please check your email!</p>
+{% endfor %}