From d9ec42c425ffb270d5ccf49e81afb5f130282075 Mon Sep 17 00:00:00 2001 From: Adeel Khan Date: Thu, 18 Feb 2021 08:20:00 +0500 Subject: [PATCH] Add throttling to validate token and reset password end points VAN-312 --- cms/envs/common.py | 3 ++ cms/envs/test.py | 3 +- lms/envs/common.py | 2 + lms/envs/production.py | 4 ++ lms/envs/test.py | 2 + .../user_authn/views/password_reset.py | 43 +++++++++++++++++++ .../views/tests/test_reset_password.py | 41 ++++++++++++++++++ 7 files changed, 97 insertions(+), 1 deletion(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 04c8ee5e52..746f08a5ae 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2340,6 +2340,9 @@ DISABLE_DEPRECATED_SIGNUP_URL = False LOGISTRATION_RATELIMIT_RATE = '100/5m' LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '30/5m' LOGISTRATION_API_RATELIMIT = '20/m' +RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT = '30/7d' +RESET_PASSWORD_API_RATELIMIT = '30/7d' + ##### REGISTRATION RATE LIMIT SETTINGS ##### REGISTRATION_VALIDATION_RATELIMIT = '30/7d' diff --git a/cms/envs/test.py b/cms/envs/test.py index 4fd06839f4..09c9919a35 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -331,7 +331,8 @@ LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '6/5m' LOGISTRATION_API_RATELIMIT = '5/m' REGISTRATION_VALIDATION_RATELIMIT = '5/minute' - +RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT = '2/m' +RESET_PASSWORD_API_RATELIMIT = '2/m' # Don't tolerate deprecated edx-platform import usage in tests. ERROR_ON_DEPRECATED_EDX_PLATFORM_IMPORTS = True diff --git a/lms/envs/common.py b/lms/envs/common.py index 91fa13247c..8dcd607b53 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4370,6 +4370,8 @@ RATELIMIT_RATE = '120/m' LOGISTRATION_RATELIMIT_RATE = '100/5m' LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '30/5m' LOGISTRATION_API_RATELIMIT = '20/m' +RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT = '30/7d' +RESET_PASSWORD_API_RATELIMIT = '30/7d' ##### PASSWORD RESET RATE LIMIT SETTINGS ##### PASSWORD_RESET_IP_RATE = '1/m' diff --git a/lms/envs/production.py b/lms/envs/production.py index f0b3ed5635..f76f774f68 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -605,6 +605,10 @@ MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS = ENV_TOKENS.get( ##### LOGISTRATION RATE LIMIT SETTINGS ##### LOGISTRATION_RATELIMIT_RATE = ENV_TOKENS.get('LOGISTRATION_RATELIMIT_RATE', LOGISTRATION_RATELIMIT_RATE) LOGISTRATION_API_RATELIMIT = ENV_TOKENS.get('LOGISTRATION_API_RATELIMIT', LOGISTRATION_API_RATELIMIT) +RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT = ENV_TOKENS.get( + 'RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT', RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT +) +RESET_PASSWORD_API_RATELIMIT = ENV_TOKENS.get('RESET_PASSWORD_API_RATELIMIT', RESET_PASSWORD_API_RATELIMIT) ##### REGISTRATION RATE LIMIT SETTINGS ##### REGISTRATION_VALIDATION_RATELIMIT = ENV_TOKENS.get( diff --git a/lms/envs/test.py b/lms/envs/test.py index f3f1d3974b..47583c8d0f 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -599,6 +599,8 @@ LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '6/5m' LOGISTRATION_API_RATELIMIT = '5/m' REGISTRATION_VALIDATION_RATELIMIT = '5/minute' +RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT = '2/m' +RESET_PASSWORD_API_RATELIMIT = '2/m' # Don't tolerate deprecated edx-platform import usage in tests. ERROR_ON_DEPRECATED_EDX_PLATFORM_IMPORTS = True diff --git a/openedx/core/djangoapps/user_authn/views/password_reset.py b/openedx/core/djangoapps/user_authn/views/password_reset.py index bf6c9da8b9..1f6b2dd425 100644 --- a/openedx/core/djangoapps/user_authn/views/password_reset.py +++ b/openedx/core/djangoapps/user_authn/views/password_reset.py @@ -26,6 +26,7 @@ from edx_ace.recipient import Recipient from eventtracking import tracker from ratelimit.decorators import ratelimit from rest_framework.response import Response +from rest_framework.throttling import AnonRateThrottle from rest_framework.views import APIView from common.djangoapps.edxmako.shortcuts import render_to_string @@ -642,7 +643,35 @@ def password_change_request_handler(request): return HttpResponseBadRequest(_("No email address provided.")) +def _get_rate(rate): + """ + Given the request rate string, return a two tuple of: + , + """ + + requests, duration = rate.split('/') + num_requests = int(requests) + num = int(duration[:-1] if duration[:-1] else 1) + symbol = duration[-1:] + duration = {'s': 1, 'm': 60, 'h': 3600, 'd': 86400}[symbol] * num + return (num_requests, duration) + + +class ResetTokenValidationThrottle(AnonRateThrottle): + """ + Setting rate limit for token validation + """ + rate = settings.RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT + + def parse_rate(self, rate): + return _get_rate(rate) + + class PasswordResetTokenValidation(APIView): # lint-amnesty, pylint: disable=missing-class-docstring + """ + API to validate generated password reset token + """ + throttle_classes = [ResetTokenValidationThrottle] def post(self, request): """ HTTP end-point to validate password reset token. """ @@ -668,7 +697,21 @@ class PasswordResetTokenValidation(APIView): # lint-amnesty, pylint: disable=mi return Response({'is_valid': is_valid}) +class PasswordResetThrottle(AnonRateThrottle): + """ + Setting rate limit for password reset + """ + rate = settings.RESET_PASSWORD_API_RATELIMIT + + def parse_rate(self, rate): + return _get_rate(rate) + + class LogistrationPasswordResetView(APIView): # lint-amnesty, pylint: disable=missing-class-docstring + """ + API to update new password credentials for a correct token + """ + throttle_classes = [PasswordResetThrottle] def post(self, request, **kwargs): """ Reset learner password using passed token and new credentials """ 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 0f3798a2b7..bce2154e56 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 @@ -709,6 +709,24 @@ class PasswordResetTokenValidateViewTest(UserAPITestCase): self.user = User.objects.get(pk=self.user.pk) assert not self.user.is_active + @override_settings( + CACHES={ + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'validate_token', + } + } + ) + def test_reset_password_token_api_throttle(self): + """ + Test that the reset password token validation endpoint is throttling + """ + for _ in range(int(settings.RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT.split('/')[0])): + response = self.client.post(self.url, data={'token': self.token}) + assert response.status_code != 429 + response = self.client.post(self.url, data={'token': self.token}) + assert response.status_code == 429 + @ddt.ddt @unittest.skipUnless( @@ -831,3 +849,26 @@ class ResetPasswordAPITests(EventTestMixin, CacheIsolationTestCase): assert sent_message.from_email == from_email assert len(sent_message.to) == 1 assert updated_user.email in sent_message.to[0] + + @override_settings( + CACHES={ + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'reset_password', + } + } + ) + def test_password_reset_api_throttle(self): + """ + Test that the reset password end point is throttling + """ + path = reverse( + "logistration_password_reset", + kwargs={"uidb36": self.uidb36, "token": self.token} + ) + request_param = {'new_password1': 'new_password1', 'new_password2': 'new_password1'} + for _ in range(int(settings.RESET_PASSWORD_API_RATELIMIT.split('/')[0])): + response = self.client.post(path, request_param) + assert response.status_code != 429 + response = self.client.post(path, request_param) + assert response.status_code == 429