Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

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

Closed
wants to merge 1 commit into from

1 participant

@timgraham timgraham Fixed #21649 -- Added invalidation of old sessions when user password…
… changes.

Thanks Paul McMillan for guidance and reviews.
38cf73c
@timgraham
Owner

This needs to be revised as noted in the ticket; will reopen when complete.

@timgraham timgraham closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 25, 2013
  1. @timgraham

    Fixed #21649 -- Added invalidation of old sessions when user password…

    timgraham authored
    … changes.
    
    Thanks Paul McMillan for guidance and reviews.
This page is out of date. Refresh to see the latest.
View
25 django/contrib/auth/__init__.py
@@ -3,13 +3,15 @@
from django.conf import settings
from django.core.exceptions import ImproperlyConfigured, PermissionDenied
-from django.utils.module_loading import import_by_path
from django.middleware.csrf import rotate_token
+from django.utils.crypto import constant_time_compare
+from django.utils.module_loading import import_by_path
from .signals import user_logged_in, user_logged_out, user_login_failed
SESSION_KEY = '_auth_user_id'
BACKEND_SESSION_KEY = '_auth_user_backend'
+SESSION_HASH_KEY = '_auth_user_hash'
REDIRECT_FIELD_NAME = 'next'
@@ -74,11 +76,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[SESSION_HASH_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.
@@ -87,6 +94,7 @@ def login(request, user):
request.session.cycle_key()
request.session[SESSION_KEY] = user.pk
request.session[BACKEND_SESSION_KEY] = user.backend
+ request.session[SESSION_HASH_KEY] = session_auth_hash
if hasattr(request, 'user'):
request.user = user
rotate_token(request)
@@ -144,9 +152,18 @@ def get_user(request):
try:
user_id = request.session[SESSION_KEY]
backend_path = request.session[BACKEND_SESSION_KEY]
- assert backend_path in settings.AUTHENTICATION_BACKENDS
+ if backend_path not in settings.AUTHENTICATION_BACKENDS:
+ raise AssertionError
backend = load_backend(backend_path)
user = backend.get_user(user_id) or AnonymousUser()
+ if hasattr(user, 'get_session_auth_hash'):
+ session_hash = request.session.get(SESSION_HASH_KEY)
+ session_hash_verified = session_hash and constant_time_compare(
+ session_hash,
+ user.get_session_auth_hash()
+ )
+ if not session_hash_verified:
+ raise AssertionError
except (KeyError, AssertionError):
user = AnonymousUser()
return user
View
5 django/contrib/auth/admin.py
@@ -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 SESSION_HASH_KEY
from django.contrib.auth.forms import (UserCreationForm, UserChangeForm,
AdminPasswordChangeForm)
from django.contrib.auth.models import User, Group
@@ -132,6 +133,10 @@ 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)
+ # when an admin changes their own password, update their
+ # session to prevent a passsword change from logging him out.
+ if hasattr(form.user, 'get_session_auth_hash') and request.user == form.user:
+ request.session[SESSION_HASH_KEY] = form.user.get_session_auth_hash()
return HttpResponseRedirect('..')
else:
form = self.change_password_form(user)
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
22 django/contrib/auth/tests/test_auth_backends.py
@@ -445,6 +445,7 @@ class ChangedBackendSettingsTest(TestCase):
TEST_USERNAME = 'test_user'
TEST_PASSWORD = 'test_password'
TEST_EMAIL = 'test@example.com'
+ NEW_TEST_PASSWORD = 'new_test_password'
def setUp(self):
User.objects.create_user(self.TEST_USERNAME,
@@ -479,6 +480,27 @@ def test_changed_backend_settings(self):
self.assertIsNotNone(user)
self.assertTrue(user.is_anonymous())
+ def test_changed_password_invalidates_session(self):
+ """
+ Tests that changing a user's password invalidates the session.
+ """
+ self.assertTrue(self.client.login(
+ username=self.TEST_USERNAME,
+ password=self.TEST_PASSWORD)
+ )
+ request = HttpRequest()
+ request.session = self.client.session
+ user = get_user(request)
+ self.assertIsNotNone(user)
+ self.assertFalse(user.is_anonymous())
+
+ # After password change, user should be anonymous
+ user.set_password(self.NEW_TEST_PASSWORD)
+ user.save()
+ user = get_user(request)
+ self.assertIsNotNone(user)
+ self.assertTrue(user.is_anonymous())
+
class TypeErrorBackend(object):
"""
View
7 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, SESSION_HASH_KEY)
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,10 @@ 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
+ # besides the current one.
+ if hasattr(form.user, 'get_session_auth_hash') and request.user == form.user:
+ request.session[SESSION_HASH_KEY] = form.user.get_session_auth_hash()
return HttpResponseRedirect(post_change_redirect)
else:
form = password_change_form(user=request.user)
View
11 docs/releases/1.7.txt
@@ -670,6 +670,17 @@ Fixing the issues introduced some backward incompatible changes:
.. _pytz: https://pypi.python.org/pypi/pytz/
+Session signing and invalidation on password change
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+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. As a side effect of this change, users
+will be logged out when upgrading from an older version of Django. See
+:ref:`session-invalidation-on-password-change` for more details.
+
Miscellaneous
~~~~~~~~~~~~~
View
7 docs/topics/auth/customizing.txt
@@ -599,6 +599,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
10 docs/topics/auth/default.txt
@@ -111,6 +111,11 @@ 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.
+.. versionchanged:: 1.7
+
+Changing a user's password will log out all his sessions. See
+:ref:`session-invalidation-on-password-change` for details.
+
Authenticating Users
--------------------
@@ -1231,3 +1236,8 @@ User passwords are not displayed in the admin (nor stored in the database), but
the :doc:`password storage details </topics/auth/passwords>` are displayed.
Included in the display of this information is a link to
a password change form that allows admins to change user passwords.
+
+.. versionchanged:: 1.7
+
+Changing a user's password will log out all his sessions. See
+:ref:`session-invalidation-on-password-change` for details.
View
32 docs/topics/http/sessions.txt
@@ -691,3 +691,35 @@ not fall back to putting session IDs in URLs as a last resort, as PHP does.
This is an intentional design decision. Not only does that behavior make URLs
ugly, it makes your site vulnerable to session-ID theft via the "Referer"
header.
+
+.. _session-invalidation-on-password-change:
+
+Session invalidation on password change
+=======================================
+
+.. versionadded:: 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, the session will include the token returned by this function. In the
+:class:`~django.contrib.auth.models.AbstractBaseUser` case, this is an HMAC of
+the password field. Django verifies that the token sent along with each request
+matches the one that's computed server-side. This allows a user to log all
+other sessions out of his account by changing their password.
+
+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.
+
+The default password change views included with Django,
+:func:`django.contrib.auth.views.password_change` and and the
+``user_change_password`` view in the ``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 view and wish to implement similar logic,
+do something like this::
+
+ from django.contrib.auth import SESSION_HASH_KEY
+
+ if hasattr(form.user, 'get_session_auth_hash') and request.user == form.user:
+ request.session[SESSION_HASH_KEY] = form.user.get_session_auth_hash()
Something went wrong with that request. Please try again.