mirror of
https://github.com/django/django.git
synced 2025-03-26 09:10:50 +00:00
[1.6.x] Fixed #23066 -- Modified RemoteUserMiddleware to logout on REMOTE_USE change.
This is a security fix. Disclosure following shortly.
This commit is contained in:
parent
dd0c3f4ee1
commit
0268b855f9
@ -53,14 +53,7 @@ class RemoteUserMiddleware(object):
|
|||||||
# authenticated remote-user, or return (leaving request.user set to
|
# authenticated remote-user, or return (leaving request.user set to
|
||||||
# AnonymousUser by the AuthenticationMiddleware).
|
# AnonymousUser by the AuthenticationMiddleware).
|
||||||
if request.user.is_authenticated():
|
if request.user.is_authenticated():
|
||||||
try:
|
self._remove_invalid_user(request)
|
||||||
stored_backend = load_backend(request.session.get(
|
|
||||||
auth.BACKEND_SESSION_KEY, ''))
|
|
||||||
if isinstance(stored_backend, RemoteUserBackend):
|
|
||||||
auth.logout(request)
|
|
||||||
except ImproperlyConfigured as e:
|
|
||||||
# backend failed to load
|
|
||||||
auth.logout(request)
|
|
||||||
return
|
return
|
||||||
# If the user is already authenticated and that user is the user we are
|
# 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
|
# getting passed in the headers, then the correct user is already
|
||||||
@ -68,6 +61,11 @@ class RemoteUserMiddleware(object):
|
|||||||
if request.user.is_authenticated():
|
if request.user.is_authenticated():
|
||||||
if request.user.get_username() == self.clean_username(username, request):
|
if request.user.get_username() == self.clean_username(username, request):
|
||||||
return
|
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
|
# We are seeing this user for the first time in this session, attempt
|
||||||
# to authenticate the user.
|
# to authenticate the user.
|
||||||
user = auth.authenticate(remote_user=username)
|
user = auth.authenticate(remote_user=username)
|
||||||
@ -89,3 +87,17 @@ class RemoteUserMiddleware(object):
|
|||||||
except AttributeError: # Backend has no clean_username method.
|
except AttributeError: # Backend has no clean_username method.
|
||||||
pass
|
pass
|
||||||
return username
|
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 ImproperlyConfigured:
|
||||||
|
# backend failed to load
|
||||||
|
auth.logout(request)
|
||||||
|
else:
|
||||||
|
if isinstance(stored_backend, RemoteUserBackend):
|
||||||
|
auth.logout(request)
|
||||||
|
@ -118,6 +118,24 @@ class RemoteUserTest(TestCase):
|
|||||||
response = self.client.get('/remote_user/')
|
response = self.client.get('/remote_user/')
|
||||||
self.assertEqual(response.context['user'].username, 'modeluser')
|
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/',
|
||||||
|
**{'REMOTE_USER': 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/',
|
||||||
|
**{'REMOTE_USER': "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):
|
def tearDown(self):
|
||||||
"""Restores settings to avoid breaking other tests."""
|
"""Restores settings to avoid breaking other tests."""
|
||||||
settings.MIDDLEWARE_CLASSES = self.curr_middleware
|
settings.MIDDLEWARE_CLASSES = self.curr_middleware
|
||||||
|
@ -38,3 +38,12 @@ if a file with the uploaded name already exists.
|
|||||||
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
|
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
|
||||||
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
|
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
|
||||||
``"_2"``, etc.).
|
``"_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.
|
||||||
|
@ -38,3 +38,12 @@ if a file with the uploaded name already exists.
|
|||||||
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
|
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
|
||||||
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
|
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
|
||||||
``"_2"``, etc.).
|
``"_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.
|
||||||
|
@ -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"``,
|
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
|
||||||
``"_2"``, etc.).
|
``"_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
|
Bugfixes
|
||||||
========
|
========
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user