From 6308c968db639aabeb6e140a6d093d00744cc073 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 8 May 2024 15:05:19 -0400 Subject: [PATCH] refactor: always define CORS_ALLOW_HEADERS centrally 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?). --- cms/envs/common.py | 14 +++++++++----- cms/envs/devstack.py | 6 ------ cms/envs/production.py | 6 ------ lms/envs/common.py | 10 +++++++--- lms/envs/devstack.py | 5 ----- lms/envs/production.py | 4 ---- 6 files changed, 16 insertions(+), 29 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index ddd79f6d16..e684946d74 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2575,11 +2575,15 @@ if FEATURES.get('ENABLE_CORS_HEADERS'): 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 = [] diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 39c28a37d2..3fe10377d7 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -5,7 +5,6 @@ Specific overrides to the base prod settings to make development easier. 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 @@ -256,11 +255,6 @@ SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' 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 diff --git a/cms/envs/production.py b/cms/envs/production.py index 2ac80b94c3..67524e7608 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -13,7 +13,6 @@ import os 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 @@ -598,11 +597,6 @@ if FEATURES.get('ENABLE_CORS_HEADERS'): 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) diff --git a/lms/envs/common.py b/lms/envs/common.py index 02e52ea5a3..6acf880d4f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3675,9 +3675,13 @@ if FEATURES.get('ENABLE_CORS_HEADERS'): 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 diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 7f69641055..b783a276fe 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -7,8 +7,6 @@ Specific overrides to the base prod settings to make development easier. 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 @@ -295,9 +293,6 @@ FEATURES['ENABLE_CORS_HEADERS'] = True 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, diff --git a/lms/envs/production.py b/lms/envs/production.py index d56a5631bb..948e81977c 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -22,7 +22,6 @@ import datetime 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 @@ -382,9 +381,6 @@ if FEATURES.get('ENABLE_CORS_HEADERS') or FEATURES.get('ENABLE_CROSS_DOMAIN_CSRF 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