From 849037af36000d53b0b3b52f780ff475534e195b Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 2 Sep 2015 20:50:34 -0400 Subject: [PATCH] Refs #23957 -- Required session verification per deprecation timeline. --- django/conf/__init__.py | 12 ----- .../project_template/project_name/settings.py | 1 - django/contrib/auth/__init__.py | 6 +-- django/contrib/auth/middleware.py | 4 +- django/contrib/auth/views.py | 4 +- docs/ref/middleware.txt | 7 --- docs/ref/settings.txt | 4 +- docs/releases/1.10.txt | 4 +- docs/releases/1.7.2.txt | 2 +- docs/releases/1.7.txt | 7 ++- docs/releases/1.8.txt | 2 +- docs/topics/auth/default.txt | 40 +++++------------ docs/topics/auth/index.txt | 2 - docs/topics/http/middleware.txt | 1 - tests/admin_views/tests.py | 2 +- tests/auth_tests/test_middleware.py | 37 ++++----------- tests/auth_tests/test_views.py | 8 +--- tests/auth_tests/urls_custom_user_admin.py | 2 + tests/settings_tests/tests.py | 45 ------------------- 19 files changed, 38 insertions(+), 152 deletions(-) diff --git a/django/conf/__init__.py b/django/conf/__init__.py index c0e44e24b2..76cac3e721 100644 --- a/django/conf/__init__.py +++ b/django/conf/__init__.py @@ -9,11 +9,9 @@ a list of all possible variables. import importlib import os import time -import warnings from django.conf import global_settings from django.core.exceptions import ImproperlyConfigured -from django.utils.deprecation import RemovedInDjango110Warning from django.utils.functional import LazyObject, empty ENVIRONMENT_VARIABLE = "DJANGO_SETTINGS_MODULE" @@ -118,16 +116,6 @@ class Settings(BaseSettings): if not self.SECRET_KEY: raise ImproperlyConfigured("The SECRET_KEY setting must not be empty.") - if ('django.contrib.auth.middleware.AuthenticationMiddleware' in self.MIDDLEWARE_CLASSES and - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware' not in self.MIDDLEWARE_CLASSES): - warnings.warn( - "Session verification will become mandatory in Django 1.10. " - "Please add 'django.contrib.auth.middleware.SessionAuthenticationMiddleware' " - "to your MIDDLEWARE_CLASSES setting when you are ready to opt-in after " - "reading the upgrade considerations in the 1.8 release notes.", - RemovedInDjango110Warning - ) - if hasattr(time, 'tzset') and self.TIME_ZONE: # When we can, attempt to validate the timezone. If we can't find # this file, no check happens and it's harmless. diff --git a/django/conf/project_template/project_name/settings.py b/django/conf/project_template/project_name/settings.py index aa9bae99ac..3ef0ab7900 100644 --- a/django/conf/project_template/project_name/settings.py +++ b/django/conf/project_template/project_name/settings.py @@ -45,7 +45,6 @@ MIDDLEWARE_CLASSES = [ 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', ] diff --git a/django/contrib/auth/__init__.py b/django/contrib/auth/__init__.py index be0ab52761..7e663f545b 100644 --- a/django/contrib/auth/__init__.py +++ b/django/contrib/auth/__init__.py @@ -173,8 +173,7 @@ def get_user(request): backend = load_backend(backend_path) user = backend.get_user(user_id) # Verify the session - if ('django.contrib.auth.middleware.SessionAuthenticationMiddleware' - in settings.MIDDLEWARE_CLASSES and hasattr(user, 'get_session_auth_hash')): + if hasattr(user, 'get_session_auth_hash'): session_hash = request.session.get(HASH_SESSION_KEY) session_hash_verified = session_hash and constant_time_compare( session_hash, @@ -196,8 +195,7 @@ def get_permission_codename(action, opts): def update_session_auth_hash(request, user): """ - Updating a user's password logs out all sessions for the user if - django.contrib.auth.middleware.SessionAuthenticationMiddleware is enabled. + Updating a user's password logs out all sessions for the user. This function takes the current request and the updated user object from which the new session hash will be derived and updates the session hash diff --git a/django/contrib/auth/middleware.py b/django/contrib/auth/middleware.py index e13eb5374b..c829038f89 100644 --- a/django/contrib/auth/middleware.py +++ b/django/contrib/auth/middleware.py @@ -28,8 +28,8 @@ class SessionAuthenticationMiddleware(object): correspond to the user's current session authentication hash. However, it caused the "Vary: Cookie" header on all responses. - Now a backwards compatibility shim that enables session verification in - auth.get_user() if this middleware is in MIDDLEWARE_CLASSES. + It's now a shim to allow a single settings file to more easily support + multiple versions of Django. Will be RemovedInDjango20Warning. """ def process_request(self, request): pass diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index 4a748e0f4a..fc8b37823c 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -303,9 +303,7 @@ def password_change(request, if form.is_valid(): form.save() # Updating the password logs out all other sessions for the user - # except the current one if - # django.contrib.auth.middleware.SessionAuthenticationMiddleware - # is enabled. + # except the current one. update_session_auth_hash(request, form.user) return HttpResponseRedirect(post_change_redirect) else: diff --git a/docs/ref/middleware.txt b/docs/ref/middleware.txt index 2adcc0a600..b91aa03f22 100644 --- a/docs/ref/middleware.txt +++ b/docs/ref/middleware.txt @@ -382,13 +382,6 @@ Middleware for utilizing Web server provided authentication when enabled only on the login page. See :ref:`persistent-remote-user-middleware-howto` for usage details. -.. class:: SessionAuthenticationMiddleware - -Allows a user's sessions to be invalidated when their password changes. See -:ref:`session-invalidation-on-password-change` for details. This middleware must -appear after :class:`django.contrib.auth.middleware.AuthenticationMiddleware` -in :setting:`MIDDLEWARE_CLASSES`. - CSRF protection middleware -------------------------- diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index a5d8715e69..d4b175e198 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -1943,9 +1943,7 @@ The secret key is used for: * All :doc:`sessions ` if you are using any other session backend than ``django.contrib.sessions.backends.cache``, - or if you use - :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` - and are using the default + or are using the default :meth:`~django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash()`. * All :doc:`messages ` if you are using :class:`~django.contrib.messages.storage.cookie.CookieStorage` or diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index ddb4e4b98b..214a1c6b06 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -349,7 +349,9 @@ removed in Django 1.10 (please see the :ref:`deprecation timeline * Session verification is enabled regardless of whether or not ``'django.contrib.auth.middleware.SessionAuthenticationMiddleware'`` is in - ``MIDDLEWARE_CLASSES``. + ``MIDDLEWARE_CLASSES``. ``SessionAuthenticationMiddleware`` no longer has + any purpose and can be removed from ``MIDDLEWARE_CLASSES``. It's kept as + a stub until Django 2.0 as a courtesy for users who don't read this note. * Private attribute ``django.db.models.Field.related`` is removed. diff --git a/docs/releases/1.7.2.txt b/docs/releases/1.7.2.txt index 81547e2313..040c983fcb 100644 --- a/docs/releases/1.7.2.txt +++ b/docs/releases/1.7.2.txt @@ -106,7 +106,7 @@ Bugfixes (:ticket:`23950`). * Prevented the - :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` from + ``django.contrib.auth.middleware.SessionAuthenticationMiddleware`` from setting a ``"Vary: Cookie"`` header on all responses (:ticket:`23939`). * Fixed a crash when adding ``blank=True`` to ``TextField()`` on MySQL diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 993215ce34..e504136bf6 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -435,9 +435,8 @@ Minor features method was added and if your :setting:`AUTH_USER_MODEL` inherits from :class:`~django.contrib.auth.models.AbstractBaseUser`, changing a user's password now invalidates old sessions if the - :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` is - enabled. See :ref:`session-invalidation-on-password-change` for more details - including upgrade considerations when enabling this new middleware. + ``django.contrib.auth.middleware.SessionAuthenticationMiddleware`` is + enabled. See :ref:`session-invalidation-on-password-change` for more details. ``django.contrib.formtools`` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -1455,7 +1454,7 @@ Miscellaneous when the input is not valid UTF-8. * With the addition of the - :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` to + ``django.contrib.auth.middleware.SessionAuthenticationMiddleware`` to the default project template (pre-1.7.2 only), a database must be created before accessing a page using :djadmin:`runserver`. diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index da31963876..fe8874f3fc 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -1621,7 +1621,7 @@ attribute will change from ``True`` to ``False`` in Django 1.9. Using ``AuthenticationMiddleware`` without ``SessionAuthenticationMiddleware`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -:class:`django.contrib.auth.middleware.SessionAuthenticationMiddleware` was +``django.contrib.auth.middleware.SessionAuthenticationMiddleware`` was added in Django 1.7. In Django 1.7.2, its functionality was moved to ``auth.get_user()`` and, for backwards compatibility, enabled only if ``'django.contrib.auth.middleware.SessionAuthenticationMiddleware'`` appears in diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 5d8d447a59..9eeae69ed2 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -108,9 +108,8 @@ Django also provides :ref:`views ` and :ref:`forms ` that may be used to allow users to change their own passwords. -Changing a user's password will log out all their sessions if the -:class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` is -enabled. See :ref:`session-invalidation-on-password-change` for details. +Changing a user's password will log out all their sessions. See +:ref:`session-invalidation-on-password-change` for details. Authenticating Users -------------------- @@ -801,29 +800,23 @@ user to the login page or issue an HTTP 403 Forbidden response. Session invalidation on password change ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -.. warning:: +.. versionchanged:: 1.10 - This protection only applies if - :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` - is enabled in :setting:`MIDDLEWARE_CLASSES`. It's included if - ``settings.py`` was generated by :djadmin:`startproject` on Django ≥ 1.7. - - Session verification will become mandatory in Django 1.10 regardless of - whether or not ``SessionAuthenticationMiddleware`` is enabled. If you have - a pre-1.7 project or one generated using a template that doesn't include - ``SessionAuthenticationMiddleware``, consider enabling it before then after - reading the upgrade considerations below. + Session verification is enabled and mandatory in Django 1.10 (there's no + way to disable it) regardless of whether or not + ``SessionAuthenticationMiddleware`` is enabled. In older + versions, this protection only applies if + ``django.contrib.auth.middleware.SessionAuthenticationMiddleware`` + is enabled in :setting:`MIDDLEWARE_CLASSES`. If your :setting:`AUTH_USER_MODEL` inherits from :class:`~django.contrib.auth.models.AbstractBaseUser` or implements its own :meth:`~django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash()` method, authenticated sessions will include the hash returned by this function. In the :class:`~django.contrib.auth.models.AbstractBaseUser` case, this is an -HMAC of the password field. If the -:class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` is -enabled, Django verifies that the hash sent along with each request matches -the one that's computed server-side. This allows a user to log out all of their -sessions by changing their password. +HMAC of the password field. Django verifies that the hash sent along with each +request matches the one that's computed server-side. This allows a user to log +out all of their sessions by changing their password. The default password change views included with Django, :func:`django.contrib.auth.views.password_change` and the @@ -849,15 +842,6 @@ and wish to have similar behavior, use this function: else: ... -If you are upgrading an existing site and wish to enable this middleware without -requiring all your users to re-login afterward, you should first upgrade to -Django 1.7 and run it for a while so that as sessions are naturally recreated -as users login, they include the session hash as described above. Once you -start running your site with -:class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware`, any -users who have not logged in and had their session updated with the verification -hash will have their existing session invalidated and be required to login. - .. note:: Since diff --git a/docs/topics/auth/index.txt b/docs/topics/auth/index.txt index b67bd2fd75..b57c2e7436 100644 --- a/docs/topics/auth/index.txt +++ b/docs/topics/auth/index.txt @@ -66,8 +66,6 @@ and these items in your :setting:`MIDDLEWARE_CLASSES` setting: :doc:`sessions ` across requests. 2. :class:`~django.contrib.auth.middleware.AuthenticationMiddleware` associates users with requests using sessions. -3. :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` - logs users out of their other sessions after a password change. With these settings in place, running the command ``manage.py migrate`` creates the necessary database tables for auth related models and permissions for any diff --git a/docs/topics/http/middleware.txt b/docs/topics/http/middleware.txt index 3cd6baa520..c9812268a0 100644 --- a/docs/topics/http/middleware.txt +++ b/docs/topics/http/middleware.txt @@ -33,7 +33,6 @@ here's the default value created by :djadmin:`django-admin startproject 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', ] diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 87d6f86ae4..7b7e09a25f 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -773,7 +773,7 @@ class AdminViewBasicTest(AdminViewBasicTestCase): user = User.objects.get(username='super') user.set_unusable_password() user.save() - + self.client.force_login(user) response = self.client.get(reverse('admin:index')) self.assertNotContains(response, reverse('admin:password_change'), msg_prefix='The "change password" link should not be displayed if a user does not have a usable password.') diff --git a/tests/auth_tests/test_middleware.py b/tests/auth_tests/test_middleware.py index e67a89ef26..9ebb1e46ee 100644 --- a/tests/auth_tests/test_middleware.py +++ b/tests/auth_tests/test_middleware.py @@ -4,47 +4,26 @@ from django.http import HttpRequest from django.test import TestCase -class TestSessionAuthenticationMiddleware(TestCase): +class TestAuthenticationMiddleware(TestCase): def setUp(self): - self.user_password = 'test_password' - self.user = User.objects.create_user('test_user', - 'test@example.com', - self.user_password) - + self.user = User.objects.create_user('test_user', 'test@example.com', 'test_password') self.middleware = AuthenticationMiddleware() - self.assertTrue(self.client.login( - username=self.user.username, - password=self.user_password, - )) + self.client.force_login(self.user) self.request = HttpRequest() self.request.session = self.client.session - def test_changed_password_doesnt_invalidate_session(self): - """ - Changing a user's password shouldn't invalidate the session if session - verification isn't activated. - """ - session_key = self.request.session.session_key + def test_no_password_change_doesnt_invalidate_session(self): + self.request.session = self.client.session self.middleware.process_request(self.request) self.assertIsNotNone(self.request.user) self.assertFalse(self.request.user.is_anonymous()) - # After password change, user should remain logged in. + def test_changed_password_invalidates_session(self): + # After password change, user should be anonymous self.user.set_password('new_password') self.user.save() self.middleware.process_request(self.request) self.assertIsNotNone(self.request.user) - self.assertFalse(self.request.user.is_anonymous()) - self.assertEqual(session_key, self.request.session.session_key) - - def test_changed_password_invalidates_session_with_middleware(self): - with self.modify_settings( - MIDDLEWARE_CLASSES={'append': ['django.contrib.auth.middleware.SessionAuthenticationMiddleware']}): - # After password change, user should be anonymous - self.user.set_password('new_password') - self.user.save() - self.middleware.process_request(self.request) - self.assertIsNotNone(self.request.user) - self.assertTrue(self.request.user.is_anonymous()) + self.assertTrue(self.request.user.is_anonymous()) # session should be flushed self.assertIsNone(self.request.session.session_key) diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index b23d895152..19a47a2697 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -24,7 +24,7 @@ from django.core.urlresolvers import NoReverseMatch, reverse, reverse_lazy from django.db import connection from django.http import HttpRequest, QueryDict from django.middleware.csrf import CsrfViewMiddleware, get_token -from django.test import TestCase, modify_settings, override_settings +from django.test import TestCase, override_settings from django.test.utils import patch_logger from django.utils.encoding import force_text from django.utils.http import urlquote @@ -506,9 +506,6 @@ class ChangePasswordTest(AuthViewsTestCase): self.assertURLEqual(response.url, '/password_reset/') -@modify_settings(MIDDLEWARE_CLASSES={ - 'append': 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', -}) class SessionAuthenticationTests(AuthViewsTestCase): def test_user_password_change_updates_session(self): """ @@ -876,9 +873,6 @@ class LogoutTest(AuthViewsTestCase): # Redirect in test_user_change_password will fail if session auth hash # isn't updated after password change (#21649) -@modify_settings(MIDDLEWARE_CLASSES={ - 'append': 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', -}) @override_settings( PASSWORD_HASHERS=['django.contrib.auth.hashers.SHA1PasswordHasher'], ROOT_URLCONF='auth_tests.urls_admin', diff --git a/tests/auth_tests/urls_custom_user_admin.py b/tests/auth_tests/urls_custom_user_admin.py index dc47be68c7..de33984fa5 100644 --- a/tests/auth_tests/urls_custom_user_admin.py +++ b/tests/auth_tests/urls_custom_user_admin.py @@ -10,8 +10,10 @@ class CustomUserAdmin(UserAdmin): def log_change(self, request, object, message): # LogEntry.user column doesn't get altered to expect a UUID, so set an # integer manually to avoid causing an error. + original_pk = request.user.pk request.user.pk = 1 super(CustomUserAdmin, self).log_change(request, object, message) + request.user.pk = original_pk site.register(get_user_model(), CustomUserAdmin) diff --git a/tests/settings_tests/tests.py b/tests/settings_tests/tests.py index 90b2b8e580..bf4f071c2f 100644 --- a/tests/settings_tests/tests.py +++ b/tests/settings_tests/tests.py @@ -12,7 +12,6 @@ from django.test import ( override_settings, signals, ) from django.utils import six -from django.utils.encoding import force_text @modify_settings(ITEMS={ @@ -489,47 +488,3 @@ class TestListSettings(unittest.TestCase): finally: del sys.modules['fake_settings_module'] delattr(settings_module, setting) - - -class TestSessionVerification(unittest.TestCase): - - def setUp(self): - self.settings_module = ModuleType('fake_settings_module') - self.settings_module.SECRET_KEY = 'foo' - - def tearDown(self): - if 'fake_settings_module' in sys.modules: - del sys.modules['fake_settings_module'] - - def test_session_verification_deprecation_no_verification(self): - self.settings_module.MIDDLEWARE_CLASSES = ['django.contrib.auth.middleware.AuthenticationMiddleware'] - sys.modules['fake_settings_module'] = self.settings_module - with warnings.catch_warnings(record=True) as warn: - warnings.filterwarnings('always') - Settings('fake_settings_module') - self.assertEqual( - force_text(warn[0].message), - "Session verification will become mandatory in Django 1.10. " - "Please add 'django.contrib.auth.middleware.SessionAuthenticationMiddleware' " - "to your MIDDLEWARE_CLASSES setting when you are ready to opt-in after " - "reading the upgrade considerations in the 1.8 release notes.", - ) - - def test_session_verification_deprecation_both(self): - self.settings_module.MIDDLEWARE_CLASSES = [ - 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.auth.middleware.SessionAuthenticationMiddleware', - ] - sys.modules['fake_settings_module'] = self.settings_module - with warnings.catch_warnings(record=True) as warn: - warnings.filterwarnings('always') - Settings('fake_settings_module') - self.assertEqual(len(warn), 0) - - def test_session_verification_deprecation_neither(self): - self.settings_module.MIDDLEWARE_CLASSES = [] - sys.modules['fake_settings_module'] = self.settings_module - with warnings.catch_warnings(record=True) as warn: - warnings.filterwarnings('always') - Settings('fake_settings_module') - self.assertEqual(len(warn), 0)