diff --git a/cms/envs/common.py b/cms/envs/common.py index 8ab6f935ce..47b1d49e63 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2325,6 +2325,7 @@ DISABLE_DEPRECATED_SIGNUP_URL = False ##### LOGISTRATION RATE LIMIT SETTINGS ##### LOGISTRATION_RATELIMIT_RATE = '100/5m' +LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '30/5m' LOGISTRATION_API_RATELIMIT = '20/m' ##### REGISTRATION RATE LIMIT SETTINGS ##### diff --git a/cms/envs/test.py b/cms/envs/test.py index 048c623f0d..4fd06839f4 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -327,6 +327,7 @@ PROCTORING_USER_OBFUSCATION_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' ##### LOGISTRATION RATE LIMIT SETTINGS ##### LOGISTRATION_RATELIMIT_RATE = '5/5m' +LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '6/5m' LOGISTRATION_API_RATELIMIT = '5/m' REGISTRATION_VALIDATION_RATELIMIT = '5/minute' diff --git a/lms/envs/common.py b/lms/envs/common.py index c2fa5e5935..d7d1a0bda4 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4268,6 +4268,7 @@ RATELIMIT_RATE = '120/m' ##### LOGISTRATION RATE LIMIT SETTINGS ##### LOGISTRATION_RATELIMIT_RATE = '100/5m' +LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '30/5m' LOGISTRATION_API_RATELIMIT = '20/m' ##### PASSWORD RESET RATE LIMIT SETTINGS ##### diff --git a/lms/envs/test.py b/lms/envs/test.py index 4e4f4470e0..f3f1d3974b 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -595,6 +595,7 @@ RATELIMIT_RATE = '2/m' ##### LOGISTRATION RATE LIMIT SETTINGS ##### LOGISTRATION_RATELIMIT_RATE = '5/5m' +LOGISTRATION_PER_EMAIL_RATELIMIT_RATE = '6/5m' LOGISTRATION_API_RATELIMIT = '5/m' REGISTRATION_VALIDATION_RATELIMIT = '5/minute' diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 6f17cb9c3e..5f0a95cf7c 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -25,7 +25,6 @@ from django.views.decorators.debug import sensitive_post_parameters from django.views.decorators.http import require_http_methods from edx_django_utils.monitoring import set_custom_attribute from ratelimit.decorators import ratelimit -from ratelimitbackend.exceptions import RateLimitException from rest_framework.views import APIView from common.djangoapps.edxmako.shortcuts import render_to_response @@ -180,6 +179,9 @@ def _authenticate_first_party(request, unauthenticated_user, third_party_auth_re """ Use Django authentication on the given request, using rate limiting if configured """ + should_be_rate_limited = getattr(request, 'limited', False) + if should_be_rate_limited: + raise AuthFailedError(_('Too many failed login attempts. Try again later.')) # lint-amnesty, pylint: disable=raise-missing-from # If the user doesn't exist, we want to set the username to an invalid username so that authentication is guaranteed # to fail and we can take advantage of the ratelimited backend @@ -191,17 +193,12 @@ def _authenticate_first_party(request, unauthenticated_user, third_party_auth_re if not third_party_auth_requested: _check_user_auth_flow(request.site, unauthenticated_user) - try: - password = normalize_password(request.POST['password']) - return authenticate( - username=username, - password=password, - request=request - ) - - # This occurs when there are too many attempts from the same IP address - except RateLimitException: - raise AuthFailedError(_('Too many failed login attempts. Try again later.')) # lint-amnesty, pylint: disable=raise-missing-from + password = normalize_password(request.POST['password']) + return authenticate( + username=username, + password=password, + request=request + ) def _handle_failed_authentication(user, authenticated_user): @@ -389,11 +386,15 @@ def finish_auth(request): @ensure_csrf_cookie @require_http_methods(['POST']) +@ratelimit( + key='openedx.core.djangoapps.util.ratelimit.request_post_email', + rate=settings.LOGISTRATION_PER_EMAIL_RATELIMIT_RATE, + method='POST', +) # lint-amnesty, pylint: disable=too-many-statements @ratelimit( key='openedx.core.djangoapps.util.ratelimit.real_ip', rate=settings.LOGISTRATION_RATELIMIT_RATE, method='POST', - block=True ) # lint-amnesty, pylint: disable=too-many-statements def login_user(request): """ diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 338248bb49..b311628cf4 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -20,6 +20,7 @@ from django.test.client import Client from django.test.utils import override_settings from django.urls import NoReverseMatch, reverse from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch +from freezegun import freeze_time from mock import Mock, patch from common.djangoapps.student.tests.factories import RegistrationFactory, UserFactory, UserProfileFactory @@ -390,15 +391,40 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): response, _audit_log = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) - @override_settings(RATELIMIT_ENABLE=False) - def test_excessive_login_attempts(self): - # try logging in 30 times, the default limit in the number of failed + @patch('openedx.core.djangoapps.util.ratelimit.real_ip') + def test_excessive_login_attempts_by_email(self, real_ip_mock): + # try logging in 6 times, the defalutlimit for the number of failed # login attempts in one 5 minute period before the rate gets limited - for i in range(30): - password = u'test_password{0}'.format(i) - self._login_response(self.user_email, password) - # check to see if this response indicates that this was ratelimited - response, _audit_log = self._login_response(self.user_email, 'wrong_password') + # for a specific e-mail address. + + # We freeze time to deal with the fact that rate limit time boundaries + # are not predictable and we don't want the test to be flaky. + with freeze_time(): + for i in range(6): + password = u'test_password{0}'.format(i) + # Provide unique IPs so we don't get ip rate limited. + real_ip_mock.return_value = f'192.168.1.{i}' + self._login_response(self.user_email, password) + # check to see if this response indicates that this was ratelimited + response, _audit_log = self._login_response(self.user_email, 'wrong_password') + self._assert_response(response, success=False, value='Too many failed login attempts') + + def test_excessive_login_attempts_by_ip(self): + # try logging in 5 times, the default limit for the number of failed + # login attempts in one 5 minute period before the rate gets limited + # from a specific ip address. + + # We freeze time to deal with the fact that rate limit time boundaries + # are not predictable and we don't want the test to be flaky. + with freeze_time(): + for i in range(5): + # provide different e-mail addresses so we don't get rate-limited + # by e-mail. + email = f'test_email{i}@example.com' + password = f'test_password{i}' + self._login_response(email, password) + # check to see if this response indicates that this was ratelimited + response, _audit_log = self._login_response(self.user_email, 'wrong_password') self._assert_response(response, success=False, value='Too many failed login attempts') @patch.dict("django.conf.settings.FEATURES", {"DISABLE_SET_JWT_COOKIES_FOR_TESTS": False}) diff --git a/openedx/core/djangoapps/util/ratelimit.py b/openedx/core/djangoapps/util/ratelimit.py index d3ef9bf9f8..9dc1ea858c 100644 --- a/openedx/core/djangoapps/util/ratelimit.py +++ b/openedx/core/djangoapps/util/ratelimit.py @@ -3,8 +3,30 @@ Code to get ip from request. """ +from uuid import uuid4 + from ipware.ip import get_ip -def real_ip(group, request): # lint-amnesty, pylint: disable=unused-argument +def real_ip(group, request): # pylint: disable=unused-argument return get_ip(request) + + +def request_post_email(group, request) -> str: # pylint: disable=unused-argument + """ + Return the the email post param if it exists, otherwise return a + random id. + + If the request doesn't have an email post body param, treat it as + a unique key. This will probably mean that it will not get rate limited. + + This ratelimit key function is meant to be used with the user_authn/views/login.py::login_user + function. To rate-limit any first party auth. For 3rd party auth, there is separate rate limiting + currently in place so we don't do any rate limiting for that case here. + """ + + email = request.POST.get('email') + if not email: + email = str(uuid4()) + + return email