From 72ea1b7d4fffe8b01dcf61aecdf941acd33b7349 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 21 May 2020 11:40:47 -0400 Subject: [PATCH 1/2] Revert "Increase requests limit for logistration rate limit." This reverts commit a1c018823d10e03e4b31a83b7983b7ca83ae8426. --- common/djangoapps/util/request_rate_limiter.py | 8 -------- openedx/core/djangoapps/user_authn/views/login_form.py | 4 ++-- .../user_authn/views/tests/test_logistration.py | 4 ++-- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/common/djangoapps/util/request_rate_limiter.py b/common/djangoapps/util/request_rate_limiter.py index 76ed937993..384ac28276 100644 --- a/common/djangoapps/util/request_rate_limiter.py +++ b/common/djangoapps/util/request_rate_limiter.py @@ -101,11 +101,3 @@ class PasswordResetEmailRateLimiter(RequestRateLimiter): """ for key in self.keys_to_check(request): self.cache_incr(key) - - -class LoginAndRegisterRateLimiter(RequestRateLimiter): - """ - Rate limiting backend for login and register endpoint which - allows 50 requests per IP for every 5 minutes. - """ - requests = 50 diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 16e61aab29..1322d2eb44 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -35,7 +35,7 @@ from student.helpers import get_next_url_for_login_page from third_party_auth import pipeline from third_party_auth.decorators import xframe_allow_whitelisted from util.password_policy_validators import DEFAULT_MAX_PASSWORD_LENGTH -from util.request_rate_limiter import LoginAndRegisterRateLimiter +from util.request_rate_limiter import BadRequestRateLimiter log = logging.getLogger(__name__) @@ -138,7 +138,7 @@ def login_and_registration_form(request, initial_mode="login"): """ - limiter = LoginAndRegisterRateLimiter() + limiter = BadRequestRateLimiter() if limiter.is_rate_limit_exceeded(request): log.warning("Rate limit exceeded in login and registration with initial mode [%s]", initial_mode) return HttpResponseForbidden("Rate limit exceeded") diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py index 3e83c94015..3c0d488118 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py @@ -75,10 +75,10 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto def test_login_and_registration_form_ratelimited(self): """ - Test that login enpoint allow only 50 requests for every 5 minutes. + Test that login enpoint allow only 30 requests for every 5 minutes. """ login_url = reverse('signin_user') - for i in range(50): + for i in range(30): response = self.client.get(login_url) self.assertEqual(response.status_code, 200) From c06f7b2fd7f108f3dff9d5027d3b32ae8f9ee77c Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 21 May 2020 11:41:09 -0400 Subject: [PATCH 2/2] Revert "Rate limit logistration endpoints." This reverts commit 74bc970edc0f5fba4588e7765211d8c4d4d55745. --- .../djangoapps/user_authn/views/login_form.py | 10 -------- .../views/tests/test_logistration.py | 23 +------------------ 2 files changed, 1 insertion(+), 32 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 1322d2eb44..0d850eae30 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -7,7 +7,6 @@ import logging import six from django.conf import settings from django.contrib import messages -from django.http import HttpResponseForbidden from django.shortcuts import redirect from django.urls import reverse from django.utils.translation import ugettext as _ @@ -35,7 +34,6 @@ from student.helpers import get_next_url_for_login_page from third_party_auth import pipeline from third_party_auth.decorators import xframe_allow_whitelisted from util.password_policy_validators import DEFAULT_MAX_PASSWORD_LENGTH -from util.request_rate_limiter import BadRequestRateLimiter log = logging.getLogger(__name__) @@ -137,12 +135,6 @@ def login_and_registration_form(request, initial_mode="login"): initial_mode (string): Either "login" or "register". """ - - limiter = BadRequestRateLimiter() - if limiter.is_rate_limit_exceeded(request): - log.warning("Rate limit exceeded in login and registration with initial mode [%s]", initial_mode) - return HttpResponseForbidden("Rate limit exceeded") - # Determine the URL to redirect to following login/registration/third_party_auth redirect_to = get_next_url_for_login_page(request) @@ -238,8 +230,6 @@ def login_and_registration_form(request, initial_mode="login"): response = render_to_response('student_account/login_and_register.html', context) handle_enterprise_cookies_for_logistration(request, response, context) - limiter.tick_request_counter(request) - return response diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py index 3c0d488118..1e0c4612e8 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- """ Tests for Logistration views. """ -from datetime import datetime, timedelta + from http.cookies import SimpleCookie import ddt @@ -17,8 +17,6 @@ from django.test.client import RequestFactory from django.test.utils import override_settings from django.urls import reverse from django.utils.translation import ugettext as _ -from freezegun import freeze_time -from pytz import UTC from six.moves.urllib.parse import urlencode # pylint: disable=import-error from course_modes.models import CourseMode @@ -73,25 +71,6 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto expected_data = u'"initial_mode": "{mode}"'.format(mode=initial_mode) self.assertContains(response, expected_data) - def test_login_and_registration_form_ratelimited(self): - """ - Test that login enpoint allow only 30 requests for every 5 minutes. - """ - login_url = reverse('signin_user') - for i in range(30): - response = self.client.get(login_url) - self.assertEqual(response.status_code, 200) - - # then the rate limiter should kick in and give a HttpForbidden response - response = self.client.get(login_url) - self.assertEqual(response.status_code, 403) - - # now reset the time to 6 mins from now in future in order to unblock - reset_time = datetime.now(UTC) + timedelta(seconds=361) - with freeze_time(reset_time): - response = self.client.get(login_url) - self.assertEqual(response.status_code, 200) - @ddt.data("signin_user", "register_user") def test_login_and_registration_form_already_authenticated(self, url_name): # call the account registration api that sets the login cookies