8000 [1.5.x] Fixed #23066 -- Modified RemoteUserMiddleware to logout on RE… · alex-python/django@dd68f31 · GitHub
[go: up one dir, main page]
Skip to content

Commit dd68f31

Browse files
ptonetimgraham
ptone
authored and
timgraham
committed
[1.5.x] Fixed #23066 -- Modified RemoteUserMiddleware to logout on REMOTE_USE change.
This is a security fix. Disclosure following shortly.
1 parent 26cd48e commit dd68f31

File tree

4 files changed

+56
-8
lines changed

4 files changed

+56
-8
lines changed

‎django/contrib/auth/middleware.py

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,19 @@ def process_request(self, request):
5353
# authenticated remote-user, or return (leaving request.user set to
5454
# AnonymousUser by the AuthenticationMiddleware).
5555
if request.user.is_authenticated():
56-
try:
57-
stored_backend = load_backend(request.session.get(
58-
auth.BACKEND_SESSION_KEY, ''))
59-
if isinstance(stored_backend, RemoteUserBackend):
60-
auth.logout(request)
61-
except ImproperlyConfigured as e:
62-
# backend failed to load
63-
auth.logout(request)
56+
self._remove_invalid_user(request)
6457
return
6558
# If the user is already authenticated and that user is the user we are
6659
# getting passed in the headers, then the correct user is already
6760
# persisted in the session and we don't need to continue.
6861
if request.user.is_authenticated():
6962
if request.user.get_username() == self.clean_username(username, request):
7063
return
64+
else:
65+
# An authenticated user is associated with the request, but
66+
# it does not match the authorized user in the header.
67+
self._remove_invalid_user(request)
68+
7169
# We are seeing this user for the first time in this session, attempt
7270
# to authenticate the user.
7371
user = auth.authenticate(remote_user=username)
@@ -89,3 +87,17 @@ def clean_username(self, username, request):
8987
except AttributeError: # Backend has no clean_username method.
9088
pass
9189
return username
90+
91+
def _remove_invalid_user(self, request):
92+
"""
93+
Removes the current authenticated user in the request which is invalid
94+
but only if the user is authenticated via the RemoteUserBackend.
95+
"""
96+
try:
97+
stored_backend = load_backend(request.session.get(auth.BACKEND_SESSION_KEY, ''))
98+
except ImproperlyConfigured:
99+
# backend failed to load
100+
auth.logout(request)
101+
else:
102+
if isinstance(stored_backend, RemoteUserBackend):
103+
auth.logout(request)

‎django/contrib/auth/tests/remote_user.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,24 @@ def test_header_disappears(self):
118118
response = self.client.get('/remote_user/')
119119
self.assertEqual(response.context['user'].username, 'modeluser')
120120

121+
def test_user_switch_forces_new_login(self):
122+
"""
123+
Tests that if the username in the header changes between requests
124+
that the original user is logged out
125+
"""
126+
User.objects.create(username='knownuser')
127+
# Known user authenticates
128+
response = self.client.get('/remote_user/',
129+
**{'REMOTE_USER': self.known_user})
130+
self.assertEqual(response.context['user'].username, 'knownuser')
131+
# During the session, the REMOTE_USER changes to a different user.
132+
response = self.client.get('/remote_user/',
133+
**{'REMOTE_USER': "newnewuser"})
134+
# Ensure that the current user is not the prior remote_user
135+
# In backends that create a new user, username is "newnewuser"
136+
# In backends that do not create new users, it is '' (anonymous user)
137+
self.assertNotEqual(response.context['user'].username, 'knownuser')
138+
121139
def tearDown(self):
122140
"""Restores settings to avoid breaking other tests."""
123141
settings.MIDDLEWARE_CLASSES = self.curr_middleware

‎docs/releases/1.4.14.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,12 @@ if a file with the uploaded name already exists.
3838
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
3939
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
4040
``"_2"``, etc.).
41+
42+
``RemoteUserMiddleware`` session hijacking
43+
==========================================
44+
45+
When using the :class:`~django.contrib.auth.middleware.RemoteUserMiddleware`
46+
and the ``RemoteUserBackend``, a change to the ``REMOTE_USER`` header between
47+
requests without an intervening logout could result in the prior user's session
48+
being co-opted by the subsequent user. The middleware now logs the user out on
49+
a failed login attempt.

‎docs/releases/1.5.9.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,12 @@ if a file with the uploaded name already exists.
3838
underscore plus a random 7 character alphanumeric string (e.g. ``"_x3a1gho"``),
3939
rather than iterating through an underscore followed by a number (e.g. ``"_1"``,
4040
``"_2"``, etc.).
41+
42+
``RemoteUserMiddleware`` session hijacking
43+
==========================================
44+
45+
When using the :class:`~django.contrib.auth.middleware.RemoteUserMiddleware`
46+
and the ``RemoteUserBackend``, a change to the ``REMOTE_USER`` header between
47+
requests without an intervening logout could result in the prior user's session
48+
being co-opted by the subsequent user. The middleware now logs the user out on
49+
a failed login attempt.

0 commit comments

Comments
 (0)
0