diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py
index 9286b49ecfbf29e6bee17ba72c118d0827b83249..8d1dab05ee1874aaa226970399690adf1fcd2e8a 100644
--- a/common/djangoapps/student/views/management.py
+++ b/common/djangoapps/student/views/management.py
@@ -85,7 +85,7 @@ from student.models import (
from student.signals import REFUND_ORDER
from student.tasks import send_activation_email
from student.text_me_the_app import TextMeTheAppFragmentView
-from util.bad_request_rate_limiter import BadRequestRateLimiter
+from util.request_rate_limiter import BadRequestRateLimiter, PasswordResetEmailRateLimiter
from util.db import outer_atomic
from util.json_request import JsonResponse
from util.password_policy_validators import normalize_password, validate_password
@@ -664,10 +664,14 @@ def password_change_request_handler(request):
"""
- limiter = BadRequestRateLimiter()
- if limiter.is_rate_limit_exceeded(request):
+ password_reset_email_limiter = PasswordResetEmailRateLimiter()
+
+ if password_reset_email_limiter.is_rate_limit_exceeded(request):
AUDIT_LOG.warning("Password reset rate limit exceeded")
- return HttpResponseForbidden()
+ return HttpResponse(
+ _("Your previous request is in progress, please try again in a few moments."),
+ status=403
+ )
user = request.user
# Prefer logged-in user's email
@@ -681,9 +685,6 @@ def password_change_request_handler(request):
destroy_oauth_tokens(user)
except UserNotFound:
AUDIT_LOG.info("Invalid password reset attempt")
- # Increment the rate limit counter
- limiter.tick_bad_request_counter(request)
-
# If enabled, send an email saying that a password reset was attempted, but that there is
# no user associated with the email
if configuration_helpers.get_value('ENABLE_PASSWORD_RESET_FAILURE_EMAIL',
@@ -703,13 +704,13 @@ def password_change_request_handler(request):
language=settings.LANGUAGE_CODE,
user_context=message_context,
)
-
ace.send(msg)
except UserAPIInternalError as err:
log.exception('Error occured during password change for user {email}: {error}'
.format(email=email, error=err))
return HttpResponse(_("Some error occured during password change. Please try again"), status=500)
+ password_reset_email_limiter.tick_request_counter(request)
return HttpResponse(status=200)
else:
return HttpResponseBadRequest(_("No email address provided."))
@@ -770,7 +771,7 @@ def password_reset(request):
else:
# bad user? tick the rate limiter counter
AUDIT_LOG.info("Bad password_reset user passed in.")
- limiter.tick_bad_request_counter(request)
+ limiter.tick_request_counter(request)
return JsonResponse({
'success': True,
diff --git a/common/djangoapps/util/bad_request_rate_limiter.py b/common/djangoapps/util/bad_request_rate_limiter.py
deleted file mode 100644
index 10bb67458d2f7769b9f772bf515eb9316547af26..0000000000000000000000000000000000000000
--- a/common/djangoapps/util/bad_request_rate_limiter.py
+++ /dev/null
@@ -1,26 +0,0 @@
-"""
-A utility class which wraps the RateLimitMixin 3rd party class to do bad request counting
-which can be used for rate limiting
-"""
-from __future__ import absolute_import
-
-from ratelimitbackend.backends import RateLimitMixin
-
-
-class BadRequestRateLimiter(RateLimitMixin):
- """
- Use the 3rd party RateLimitMixin to help do rate limiting on the Password Reset flows
- """
-
- def is_rate_limit_exceeded(self, request):
- """
- Returns if the client has been rated limited
- """
- counts = self.get_counters(request)
- return sum(counts.values()) >= self.requests
-
- def tick_bad_request_counter(self, request):
- """
- Ticks any counters used to compute when rate limt has been reached
- """
- self.cache_incr(self.get_cache_key(request))
diff --git a/common/djangoapps/util/request_rate_limiter.py b/common/djangoapps/util/request_rate_limiter.py
new file mode 100644
index 0000000000000000000000000000000000000000..0d3c0405cbf6331c9db87a2a84addf64a85e7f8c
--- /dev/null
+++ b/common/djangoapps/util/request_rate_limiter.py
@@ -0,0 +1,59 @@
+"""
+A utility class which wraps the RateLimitMixin 3rd party class to do bad request counting
+which can be used for rate limiting
+"""
+from __future__ import absolute_import
+
+from django.conf import settings
+from ratelimitbackend.backends import RateLimitMixin
+
+
+class RequestRateLimiter(RateLimitMixin):
+ """
+ Use the 3rd party RateLimitMixin to help do rate limiting.
+ """
+ def is_rate_limit_exceeded(self, request):
+ """
+ Returns if the client has been rated limited
+ """
+ counts = self.get_counters(request)
+ return sum(counts.values()) >= self.requests
+
+ def tick_request_counter(self, request):
+ """
+ Ticks any counters used to compute when rate limt has been reached
+ """
+ self.cache_incr(self.get_cache_key(request))
+
+
+class BadRequestRateLimiter(RequestRateLimiter):
+ """
+ Default rate limit is 30 requests for every 5 minutes.
+ """
+ pass
+
+
+class PasswordResetEmailRateLimiter(RequestRateLimiter):
+ """
+ Rate limiting requests to send password reset emails.
+ """
+ email_rate_limit = getattr(settings, 'PASSWORD_RESET_EMAIL_RATE_LIMIT', {})
+ requests = email_rate_limit.get('no_of_emails', 1)
+ cache_timeout_seconds = email_rate_limit.get('per_seconds', 60)
+ reset_email_cache_prefix = 'resetemail'
+
+ def key(self, request, dt):
+ """
+ Returns cache key.
+ """
+ return '%s-%s-%s' % (
+ self.reset_email_cache_prefix,
+ self.get_ip(request),
+ dt.strftime('%Y%m%d%H%M'),
+ )
+
+ def expire_after(self):
+ """
+ Returns timeout for cache keys.
+ """
+ return self.cache_timeout_seconds
diff --git a/lms/djangoapps/certificates/views/xqueue.py b/lms/djangoapps/certificates/views/xqueue.py
index 3ad0fe71c589d82c01259c14b8c950ed6293fd5a..9f58bf5ac4c9c0c2236cd11d761b09cdd574ed5f 100644
--- a/lms/djangoapps/certificates/views/xqueue.py
+++ b/lms/djangoapps/certificates/views/xqueue.py
@@ -21,7 +21,7 @@ from lms.djangoapps.certificates.models import (
GeneratedCertificate,
certificate_status_for_student
)
-from util.bad_request_rate_limiter import BadRequestRateLimiter
+from util.request_rate_limiter import BadRequestRateLimiter
from util.json_request import JsonResponse, JsonResponseBadRequest
from xmodule.modulestore.django import modulestore
@@ -171,12 +171,12 @@ def update_example_certificate(request):
if 'xqueue_body' not in request.POST:
log.info(u"Missing parameter 'xqueue_body' for update example certificate end-point")
- rate_limiter.tick_bad_request_counter(request)
+ rate_limiter.tick_request_counter(request)
return JsonResponseBadRequest("Parameter 'xqueue_body' is required.")
if 'xqueue_header' not in request.POST:
log.info(u"Missing parameter 'xqueue_header' for update example certificate end-point")
- rate_limiter.tick_bad_request_counter(request)
+ rate_limiter.tick_request_counter(request)
return JsonResponseBadRequest("Parameter 'xqueue_header' is required.")
try:
@@ -184,7 +184,7 @@ def update_example_certificate(request):
xqueue_header = json.loads(request.POST['xqueue_header'])
except (ValueError, TypeError):
log.info(u"Could not decode params to example certificate end-point as JSON.")
- rate_limiter.tick_bad_request_counter(request)
+ rate_limiter.tick_request_counter(request)
return JsonResponseBadRequest("Parameters must be JSON-serialized.")
# Attempt to retrieve the example certificate record
@@ -199,7 +199,7 @@ def update_example_certificate(request):
# from the XQueue. Return a 404 and increase the bad request counter
# to protect against a DDOS attack.
log.info(u"Could not find example certificate with uuid '%s' and access key '%s'", uuid, access_key)
- rate_limiter.tick_bad_request_counter(request)
+ rate_limiter.tick_request_counter(request)
raise Http404
if 'error' in xqueue_body:
@@ -217,7 +217,7 @@ def update_example_certificate(request):
# so we can display the example certificate.
download_url = xqueue_body.get('url')
if download_url is None:
- rate_limiter.tick_bad_request_counter(request)
+ rate_limiter.tick_request_counter(request)
log.warning(u"No download URL provided for example certificate with uuid '%s'.", uuid)
return JsonResponseBadRequest(
"Parameter 'download_url' is required for successfully generated certificates."
diff --git a/lms/djangoapps/shoppingcart/views.py b/lms/djangoapps/shoppingcart/views.py
index afeaff1d2cb392bf0fa24e9396967f0d24945241..3cd29b72eb9fc2a8f192ee7a490cdb1a980b0178 100644
--- a/lms/djangoapps/shoppingcart/views.py
+++ b/lms/djangoapps/shoppingcart/views.py
@@ -39,7 +39,7 @@ from shoppingcart.reports import (
UniversityRevenueShareReport
)
from student.models import AlreadyEnrolledError, CourseEnrollment, CourseFullError, EnrollmentClosedError
-from util.bad_request_rate_limiter import BadRequestRateLimiter
+from util.request_rate_limiter import BadRequestRateLimiter
from util.date_utils import get_default_time_display
from util.json_request import JsonResponse
@@ -319,7 +319,7 @@ def get_reg_code_validity(registration_code, request, limiter):
if not reg_code_is_valid:
# tick the rate limiter counter
AUDIT_LOG.info(u"Redemption of a invalid RegistrationCode %s", registration_code)
- limiter.tick_bad_request_counter(request)
+ limiter.tick_request_counter(request)
raise Http404()
return reg_code_is_valid, reg_code_already_redeemed, course_registration
diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py
index 1d19a45624d9622e811e574fc5648b0a407d80bb..d4c1dc733cbf79ab20684f7d5833c65fe5392691 100644
--- a/lms/envs/bok_choy.py
+++ b/lms/envs/bok_choy.py
@@ -60,6 +60,12 @@ CAPTURE_CONSOLE_LOG = True
PLATFORM_NAME = ugettext_lazy(u"édX")
PLATFORM_DESCRIPTION = ugettext_lazy(u"Open édX Platform")
+# We need to test different scenarios, following setting effectively disbale rate limiting
+PASSWORD_RESET_EMAIL_RATE_LIMIT = {
+ 'no_of_emails': 1,
+ 'per_seconds': 1
+}
+
############################ STATIC FILES #############################
# Enable debug so that static assets are served by Django
diff --git a/lms/envs/common.py b/lms/envs/common.py
index c267b0dd647701cdcec51264c0f0f1e4771ef82d..d368a731d3bbe69368003465035b0d2d42ae36a1 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -445,7 +445,10 @@ XQUEUE_WAITTIME_BETWEEN_REQUESTS = 5 # seconds
# Used with Email sending
RETRY_ACTIVATION_EMAIL_MAX_ATTEMPTS = 5
RETRY_ACTIVATION_EMAIL_TIMEOUT = 0.5
-
+PASSWORD_RESET_EMAIL_RATE_LIMIT = {
+ 'no_of_emails': 1,
+ 'per_seconds': 60
+}
# Deadline message configurations
COURSE_MESSAGE_ALERT_DURATION_IN_DAYS = 14
diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js
index f385685992f19fecd1062a4d269f16311dbcf4a0..99ee8c4564cd9c4b63c2c3e91d15c5e1a38306c9 100644
--- a/lms/static/js/student_account/views/LoginView.js
+++ b/lms/static/js/student_account/views/LoginView.js
@@ -145,7 +145,7 @@
successTitle = gettext('Check Your Email'),
successMessageHtml = HtmlUtils.interpolateHtml(
gettext('{paragraphStart}You entered {boldStart}{email}{boldEnd}. If this email address is associated with your {platform_name} account, we will send a message with password recovery instructions to this email address.{paragraphEnd}' + // eslint-disable-line max-len
- '{paragraphStart}If you do not receive a password reset message, verify that you entered the correct email address, or check your spam folder.{paragraphEnd}' + // eslint-disable-line max-len
+ '{paragraphStart}If you do not receive a password reset message after 1 minute, verify that you entered the correct email address, or check your spam folder.{paragraphEnd}' + // eslint-disable-line max-len
'{paragraphStart}If you need further assistance, {anchorStart}contact technical support{anchorEnd}.{paragraphEnd}'), { // eslint-disable-line max-len
boldStart: HtmlUtils.HTML('<b>'),
boldEnd: HtmlUtils.HTML('</b>'),
diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_views.py b/openedx/core/djangoapps/user_authn/views/tests/test_views.py
index 6340c47cc7a09dee03330d068e27bc7b9b67f905..d396c2f345cde7614f7a840a2e537d3b5e39055e 100644
--- a/openedx/core/djangoapps/user_authn/views/tests/test_views.py
+++ b/openedx/core/djangoapps/user_authn/views/tests/test_views.py
@@ -68,7 +68,6 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
OLD_EMAIL = u"walter@graymattertech.com"
NEW_EMAIL = u"walt@savewalterwhite.com"
- INVALID_ATTEMPTS = 100
INVALID_KEY = u"123abc"
URLCONF_MODULES = ['student_accounts.urls']
@@ -238,17 +237,23 @@ class UserAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin):
logger.check((LOGGER_NAME, 'INFO', 'Invalid password reset attempt'))
def test_password_change_rate_limited(self):
+ """
+ Tests that consective password reset requests are rate limited.
+ """
# Log out the user created during test setup, to prevent the view from
# selecting the logged-in user's email address over the email provided
# in the POST data
self.client.logout()
+ for status in [200, 403]:
+ response = self._change_password(email=self.NEW_EMAIL)
+ self.assertEqual(response.status_code, status)
- # Make many consecutive bad requests in an attempt to trigger the rate limiter
- for __ in range(self.INVALID_ATTEMPTS):
- self._change_password(email=self.NEW_EMAIL)
-
- response = self._change_password(email=self.NEW_EMAIL)
- self.assertEqual(response.status_code, 403)
+ with mock.patch(
+ 'util.request_rate_limiter.PasswordResetEmailRateLimiter.is_rate_limit_exceeded',
+ return_value=False
+ ):
+ response = self._change_password(email=self.NEW_EMAIL)
+ self.assertEqual(response.status_code, 200)
@ddt.data(
('post', 'password_change_request', []),