diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index ad243c1a5a..f7b32550aa 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -7,13 +7,14 @@ from django.urls import reverse from django.test import RequestFactory from django.test.utils import override_settings from nose.plugins.attrib import attr +from rest_framework import status from search.tests.test_course_discovery import DemoCourse from search.tests.tests import TEST_INDEX_NAME from search.tests.utils import SearcherMixin from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase -from ..views import CourseDetailView +from ..views import CourseDetailView, CourseListUserThrottle from .mixins import TEST_PASSWORD, CourseApiFactoryMixin @@ -100,6 +101,15 @@ class CourseListViewTestCase(CourseApiTestViewMixin, SharedModuleStoreTestCase): self.client.logout() self.verify_response() + def test_throttle_for_user(self): + """Make sure a user requests do not exceed the maximum number of requests""" + throttle = CourseListUserThrottle() + rate_limit, __ = throttle.parse_rate(throttle.rate) + self.setup_user(self.honor_user) + for attempt in xrange(rate_limit + 2): + expected_status = status.HTTP_429_TOO_MANY_REQUESTS if attempt >= rate_limit else status.HTTP_200_OK + self.verify_response(expected_status_code=expected_status, params={'username': self.honor_user.username}) + @attr(shard=9) class CourseListViewTestCaseMultipleCourses(CourseApiTestViewMixin, ModuleStoreTestCase): diff --git a/lms/djangoapps/course_api/views.py b/lms/djangoapps/course_api/views.py index 0caeeed376..9eb75bdfae 100644 --- a/lms/djangoapps/course_api/views.py +++ b/lms/djangoapps/course_api/views.py @@ -6,6 +6,7 @@ import search from django.conf import settings from django.core.exceptions import ValidationError from rest_framework.generics import ListAPIView, RetrieveAPIView +from rest_framework.throttling import UserRateThrottle from edx_rest_framework_extensions.paginators import NamespacedPageNumberPagination from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes @@ -123,6 +124,28 @@ class CourseDetailView(DeveloperErrorViewMixin, RetrieveAPIView): ) +class CourseListUserThrottle(UserRateThrottle): + """Limit the number of requests users can make to the course list API.""" + # The course list endpoint is likely being inefficient with how it's querying + # various parts of the code and can take courseware down, it needs to be rate + # limited until optimized. LEARNER-5527 + + THROTTLE_RATES = { + 'user': '2/minute', + 'staff': '10/minute', + } + + def allow_request(self, request, view): + # Use a special scope for staff to allow for a separate throttle rate + user = request.user + if user.is_authenticated and (user.is_staff or user.is_superuser): + self.scope = 'staff' + self.rate = self.get_rate() + self.num_requests, self.duration = self.parse_rate(self.rate) + + return super(CourseListUserThrottle, self).allow_request(request, view) + + @view_auth_classes(is_authenticated=False) class CourseListView(DeveloperErrorViewMixin, ListAPIView): """ @@ -196,6 +219,7 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView): pagination_class = NamespacedPageNumberPagination serializer_class = CourseSerializer + throttle_classes = CourseListUserThrottle, # Return all the results, 10K is the maximum allowed value for ElasticSearch. # We should use 0 after upgrading to 1.1+: