From 09dfd8718e35d6c55e379b9766ce63475f479f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Thu, 23 Nov 2023 18:52:16 +0100 Subject: [PATCH] fix: CSRF errors in POST requests to LMS (#33727) POST requests to the LMS are failing systematically when HTTPS is enabled. This issue is observed in the Quince release branch. Here is the root cause analysis: - CorsCSRFMiddleware overrides the `is_secure` attribute by setting it to "false". - CorsCSRFMiddleware calls the parent `process_view` method, from the CsrfViewMiddleware. - CsrfViewMiddleware checks the Origin header, including the scheme. It is equal to "https://LMSHOST". But because the request is not considered secure, the expected origin is "http://LMSHOST". - The check fails with "Origin checking failed" We resolve this issue by running the CsrfViewMiddleware *before* the custom CorsCSRFMiddleware. After a successful check of the CsrfViewMiddleware, the request has the "csrf_processing_done = True" attribute, and CorsCSRFMiddleware is short-circuited. This issue did not happen in the following environments: - in Palm because the CsrfViewMiddleware did not check the "Origin" header in Django 3. - in the Studio, because the Studio already runs the CsrfViewMiddleware before the CorsCSRFMiddleware. - in the master branch because the master branch does not yet run on Django 4. But the issue will happen in the master branch without this proposed change. To bypass this issue in the master branch, it was proposed that we add "https://LMSHOST" to CSRF_TRUSTED_ORIGINS. This would effectily bypass CSRF checking entirely for all requests that originate from the LMS. Such a solution would not be acceptable, as we would lose the security guarantees offered by CSRF. See discussion: https://github.com/openedx/wg-build-test-release/issues/325 --- lms/envs/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index a5949279df24..f738f57e6381 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2235,10 +2235,10 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring 'common.djangoapps.track.middleware.TrackMiddleware', # CORS and CSRF + 'django.middleware.csrf.CsrfViewMiddleware', 'corsheaders.middleware.CorsMiddleware', 'openedx.core.djangoapps.cors_csrf.middleware.CorsCSRFMiddleware', 'openedx.core.djangoapps.cors_csrf.middleware.CsrfCrossDomainCookieMiddleware', - 'django.middleware.csrf.CsrfViewMiddleware', 'splash.middleware.SplashMiddleware',