From eeb35733bb661e6d8f8e49d93a96395835a61014 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Fri, 15 Sep 2023 10:34:25 -0400 Subject: [PATCH] fix: Update cors_csrf middleware for Django 4.0 (#33262) Remove use of Django's `CSRF_COOKIE_USED`, which is no longer leaked outside of the dynamic scope of the Django csrf middleware as of Django 4.0. Specifically, https://github.com/django/django/pull/14688 replaced that META entry and a request attribute with a single META entry `CSRF_COOKIE_NEEDS_UPDATE`, which is then set back to False once the CSRF cookie is set by the middleware's process_response. We'll send the cross-domain cookie if the decorator requests it and the value is present, regardless of whether the same-domain cookie would have been sent. (And we'll still *set* `CSRF_COOKIE_NEEDS_UPDATE` to ensure that a cookie gets generated.) See https://github.com/openedx/edx-platform/issues/33207 --- openedx/core/djangoapps/cors_csrf/middleware.py | 14 ++++++++++---- .../djangoapps/cors_csrf/tests/test_decorators.py | 7 ++++++- .../djangoapps/cors_csrf/tests/test_middleware.py | 1 - 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/cors_csrf/middleware.py b/openedx/core/djangoapps/cors_csrf/middleware.py index 072a333c45..8122231e32 100644 --- a/openedx/core/djangoapps/cors_csrf/middleware.py +++ b/openedx/core/djangoapps/cors_csrf/middleware.py @@ -120,12 +120,18 @@ class CsrfCrossDomainCookieMiddleware(MiddlewareMixin): log.debug("Could not set cross-domain CSRF cookie.") return response - # Check whether (a) the CSRF middleware has already set a cookie, and - # (b) this is a view decorated with `@ensure_cross_domain_csrf_cookie` - # If so, we can send the cross-domain CSRF cookie. + # Send the cross-domain CSRF cookie if this is a view decorated with + # `@ensure_cross_domain_csrf_cookie` and the same-domain CSRF cookie + # value is available. + # + # Because CSRF_COOKIE can be set either by an inbound CSRF token or + # by the middleware generating a new one or echoing the old one for + # the response, this might result in sending the cookie more often + # than the CSRF value actually changes, but as of Django 4.0 we no + # longer have a good way of finding out when the csrf middleware has + # updated the value. should_set_cookie = ( request.META.get('CROSS_DOMAIN_CSRF_COOKIE_USED', False) and - request.META.get('CSRF_COOKIE_USED', False) and request.META.get('CSRF_COOKIE') is not None ) diff --git a/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py b/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py index 64da1a5639..917b62e0e5 100644 --- a/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py +++ b/openedx/core/djangoapps/cors_csrf/tests/test_decorators.py @@ -3,6 +3,8 @@ import json from unittest import mock + +import django from django.http import HttpResponse from django.test import TestCase @@ -25,4 +27,7 @@ class TestEnsureCsrfCookieCrossDomain(TestCase): response = wrapped_view(request) response_meta = json.loads(response.content.decode('utf-8')) assert response_meta['CROSS_DOMAIN_CSRF_COOKIE_USED'] is True - assert response_meta['CSRF_COOKIE_USED'] is True + if django.VERSION < (4, 0): + assert response_meta['CSRF_COOKIE_USED'] is True + else: + assert response_meta['CSRF_COOKIE_NEEDS_UPDATE'] is True diff --git a/openedx/core/djangoapps/cors_csrf/tests/test_middleware.py b/openedx/core/djangoapps/cors_csrf/tests/test_middleware.py index 71be51ce7f..a546cbbcd4 100644 --- a/openedx/core/djangoapps/cors_csrf/tests/test_middleware.py +++ b/openedx/core/djangoapps/cors_csrf/tests/test_middleware.py @@ -258,7 +258,6 @@ class TestCsrfCrossDomainCookieMiddleware(TestCase): del request.META['HTTP_REFERER'] if csrf_cookie_used: - request.META['CSRF_COOKIE_USED'] = True request.META['CSRF_COOKIE'] = self.COOKIE_VALUE if cross_domain_decorator: