Skip to content

Commit

Permalink
fix: CSRF errors in POST requests to LMS (#33727)
Browse files Browse the repository at this point in the history
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: openedx/wg-build-test-release#325
  • Loading branch information
regisb authored Nov 23, 2023
1 parent e9ca49d commit 09dfd87
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',

Expand Down

0 comments on commit 09dfd87

Please sign in to comment.