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

Thanks Paul McMillan, Aymeric Augustin, and Erik Romijn for reviews.
This commit is contained in:
Tim Graham 2014-03-31 20:16:09 -04:00
parent 9494f29d4f
commit fd23c06023
12 changed files with 246 additions and 6 deletions

View File

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

View File

@ -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'

View File

@ -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)

View File

@ -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.

View File

@ -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):

View File

@ -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 File

@ -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):

View File

@ -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)

View File

@ -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 File

@ -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 File

@ -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 File

@ -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