From 06198af5b9ceece4f319cc147a83c16fd13dad7c Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Mon, 30 Dec 2019 02:21:51 +0100 Subject: [PATCH] auth: Handle rate limiting in OurAuthenticationForm and user_settings. These parts of the code should catch the RateLimited exception and generate their own, apprioprate user-facing error message. --- zerver/forms.py | 14 +++++++++-- zerver/lib/rate_limiter.py | 3 +++ zerver/tests/test_settings.py | 45 +++++++++++++++++++++++++++++++++++ zerver/tests/test_signup.py | 27 +++++++++++++++++++++ zerver/views/user_settings.py | 17 ++++++++++--- 5 files changed, 101 insertions(+), 5 deletions(-) diff --git a/zerver/forms.py b/zerver/forms.py index bd28aae258..8c70539310 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -17,6 +17,7 @@ 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.request import JsonableError from zerver.lib.send_email import send_email, FromAddress from zerver.lib.subdomains import get_subdomain, is_root_domain_available @@ -45,6 +46,9 @@ WRONG_SUBDOMAIN_ERROR = "Your Zulip account is not a member of the " + \ DEACTIVATED_ACCOUNT_ERROR = u"Your account is no longer active. " + \ u"Please contact your organization administrator to reactivate it." PASSWORD_TOO_WEAK_ERROR = u"The password is too weak." +AUTHENTICATION_RATE_LIMITED_ERROR = "You're making too many attempts to sign in. " + \ + "Try again in %s seconds or contact your organization administrator " + \ + "for help." def email_is_not_mit_mailing_list(email: str) -> None: """Prevent MIT mailing lists from signing up for Zulip""" @@ -305,8 +309,14 @@ class OurAuthenticationForm(AuthenticationForm): raise ValidationError("Realm does not exist") return_data = {} # type: Dict[str, Any] - self.user_cache = authenticate(request=self.request, username=username, password=password, - realm=realm, return_data=return_data) + try: + self.user_cache = authenticate(request=self.request, username=username, password=password, + realm=realm, return_data=return_data) + except RateLimited as e: + entity_type = str(e) + secs_to_freedom = int(get_rate_limit_result_from_request(self.request, + entity_type).secs_to_freedom) + raise ValidationError(AUTHENTICATION_RATE_LIMITED_ERROR % (secs_to_freedom,)) if return_data.get("inactive_realm"): raise AssertionError("Programming error: inactive realm in authentication form") diff --git a/zerver/lib/rate_limiter.py b/zerver/lib/rate_limiter.py index 5ac64bbdcb..f908fa86f9 100644 --- a/zerver/lib/rate_limiter.py +++ b/zerver/lib/rate_limiter.py @@ -300,3 +300,6 @@ class RateLimitResult: self.secs_to_freedom = secs_to_freedom self.over_limit = over_limit self.remaining = remaining + +def get_rate_limit_result_from_request(request: HttpRequest, entity_type: str) -> RateLimitResult: + return request._ratelimit[entity_type] diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index af789828cd..2ce3e0b3ec 100644 --- a/zerver/tests/test_settings.py +++ b/zerver/tests/test_settings.py @@ -1,3 +1,5 @@ +import mock +import time import ujson from django.http import HttpResponse @@ -8,6 +10,7 @@ from zerver.lib.initial_password import initial_password from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import get_test_image_file from zerver.lib.users import get_all_api_keys +from zerver.lib.rate_limiter import add_ratelimit_rule, remove_ratelimit_rule from zerver.models import get_realm, UserProfile, \ get_user_profile_by_api_key @@ -203,6 +206,48 @@ class ChangeSettingsTest(ZulipTestCase): )) self.assert_json_error(result, "Wrong password!") + def test_wrong_old_password_rate_limiter(self) -> None: + self.login(self.example_email("hamlet")) + with self.settings(RATE_LIMITING_AUTHENTICATE=True): + add_ratelimit_rule(10, 2, domain='authenticate') + start_time = time.time() + with mock.patch('time.time', return_value=start_time): + result = self.client_patch( + "/json/settings", + dict( + old_password='bad_password', + new_password="ignored", + )) + self.assert_json_error(result, "Wrong password!") + result = self.client_patch( + "/json/settings", + dict( + old_password='bad_password', + new_password="ignored", + )) + self.assert_json_error(result, "Wrong password!") + + # We're over the limit, so we'll get blocked even with the correct password. + result = self.client_patch( + "/json/settings", + dict( + old_password=initial_password(self.example_email("hamlet")), + new_password="ignored", + )) + self.assert_json_error(result, "You're making too many attempts! Try again in 10 seconds.") + + # After time passes, we should be able to succeed if we give the correct password. + with mock.patch('time.time', return_value=start_time + 11): + json_result = self.client_patch( + "/json/settings", + dict( + old_password=initial_password(self.example_email("hamlet")), + new_password='foobar1', + )) + self.assert_json_success(json_result) + + remove_ratelimit_rule(10, 2, domain='authenticate') + @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend', 'zproject.backends.EmailAuthBackend', 'zproject.backends.ZulipDummyBackend')) diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index c43145fa53..17a73f3880 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -52,6 +52,7 @@ from zerver.lib.mobile_auth_otp import xor_hex_strings, ascii_to_hex, \ otp_encrypt_api_key, is_valid_otp, hex_to_ascii, otp_decrypt_api_key from zerver.lib.email_notifications import enqueue_welcome_emails, \ followup_day2_email_delay +from zerver.lib.rate_limiter import add_ratelimit_rule, remove_ratelimit_rule from zerver.lib.subdomains import is_root_domain_available from zerver.lib.stream_subscription import get_stream_subscriptions_for_user from zerver.lib.test_helpers import find_key_by_email, queries_captured, \ @@ -64,6 +65,7 @@ from zerver.context_processors import common_context import re import smtplib +import time import ujson from typing import Any, List, Optional @@ -487,6 +489,31 @@ class LoginTest(ZulipTestCase): self.assert_in_success_response([email], result) self.assert_logged_in_user_id(None) + @override_settings(RATE_LIMITING_AUTHENTICATE=True) + def test_login_bad_password_rate_limiter(self) -> None: + user_profile = self.example_user("hamlet") + email = user_profile.email + add_ratelimit_rule(10, 2, domain='authenticate') + + start_time = time.time() + with patch('time.time', return_value=start_time): + self.login_with_return(email, password="wrongpassword") + self.assert_logged_in_user_id(None) + self.login_with_return(email, password="wrongpassword") + self.assert_logged_in_user_id(None) + + # We're over the allowed limit, so the next attempt, even with the correct + # password, will get blocked. + result = self.login_with_return(email) + self.assert_in_success_response(["Try again in 10 seconds"], result) + + # After time passes, we should be able to log in. + with patch('time.time', return_value=start_time + 11): + self.login_with_return(email) + self.assert_logged_in_user_id(user_profile.id) + + remove_ratelimit_rule(10, 2, domain='authenticate') + def test_login_nonexist_user(self) -> None: result = self.login_with_return("xxx@zulip.com", "xxx") self.assertEqual(result.status_code, 200) diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index f01322bf39..8fe9a67bbc 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -19,6 +19,7 @@ from zerver.lib.i18n import get_available_language_codes from zerver.lib.response import json_success, json_error from zerver.lib.upload import upload_avatar_image from zerver.lib.validator import check_bool, check_string, check_int +from zerver.lib.rate_limiter import RateLimited, get_rate_limit_result_from_request from zerver.lib.request import JsonableError from zerver.lib.timezone import get_all_timezones from zerver.models import UserProfile, name_changes_disabled, avatar_changes_disabled @@ -68,11 +69,21 @@ def json_change_settings(request: HttpRequest, user_profile: UserProfile, return_data = {} # type: Dict[str, Any] if email_belongs_to_ldap(user_profile.realm, user_profile.delivery_email): return json_error(_("Your Zulip password is managed in LDAP")) - if not authenticate(request=request, username=user_profile.delivery_email, password=old_password, - realm=user_profile.realm, return_data=return_data): - return json_error(_("Wrong password!")) + + try: + if not authenticate(request, username=user_profile.delivery_email, password=old_password, + realm=user_profile.realm, return_data=return_data): + return json_error(_("Wrong password!")) + except RateLimited as e: + entity_type = str(e) + secs_to_freedom = int(get_rate_limit_result_from_request(request, entity_type).secs_to_freedom) + return json_error( + _("You're making too many attempts! Try again in %s seconds.") % (secs_to_freedom,) + ) + if not check_password_strength(new_password): return json_error(_("New password is too weak!")) + do_change_password(user_profile, new_password) # In Django 1.10, password changes invalidates sessions, see # https://docs.djangoproject.com/en/1.10/topics/auth/default/#session-invalidation-on-password-change