diff --git a/CHANGELOG.md b/CHANGELOG.md index fa4f3024..a6566252 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ All notable changes to this project will be documented in this file. - Upgrade to Django 2.2 - Can configure the email integration to only report the "down" events (#231) - Add "Test!" function in the Integrations page (#207) -- Rate limiting for the "Log In" emails +- Rate limiting for the log in attempts ## 1.6.0 - 2019-04-01 diff --git a/hc/accounts/forms.py b/hc/accounts/forms.py index 5f84d941..19a55ced 100644 --- a/hc/accounts/forms.py +++ b/hc/accounts/forms.py @@ -1,8 +1,8 @@ from datetime import timedelta as td from django import forms - from django.contrib.auth import authenticate from django.contrib.auth.models import User +from hc.api.models import TokenBucket class LowercaseEmailField(forms.EmailField): @@ -25,13 +25,16 @@ class AvailableEmailForm(forms.Form): return v -class ExistingEmailForm(forms.Form): +class EmailLoginForm(forms.Form): # Call it "identity" instead of "email" # to avoid some of the dumber bots identity = LowercaseEmailField() def clean_identity(self): v = self.cleaned_data["identity"] + if not TokenBucket.authorize_login_email(v): + raise forms.ValidationError("Too many attempts, please try later.") + try: self.user = User.objects.get(email=v) except User.DoesNotExist: @@ -40,7 +43,7 @@ class ExistingEmailForm(forms.Form): return v -class EmailPasswordForm(forms.Form): +class PasswordLoginForm(forms.Form): email = LowercaseEmailField() password = forms.CharField() @@ -49,11 +52,12 @@ class EmailPasswordForm(forms.Form): password = self.cleaned_data.get('password') if username and password: + if not TokenBucket.authorize_login_password(username): + raise forms.ValidationError("Too many attempts, please try later.") + self.user = authenticate(username=username, password=password) - if self.user is None: - raise forms.ValidationError("Incorrect email or password") - if not self.user.is_active: - raise forms.ValidationError("Account is inactive") + if self.user is None or not self.user.is_active: + raise forms.ValidationError("Incorrect email or password.") return self.cleaned_data diff --git a/hc/accounts/tests/test_login.py b/hc/accounts/tests/test_login.py index 5f7c21cf..cd4b1351 100644 --- a/hc/accounts/tests/test_login.py +++ b/hc/accounts/tests/test_login.py @@ -43,22 +43,7 @@ class LoginTestCase(BaseTestCase): form = {"identity": "alice@example.org"} r = self.client.post("/accounts/login/", form) - self.assertContains(r, "Too Many Requests") - - # No email should have been sent - self.assertEqual(len(mail.outbox), 0) - - @override_settings(SECRET_KEY="test-secret") - def test_it_rate_limits_ips(self): - # 60be.... is sha1("127.0.0.1test-secret") - obj = TokenBucket(value="ip-60be45f44bd9ab3805871fb1137594e708c993ff") - obj.tokens = 0 - obj.save() - - form = {"identity": "alice@example.org"} - - r = self.client.post("/accounts/login/", form) - self.assertContains(r, "Too Many Requests") + self.assertContains(r, "Too many attempts") # No email should have been sent self.assertEqual(len(mail.outbox), 0) @@ -87,6 +72,22 @@ class LoginTestCase(BaseTestCase): r = self.client.post("/accounts/login/", form) self.assertRedirects(r, self.checks_url) + @override_settings(SECRET_KEY="test-secret") + def test_it_rate_limits_password_attempts(self): + # "d60d..." is sha1("alice@example.orgtest-secret") + obj = TokenBucket(value="pw-d60db3b2343e713a4de3e92d4eb417e4f05f06ab") + obj.tokens = 0 + obj.save() + + form = { + "action": "login", + "email": "alice@example.org", + "password": "password" + } + + r = self.client.post("/accounts/login/", form) + self.assertContains(r, "Too many attempts") + def test_it_handles_password_login_with_redirect(self): check = Check.objects.create(project=self.project) diff --git a/hc/accounts/views.py b/hc/accounts/views.py index ef2f9602..6a5eb46e 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -16,11 +16,11 @@ from django.utils.timezone import now from django.urls import resolve, Resolver404 from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_POST -from hc.accounts.forms import (ChangeEmailForm, EmailPasswordForm, +from hc.accounts.forms import (ChangeEmailForm, PasswordLoginForm, InviteTeamMemberForm, RemoveTeamMemberForm, ReportSettingsForm, SetPasswordForm, ProjectNameForm, AvailableEmailForm, - ExistingEmailForm) + EmailLoginForm) from hc.accounts.models import Profile, Project, Member from hc.api.models import Channel, Check, TokenBucket from hc.payments.models import Subscription @@ -89,30 +89,24 @@ def _redirect_after_login(request): def login(request): - form = EmailPasswordForm() - magic_form = ExistingEmailForm() + form = PasswordLoginForm() + magic_form = EmailLoginForm() if request.method == 'POST': if request.POST.get("action") == "login": - form = EmailPasswordForm(request.POST) + form = PasswordLoginForm(request.POST) if form.is_valid(): auth_login(request, form.user) return _redirect_after_login(request) else: - magic_form = ExistingEmailForm(request.POST) + magic_form = EmailLoginForm(request.POST) if magic_form.is_valid(): - user = magic_form.user - if not TokenBucket.authorize_login_email(user.email): - return render(request, "try_later.html") - if not TokenBucket.authorize_login_ip(request): - return render(request, "try_later.html") - redirect_url = request.GET.get("next") if not _is_whitelisted(redirect_url): redirect_url = None - profile = Profile.objects.for_user(user) + profile = Profile.objects.for_user(magic_form.user) profile.send_instant_login_link(redirect_url=redirect_url) return redirect("hc-login-link-sent") diff --git a/hc/api/models.py b/hc/api/models.py index d0d3d0da..90c4cf9b 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -635,20 +635,17 @@ class TokenBucket(models.Model): # 20 login attempts for a single email per hour: return TokenBucket.authorize(value, 20, 3600) - @staticmethod - def authorize_login_ip(request): - headers = request.META - ip = headers.get("HTTP_X_FORWARDED_FOR", headers["REMOTE_ADDR"]) - ip = ip.split(",")[0] - salted_encoded = (ip + settings.SECRET_KEY).encode() - value = "ip-%s" % hashlib.sha1(salted_encoded).hexdigest() - - # 20 login attempts from a single IP per hour: - return TokenBucket.authorize(value, 20, 3600) - @staticmethod def authorize_invite(user): value = "invite-%d" % user.id # 20 invites per day + return TokenBucket.authorize(value, 2, 3600 * 24) + + @staticmethod + def authorize_login_password(email): + salted_encoded = (email + settings.SECRET_KEY).encode() + value = "pw-%s" % hashlib.sha1(salted_encoded).hexdigest() + + # 20 password attempts per day return TokenBucket.authorize(value, 20, 3600 * 24) diff --git a/templates/accounts/login.html b/templates/accounts/login.html index 3dccb463..4f61f488 100644 --- a/templates/accounts/login.html +++ b/templates/accounts/login.html @@ -20,8 +20,8 @@