Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #21649 -- Added optional invalidation of sessions when user pas…

…sword changes.

Thanks Paul McMillan, Aymeric Augustin, and Erik Romijn for reviews.
  • Loading branch information...
commit fd23c06023a0585ee743c0752dc94da66694cf63 1 parent 9494f29
@timgraham timgraham authored
View
1  django/conf/project_template/project_name/settings.py
@@ -43,6 +43,7 @@
'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',
)
View
24 django/contrib/auth/__init__.py
@@ -12,6 +12,7 @@
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'
View
2  django/contrib/auth/admin.py
@@ -3,6 +3,7 @@
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 @@ def user_change_password(self, request, id, form_url=''):
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)
View
19 django/contrib/auth/middleware.py
@@ -2,6 +2,7 @@
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 @@ def process_request(self, request):
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.
View
9 django/contrib/auth/models.py
@@ -4,7 +4,7 @@
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 @@ def get_full_name(self):
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):
View
35 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())
View
58 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 @@ def test_password_change_redirect_custom_named(self):
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 @@ def test_login_csrf_rotate(self, password='password'):
# 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 @@ def test_logout_preserve_language(self):
@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):
View
8 django/contrib/auth/views.py
@@ -11,7 +11,8 @@
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)
View
9 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
--------------------------
View
9 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`
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
View
7 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
View
71 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
Please sign in to comment.
Something went wrong with that request. Please try again.