refactor Replace django-ratelimit-backend with django-ratelimit

We use django-ratelimit to limit per IP login attempts, and then we use
django-ratelimit-backend to limit per username login attempts. This
change replaces the usage of django-ratelimit-backend with another
instance of django-ratelimit so that both limits can be managed by one
library.

This is the first step in being able to fully excise
django-ratelimit-backend from edx-platform. Note that we're still using
the `RateLimitMixin` in openedx/core/djangoapps/oauth_dispatch/dot_overrides/backends.py
because studio and the admin UI still relies on that for rate limiting.
Those login paths will have to be updated before we can remove the mixin
from our auth backend.
This commit is contained in:
Feanil Patel
2021-02-11 14:12:45 -05:00
parent 01ac3c2ed3
commit 6fb93463a8
7 changed files with 75 additions and 22 deletions

View File

@@ -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 #####

View File

@@ -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'

View File

@@ -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 #####

View File

@@ -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'

View File

@@ -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):
"""

View File

@@ -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})

View File

@@ -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