Attachment 926642 Details for Bug 1129954 – 1.5 patch

[patch] 1.5 patch

remote-user-1.5.diff (text/plain), 5.77 KB, created by Murray McAllister on 2014-08-14 04:51:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Murray McAllister

Created: 2014-08-14 04:51:36 UTC

Size: 5.77 KB

patch

obsolete

>commit 2b52eb9eb3c6956dbff44fa7cd8b04c9a06b6ff0
>Author: Preston Holmes <preston@ptone.com>
>Date:   Mon Aug 11 12:04:53 2014 -0400
>
>    [1.5.x] Fixed #23066 -- Modified RemoteUserMiddleware to logout on REMOTE_USE change.
>    
>    This is a security fix. Disclosure following shortly.
>
>diff --git a/django/contrib/auth/middleware.py b/django/contrib/auth/middleware.py
>index f38efdd..023e082 100644
>--- a/django/contrib/auth/middleware.py
>+++ b/django/contrib/auth/middleware.py
>@@ -53,14 +53,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 ImproperlyConfigured as e:
>-                    # 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
>@@ -68,6 +61,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)
>@@ -89,3 +87,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 ImproperlyConfigured:
>+            # backend failed to load
>+            auth.logout(request)
>+        else:
>+            if isinstance(stored_backend, RemoteUserBackend):
>+                auth.logout(request)
>diff --git a/django/contrib/auth/tests/remote_user.py b/django/contrib/auth/tests/remote_user.py
>index 0e59b29..572e330 100644
>--- a/django/contrib/auth/tests/remote_user.py
>+++ b/django/contrib/auth/tests/remote_user.py
>@@ -118,6 +118,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/',
>+                                   **{'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):
>         """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 6c140ee..811c3f6 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 232cbb0..5070f0e 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.

Actions: View | Diff

Attachments on bug 1129954: 926641 | 926642 | 926643