mirror of
				https://github.com/django/django.git
				synced 2025-10-31 09:41:08 +00:00 
			
		
		
		
	Fixed #23066 -- Modified RemoteUserMiddleware to logout on REMOTE_USER change.
This is a security fix. Disclosure following shortly.
This commit is contained in:
		
				
					committed by
					
						 Tim Graham
						Tim Graham
					
				
			
			
				
	
			
			
			
						parent
						
							0d8d30b7dd
						
					
				
				
					commit
					5307ce565f
				
			| @@ -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) | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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. | ||||
|   | ||||
| @@ -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. | ||||
|   | ||||
| @@ -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 | ||||
| ======== | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user