From 7c78d8a9660160ec123d054cb8e8224c2d585a1a Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Mon, 30 Dec 2019 21:13:02 +0100 Subject: [PATCH] rate_limiter: Limit the amount of password reset emails to one address. This limits the possibility to use the password reset form to make us spam an email address with password reset emails. --- zerver/forms.py | 31 +++++++++++++++++++++++++++++-- zerver/tests/test_signup.py | 31 +++++++++++++++++++++++++++++++ zproject/settings.py | 4 ++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/zerver/forms.py b/zerver/forms.py index 8c70539310..cbc8f95244 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -17,7 +17,8 @@ from jinja2 import Markup as mark_safe from zerver.lib.actions import do_change_password, email_not_system_bot, \ validate_email_for_realm from zerver.lib.name_restrictions import is_reserved_subdomain, is_disposable_domain -from zerver.lib.rate_limiter import RateLimited, get_rate_limit_result_from_request +from zerver.lib.rate_limiter import RateLimited, get_rate_limit_result_from_request, \ + RateLimitedObject, rate_limit_entity from zerver.lib.request import JsonableError from zerver.lib.send_email import send_email, FromAddress from zerver.lib.subdomains import get_subdomain, is_root_domain_available @@ -32,7 +33,7 @@ import logging import re import DNS -from typing import Any, List, Optional, Dict +from typing import Any, List, Optional, Dict, Tuple from two_factor.forms import AuthenticationTokenForm as TwoFactorAuthenticationTokenForm from two_factor.utils import totp_digits @@ -255,6 +256,14 @@ class ZulipPasswordResetForm(PasswordResetForm): logging.info("Realm is deactivated") return + if settings.RATE_LIMITING: + try: + rate_limit_password_reset_form_by_email(email) + except RateLimited: + # TODO: Show an informative, user-facing error message. + logging.info("Too many password reset attempts for email %s" % (email,)) + return + user = None # type: Optional[UserProfile] try: user = get_user_by_delivery_email(email, realm) @@ -290,6 +299,24 @@ class ZulipPasswordResetForm(PasswordResetForm): language=request.LANGUAGE_CODE, context=context) +class RateLimitedPasswordResetByEmail(RateLimitedObject): + def __init__(self, email: str) -> None: + self.email = email + + def __str__(self) -> str: + return "Email: {}".format(self.email) + + def key_fragment(self) -> str: + return "{}:{}".format(type(self), self.email) + + def rules(self) -> List[Tuple[int, int]]: + return settings.RATE_LIMITING_RULES['password_reset_form_by_email'] + +def rate_limit_password_reset_form_by_email(email: str) -> None: + ratelimited, _ = rate_limit_entity(RateLimitedPasswordResetByEmail(email)) + if ratelimited: + raise RateLimited + class CreateUserForm(forms.Form): full_name = forms.CharField(max_length=100) email = forms.EmailField() diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 17a73f3880..ec0a5845e1 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -350,6 +350,37 @@ class PasswordResetTest(ZulipTestCase): from django.core.mail import outbox self.assertEqual(len(outbox), 0) + @override_settings(RATE_LIMITING=True) + def test_rate_limiting(self) -> None: + user_profile = self.example_user("hamlet") + email = user_profile.email + from django.core.mail import outbox + + add_ratelimit_rule(10, 2, domain='password_reset_form_by_email') + start_time = time.time() + with patch('time.time', return_value=start_time): + self.client_post('/accounts/password/reset/', {'email': email}) + self.client_post('/accounts/password/reset/', {'email': email}) + self.assert_length(outbox, 2) + + # Too many password reset emails sent to the address, we won't send more. + self.client_post('/accounts/password/reset/', {'email': email}) + self.assert_length(outbox, 2) + + # Resetting for a different address works though. + self.client_post('/accounts/password/reset/', {'email': self.example_email("othello")}) + self.assert_length(outbox, 3) + self.client_post('/accounts/password/reset/', {'email': self.example_email("othello")}) + self.assert_length(outbox, 4) + + # After time, password reset emails can be sent again. + with patch('time.time', return_value=start_time + 11): + self.client_post('/accounts/password/reset/', {'email': email}) + self.client_post('/accounts/password/reset/', {'email': email}) + self.assert_length(outbox, 6) + + remove_ratelimit_rule(10, 2, domain='password_reset_form_by_email') + def test_wrong_subdomain(self) -> None: email = self.example_email("hamlet") diff --git a/zproject/settings.py b/zproject/settings.py index 9a2bb681e5..bd82f66b4b 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -360,6 +360,10 @@ RATE_LIMITING_RULES = { 'authenticate': [ (1800, 5), # 5 login attempts within 30 minutes ], + 'password_reset_form_by_email': [ + (3600, 2), # 2 reset emails per hour + (86400, 5), # 5 per day + ], } RATE_LIMITING_MIRROR_REALM_RULES = [