diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 15310ebb9e..cba3d2dbf8 100644 --- a/CHANGELOG.rst +++ b/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 diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 1f2a4185a3..0cbc82cbf1 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -1,4 +1,5 @@ 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 diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 65473d8bde..0f9a189bb7 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/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 diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 91a28d77ae..f28fec5a44 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/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) diff --git a/cms/envs/common.py b/cms/envs/common.py index 155c4d46d8..40084c20ae 100644 --- a/cms/envs/common.py +++ b/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 diff --git a/cms/envs/test.py b/cms/envs/test.py index efc7c5a7ef..4f3b0caee0 100644 --- a/cms/envs/test.py +++ b/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 diff --git a/cms/urls.py b/cms/urls.py index 5945394f55..0beb21362e 100644 --- a/cms/urls.py +++ b/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 diff --git a/common/djangoapps/external_auth/admin.py b/common/djangoapps/external_auth/admin.py index 1ee18dadc1..fad0917604 100644 --- a/common/djangoapps/external_auth/admin.py +++ b/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): diff --git a/common/djangoapps/external_auth/tests/test_openid_provider.py b/common/djangoapps/external_auth/tests/test_openid_provider.py index 1f7f201087..2024da469f 100644 --- a/common/djangoapps/external_auth/tests/test_openid_provider.py +++ b/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): """ diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 9065ea78d6..37dcd5313a 100644 --- a/common/djangoapps/external_auth/views.py +++ b/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" diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 4d6976d7d4..209f7cf6c0 100644 --- a/common/djangoapps/student/admin.py +++ b/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) diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 5a6cd043ae..e8def470c0 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/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} diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 40bd4a3034..223c124e46 100644 --- a/common/djangoapps/student/views.py +++ b/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 diff --git a/common/djangoapps/track/admin.py b/common/djangoapps/track/admin.py index d75f206846..e0835f5a8a 100644 --- a/common/djangoapps/track/admin.py +++ b/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) diff --git a/lms/djangoapps/courseware/admin.py b/lms/djangoapps/courseware/admin.py index 743d1fed52..147630dbc5 100644 --- a/lms/djangoapps/courseware/admin.py +++ b/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) diff --git a/lms/envs/common.py b/lms/envs/common.py index 95f2640bce..653040297c 100644 --- a/lms/envs/common.py +++ b/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 ####################################### diff --git a/lms/envs/dev_edx4edx.py b/lms/envs/dev_edx4edx.py deleted file mode 100644 index 13a66ed1e8..0000000000 --- a/lms/envs/dev_edx4edx.py +++ /dev/null @@ -1,79 +0,0 @@ -""" -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 + '/' diff --git a/lms/envs/test.py b/lms/envs/test.py index 1df9c0491f..704c78b177 100644 --- a/lms/envs/test.py +++ b/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 diff --git a/lms/urls.py b/lms/urls.py index bcb46f919c..380b5ea91d 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1,6 +1,6 @@ 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. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 32827d052a..ffa2977dbd 100644 --- a/requirements/edx/base.txt +++ b/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