diff --git a/common/djangoapps/util/request_rate_limiter.py b/common/djangoapps/util/request_rate_limiter.py index 195117cbc3..384ac28276 100644 --- a/common/djangoapps/util/request_rate_limiter.py +++ b/common/djangoapps/util/request_rate_limiter.py @@ -80,7 +80,7 @@ class PasswordResetEmailRateLimiter(RequestRateLimiter): def keys_to_check(self, request): """ - Retun list of IP and email based keys. + Return list of IP and email based keys. """ keys = super(PasswordResetEmailRateLimiter, self).keys_to_check(request) @@ -97,7 +97,7 @@ class PasswordResetEmailRateLimiter(RequestRateLimiter): def tick_request_counter(self, request): """ - Ticks any counters used to compute when rate limt has been reached + Ticks any counters used to compute when rate limit has been reached. """ for key in self.keys_to_check(request): self.cache_incr(key) 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 5d7bb78539..8450540a8b 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 @@ -160,13 +160,9 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): cache.clear() - @patch( - 'openedx.core.djangoapps.user_authn.views.password_reset.render_to_string', - Mock(side_effect=mock_render_to_string, autospec=True) - ) def test_ratelimitted_from_same_ip_with_different_email(self): """ - Test that password reset endpoint allow one request per minute per IP. + Test that password reset endpoint allow only one request per minute per IP. """ cache.clear() good_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'}) @@ -183,6 +179,34 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): cache.clear() + def test_ratelimited_from_different_ips_with_same_email(self): + """ + Test that password reset endpoint allow only one request per minute + per email address. + """ + cache.clear() + good_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'}) + good_req.user = AnonymousUser() + good_resp = password_reset(good_req) + self.assertEqual(good_resp.status_code, 200) + + # change the IP and verify that the rate limiter should kick in and + # give a Forbidden response if the request is for same email address. + new_ip = "8.8.8.8" + self.assertNotEqual(good_req.META.get('REMOTE_ADDR'), new_ip) + + bad_req = self.request_factory.post( + '/password_reset/', + {'email': 'thisdoesnotexist@foo.com'}, + REMOTE_ADDR=new_ip + ) + bad_req.user = AnonymousUser() + bad_resp = password_reset(bad_req) + self.assertEqual(bad_resp.status_code, 403) + self.assertEqual(bad_req.META.get('REMOTE_ADDR'), new_ip) + + cache.clear() + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', "Test only valid in LMS") @ddt.data(('plain_text', "You're receiving this e-mail because you requested a password reset"), ('html', "You're receiving this e-mail because you requested a password reset"))