[1.7.x] Fixed #23939 -- Moved session verification out of SessionAuthenticationMiddleware.

Thanks andrewbadr for the report and Carl Meyer for the review.

Backport of b06dfad88f from master

django/contrib/auth/__init__.py

@@ -4,9 +4,10 @@ import re
 from django.apps import apps as django_apps
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured, PermissionDenied
+from django.middleware.csrf import rotate_token
+from django.utils.crypto import constant_time_compare
 from django.utils.module_loading import import_string
 from django.utils.translation import LANGUAGE_SESSION_KEY
-from django.middleware.csrf import rotate_token
 
 from .signals import user_logged_in, user_logged_out, user_login_failed
 
@@ -156,6 +157,18 @@ def get_user(request):
         if backend_path in settings.AUTHENTICATION_BACKENDS:
             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')):
+                session_hash = request.session.get(HASH_SESSION_KEY)
+                session_hash_verified = session_hash and constant_time_compare(
+                    session_hash,
+                    user.get_session_auth_hash()
+                )
+                if not session_hash_verified:
+                    request.session.flush()
+                    user = None
+
     return user or AnonymousUser()
 
 

django/contrib/auth/middleware.py

@@ -2,7 +2,6 @@ from django.contrib import auth
 from django.contrib.auth import load_backend
 from django.contrib.auth.backends import RemoteUserBackend
 from django.core.exceptions import ImproperlyConfigured
-from django.utils.crypto import constant_time_compare
 from django.utils.functional import SimpleLazyObject
 
 
@@ -25,20 +24,15 @@ class AuthenticationMiddleware(object):
 
 class SessionAuthenticationMiddleware(object):
     """
-    Middleware for invalidating a user's sessions that don't correspond to the
-    user's current session authentication hash (generated based on the user's
-    password for AbstractUser).
+    Formerly, a middleware for invalidating a user's sessions that don't
+    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.
     """
     def process_request(self, request):
-        user = request.user
-        if user and hasattr(user, 'get_session_auth_hash'):
-            session_hash = request.session.get(auth.HASH_SESSION_KEY)
-            session_hash_verified = session_hash and constant_time_compare(
-                session_hash,
-                user.get_session_auth_hash()
-            )
-            if not session_hash_verified:
-                auth.logout(request)
+        pass
 
 
 class RemoteUserMiddleware(object):

django/contrib/auth/tests/test_middleware.py

@@ -1,4 +1,4 @@
-from django.contrib.auth.middleware import SessionAuthenticationMiddleware
+from django.contrib.auth.middleware import AuthenticationMiddleware
 from django.contrib.auth.models import User
 from django.http import HttpRequest
 from django.test import TestCase
@@ -11,25 +11,39 @@ class TestSessionAuthenticationMiddleware(TestCase):
                                              'test@example.com',
                                              self.user_password)
 
-    def test_changed_password_invalidates_session(self):
-        """
-        Tests that changing a user's password invalidates the session.
-        """
-        verification_middleware = SessionAuthenticationMiddleware()
+        self.middleware = AuthenticationMiddleware()
         self.assertTrue(self.client.login(
             username=self.user.username,
             password=self.user_password,
         ))
-        request = HttpRequest()
-        request.session = self.client.session
-        request.user = self.user
-        verification_middleware.process_request(request)
-        self.assertIsNotNone(request.user)
-        self.assertFalse(request.user.is_anonymous())
+        self.request = HttpRequest()
+        self.request.session = self.client.session
 
-        # After password change, user should be anonymous
-        request.user.set_password('new_password')
-        request.user.save()
-        verification_middleware.process_request(request)
-        self.assertIsNotNone(request.user)
-        self.assertTrue(request.user.is_anonymous())
+    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
+        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.
+        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())
+        # session should be flushed
+        self.assertIsNone(self.request.session.session_key)

docs/releases/1.7.2.txt

@@ -104,3 +104,7 @@ Bugfixes
 
 * Fixed serialization of ``type`` when adding a ``deconstruct()`` method
   (:ticket:`23950`).
+
+* Prevented the
+  :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` from
+  setting a ``"Vary: Cookie"`` header on all responses.

docs/releases/1.7.txt

@@ -1461,8 +1461,8 @@ Miscellaneous
 
 * With the addition of the
   :class:`~django.contrib.auth.middleware.SessionAuthenticationMiddleware` to
-  the default project template, a database must be created before accessing
-  a page using :djadmin:`runserver`.
+  the default project template (pre-1.7.2 only), a database must be created
+  before accessing a page using :djadmin:`runserver`.
 
 .. _deprecated-features-1.7: