From f283ffaa84ef0a558eb466b8fc3fae7e6fbb547c Mon Sep 17 00:00:00 2001 From: Colton Hicks Date: Fri, 31 Jan 2020 23:42:24 -0800 Subject: [PATCH] Fixed #28699 -- Fixed CSRF validation with remote user middleware. Ensured process_view() always accesses the CSRF token from the session or cookie, rather than the request, as rotate_token() may have been called by an authentication middleware during the process_request() phase. --- AUTHORS | 1 + django/middleware/csrf.py | 5 ++++- tests/auth_tests/test_remote_user.py | 32 +++++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index b2ea40e565b..afc4ad03063 100644 --- a/AUTHORS +++ b/AUTHORS @@ -201,6 +201,7 @@ answer newbie questions, and generally made Django that much better: Colin Wood Collin Anderson Collin Grady + Colton Hicks Craig Blaszczyk crankycoder@gmail.com Curtis Maloney (FunkyBob) diff --git a/django/middleware/csrf.py b/django/middleware/csrf.py index 4ac2f23019f..368b51f3165 100644 --- a/django/middleware/csrf.py +++ b/django/middleware/csrf.py @@ -280,7 +280,10 @@ class CsrfViewMiddleware(MiddlewareMixin): reason = REASON_BAD_REFERER % referer.geturl() return self._reject(request, reason) - csrf_token = request.META.get('CSRF_COOKIE') + # Access csrf_token via self._get_token() as rotate_token() may + # have been called by an authentication middleware during the + # process_request() phase. + csrf_token = self._get_token(request) if csrf_token is None: # No CSRF cookie. For POST requests, we insist on a CSRF cookie, # and in this way we can avoid all CSRF attacks, including login diff --git a/tests/auth_tests/test_remote_user.py b/tests/auth_tests/test_remote_user.py index 6260c460afb..ee1f2e1cdf0 100644 --- a/tests/auth_tests/test_remote_user.py +++ b/tests/auth_tests/test_remote_user.py @@ -5,7 +5,8 @@ from django.contrib.auth import authenticate from django.contrib.auth.backends import RemoteUserBackend from django.contrib.auth.middleware import RemoteUserMiddleware from django.contrib.auth.models import User -from django.test import TestCase, modify_settings, override_settings +from django.middleware.csrf import _get_new_csrf_string, _mask_cipher_secret +from django.test import Client, TestCase, modify_settings, override_settings from django.utils import timezone @@ -50,6 +51,35 @@ class RemoteUserTest(TestCase): self.assertTrue(response.context['user'].is_anonymous) self.assertEqual(User.objects.count(), num_users) + def test_csrf_validation_passes_after_process_request_login(self): + """ + CSRF check must access the CSRF token from the session or cookie, + rather than the request, as rotate_token() may have been called by an + authentication middleware during the process_request() phase. + """ + csrf_client = Client(enforce_csrf_checks=True) + csrf_secret = _get_new_csrf_string() + csrf_token = _mask_cipher_secret(csrf_secret) + csrf_token_form = _mask_cipher_secret(csrf_secret) + headers = {self.header: 'fakeuser'} + data = {'csrfmiddlewaretoken': csrf_token_form} + + # Verify that CSRF is configured for the view + csrf_client.cookies.load({settings.CSRF_COOKIE_NAME: csrf_token}) + response = csrf_client.post('/remote_user/', **headers) + self.assertEqual(response.status_code, 403) + self.assertIn(b'CSRF verification failed.', response.content) + + # This request will call django.contrib.auth.login() which will call + # django.middleware.csrf.rotate_token() thus changing the value of + # request.META['CSRF_COOKIE'] from the user submitted value set by + # CsrfViewMiddleware.process_request() to the new csrftoken value set + # by rotate_token(). Csrf validation should still pass when the view is + # later processed by CsrfViewMiddleware.process_view() + csrf_client.cookies.load({settings.CSRF_COOKIE_NAME: csrf_token}) + response = csrf_client.post('/remote_user/', data, **headers) + self.assertEqual(response.status_code, 200) + def test_unknown_user(self): """ Tests the case where the username passed in the header does not exist