From 8cc5f13dafac06dc8967af695debee850136aa57 Mon Sep 17 00:00:00 2001 From: Zainab Amir Date: Thu, 25 Mar 2021 16:28:30 +0500 Subject: [PATCH] Add rate limit to registration endpoint (#27060) Currently the registration endpoint has no rate limit. Added a new ratelimit variable to support the change, it's value is set to 60/7d. VAN-302 --- cms/envs/common.py | 1 + cms/envs/production.py | 1 + cms/envs/test.py | 2 ++ .../student/tests/test_long_username_email.py | 2 ++ .../student/tests/test_password_policy.py | 2 ++ lms/envs/common.py | 1 + lms/envs/production.py | 2 ++ lms/envs/test.py | 2 ++ .../djangoapps/user_authn/views/register.py | 5 +++ .../user_authn/views/tests/test_register.py | 32 +++++++++++++++++++ 10 files changed, 50 insertions(+) diff --git a/cms/envs/common.py b/cms/envs/common.py index 1b189d536b..337122feca 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2380,6 +2380,7 @@ RESET_PASSWORD_API_RATELIMIT = '30/7d' ##### REGISTRATION RATE LIMIT SETTINGS ##### REGISTRATION_VALIDATION_RATELIMIT = '30/7d' +REGISTRATION_RATELIMIT = '60/7d' ##### PASSWORD RESET RATE LIMIT SETTINGS ##### PASSWORD_RESET_IP_RATE = '1/m' diff --git a/cms/envs/production.py b/cms/envs/production.py index 0e1f29cc4a..251bb66f99 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -266,6 +266,7 @@ TIME_ZONE = ENV_TOKENS.get('CELERY_TIMEZONE', CELERY_TIMEZONE) REGISTRATION_VALIDATION_RATELIMIT = ENV_TOKENS.get( 'REGISTRATION_VALIDATION_RATELIMIT', REGISTRATION_VALIDATION_RATELIMIT ) +REGISTRATION_RATELIMIT = ENV_TOKENS.get('REGISTRATION_RATELIMIT', REGISTRATION_RATELIMIT) # Push to LMS overrides GIT_REPO_EXPORT_DIR = ENV_TOKENS.get('GIT_REPO_EXPORT_DIR', '/edx/var/edxapp/export_course_repos') diff --git a/cms/envs/test.py b/cms/envs/test.py index 7da57c79bd..593132784b 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -330,6 +330,8 @@ LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '6/5m' LOGISTRATION_API_RATELIMIT = '5/m' REGISTRATION_VALIDATION_RATELIMIT = '5/minute' +REGISTRATION_RATELIMIT = '5/minute' + RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT = '2/m' RESET_PASSWORD_API_RATELIMIT = '2/m' diff --git a/common/djangoapps/student/tests/test_long_username_email.py b/common/djangoapps/student/tests/test_long_username_email.py index bee416100e..337929acb2 100644 --- a/common/djangoapps/student/tests/test_long_username_email.py +++ b/common/djangoapps/student/tests/test_long_username_email.py @@ -2,11 +2,13 @@ import json from django.test import TestCase +from django.test.utils import override_settings from django.urls import reverse from openedx.core.djangoapps.user_api.accounts import USERNAME_BAD_LENGTH_MSG +@override_settings(RATELIMIT_ENABLE=False) class TestLongUsernameEmail(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring def setUp(self): diff --git a/common/djangoapps/student/tests/test_password_policy.py b/common/djangoapps/student/tests/test_password_policy.py index f380095608..c758e48f4c 100644 --- a/common/djangoapps/student/tests/test_password_policy.py +++ b/common/djangoapps/student/tests/test_password_policy.py @@ -11,6 +11,7 @@ from django.urls import reverse from common.djangoapps.util.password_policy_validators import create_validator_config +@override_settings(RATELIMIT_ENABLE=False) class TestPasswordPolicy(TestCase): """ Go through some password policy tests to make sure things are properly working @@ -226,6 +227,7 @@ class TestPasswordPolicy(TestCase): assert obj['success'] +@override_settings(RATELIMIT_ENABLE=False) class TestUsernamePasswordNonmatch(TestCase): """ Test that registration username and password fields differ diff --git a/lms/envs/common.py b/lms/envs/common.py index 9e345e2506..d6ab6a1deb 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3160,6 +3160,7 @@ REST_FRAMEWORK = { } REGISTRATION_VALIDATION_RATELIMIT = '30/7d' +REGISTRATION_RATELIMIT = '60/7d' SWAGGER_SETTINGS = { 'DEFAULT_INFO': 'openedx.core.apidocs.api_info', diff --git a/lms/envs/production.py b/lms/envs/production.py index 0e30918187..444e2589f6 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -613,6 +613,8 @@ REGISTRATION_VALIDATION_RATELIMIT = ENV_TOKENS.get( 'REGISTRATION_VALIDATION_RATELIMIT', REGISTRATION_VALIDATION_RATELIMIT ) +REGISTRATION_RATELIMIT = ENV_TOKENS.get('REGISTRATION_RATELIMIT', REGISTRATION_RATELIMIT) + #### PASSWORD POLICY SETTINGS ##### AUTH_PASSWORD_VALIDATORS = ENV_TOKENS.get("AUTH_PASSWORD_VALIDATORS", AUTH_PASSWORD_VALIDATORS) diff --git a/lms/envs/test.py b/lms/envs/test.py index 00663f732d..5448e3b48a 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -597,5 +597,7 @@ LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '6/5m' LOGISTRATION_API_RATELIMIT = '5/m' REGISTRATION_VALIDATION_RATELIMIT = '5/minute' +REGISTRATION_RATELIMIT = '5/minute' + RESET_PASSWORD_TOKEN_VALIDATE_API_RATELIMIT = '2/m' RESET_PASSWORD_API_RATELIMIT = '2/m' diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index c2920fd1ca..ed07d3a237 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -492,6 +492,7 @@ class RegistrationView(APIView): content_type="application/json") @method_decorator(csrf_exempt) + @method_decorator(ratelimit(key=REAL_IP_KEY, rate=settings.REGISTRATION_RATELIMIT, method='POST')) def post(self, request): """Create the user's account. @@ -510,6 +511,10 @@ class RegistrationView(APIView): address already exists HttpResponse: 403 operation not allowed """ + should_be_rate_limited = getattr(request, 'limited', False) + if should_be_rate_limited: + return JsonResponse({'error_code': 'forbidden-request'}, status=403) + if is_require_third_party_auth_enabled() and not pipeline.running(request): # if request is not running a third-party auth pipeline return HttpResponseForbidden( diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index ccb089087b..adf510e9eb 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -1632,6 +1632,38 @@ class RegistrationViewTestV1(ThirdPartyAuthTestMixin, UserAPITestCase): response = self.client.post(self.url, {"email": self.EMAIL, "username": self.USERNAME}) assert response.status_code == 403 + @override_settings( + CACHES={ + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'registration_proxy', + } + } + ) + def test_rate_limiting_registration_view(self): + """ + Confirm rate limits work as expected for registration + end point. + Note that drf's rate limiting makes use of the default cache + to enforce limits; that's why this test needs a "real" + default cache (as opposed to the usual-for-tests DummyCache) + """ + payload = { + "email": 'email', + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "honor_code": "true", + } + + for _ in range(int(settings.REGISTRATION_RATELIMIT.split('/')[0])): + response = self.client.post(self.url, payload) + assert response.status_code != 403 + + response = self.client.post(self.url, payload) + assert response.status_code == 403 + cache.clear() + def _assert_fields_match(self, actual_field, expected_field): """ Assert that the actual field and the expected field values match.