소스 검색

Fixed #21649 -- Added optional invalidation of sessions when user password changes.

Thanks Paul McMillan, Aymeric Augustin, and Erik Romijn for reviews.
Tim Graham 11 년 전
부모
커밋
fd23c06023

+ 1 - 0
django/conf/project_template/project_name/settings.py

@@ -43,6 +43,7 @@ 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',
 )

+ 22 - 2
django/contrib/auth/__init__.py

@@ -12,6 +12,7 @@ from .signals import user_logged_in, user_logged_out, user_login_failed
 
 SESSION_KEY = '_auth_user_id'
 BACKEND_SESSION_KEY = '_auth_user_backend'
+HASH_SESSION_KEY = '_auth_user_hash'
 REDIRECT_FIELD_NAME = 'next'
 
 
@@ -76,11 +77,16 @@ def login(request, user):
     have to reauthenticate on every request. Note that data set during
     the anonymous session is retained when the user logs in.
     """
+    session_auth_hash = ''
     if user is None:
         user = request.user
-    # TODO: It would be nice to support different login methods, like signed cookies.
+    if hasattr(user, 'get_session_auth_hash'):
+        session_auth_hash = user.get_session_auth_hash()
+
     if SESSION_KEY in request.session:
-        if request.session[SESSION_KEY] != user.pk:
+        if request.session[SESSION_KEY] != user.pk or (
+                session_auth_hash and
+                request.session[HASH_SESSION_KEY] != session_auth_hash):
             # To avoid reusing another user's session, create a new, empty
             # session if the existing session corresponds to a different
             # authenticated user.
@@ -89,6 +95,7 @@ def login(request, user):
         request.session.cycle_key()
     request.session[SESSION_KEY] = user.pk
     request.session[BACKEND_SESSION_KEY] = user.backend
+    request.session[HASH_SESSION_KEY] = session_auth_hash
     if hasattr(request, 'user'):
         request.user = user
     rotate_token(request)
@@ -158,4 +165,17 @@ def get_permission_codename(action, opts):
     return '%s_%s' % (action, opts.model_name)
 
 
+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.
+
+    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
+    appropriately to prevent a password change from logging out the session
+    from which the password was changed.
+    """
+    if hasattr(user, 'get_session_auth_hash') and request.user == user:
+        request.session[HASH_SESSION_KEY] = user.get_session_auth_hash()
+
 default_app_config = 'django.contrib.auth.apps.AuthConfig'

+ 2 - 0
django/contrib/auth/admin.py

@@ -3,6 +3,7 @@ from django.conf import settings
 from django.conf.urls import url
 from django.contrib import admin
 from django.contrib.admin.options import IS_POPUP_VAR
+from django.contrib.auth import update_session_auth_hash
 from django.contrib.auth.forms import (UserCreationForm, UserChangeForm,
     AdminPasswordChangeForm)
 from django.contrib.auth.models import User, Group
@@ -131,6 +132,7 @@ class UserAdmin(admin.ModelAdmin):
                 self.log_change(request, request.user, change_message)
                 msg = ugettext('Password changed successfully.')
                 messages.success(request, msg)
+                update_session_auth_hash(request, form.user)
                 return HttpResponseRedirect('..')
         else:
             form = self.change_password_form(user)

+ 19 - 0
django/contrib/auth/middleware.py

@@ -2,6 +2,7 @@ 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
 
 
@@ -22,6 +23,24 @@ class AuthenticationMiddleware(object):
         request.user = SimpleLazyObject(lambda: get_user(request))
 
 
+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).
+    """
+    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)
+
+
 class RemoteUserMiddleware(object):
     """
     Middleware for utilizing Web-server-provided authentication.

+ 8 - 1
django/contrib/auth/models.py

@@ -4,7 +4,7 @@ from django.core.mail import send_mail
 from django.core import validators
 from django.db import models
 from django.db.models.manager import EmptyManager
-from django.utils.crypto import get_random_string
+from django.utils.crypto import get_random_string, salted_hmac
 from django.utils import six
 from django.utils.translation import ugettext_lazy as _
 from django.utils import timezone
@@ -249,6 +249,13 @@ class AbstractBaseUser(models.Model):
     def get_short_name(self):
         raise NotImplementedError('subclasses of AbstractBaseUser must provide a get_short_name() method.')
 
+    def get_session_auth_hash(self):
+        """
+        Returns an HMAC of the password field.
+        """
+        key_salt = "django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash"
+        return salted_hmac(key_salt, self.password).hexdigest()
+
 
 # A few helper functions for common logic between User and AnonymousUser.
 def _user_get_all_permissions(user, obj):

+ 35 - 0
django/contrib/auth/tests/test_middleware.py

@@ -0,0 +1,35 @@
+from django.contrib.auth.middleware import SessionAuthenticationMiddleware
+from django.contrib.auth.models import User
+from django.http import HttpRequest
+from django.test import TestCase
+
+
+class TestSessionAuthenticationMiddleware(TestCase):
+    def setUp(self):
+        self.user_password = 'test_password'
+        self.user = User.objects.create_user('test_user',
+                                             '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.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())
+
+        # 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())

+ 56 - 2
django/contrib/auth/tests/test_views.py

@@ -49,9 +49,9 @@ class AuthViewsTestCase(TestCase):
     fixtures = ['authtestdata.json']
     urls = 'django.contrib.auth.tests.urls'
 
-    def login(self, password='password'):
+    def login(self, username='testclient', password='password'):
         response = self.client.post('/login/', {
-            'username': 'testclient',
+            'username': username,
             'password': password,
         })
         self.assertTrue(SESSION_KEY in self.client.session)
@@ -443,6 +443,25 @@ class ChangePasswordTest(AuthViewsTestCase):
         self.assertURLEqual(response.url, '/password_reset/')
 
 
+@override_settings(MIDDLEWARE_CLASSES=list(settings.MIDDLEWARE_CLASSES) + [
+    'django.contrib.auth.middleware.SessionAuthenticationMiddleware'
+])
+class SessionAuthenticationTests(AuthViewsTestCase):
+    def test_user_password_change_updates_session(self):
+        """
+        #21649 - Ensure contrib.auth.views.password_change updates the user's
+        session auth hash after a password change so the session isn't logged out.
+        """
+        self.login()
+        response = self.client.post('/password_change/', {
+            'old_password': 'password',
+            'new_password1': 'password1',
+            'new_password2': 'password1',
+        })
+        # if the hash isn't updated, retrieving the redirection page will fail.
+        self.assertRedirects(response, '/password_change/done/')
+
+
 @skipIfCustomUser
 class LoginTest(AuthViewsTestCase):
 
@@ -546,6 +565,36 @@ class LoginTest(AuthViewsTestCase):
         # Check the CSRF token switched
         self.assertNotEqual(token1, token2)
 
+    def test_session_key_flushed_on_login(self):
+        """
+        To avoid reusing another user's session, ensure a new, empty session is
+        created if the existing session corresponds to a different authenticated
+        user.
+        """
+        self.login()
+        original_session_key = self.client.session.session_key
+
+        self.login(username='staff')
+        self.assertNotEqual(original_session_key, self.client.session.session_key)
+
+    def test_session_key_flushed_on_login_after_password_change(self):
+        """
+        As above, but same user logging in after a password change.
+        """
+        self.login()
+        original_session_key = self.client.session.session_key
+
+        # If no password change, session key should not be flushed.
+        self.login()
+        self.assertEqual(original_session_key, self.client.session.session_key)
+
+        user = User.objects.get(username='testclient')
+        user.set_password('foobar')
+        user.save()
+
+        self.login(password='foobar')
+        self.assertNotEqual(original_session_key, self.client.session.session_key)
+
 
 @skipIfCustomUser
 class LoginURLSettings(AuthViewsTestCase):
@@ -731,6 +780,11 @@ class LogoutTest(AuthViewsTestCase):
 
 @skipIfCustomUser
 @override_settings(
+    # Redirect in test_user_change_password will fail if session auth hash
+    # isn't updated after password change (#21649)
+    MIDDLEWARE_CLASSES=list(settings.MIDDLEWARE_CLASSES) + [
+        'django.contrib.auth.middleware.SessionAuthenticationMiddleware'
+    ],
     PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',),
 )
 class ChangelistTests(AuthViewsTestCase):

+ 7 - 1
django/contrib/auth/views.py

@@ -11,7 +11,8 @@ from django.views.decorators.cache import never_cache
 from django.views.decorators.csrf import csrf_protect
 
 # Avoid shadowing the login() and logout() views below.
-from django.contrib.auth import REDIRECT_FIELD_NAME, login as auth_login, logout as auth_logout, get_user_model
+from django.contrib.auth import (REDIRECT_FIELD_NAME, login as auth_login,
+    logout as auth_logout, get_user_model, update_session_auth_hash)
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.forms import AuthenticationForm, PasswordResetForm, SetPasswordForm, PasswordChangeForm
 from django.contrib.auth.tokens import default_token_generator
@@ -264,6 +265,11 @@ def password_change(request,
         form = password_change_form(user=request.user, data=request.POST)
         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.
+            update_session_auth_hash(request, form.user)
             return HttpResponseRedirect(post_change_redirect)
     else:
         form = password_change_form(user=request.user)

+ 9 - 0
docs/ref/middleware.txt

@@ -204,6 +204,15 @@ Adds the ``user`` attribute, representing the currently-logged-in user, to
 every incoming ``HttpRequest`` object. See :ref:`Authentication in Web requests
 <auth-web-requests>`.
 
+.. class:: SessionAuthenticationMiddleware
+
+.. versionadded:: 1.7
+
+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
 --------------------------
 

+ 9 - 0
docs/releases/1.7.txt

@@ -331,6 +331,15 @@ Minor features
   ``html_email_template_name`` parameter used to send a multipart HTML email
   for password resets.
 
+* The :meth:`AbstractBaseUser.get_session_auth_hash()
+  <django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash>`
+  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.
+
 :mod:`django.contrib.formtools`
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

+ 7 - 0
docs/topics/auth/customizing.txt

@@ -591,6 +591,13 @@ The following methods are available on any subclass of
         :meth:`~django.contrib.auth.models.AbstractBaseUser.set_unusable_password()` has
         been called for this user.
 
+    .. method:: models.AbstractBaseUser.get_session_auth_hash()
+
+        .. versionadded:: 1.7
+
+        Returns an HMAC of the password field. Used for
+        :ref:`session-invalidation-on-password-change`.
+
 You should also define a custom manager for your ``User`` model. If your
 ``User`` model defines ``username``, ``email``, ``is_staff``, ``is_active``,
 ``is_superuser``, ``last_login``, and ``date_joined`` fields the same as

+ 71 - 0
docs/topics/auth/default.txt

@@ -111,6 +111,12 @@ Django also provides :ref:`views <built-in-auth-views>` and :ref:`forms
 <built-in-auth-forms>` that may be used to allow users to change their own
 passwords.
 
+.. versionadded:: 1.7
+
+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.
+
 Authenticating Users
 --------------------
 
@@ -575,6 +581,71 @@ To apply a permission to a :doc:`class-based generic view
 :ref:`decorating-class-based-views` for details. Another approach is to
 :ref:`write a mixin that wraps as_view() <mixins_that_wrap_as_view>`.
 
+.. _session-invalidation-on-password-change:
+
+Session invalidation on password change
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. versionadded:: 1.7
+
+.. warning::
+
+    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.
+
+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.
+
+The default password change views included with Django,
+:func:`django.contrib.auth.views.password_change` and the
+``user_change_password`` view in the :mod:`django.contrib.auth` admin, update
+the session with the new password hash so that a user changing their own
+password won't log themselves out. If you have a custom password change view
+and wish to have similar behavior, use this function:
+
+.. function:: update_session_auth_hash(request, 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
+    appropriately. Example usage::
+
+        from django.contrib.auth import update_session_auth_hash
+
+        def password_change(request):
+            if request.method == 'POST':
+                form = PasswordChangeForm(user=request.user, data=request.POST)
+                if form.is_valid():
+                    form.save()
+                    update_session_auth_hash(request, form.user)
+            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
+    :meth:`~django.contrib.auth.models.AbstractBaseUser.get_session_auth_hash()`
+    is based on :setting:`SECRET_KEY`, updating your site to use a new secret
+    will invalidate all existing sessions.
+
 .. _built-in-auth-views:
 
 Authentication Views