summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPreston Holmes <preston@ptone.com>2014-07-27 21:54:29 -0700
committerTim Graham <timograham@gmail.com>2014-08-20 14:42:48 -0400
commit1a45d059c70385fcd6f4a3955f3b4e4cc96d0150 (patch)
tree7cf698c1c9a4782c0f7ed4a63d99939113677d66
parent3123f8452cf49071be9110e277eea60ba0032216 (diff)
[1.7.x] Fixed #23066 -- Modified RemoteUserMiddleware to logout on REMOTE_USER change.
This is a security fix. Disclosure following shortly.
-rw-r--r--django/contrib/auth/middleware.py28
-rw-r--r--django/contrib/auth/tests/test_remote_user.py18
-rw-r--r--docs/releases/1.4.14.txt9
-rw-r--r--docs/releases/1.5.9.txt9
-rw-r--r--docs/releases/1.6.6.txt9
5 files changed, 65 insertions, 8 deletions
diff --git a/django/contrib/auth/middleware.py b/django/contrib/auth/middleware.py
index b2f392262a..b34da60299 100644
--- a/django/contrib/auth/middleware.py
+++ b/django/contrib/auth/middleware.py
@@ -76,14 +76,7 @@ class RemoteUserMiddleware(object):
# authenticated remote-user, or return (leaving request.user set to
# AnonymousUser by the AuthenticationMiddleware).
if request.user.is_authenticated():
- try:
- stored_backend = load_backend(request.session.get(
- auth.BACKEND_SESSION_KEY, ''))
- if isinstance(stored_backend, RemoteUserBackend):
- auth.logout(request)
- except ImportError:
- # backend failed to load
- auth.logout(request)
+ self._remove_invalid_user(request)
return
# If the user is already authenticated and that user is the user we are
# getting passed in the headers, then the correct user is already
@@ -91,6 +84,11 @@ class RemoteUserMiddleware(object):
if request.user.is_authenticated():
if request.user.get_username() == self.clean_username(username, request):
return
+ else:
+ # An authenticated user is associated with the request, but
+ # it does not match the authorized user in the header.
+ self._remove_invalid_user(request)
+
# We are seeing this user for the first time in this session, attempt
# to authenticate the user.
user = auth.authenticate(remote_user=username)
@@ -112,3 +110,17 @@ class RemoteUserMiddleware(object):
except AttributeError: # Backend has no clean_username method.
pass
return username
+
+ def _remove_invalid_user(self, request):
+ """
+ Removes the current authenticated user in the request which is invalid
+ but only if the user is authenticated via the RemoteUserBackend.
+ """
+ try:
+ stored_backend = load_backend(request.session.get(auth.BACKEND_SESSION_KEY, ''))
+ except ImportError:
+ # backend failed to load
+ auth.logout(request)
+ else:
+ if isinstance(stored_backend, RemoteUserBackend):
+ auth.logout(request)
diff --git a/django/contrib/auth/tests/test_remote_user.py b/django/contrib/auth/tests/test_remote_user.py
index 5b743ac8c1..45e96133ce 100644
--- a/django/contrib/auth/tests/test_remote_user.py
+++ b/django/contrib/auth/tests/test_remote_user.py
@@ -125,6 +125,24 @@ class RemoteUserTest(TestCase):
response = self.client.get('/remote_user/')
self.assertEqual(response.context['user'].username, 'modeluser')
+ def test_user_switch_forces_new_login(self):
+ """
+ Tests that if the username in the header changes between requests
+ that the original user is logged out
+ """
+ User.objects.create(username='knownuser')
+ # Known user authenticates
+ response = self.client.get('/remote_user/',
+ **{self.header: self.known_user})
+ self.assertEqual(response.context['user'].username, 'knownuser')
+ # During the session, the REMOTE_USER changes to a different user.
+ response = self.client.get('/remote_user/',
+ **{self.header: "newnewuser"})
+ # Ensure that the current user is not the prior remote_user
+ # In backends that create a new user, username is "newnewuser"
+ # In backends that do not create new users, it is '' (anonymous user)
+ self.assertNotEqual(response.context['user'].username, 'knownuser')
+
def tearDown(self):
"""Restores settings to avoid breaking other tests."""
settings.MIDDLEWARE_CLASSES = self.curr_middleware
diff --git a/docs/releases/1.4.14.txt b/docs/releases/1.4.14.txt
index 6c140ee6dc..811c3f67ea 100644
--- a/docs/releases/1.4.14.txt
+++ b/docs/releases/1.4.14.txt
@@ -38,3 +38,12 @@ if a file with the uploaded name already exists.
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
``"_2"``, etc.).
+
+``RemoteUserMiddleware`` session hijacking
+==========================================
+
+When using the :class:`~django.contrib.auth.middleware.RemoteUserMiddleware`
+and the ``RemoteUserBackend``, a change to the ``REMOTE_USER`` header between
+requests without an intervening logout could result in the prior user's session
+being co-opted by the subsequent user. The middleware now logs the user out on
+a failed login attempt.
diff --git a/docs/releases/1.5.9.txt b/docs/releases/1.5.9.txt
index 232cbb0767..5070f0eca1 100644
--- a/docs/releases/1.5.9.txt
+++ b/docs/releases/1.5.9.txt
@@ -38,3 +38,12 @@ if a file with the uploaded name already exists.
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
``"_2"``, etc.).
+
+``RemoteUserMiddleware`` session hijacking
+==========================================
+
+When using the :class:`~django.contrib.auth.middleware.RemoteUserMiddleware`
+and the ``RemoteUserBackend``, a change to the ``REMOTE_USER`` header between
+requests without an intervening logout could result in the prior user's session
+being co-opted by the subsequent user. The middleware now logs the user out on
+a failed login attempt.
diff --git a/docs/releases/1.6.6.txt b/docs/releases/1.6.6.txt
index c2ebdb9efb..e52ea6b23f 100644
--- a/docs/releases/1.6.6.txt
+++ b/docs/releases/1.6.6.txt
@@ -39,6 +39,15 @@ underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
``"_2"``, etc.).
+``RemoteUserMiddleware`` session hijacking
+==========================================
+
+When using the :class:`~django.contrib.auth.middleware.RemoteUserMiddleware`
+and the ``RemoteUserBackend``, a change to the ``REMOTE_USER`` header between
+requests without an intervening logout could result in the prior user's session
+being co-opted by the subsequent user. The middleware now logs the user out on
+a failed login attempt.
+
Bugfixes
========