Merge pull request #20794 from edx/private_to_public_55dd3e1

Mergeback PR from private to public.
This commit is contained in:
edx-pipeline-bot
2019-06-11 19:17:13 +05:00
committed by GitHub
9 changed files with 100 additions and 52 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@@ -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', []),