Skip to content

Commit

Permalink
refactor: always define CORS_ALLOW_HEADERS centrally
Browse files Browse the repository at this point in the history
The LMS and Studio need to set values for CORS_ALLOW_HEADERS so that the
MFEs can work properly, since preflight requests will need to send over
extra headers. Prior to this commit, CORS_ALLOW_HEADERS was being
redefined in multiple places in edx-platform and again in Tutor's config
because it was only being conditionally set if ENABLE_CORS_HEADERS was
True (which was a policy setting). But CORS_ALLOW_HEADERS is application
logic in that the value is determined by what the view needs, and won't
vary by deployment.

By consolidating this to always be defined in the common.py files, we
make sure that deployment environments don't have to define it. An
example of where this bit us was when course import in the course
authoring MFE did not work because Tutor was using an outdated value for
this setting.

A followup to this would be to just rip out the ENABLE_CORS_HEADERS
setting entirely, and just always have it on. But that would benefit
from a little more discovery to make sure there's no weird use case that
still requires it to be False (maybe something in the test suite?).
  • Loading branch information
ormsbee committed May 8, 2024
1 parent 634ab25 commit 6308c96
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 29 deletions.
14 changes: 9 additions & 5 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2575,11 +2575,15 @@
CORS_ORIGIN_WHITELIST = ()
CORS_ORIGIN_ALLOW_ALL = False
CORS_ALLOW_INSECURE = False
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
'content-range',
'content-disposition',
)

# Set CORS_ALLOW_HEADERS regardless of whether we've enabled ENABLE_CORS_HEADERS
# because that decision might happen in a later config file. (The headers to
# allow is an application logic, and not site policy.)
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
'content-range',
'content-disposition',
)

LOGIN_REDIRECT_WHITELIST = []

Expand Down
6 changes: 0 additions & 6 deletions cms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import logging
from os.path import abspath, dirname, join
from corsheaders.defaults import default_headers as corsheaders_default_headers

from .production import * # pylint: disable=wildcard-import, unused-wildcard-import

Expand Down Expand Up @@ -256,11 +255,6 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing
FEATURES['ENABLE_CORS_HEADERS'] = True
CORS_ALLOW_CREDENTIALS = True
CORS_ORIGIN_ALLOW_ALL = True
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
'content-range',
'content-disposition',
)

################### Special Exams (Proctoring) and Prereqs ###################
FEATURES['ENABLE_SPECIAL_EXAMS'] = True
Expand Down
6 changes: 0 additions & 6 deletions cms/envs/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import warnings
import yaml

from corsheaders.defaults import default_headers as corsheaders_default_headers
import django
from django.core.exceptions import ImproperlyConfigured
from django.urls import reverse_lazy
Expand Down Expand Up @@ -598,11 +597,6 @@ def get_env_setting(setting):

CORS_ORIGIN_ALLOW_ALL = ENV_TOKENS.get('CORS_ORIGIN_ALLOW_ALL', False)
CORS_ALLOW_INSECURE = ENV_TOKENS.get('CORS_ALLOW_INSECURE', False)
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
'content-range',
'content-disposition',
)

################# Settings for brand logos. #################
LOGO_URL = ENV_TOKENS.get('LOGO_URL', LOGO_URL)
Expand Down
10 changes: 7 additions & 3 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3675,9 +3675,13 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
CORS_ORIGIN_WHITELIST = ()
CORS_ORIGIN_ALLOW_ALL = False
CORS_ALLOW_INSECURE = False
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
)

# Set CORS_ALLOW_HEADERS regardless of whether we've enabled ENABLE_CORS_HEADERS
# because that decision might happen in a later config file. (The headers to
# allow is an application logic, and not site policy.)
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
)

# Default cache expiration for the cross-domain proxy HTML page.
# This is a static page that can be iframed into an external page
Expand Down
5 changes: 0 additions & 5 deletions lms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
import logging
from os.path import abspath, dirname, join

from corsheaders.defaults import default_headers as corsheaders_default_headers

# pylint: enable=unicode-format-string # lint-amnesty, pylint: disable=bad-option-value
#####################################################################
from edx_django_utils.plugins import add_plugins
Expand Down Expand Up @@ -295,9 +293,6 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing
CORS_ALLOW_CREDENTIALS = True
CORS_ORIGIN_WHITELIST = ()
CORS_ORIGIN_ALLOW_ALL = True
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
)

LOGIN_REDIRECT_WHITELIST.extend([
CMS_BASE,
Expand Down
4 changes: 0 additions & 4 deletions lms/envs/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import os

import yaml
from corsheaders.defaults import default_headers as corsheaders_default_headers
import django
from django.core.exceptions import ImproperlyConfigured
from edx_django_utils.plugins import add_plugins
Expand Down Expand Up @@ -382,9 +381,6 @@ def get_env_setting(setting):

CORS_ORIGIN_ALLOW_ALL = ENV_TOKENS.get('CORS_ORIGIN_ALLOW_ALL', False)
CORS_ALLOW_INSECURE = ENV_TOKENS.get('CORS_ALLOW_INSECURE', False)
CORS_ALLOW_HEADERS = corsheaders_default_headers + (
'use-jwt-cookie',
)

# If setting a cross-domain cookie, it's really important to choose
# a name for the cookie that is DIFFERENT than the cookies used
Expand Down

0 comments on commit 6308c96

Please sign in to comment.