From 05c857478bfed379ffc105830b678ed603cc8376 Mon Sep 17 00:00:00 2001 From: Xavier Antoviaque Date: Wed, 26 Mar 2014 18:19:14 -0400 Subject: [PATCH] xblock-external-ui: Adds support for CORS headers (cross-domain request) xblock-external-ui: Alternate referer check for CORS requests xblock-external-ui: Allow to disable httponly on session cookies xblock-external-ui: Add a unit test for CorsCSRFMiddleware --- cms/envs/aws.py | 1 + common/djangoapps/cors_csrf/__init__.py | 0 common/djangoapps/cors_csrf/middleware.py | 67 ++++++++++++++ common/djangoapps/cors_csrf/models.py | 0 common/djangoapps/cors_csrf/tests.py | 101 +++++++++++++++++++++ lms/djangoapps/courseware/module_render.py | 2 +- lms/envs/aws.py | 12 +++ lms/envs/common.py | 10 ++ requirements/edx/base.txt | 3 + 9 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 common/djangoapps/cors_csrf/__init__.py create mode 100644 common/djangoapps/cors_csrf/middleware.py create mode 100644 common/djangoapps/cors_csrf/models.py create mode 100644 common/djangoapps/cors_csrf/tests.py diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 29cdb486ad..863809fc16 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -144,6 +144,7 @@ if 'loc_cache' not in CACHES: } SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN') +SESSION_COOKIE_HTTPONLY = ENV_TOKENS.get('SESSION_COOKIE_HTTPONLY', True) SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE) SESSION_COOKIE_SECURE = ENV_TOKENS.get('SESSION_COOKIE_SECURE', SESSION_COOKIE_SECURE) diff --git a/common/djangoapps/cors_csrf/__init__.py b/common/djangoapps/cors_csrf/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/cors_csrf/middleware.py b/common/djangoapps/cors_csrf/middleware.py new file mode 100644 index 0000000000..c4541d5549 --- /dev/null +++ b/common/djangoapps/cors_csrf/middleware.py @@ -0,0 +1,67 @@ +""" +Middleware for handling CSRF checks with CORS requests + +When processing HTTPS requests, the default CSRF middleware checks that the referer +domain and protocol is the same as the request's domain and protocol. This is meant +to avoid a type of attack for sites which serve their content with both HTTP and HTTPS, +with a man in the middle on the HTTP requests. + +https://github.com/django/django/blob/b91c385e324f1cb94d20e2ad146372c259d51d3b/django/middleware/csrf.py#L117 + +This doesn't work well with CORS requests, which aren't vulnerable to this attack when +the server from which the request is coming uses HTTPS too, as it prevents the man in the +middle attack vector. + +We thus do the CSRF check of requests coming from an authorized CORS host separately +in this middleware, applying the same protections as the default CSRF middleware, but +without the referrer check, when both the request and the referer use HTTPS. +""" + +import logging +import urlparse + +from django.conf import settings +from django.middleware.csrf import CsrfViewMiddleware + +log = logging.getLogger(__name__) + + +class CorsCSRFMiddleware(CsrfViewMiddleware): + """ + Middleware for handling CSRF checks with CORS requests + """ + def is_enabled(self, request): + """ + Override the `is_enabled()` method to allow cross-domain HTTPS requests + """ + if not settings.FEATURES.get('ENABLE_CORS_HEADERS'): + return False + + referer = request.META.get('HTTP_REFERER') + if not referer: + return False + referer_parts = urlparse.urlparse(referer) + + if referer_parts.hostname not in getattr(settings, 'CORS_ORIGIN_WHITELIST', []): + return False + if not request.is_secure() or referer_parts.scheme != 'https': + return False + + return True + + def process_view(self, request, callback, callback_args, callback_kwargs): + if not self.is_enabled(request): + return + + is_secure_default = request.is_secure + + def is_secure_patched(): + """ + Avoid triggering the additional CSRF middleware checks on the referrer + """ + return False + request.is_secure = is_secure_patched + + res = super(CorsCSRFMiddleware, self).process_view(request, callback, callback_args, callback_kwargs) + request.is_secure = is_secure_default + return res diff --git a/common/djangoapps/cors_csrf/models.py b/common/djangoapps/cors_csrf/models.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/cors_csrf/tests.py b/common/djangoapps/cors_csrf/tests.py new file mode 100644 index 0000000000..7e6c4f4a36 --- /dev/null +++ b/common/djangoapps/cors_csrf/tests.py @@ -0,0 +1,101 @@ +""" +Tests for the CORS CSRF middleware +""" + +from mock import patch, Mock + +from django.test import TestCase +from django.test.utils import override_settings +from django.middleware.csrf import CsrfViewMiddleware + +from cors_csrf.middleware import CorsCSRFMiddleware + + +SENTINEL = object() + + +class TestCorsMiddlewareProcessRequest(TestCase): + """ + Test processing a request through the middleware + """ + def get_request(self, is_secure, http_referer): + """ + Build a test request + """ + request = Mock() + request.META = {'HTTP_REFERER': http_referer} + request.is_secure = lambda: is_secure + return request + + def setUp(self): + self.middleware = CorsCSRFMiddleware() + + def check_not_enabled(self, request): + """ + Check that the middleware does NOT process the provided request + """ + with patch.object(CsrfViewMiddleware, 'process_view') as mock_method: + res = self.middleware.process_view(request, None, None, None) + + self.assertIsNone(res) + self.assertFalse(mock_method.called) + + def check_enabled(self, request): + """ + Check that the middleware does process the provided request + """ + def cb_check_req_is_secure_false(request, callback, args, kwargs): + """ + Check that the request doesn't pass (yet) the `is_secure()` test + """ + self.assertFalse(request.is_secure()) + return SENTINEL + + with patch.object(CsrfViewMiddleware, 'process_view') as mock_method: + mock_method.side_effect = cb_check_req_is_secure_false + res = self.middleware.process_view(request, None, None, None) + + self.assertIs(res, SENTINEL) + self.assertTrue(request.is_secure()) + + @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True}, + CORS_ORIGIN_WHITELIST=['foo.com']) + def test_enabled(self): + request = self.get_request(is_secure=True, + http_referer='https://foo.com/bar') + self.check_enabled(request) + + @override_settings(FEATURES={'ENABLE_CORS_HEADERS': False}, + CORS_ORIGIN_WHITELIST=['foo.com']) + def test_disabled_no_cors_headers(self): + request = self.get_request(is_secure=True, + http_referer='https://foo.com/bar') + self.check_not_enabled(request) + + @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True}, + CORS_ORIGIN_WHITELIST=['bar.com']) + def test_disabled_wrong_cors_domain(self): + request = self.get_request(is_secure=True, + http_referer='https://foo.com/bar') + self.check_not_enabled(request) + + @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True}, + CORS_ORIGIN_WHITELIST=['foo.com']) + def test_disabled_wrong_cors_domain_reversed(self): + request = self.get_request(is_secure=True, + http_referer='https://bar.com/bar') + self.check_not_enabled(request) + + @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True}, + CORS_ORIGIN_WHITELIST=['foo.com']) + def test_disabled_http_request(self): + request = self.get_request(is_secure=False, + http_referer='https://foo.com/bar') + self.check_not_enabled(request) + + @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True}, + CORS_ORIGIN_WHITELIST=['foo.com']) + def test_disabled_http_referer(self): + request = self.get_request(is_secure=True, + http_referer='http://foo.com/bar') + self.check_not_enabled(request) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 433086e2cb..260b552a34 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -960,7 +960,7 @@ def xblock_view(request, course_id, usage_id, view_name): if not request.user.is_authenticated(): raise PermissionDenied - instance, tracking_context = _get_module_by_usage_id(request, course_id, usage_id) + instance, _ = _get_module_by_usage_id(request, course_id, usage_id) try: fragment = instance.render(view_name, context=request.GET) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 72c26b34fb..47ecffba6d 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -151,6 +151,7 @@ SITE_NAME = ENV_TOKENS['SITE_NAME'] HTTPS = ENV_TOKENS.get('HTTPS', HTTPS) SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE) SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN') +SESSION_COOKIE_HTTPONLY = ENV_TOKENS.get('SESSION_COOKIE_HTTPONLY', True) REGISTRATION_EXTRA_FIELDS = ENV_TOKENS.get('REGISTRATION_EXTRA_FIELDS', REGISTRATION_EXTRA_FIELDS) SESSION_COOKIE_SECURE = ENV_TOKENS.get('SESSION_COOKIE_SECURE', SESSION_COOKIE_SECURE) @@ -302,6 +303,17 @@ if FEATURES.get('AUTH_USE_CAS'): # Example: {'CN': 'http://api.xuetangx.com/edx/video?s3_url='} VIDEO_CDN_URL = ENV_TOKENS.get('VIDEO_CDN_URL', {}) +############# CORS headers for cross-domain requests ################# + +if FEATURES.get('ENABLE_CORS_HEADERS'): + INSTALLED_APPS += ('corsheaders', 'cors_csrf') + MIDDLEWARE_CLASSES = ( + 'corsheaders.middleware.CorsMiddleware', + 'cors_csrf.middleware.CorsCSRFMiddleware', + ) + MIDDLEWARE_CLASSES + CORS_ALLOW_CREDENTIALS = True + CORS_ORIGIN_WHITELIST = ENV_TOKENS.get('CORS_ORIGIN_WHITELIST', ()) + ############################## SECURE AUTH ITEMS ############### # Secret things: passwords, access keys, etc. diff --git a/lms/envs/common.py b/lms/envs/common.py index 9347fcc670..f5fd5b1943 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1685,6 +1685,16 @@ if FEATURES.get('AUTH_USE_CAS'): INSTALLED_APPS += ('django_cas',) MIDDLEWARE_CLASSES += ('django_cas.middleware.CASMiddleware',) +############# CORS headers for cross-domain requests ################# + +if FEATURES.get('ENABLE_CORS_HEADERS'): + INSTALLED_APPS += ('corsheaders', 'cors_csrf') + MIDDLEWARE_CLASSES = ( + 'corsheaders.middleware.CorsMiddleware', + 'cors_csrf.middleware.CorsCSRFMiddleware', + ) + MIDDLEWARE_CLASSES + CORS_ALLOW_CREDENTIALS = True + CORS_ORIGIN_WHITELIST = () ###################### Registration ################################## diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 10052b3f2c..c58f993fe7 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -115,6 +115,9 @@ transifex-client==0.10 # Ip network support for Embargo feature ipaddr==2.1.11 +# Used to allow to configure CORS headers for cross-domain requests +django-cors-headers==0.13 + # We've tried several times to update the debug toolbar to version 1.0.1, # and had problems each time, resulting in us rolling back to 0.9.4. Before # submitting another pull request to do this update, check the following: