Merge pull request #554 from edx/diana/django-ratelimit

Limit the rate of logins

CHANGELOG.rst

@@ -5,6 +5,8 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Common: Added ratelimiting to our authentication backend.
+
 Common: Add additional logging to cover login attempts and logouts.
 
 Studio: Send e-mails to new Studio users (on edge only) when their course creator

cms/djangoapps/contentstore/tests/tests.py

 from django.test.client import Client
+from django.core.cache import cache
 from django.core.urlresolvers import reverse
 
 from .utils import parse_json, user, registration
@@ -79,6 +80,8 @@ class AuthTestCase(ContentStoreTestCase):
         self.pw = 'xyz'
         self.username = 'testuser'
         self.client = Client()
+        # clear the cache so ratelimiting won't affect these tests
+        cache.clear()
 
     def check_page_get(self, url, expected):
         resp = self.client.get(url)
@@ -119,6 +122,18 @@ class AuthTestCase(ContentStoreTestCase):
         # Now login should work
         self.login(self.email, self.pw)
 
+    def test_login_ratelimited(self):
+        # try logging in 30 times, the default limit in the number of failed
+        # login attempts in one 5 minute period before the rate gets limited
+        for i in xrange(30):
+            resp = self._login(self.email, 'wrong_password{0}'.format(i))
+            self.assertEqual(resp.status_code, 200)
+        resp = self._login(self.email, 'wrong_password')
+        self.assertEqual(resp.status_code, 200)
+        data = parse_json(resp)
+        self.assertFalse(data['success'])
+        self.assertIn('Too many failed login attempts.', data['value'])
+
     def test_login_link_on_activation_age(self):
         self.create_account(self.username, self.email, self.pw)
         # we want to test the rendering of the activation page when the user isn't logged in

cms/djangoapps/course_creators/admin.py

@@ -5,7 +5,7 @@ django admin page for the course creators table
 from course_creators.models import CourseCreator, update_creator_state
 from course_creators.views import update_course_creator_group
 
-from django.contrib import admin
+from ratelimitbackend import admin
 from django.conf import settings
 from django.dispatch import receiver
 from mitxmako.shortcuts import render_to_string

cms/djangoapps/course_creators/tests/test_admin.py

@@ -43,14 +43,14 @@ class CourseCreatorAdminTest(TestCase):
         """
         Tests that updates to state impact the creator group maintained in authz.py and that e-mails are sent.
         """
-        STUDIO_REQUEST_EMAIL = 'mark@marky.mark' 
-        
+        STUDIO_REQUEST_EMAIL = 'mark@marky.mark'
+
         def change_state(state, is_creator):
             """ Helper method for changing state """
             self.table_entry.state = state
             self.creator_admin.save_model(self.request, self.table_entry, None, True)
             self.assertEqual(is_creator, is_user_in_creator_group(self.user))
-            
+
             context = {'studio_request_email': STUDIO_REQUEST_EMAIL}
             if state == CourseCreator.GRANTED:
                 template = 'emails/course_creator_granted.txt'
@@ -69,7 +69,8 @@ class CourseCreatorAdminTest(TestCase):
             {
                 "ENABLE_CREATOR_GROUP": True,
                 "STUDIO_REQUEST_EMAIL": STUDIO_REQUEST_EMAIL
-            }):
+            }
+        ):
 
             # User is initially unrequested.
             self.assertFalse(is_user_in_creator_group(self.user))
@@ -106,3 +107,18 @@ class CourseCreatorAdminTest(TestCase):
 
         self.request.user = self.user
         self.assertFalse(self.creator_admin.has_change_permission(self.request))
+
+    def test_rate_limit_login(self):
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_CREATOR_GROUP': True}):
+            post_params = {'username': self.user.username, 'password': 'wrong_password'}
+            # try logging in 30 times, the default limit in the number of failed
+            # login attempts in one 5 minute period before the rate gets limited
+            for _ in xrange(30):
+                response = self.client.post('/admin/', post_params)
+                self.assertEquals(response.status_code, 200)
+
+            response = self.client.post('/admin/', post_params)
+            # Since we are using the default rate limit behavior, we are
+            # expecting this to return a 403 error to indicate that there have
+            # been too many attempts
+            self.assertEquals(response.status_code, 403)

cms/envs/common.py

@@ -108,6 +108,11 @@ TEMPLATE_CONTEXT_PROCESSORS = (
     'django.core.context_processors.csrf'
 )
 
+# use the ratelimit backend to prevent brute force attacks
+AUTHENTICATION_BACKENDS = (
+    'ratelimitbackend.backends.RateLimitModelBackend',
+)
+
 LMS_BASE = None
 
 #################### CAPA External Code Evaluation #############################
@@ -152,7 +157,10 @@ MIDDLEWARE_CLASSES = (
     # Detects user-requested locale from 'accept-language' header in http request
     'django.middleware.locale.LocaleMiddleware',
 
-    'django.middleware.transaction.TransactionMiddleware'
+    'django.middleware.transaction.TransactionMiddleware',
+
+    # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500
+    'ratelimitbackend.middleware.RateLimitMiddleware',
 )
 
 ############################ SIGNAL HANDLERS ################################
@@ -188,8 +196,8 @@ STATICFILES_DIRS = [
     COMMON_ROOT / "static",
     PROJECT_ROOT / "static",
 
-# This is how you would use the textbook images locally
-# ("book", ENV_ROOT / "book_images")
+    # This is how you would use the textbook images locally
+    # ("book", ENV_ROOT / "book_images")
 ]
 
 # Locale/Internationalization

cms/envs/test.py

@@ -15,6 +15,7 @@ sessions. Assumes structure:
 from .common import *
 import os
 from path import path
+from warnings import filterwarnings
 
 # Nose Test Runner
 INSTALLED_APPS += ('django_nose',)
@@ -124,6 +125,9 @@ CACHES = {
     }
 }
 
+# hide ratelimit warnings while running tests
+filterwarnings('ignore', message='No request passed to the backend, unable to rate-limit')
+
 ################################# CELERY ######################################
 
 CELERY_ALWAYS_EAGER = True

cms/urls.py

@@ -6,7 +6,7 @@ from django.conf.urls import patterns, include, url
 from . import one_time_startup
 
 # There is a course creators admin table.
-from django.contrib import admin
+from ratelimitbackend import admin
 admin.autodiscover()
 
 urlpatterns = ('',  # nopep8

common/djangoapps/external_auth/admin.py

@@ -3,7 +3,7 @@ django admin pages for courseware model
 '''
 
 from external_auth.models import *
-from django.contrib import admin
+from ratelimitbackend import admin
 
 
 class ExternalAuthMapAdmin(admin.ModelAdmin):

common/djangoapps/external_auth/tests/test_openid_provider.py

@@ -9,12 +9,15 @@ from urlparse import parse_qs
 
 from django.conf import settings
 from django.test import TestCase, LiveServerTestCase
+from django.core.cache import cache
 from django.test.utils import override_settings
-# from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from django.test.client import RequestFactory
 from unittest import skipUnless
 
+from student.tests.factories import UserFactory
+from external_auth.views import provider_login
+
 
 class MyFetcher(HTTPFetcher):
     """A fetcher that uses server-internal calls for performing HTTP
@@ -199,6 +202,49 @@ class OpenIdProviderTest(TestCase):
         """ Test for 403 error code when the url"""
         self.attempt_login(403, return_to="http://apps.cs50.edx.or")
 
+    def _send_bad_redirection_login(self):
+        """
+        Attempt to log in to the provider with setup parameters
+
+        Intentionally fail the login to force a redirect
+        """
+        user = UserFactory()
+
+        factory = RequestFactory()
+        post_params = {'email': user.email, 'password': 'password'}
+        fake_url = 'fake url'
+        request = factory.post(reverse('openid-provider-login'), post_params)
+        openid_setup = {
+            'request': factory.request(),
+            'url': fake_url
+        }
+        request.session = {
+            'openid_setup': openid_setup
+        }
+        response = provider_login(request)
+        return response
+
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_login_openid_handle_redirection(self):
+        """ Test to see that we can handle login redirection properly"""
+        response = self._send_bad_redirection_login()
+        self.assertEquals(response.status_code, 302)
+
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_login_openid_handle_redirection_ratelimited(self):
+        # try logging in 30 times, the default limit in the number of failed
+        # log in attempts before the rate gets limited
+        for _ in xrange(30):
+            self._send_bad_redirection_login()
+
+        response = self._send_bad_redirection_login()
+        # verify that we are not returning the default 403
+        self.assertEquals(response.status_code, 302)
+        # clear the ratelimit cache so that we don't fail other logins
+        cache.clear()
+
 
 class OpenIdProviderLiveServerTest(LiveServerTestCase):
     """

common/djangoapps/external_auth/views.py

@@ -39,6 +39,7 @@ from openid.consumer.consumer import SUCCESS
 from openid.server.server import Server, ProtocolError, UntrustedReturnURL
 from openid.server.trustroot import TrustRoot
 from openid.extensions import ax, sreg
+from ratelimitbackend.exceptions import RateLimitException
 
 import student.views as student_views
 # Required for Pearson
@@ -191,7 +192,7 @@ def _external_login_or_signup(request,
         user.backend = auth_backend
         AUDIT_LOG.info('Linked user "%s" logged in via Shibboleth', user.email)
     else:
-        user = authenticate(username=uname, password=eamap.internal_password)
+        user = authenticate(username=uname, password=eamap.internal_password, request=request)
     if user is None:
         # we want to log the failure, but don't want to log the password attempted:
         AUDIT_LOG.warning('External Auth Login failed for "%s"', uname)
@@ -718,7 +719,12 @@ def provider_login(request):
         # Failure is again redirected to the login dialog.
         username = user.username
         password = request.POST.get('password', None)
-        user = authenticate(username=username, password=password)
+        try:
+            user = authenticate(username=username, password=password, request=request)
+        except RateLimitException:
+            AUDIT_LOG.warning('OpenID - Too many failed login attempts.')
+            return HttpResponseRedirect(openid_request_url)
+
         if user is None:
             request.session['openid_error'] = True
             msg = "OpenID login failed - password for %s is invalid"

common/djangoapps/student/admin.py

@@ -4,7 +4,7 @@ django admin pages for courseware model
 
 from student.models import UserProfile, UserTestGroup, CourseEnrollmentAllowed
 from student.models import CourseEnrollment, Registration, PendingNameChange
-from django.contrib import admin
+from ratelimitbackend import admin
 
 admin.site.register(UserProfile)
 

common/djangoapps/student/tests/test_login.py

@@ -6,6 +6,7 @@ from mock import patch
 
 from django.test import TestCase
 from django.test.client import Client
+from django.core.cache import cache
 from django.core.urlresolvers import reverse, NoReverseMatch
 from student.tests.factories import UserFactory, RegistrationFactory, UserProfileFactory
 
@@ -29,6 +30,7 @@ class LoginTest(TestCase):
 
         # Create the test client
         self.client = Client()
+        cache.clear()
 
         # Store the login url
         try:
@@ -95,6 +97,27 @@ class LoginTest(TestCase):
         self.assertEqual(response.status_code, 302)
         self._assert_audit_log(mock_audit_log, 'info', [u'Logout', u'test'])
 
+    def test_login_ratelimited_success(self):
+        # Try (and fail) logging in with fewer attempts than the limit of 30
+        # and verify that you can still successfully log in afterwards.
+        for i in xrange(20):
+            password = u'test_password{0}'.format(i)
+            response, _audit_log = self._login_response('test@edx.org', password)
+            self._assert_response(response, success=False)
+        # now try logging in with a valid password
+        response, _audit_log = self._login_response('test@edx.org', 'test_password')
+        self._assert_response(response, success=True)
+
+    def test_login_ratelimited(self):
+        # try logging in 30 times, the default limit in the number of failed
+        # login attempts in one 5 minute period before the rate gets limited
+        for i in xrange(30):
+            password = u'test_password{0}'.format(i)
+            self._login_response('test@edx.org', password)
+        # check to see if this response indicates that this was ratelimited
+        response, _audit_log = self._login_response('test@edx.org', 'wrong_password')
+        self._assert_response(response, success=False, value='Too many failed login attempts')
+
     def _login_response(self, email, password, patched_audit_log='student.views.AUDIT_LOG'):
         ''' Post the login info '''
         post_params = {'email': email, 'password': password}

common/djangoapps/student/views.py

@@ -28,6 +28,8 @@ from django.utils.http import cookie_date
 from django.utils.http import base36_to_int
 from django.utils.translation import ugettext as _
 
+from ratelimitbackend.exceptions import RateLimitException
+
 from mitxmako.shortcuts import render_to_response, render_to_string
 from bs4 import BeautifulSoup
 
@@ -421,13 +423,23 @@ def login_user(request, error=""):
         user = User.objects.get(email=email)
     except User.DoesNotExist:
         AUDIT_LOG.warning(u"Login failed - Unknown user email: {0}".format(email))
-        return HttpResponse(json.dumps({'success': False,
-                                        'value': _('Email or password is incorrect.')}))  # TODO: User error message
+        user = None
 
-    username = user.username
-    user = authenticate(username=username, password=password)
+    # 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
+    username = user.username if user else ""
+    try:
+        user = authenticate(username=username, password=password, request=request)
+    # this occurs when there are too many attempts from the same IP address
+    except RateLimitException:
+        return HttpResponse(json.dumps({'success': False,
+                                        'value': _('Too many failed login attempts. Try again later.')}))
     if user is None:
-        AUDIT_LOG.warning(u"Login failed - password for {0} is invalid".format(email))
+        # if we didn't find this username earlier, the account for this email
+        # doesn't exist, and doesn't have a corresponding password
+        if username != "":
+            AUDIT_LOG.warning(u"Login failed - password for {0} is invalid".format(email))
         return HttpResponse(json.dumps({'success': False,
                                         'value': _('Email or password is incorrect.')}))
 
@@ -942,7 +954,7 @@ def auto_auth(request):
     # if they already are a user, log in
     try:
         user = User.objects.get(username=username)
-        user = authenticate(username=username, password=password)
+        user = authenticate(username=username, password=password, request=request)
         login(request, user)
 
     # else create and activate account info

common/djangoapps/track/admin.py

@@ -3,6 +3,6 @@ django admin pages for courseware model
 '''
 
 from track.models import TrackingLog
-from django.contrib import admin
+from ratelimitbackend import admin
 
 admin.site.register(TrackingLog)

lms/djangoapps/courseware/admin.py

@@ -3,7 +3,7 @@ django admin pages for courseware model
 '''
 
 from courseware.models import StudentModule, OfflineComputedGrade, OfflineComputedGradeLog
-from django.contrib import admin
+from ratelimitbackend import admin
 from django.contrib.auth.models import User
 
 admin.site.register(StudentModule)

lms/envs/common.py

@@ -239,6 +239,10 @@ TEMPLATE_CONTEXT_PROCESSORS = (
     'mitxmako.shortcuts.marketing_link_context_processor',
 )
 
+# use the ratelimit backend to prevent brute force attacks
+AUTHENTICATION_BACKENDS = (
+    'ratelimitbackend.backends.RateLimitModelBackend',
+)
 STUDENT_FILEUPLOAD_MAX_SIZE = 4 * 1000 * 1000  # 4 MB
 MAX_FILEUPLOADS_PER_INPUT = 20
 
@@ -437,10 +441,10 @@ OPEN_ENDED_GRADING_INTERFACE = {
     'url': 'http://sandbox-grader-001.m.edx.org/peer_grading',
     'username': 'incorrect_user',
     'password': 'incorrect_pass',
-    'staff_grading' : 'staff_grading',
-    'peer_grading' : 'peer_grading',
-    'grading_controller' : 'grading_controller'
-    }
+    'staff_grading': 'staff_grading',
+    'peer_grading': 'peer_grading',
+    'grading_controller': 'grading_controller'
+}
 
 # Used for testing, debugging peer grading
 MOCK_PEER_GRADING = False
@@ -495,6 +499,9 @@ MIDDLEWARE_CLASSES = (
 
     'django_comment_client.utils.ViewNameMiddleware',
     'codejail.django_integration.ConfigureCodeJailMiddleware',
+
+    # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500
+    'ratelimitbackend.middleware.RateLimitMiddleware',
 )
 
 ############################### Pipeline #######################################

lms/envs/dev_edx4edx.py

-"""
-This config file runs the simplest dev environment using sqlite, and db-based
-sessions. Assumes structure:
-
-/envroot/
-        /db   # This is where it'll write the database file
-        /mitx # The location of this repo
-        /log  # Where we're going to write log files
-"""
-
-# We intentionally define lots of variables that aren't used, and
-# want to import all variables from base settings files
-# pylint: disable=W0401, W0614
-
-import socket
-
-if 'eecs1' in socket.gethostname():
-    MITX_ROOT_URL = '/mitx2'
-
-from .common import *
-from .dev import *
-
-if 'eecs1' in socket.gethostname():
-    MITX_ROOT_URL = '/mitx2'
-
-#-----------------------------------------------------------------------------
-# edx4edx content server
-
-EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend'
-MITX_FEATURES['REROUTE_ACTIVATION_EMAIL'] = 'ichuang@mitx.mit.edu'
-EDX4EDX_ROOT = ENV_ROOT / "data/edx4edx"
-
-#EMAIL_BACKEND = 'django_ses.SESBackend'
-
-#-----------------------------------------------------------------------------
-# ichuang
-
-DEBUG = True
-ENABLE_MULTICOURSE = True     # set to False to disable multicourse display (see lib.util.views.mitxhome)
-
-MAKO_TEMPLATES['course'] = [DATA_DIR, EDX4EDX_ROOT]
-
-#MITX_FEATURES['USE_DJANGO_PIPELINE'] = False
-MITX_FEATURES['DISPLAY_HISTOGRAMS_TO_STAFF'] = False
-MITX_FEATURES['DISPLAY_EDIT_LINK'] = True
-
-COURSE_DEFAULT = "edx4edx"
-COURSE_NAME = "edx4edx"
-COURSE_NUMBER = "edX.01"
-COURSE_TITLE = "edx4edx: edX Author Course"
-SITE_NAME = "ichuang.mitx.mit.edu"
-
-COURSE_SETTINGS =  {'edx4edx': {'number' : 'edX.01',
-                                    'title': 'edx4edx: edX Author Course',
-                                    'xmlpath': '/edx4edx/',
-                                    'github_url': 'https://github.com/MITx/edx4edx',
-                                    'active': True,
-                                    'default_chapter': 'Introduction',
-                                    'default_section': 'edx4edx_Course',
-                                    },
-                    }
-
-#-----------------------------------------------------------------------------
-
-MIDDLEWARE_CLASSES = MIDDLEWARE_CLASSES + (
-    'ssl_auth.ssl_auth.NginxProxyHeaderMiddleware',		# ssl authentication behind nginx proxy
-    )
-
-AUTHENTICATION_BACKENDS = (
-    'ssl_auth.ssl_auth.SSLLoginBackend',
-    'django.contrib.auth.backends.ModelBackend',
-    )
-
-INSTALLED_APPS = INSTALLED_APPS + (
-    'ssl_auth',
-    )
-
-LOGIN_REDIRECT_URL = MITX_ROOT_URL + '/'
-LOGIN_URL = MITX_ROOT_URL + '/'

lms/envs/test.py

@@ -15,6 +15,7 @@ sessions. Assumes structure:
 from .common import *
 import os
 from path import path
+from warnings import filterwarnings
 
 # can't test start dates with this True, but on the other hand,
 # can test everything else :)
@@ -137,6 +138,9 @@ CACHES = {
 # Dummy secret key for dev
 SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
 
+# hide ratelimit warnings while running tests
+filterwarnings('ignore', message='No request passed to the backend, unable to rate-limit')
+
 ################################## OPENID #####################################
 MITX_FEATURES['AUTH_USE_OPENID'] = True
 MITX_FEATURES['AUTH_USE_OPENID_PROVIDER'] = True

lms/urls.py

 from django.conf import settings
 from django.conf.urls import patterns, include, url
-from django.contrib import admin
+from ratelimitbackend import admin
 from django.conf.urls.static import static
 
 # Not used, the work is done in the imported module.

requirements/edx/base.txt

@@ -52,6 +52,7 @@ sorl-thumbnail==11.12
 South==0.7.6
 sympy==0.7.1
 xmltodict==0.4.1
+django-ratelimit-backend==0.6
 
 # Used for debugging
 ipython==0.13.1