diff --git a/cms/envs/common.py b/cms/envs/common.py index 5e17b93aa7..651f64a58f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -326,6 +326,10 @@ MIDDLEWARE_CLASSES = ( # Instead of AuthenticationMiddleware, we use a cache-backed version 'cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', + # Enable SessionAuthenticationMiddleware in order to invalidate + # user sessions after a password change. + 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', + 'student.middleware.UserStandingMiddleware', 'contentserver.middleware.StaticContentServer', 'crum.CurrentRequestUserMiddleware', diff --git a/common/djangoapps/cache_toolbox/middleware.py b/common/djangoapps/cache_toolbox/middleware.py index eba90210de..7c7839b9a5 100644 --- a/common/djangoapps/cache_toolbox/middleware.py +++ b/common/djangoapps/cache_toolbox/middleware.py @@ -78,8 +78,11 @@ choice for most environments but you may be happy with the trade-offs of the """ -from django.contrib.auth.models import User +from django.conf import settings +from django.contrib.auth import HASH_SESSION_KEY +from django.contrib.auth.models import User, AnonymousUser from django.contrib.auth.middleware import AuthenticationMiddleware +from django.utils.crypto import constant_time_compare from logging import getLogger from openedx.core.djangoapps.safe_sessions.middleware import SafeSessionMiddleware @@ -106,6 +109,29 @@ class CacheBackedAuthenticationMiddleware(AuthenticationMiddleware): ) # Raise an exception to fall through to the except clause below. raise Exception + self._verify_session_auth(request) except: # Fallback to constructing the User from the database. super(CacheBackedAuthenticationMiddleware, self).process_request(request) + + def _verify_session_auth(self, request): + """ + Ensure that the user's session hash hasn't changed. We check that + SessionAuthenticationMiddleware is enabled in order to match Django's + behavior. + """ + session_auth_class = 'django.contrib.auth.middleware.SessionAuthenticationMiddleware' + session_auth_enabled = session_auth_class in settings.MIDDLEWARE_CLASSES + # Auto-auth causes issues in Bok Choy tests because it resets + # the requesting user. Since session verification is a + # security feature, we can turn it off when auto-auth is + # enabled since auto-auth is highly insecure and only for + # tests. + auto_auth_enabled = settings.FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING', False) + if not auto_auth_enabled and session_auth_enabled and hasattr(request.user, 'get_session_auth_hash'): + session_hash = request.session.get(HASH_SESSION_KEY) + if not (session_hash and constant_time_compare(session_hash, request.user.get_session_auth_hash())): + # The session hash has changed due to a password + # change. Log the user out. + request.session.flush() + request.user = AnonymousUser() diff --git a/common/djangoapps/cache_toolbox/tests/test_middleware.py b/common/djangoapps/cache_toolbox/tests/test_middleware.py new file mode 100644 index 0000000000..2e6744e0ad --- /dev/null +++ b/common/djangoapps/cache_toolbox/tests/test_middleware.py @@ -0,0 +1,46 @@ +"""Tests for cached authentication middleware.""" +import unittest + +from django.conf import settings +from django.contrib.auth.models import User +from django.core.urlresolvers import reverse +from django.test import TestCase +from mock import patch + +from student.tests.factories import UserFactory + + +class CachedAuthMiddlewareTestCase(TestCase): + """Tests for CacheBackedAuthenticationMiddleware class.""" + + def setUp(self): + super(CachedAuthMiddlewareTestCase, self).setUp() + password = 'test-password' + self.user = UserFactory(password=password) + self.client.login(username=self.user.username, password=password) + + def _test_change_session_hash(self, test_url, redirect_url): + """ + Verify that if a user's session auth hash and the request's hash + differ, the user is logged out. The URL to test and the + expected redirect are passed in, since we want to test this + behavior in both LMS and CMS, but the two systems have + different URLconfs. + """ + response = self.client.get(test_url) + self.assertEqual(response.status_code, 200) + with patch.object(User, 'get_session_auth_hash', return_value='abc123'): + response = self.client.get(test_url) + self.assertRedirects(response, redirect_url) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + def test_session_change_lms(self): + """Test session verification with LMS-specific URLs.""" + dashboard_url = reverse('dashboard') + self._test_change_session_hash(dashboard_url, reverse('signin_user') + '?next=' + dashboard_url) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'cms.urls', 'Test only valid in cms') + def test_session_change_cms(self): + """Test session verification with CMS-specific URLs.""" + home_url = reverse('home') + self._test_change_session_hash(home_url, reverse('login') + '?next=' + home_url) diff --git a/lms/envs/common.py b/lms/envs/common.py index b0f2470943..b1f7002099 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1106,6 +1106,9 @@ MIDDLEWARE_CLASSES = ( # Instead of AuthenticationMiddleware, we use a cached backed version #'django.contrib.auth.middleware.AuthenticationMiddleware', 'cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', + # Enable SessionAuthenticationMiddleware in order to invalidate + # user sessions after a password change. + 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'student.middleware.UserStandingMiddleware', 'contentserver.middleware.StaticContentServer',