diff --git a/CHANGELOG.md b/CHANGELOG.md index cfe1246f..6f9bcc25 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/hc/accounts/forms.py b/hc/accounts/forms.py index a83af82a..3ec40648 100644 --- a/hc/accounts/forms.py +++ b/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 diff --git a/hc/accounts/tests/test_login.py b/hc/accounts/tests/test_login.py index ae03f5d8..dad48267 100644 --- a/hc/accounts/tests/test_login.py +++ b/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") diff --git a/hc/accounts/tests/test_signup.py b/hc/accounts/tests/test_signup.py index 96d595dd..d19872e4 100644 --- a/hc/accounts/tests/test_signup.py +++ b/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() diff --git a/hc/accounts/views.py b/hc/accounts/views.py index 9759d765..f5b9c2f9 100644 --- a/hc/accounts/views.py +++ b/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 diff --git a/templates/accounts/login_link_sent.html b/templates/accounts/login_link_sent.html index b5aebc2c..3aec22e9 100644 --- a/templates/accounts/login_link_sent.html +++ b/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> diff --git a/templates/accounts/signup_result.html b/templates/accounts/signup_result.html index 5a096fda..1c049425 100644 --- a/templates/accounts/signup_result.html +++ b/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 %} \ No newline at end of file +{% empty %} +<p class="text-success">Please check your email!</p> +{% endfor %} \ No newline at end of file