From c8badbd8585943303be1e961b5aaac7ea73bb428 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Thu, 4 Nov 2021 18:21:26 -0700 Subject: [PATCH] reset_password: Show user-facing page on rate-limit. --- zerver/forms.py | 4 ++-- zerver/tests/test_external.py | 12 ++++++++++++ zerver/views/auth.py | 20 +++++++++++++++----- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/zerver/forms.py b/zerver/forms.py index 33a589127a..07c275e953 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -326,13 +326,13 @@ class ZulipPasswordResetForm(PasswordResetForm): rate_limit_password_reset_form_by_email(email) rate_limit_request_by_ip(request, domain="sends_email_by_ip") except RateLimited: - # TODO: Show an informative, user-facing error message. logging.info( "Too many password reset attempts for email %s from %s", email, request.META["REMOTE_ADDR"], ) - return + # The view will handle the RateLimit exception and render an appropriate page + raise user: Optional[UserProfile] = None try: diff --git a/zerver/tests/test_external.py b/zerver/tests/test_external.py index ed2ea014ce..286d097f2e 100644 --- a/zerver/tests/test_external.py +++ b/zerver/tests/test_external.py @@ -227,6 +227,18 @@ class RateLimitTests(ZulipTestCase): is_json=False, ) + @rate_limit_rule(1, 5, domain="sends_email_by_ip") + def test_password_reset_rate_limiting(self) -> None: + with self.assertLogs(level="INFO") as m: + self.do_test_hit_ratelimits( + lambda: self.client_post("/accounts/password/reset/", {"email": "new@zulip.com"}), + is_json=False, + ) + self.assertEqual( + m.output, + ["INFO:root:Too many password reset attempts for email new@zulip.com from 127.0.0.1"], + ) + # Test whether submitting multiple emails is handled correctly. # The limit is set to 10 per second, so 5 requests with 2 emails # submitted in each should be allowed. diff --git a/zerver/views/auth.py b/zerver/views/auth.py index 6b1b78132b..6a52e865c2 100644 --- a/zerver/views/auth.py +++ b/zerver/views/auth.py @@ -51,6 +51,7 @@ from zerver.lib.exceptions import ( JsonableError, PasswordAuthDisabledError, PasswordResetRequiredError, + RateLimited, RealmDeactivatedError, UserDeactivatedError, ) @@ -992,11 +993,20 @@ def password_reset(request: HttpRequest) -> HttpResponse: ) return HttpResponseRedirect(redirect_url) - response = DjangoPasswordResetView.as_view( - template_name="zerver/reset.html", - form_class=ZulipPasswordResetForm, - success_url="/accounts/password/reset/done/", - )(request) + try: + response = DjangoPasswordResetView.as_view( + template_name="zerver/reset.html", + form_class=ZulipPasswordResetForm, + success_url="/accounts/password/reset/done/", + )(request) + except RateLimited as e: + assert e.secs_to_freedom is not None + return render( + request, + "zerver/rate_limit_exceeded.html", + context={"retry_after": int(e.secs_to_freedom)}, + status=429, + ) assert isinstance(response, HttpResponse) return response