From f3db71171e2a95fe1ef1e4350016ff3758ba170b Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Thu, 30 Apr 2020 17:55:34 +0500 Subject: [PATCH] Fix password reset rate limiting for existing users. PROD-1427 --- .../djangoapps/user_authn/views/password_reset.py | 3 ++- .../user_authn/views/tests/test_reset_password.py | 14 ++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/views/password_reset.py b/openedx/core/djangoapps/user_authn/views/password_reset.py index a6af532ecf..7aca85d1aa 100644 --- a/openedx/core/djangoapps/user_authn/views/password_reset.py +++ b/openedx/core/djangoapps/user_authn/views/password_reset.py @@ -273,7 +273,8 @@ def password_reset(request): else: # bad user? tick the rate limiter counter AUDIT_LOG.info("Bad password_reset user passed in.") - password_reset_email_limiter.tick_request_counter(request) + + password_reset_email_limiter.tick_request_counter(request) return JsonResponse({ 'success': True, diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py index 656d4c8564..c477b84184 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_reset_password.py @@ -123,21 +123,23 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): 'openedx.core.djangoapps.user_authn.views.password_reset.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True) ) - def test_password_reset_ratelimited(self): + @ddt.data(True, False) + def test_password_reset_ratelimited(self, existing_user): """ - Test that reset password endpoint only allow one request per minute. + Test that reset password endpoint only allow one request per minute for both + existing and non-existing users. """ cache.clear() - - password_reset_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'}) - password_reset_req.user = AnonymousUser() + email = self.user.email if existing_user else 'thisdoesnotexist@foo.com' + password_reset_req = self.request_factory.post('/password_reset/', {'email': email}) + password_reset_req.user = self.user if existing_user else AnonymousUser() + password_reset_req.site = Mock(domain='example.com') good_resp = password_reset(password_reset_req) self.assertEqual(good_resp.status_code, 200) # then the rate limiter should kick in and give a HttpForbidden response bad_resp = password_reset(password_reset_req) self.assertEqual(bad_resp.status_code, 403) - self.assert_no_events_were_emitted() cache.clear()