Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[1.7.x] Fixed #21649 -- Added optional invalidation of sessions when …
…user password changes.

Thanks Paul McMillan, Aymeric Augustin, and Erik Romijn for reviews.

Backport of fd23c06 from master
  • Loading branch information
timgraham committed Apr 5, 2014
1 parent 4ad4a23 commit 5891fd3
Show file tree
Hide file tree
Showing 12 changed files with 246 additions and 6 deletions.
1 change: 1 addition & 0 deletions django/conf/project_template/project_name/settings.py
Expand Up @@ -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',
)
Expand Down
24 changes: 22 additions & 2 deletions django/contrib/auth/__init__.py
Expand Up @@ -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'


Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -159,4 +166,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 changes: 2 additions & 0 deletions django/contrib/auth/admin.py
Expand Up @@ -2,6 +2,7 @@
from django.conf import settings
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
Expand Down Expand Up @@ -132,6 +133,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)
Expand Down
19 changes: 19 additions & 0 deletions django/contrib/auth/middleware.py
Expand Up @@ -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


Expand All @@ -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.
Expand Down
9 changes: 8 additions & 1 deletion django/contrib/auth/models.py
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
35 changes: 35 additions & 0 deletions 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())
58 changes: 56 additions & 2 deletions django/contrib/auth/tests/test_views.py
Expand Up @@ -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)
Expand Down Expand Up @@ -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):

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
8 changes: 7 additions & 1 deletion django/contrib/auth/views.py
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions docs/ref/middleware.txt
Expand Up @@ -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
--------------------------

Expand Down
9 changes: 9 additions & 0 deletions docs/releases/1.7.txt
Expand Up @@ -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`
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
7 changes: 7 additions & 0 deletions docs/topics/auth/customizing.txt
Expand Up @@ -604,6 +604,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
Expand Down

0 comments on commit 5891fd3

Please sign in to comment.