From 2ba4957951ed92bf21dacd5caf05c2bf676ad4fe Mon Sep 17 00:00:00 2001 From: Xavier Antoviaque Date: Tue, 18 Mar 2014 08:49:16 -0400 Subject: [PATCH 1/2] xblock-external-ui: Add XBlock API call to render XBlock views xblock-external-ui: Include CSRF token in the API answer xblock-external-ui: Include full path when building local_url xblock-external-ui: Fix TestHandleXBlockCallback & bok_choy, add tests xblock-external-ui: Only return `instance` in `_invoke_xblock_handler()` xblock-external-ui: Group resources by hash tag to avoid duplicate loads xblock-external-ui: PEP8 xblock-external-ui: Fail early if the XBlock view is called anonymously We used to serve anonymous requests, but most XBlocks assume that the user is logged in, which can generate a lot of errors when the user is accessed or when an XBlock ajax callback is queried. Fail early to only get one error per page load, and prevent displaying the XBlock altogether when the LMS doesn't find an active user session. xblock-external-ui: Add request params in view render context xblock-external-ui: HTTP error status when file is too large for handler xblock-external-ui: Fix unicode encodings in XBlock rendering xblock-external-ui: Feature flag for API call ENABLE_XBLOCK_VIEW_ENDPOINT --- lms/djangoapps/courseware/module_render.py | 107 ++++++++++++++---- .../courseware/tests/test_module_render.py | 36 +++++- lms/djangoapps/lms_xblock/runtime.py | 4 +- lms/envs/bok_choy.env.json | 2 +- lms/envs/common.py | 7 ++ lms/urls.py | 5 + 6 files changed, 137 insertions(+), 24 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6fe7f2b276..433086e2cb 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -1,3 +1,8 @@ +""" +Module rendering +""" + +import hashlib import json import logging import mimetypes @@ -5,6 +10,7 @@ import mimetypes import static_replace import xblock.reference.plugins +from collections import OrderedDict from functools import partial from requests.auth import HTTPBasicAuth import dogstats_wrapper as dog_stats_api @@ -13,6 +19,8 @@ from opaque_keys import InvalidKeyError from django.conf import settings from django.contrib.auth.models import User from django.core.cache import cache +from django.core.context_processors import csrf +from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.http import Http404, HttpResponse from django.views.decorators.csrf import csrf_exempt @@ -32,7 +40,7 @@ from student.models import anonymous_id_for_user, user_by_anonymous_id from xblock.core import XBlock from xblock.fields import Scope from xblock.runtime import KvsFieldData, KeyValueStore -from xblock.exceptions import NoSuchHandlerError +from xblock.exceptions import NoSuchHandlerError, NoSuchViewError from xblock.django.request import django_to_webob_request, webob_to_django_response from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor from xmodule.exceptions import NotFoundError, ProcessingError @@ -781,7 +789,7 @@ def handle_xblock_callback_noauth(request, course_id, usage_id, handler, suffix= """ request.user.known = False - return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, request.user) + return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix) def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): @@ -802,7 +810,7 @@ def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): if not request.user.is_authenticated(): return HttpResponse('Unauthenticated', status=403) - return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, request.user) + return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix) def xblock_resource(request, block_type, uri): # pylint: disable=unused-argument @@ -822,31 +830,20 @@ def xblock_resource(request, block_type, uri): # pylint: disable=unused-argumen return HttpResponse(content, mimetype=mimetype) -def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): +def _get_module_by_usage_id(request, course_id, usage_id): """ - Invoke an XBlock handler, either authenticated or not. - - Arguments: - request (HttpRequest): the current request - course_id (str): A string of the form org/course/run - usage_id (str): A string of the form i4x://org/course/category/name@revision - handler (str): The name of the handler to invoke - suffix (str): The suffix to pass to the handler when invoked - user (User): The currently logged in user + Gets a module instance based on its `usage_id` in a course, for a given request/user + Returns (instance, tracking_context) """ + user = request.user + try: course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) usage_key = course_id.make_usage_key_from_deprecated_string(unquote_slashes(usage_id)) except InvalidKeyError: raise Http404("Invalid location") - # Check submitted files - files = request.FILES or {} - error_msg = _check_files_limits(files) - if error_msg: - return HttpResponse(json.dumps({'success': error_msg})) - try: descriptor = modulestore().get_item(usage_key) descriptor_orig_usage_key, descriptor_orig_version = modulestore().get_block_original_usage(usage_key) @@ -859,13 +856,13 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): ) raise Http404 - tracking_context_name = 'module_callback_handler' tracking_context = { 'module': { 'display_name': descriptor.display_name_with_default, 'usage_key': unicode(descriptor.location), } } + # For blocks that are inherited from a content library, we add some additional metadata: if descriptor_orig_usage_key is not None: tracking_context['module']['original_usage_key'] = unicode(descriptor_orig_usage_key) @@ -884,6 +881,30 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): log.debug("No module %s for user %s -- access denied?", usage_key, user) raise Http404 + return (instance, tracking_context) + + +def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix): + """ + Invoke an XBlock handler, either authenticated or not. + + Arguments: + request (HttpRequest): the current request + course_id (str): A string of the form org/course/run + usage_id (str): A string of the form i4x://org/course/category/name@revision + handler (str): The name of the handler to invoke + suffix (str): The suffix to pass to the handler when invoked + """ + + # Check submitted files + files = request.FILES or {} + error_msg = _check_files_limits(files) + if error_msg: + return JsonResponse(object={'success': error_msg}, status=413) + + instance, tracking_context = _get_module_by_usage_id(request, course_id, usage_id) + + tracking_context_name = 'module_callback_handler' req = django_to_webob_request(request) try: with tracker.get_tracker().context(tracking_context_name, tracking_context): @@ -912,6 +933,52 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): return webob_to_django_response(resp) +def hash_resource(resource): + """ + Hash a :class:`xblock.fragment.FragmentResource + """ + md5 = hashlib.md5() + for data in resource: + md5.update(repr(data)) + return md5.hexdigest() + + +def xblock_view(request, course_id, usage_id, view_name): + """ + Returns the rendered view of a given XBlock, with related resources + + Returns a json object containing two keys: + html: The rendered html of the view + resources: A list of tuples where the first element is the resource hash, and + the second is the resource description + """ + if not settings.FEATURES.get('ENABLE_XBLOCK_VIEW_ENDPOINT', False): + log.warn("Attempt to use deactivated XBlock view endpoint -" + " see FEATURES['ENABLE_XBLOCK_VIEW_ENDPOINT']") + raise Http404 + + if not request.user.is_authenticated(): + raise PermissionDenied + + instance, tracking_context = _get_module_by_usage_id(request, course_id, usage_id) + + try: + fragment = instance.render(view_name, context=request.GET) + except NoSuchViewError: + log.exception("Attempt to render missing view on %s: %s", instance, view_name) + raise Http404 + + hashed_resources = OrderedDict() + for resource in fragment.resources: + hashed_resources[hash_resource(resource)] = resource + + return JsonResponse({ + 'html': fragment.content, + 'resources': hashed_resources.items(), + 'csrf_token': str(csrf(request)['csrf_token']), + }) + + def get_score_bucket(grade, max_grade): """ Function to split arbitrary score ranges into 3 buckets. diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 2a12a8c3ea..278a28a108 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -16,6 +16,7 @@ from django.contrib.auth.models import AnonymousUser from mock import MagicMock, patch, Mock from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey +from courseware.module_render import hash_resource from xblock.field_data import FieldData from xblock.runtime import Runtime from xblock.fields import ScopeIds @@ -246,6 +247,14 @@ class ModuleRenderTestCase(ModuleStoreTestCase, LoginEnrollmentTestCase): render.get_module_for_descriptor(self.mock_user, request, descriptor, field_data_cache, self.toy_course.id) render.get_module_for_descriptor(self.mock_user, request, descriptor, field_data_cache, self.toy_course.id) + def test_hash_resource(self): + """ + Ensure that the resource hasher works and does not fail on unicode, + decoded or otherwise. + """ + resources = ['ASCII text', u'❄ I am a special snowflake.', "❄ So am I, but I didn't tell you."] + self.assertEqual(hash_resource(resources), 'a76e27c8e80ca3efd7ce743093aa59e0') + class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): """ @@ -316,7 +325,7 @@ class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): json.dumps({ 'success': 'Submission aborted! Maximum %d files may be submitted at once' % settings.MAX_FILEUPLOADS_PER_INPUT - }) + }, indent=2) ) def test_too_large_file(self): @@ -336,7 +345,7 @@ class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): json.dumps({ 'success': 'Submission aborted! Your file "%s" is too large (max size: %d MB)' % (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE / (1000 ** 2)) - }) + }, indent=2) ) def test_xmodule_dispatch(self): @@ -399,6 +408,29 @@ class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): 'bad_dispatch', ) + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_XBLOCK_VIEW_ENDPOINT': True}) + def test_xblock_view_handler(self): + args = [ + 'edX/toy/2012_Fall', + quote_slashes('i4x://edX/toy/videosequence/Toy_Videos'), + 'student_view' + ] + xblock_view_url = reverse( + 'xblock_view', + args=args + ) + + request = self.request_factory.get(xblock_view_url) + request.user = self.mock_user + response = render.xblock_view(request, *args) + self.assertEquals(200, response.status_code) + + expected = ['csrf_token', 'html', 'resources'] + content = json.loads(response.content) + for section in expected: + self.assertIn(section, content) + self.assertIn('
[^/]*)(?:/(?P.*))?$'.format(course_key=settings.COURSE_ID_PATTERN, usage_key=settings.USAGE_ID_PATTERN), 'courseware.module_render.handle_xblock_callback', name='xblock_handler'), + url(r'^courses/{course_key}/xblock/{usage_key}/view/(?P[^/]*)$'.format( + course_key=settings.COURSE_ID_PATTERN, + usage_key=settings.USAGE_ID_PATTERN), + 'courseware.module_render.xblock_view', + name='xblock_view'), url(r'^courses/{course_key}/xblock/{usage_key}/handler_noauth/(?P[^/]*)(?:/(?P.*))?$'.format(course_key=settings.COURSE_ID_PATTERN, usage_key=settings.USAGE_ID_PATTERN), 'courseware.module_render.handle_xblock_callback_noauth', name='xblock_handler_noauth'), From 05c857478bfed379ffc105830b678ed603cc8376 Mon Sep 17 00:00:00 2001 From: Xavier Antoviaque Date: Wed, 26 Mar 2014 18:19:14 -0400 Subject: [PATCH 2/2] 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: