diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 6e86a6485b3df5631666a3285a52fbe0c678c69c..ab1375554d60ba001f6646e2094ef56a0b89344b 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -115,7 +115,7 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ An XModule ModuleSystem for use in Studio previews """ - def handler_url(self, block, handler_name, suffix='', query=''): + def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): return handler_prefix(block, handler_name, suffix) + '?' + query diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 90c7f4500985bb65bab3d9ab7598c87d5091d7c7..bfb5bd7f93e4fbd2ea091bc7404d4b9006b981d6 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -42,7 +42,7 @@ class TestModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ ModuleSystem for testing """ - def handler_url(self, block, handler, suffix='', query=''): + def handler_url(self, block, handler, suffix='', query='', thirdparty=False): return str(block.scope_ids.usage_id) + '/' + handler + '/' + suffix + '?' + query diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index d39095586c238df8255645e9ecb5bf44ea63aa31..d4b7c01439ef7cdbc5cb57615ee1d0f3bc6a6f13 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -403,6 +403,7 @@ class XModule(XModuleMixin, HTMLSnippet, XBlock): # pylint: disable=abstract-me data is a dictionary-like object with the content of the request""" return u"" + @XBlock.handler def xmodule_handler(self, request, suffix=None): """ XBlock handler that wraps `handle_ajax` diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 1539b883d34fde7a82e293a05fcc2f8de40481e1..f4bb129e3dde828449aadca910ad953edf76d594 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -14,7 +14,7 @@ from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.http import Http404 from django.http import HttpResponse -from django.views.decorators.csrf import csrf_exempt +from django.views.decorators.csrf import csrf_exempt, csrf_protect from capa.xqueue_interface import XQueueInterface from courseware.access import has_access @@ -482,6 +482,14 @@ def xqueue_callback(request, course_id, userid, mod_id, dispatch): return HttpResponse("") +@csrf_exempt +def handle_xblock_callback_noauth(request, course_id, usage_id, handler, suffix=None): + """ + Entry point for unauthenticated XBlock handlers. + """ + return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, request.user) + + def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): """ Generic view for extensions. This is where AJAX calls go. @@ -496,6 +504,17 @@ def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): the location and course_id do not identify a valid module, the module is not accessible by the user, or the module raises NotFoundError. If the module raises any other error, it will escape this function. + """ + if not request.user.is_authenticated(): + raise PermissionDenied + + return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, request.user) + + +def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, user): + """ + Invoke an XBlock handler, either authenticated or not. + """ location = unquote_slashes(usage_id) @@ -503,9 +522,6 @@ def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): if not Location.is_valid(location): raise Http404("Invalid location") - if not request.user.is_authenticated(): - raise PermissionDenied - # Check submitted files files = request.FILES or {} error_msg = _check_files_limits(files) @@ -525,15 +541,14 @@ def handle_xblock_callback(request, course_id, usage_id, handler, suffix=None): field_data_cache = FieldDataCache.cache_for_descriptor_descendents( course_id, - request.user, + user, descriptor ) - - instance = get_module(request.user, request, location, field_data_cache, course_id, grade_bucket_type='ajax') + instance = get_module(user, request, location, field_data_cache, course_id, grade_bucket_type='ajax') if instance is None: # Either permissions just changed, or someone is trying to be clever # and load something they shouldn't have access to. - log.debug("No module %s for user %s -- access denied?", location, request.user) + log.debug("No module %s for user %s -- access denied?", location, user) raise Http404 req = django_to_webob_request(request) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 3f4f3b673b2ec143b54ec119ab0c1eae39d7ccd5..38f42d1f49bcc4e7a904dc7f9eab58a675c64e36 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -185,9 +185,11 @@ class TestHandleXBlockCallback(ModuleStoreTestCase, LoginEnrollmentTestCase): return mock_file def test_invalid_location(self): + request = self.request_factory.post('dummy_url', data={'position': 1}) + request.user = self.mock_user with self.assertRaises(Http404): render.handle_xblock_callback( - None, + request, 'dummy/course/id', 'invalid Location', 'dummy_handler' diff --git a/lms/lib/xblock/runtime.py b/lms/lib/xblock/runtime.py index cecdd4d121dabeb784852705b28988a5f487dc00..4fa48be83a83dd074920406a5ef39f6394abf4f1 100644 --- a/lms/lib/xblock/runtime.py +++ b/lms/lib/xblock/runtime.py @@ -58,11 +58,31 @@ def unquote_slashes(text): return re.sub(r'(;;|;_)', _unquote_slashes, text) -def handler_url(course_id, block, handler, suffix='', query=''): +def handler_url(course_id, block, handler, suffix='', query='', thirdparty=False): """ - Return an xblock handler url for the specified course, block and handler + Return an XBlock handler url for the specified course, block and handler. + + If handler is an empty string, this function is being used to create a + prefix of the general URL, which is assumed to be followed by handler name + and suffix. + + If handler is specified, then it is checked for being a valid handler + function, and ValueError is raised if not. + """ - return reverse('xblock_handler', kwargs={ + view_name = 'xblock_handler' + if handler: + # Be sure this is really a handler. + func = getattr(block, handler, None) + if not func: + raise ValueError("{!r} is not a function name".format(handler)) + if not getattr(func, "_is_xblock_handler", False): + raise ValueError("{!r} is not a handler name".format(handler)) + + if thirdparty: + view_name = 'xblock_handler_noauth' + + return reverse(view_name, kwargs={ 'course_id': course_id, 'usage_id': quote_slashes(str(block.scope_ids.usage_id)), 'handler': handler, @@ -72,8 +92,11 @@ def handler_url(course_id, block, handler, suffix='', query=''): def handler_prefix(course_id, block): """ - Returns a prefix for use by the javascript handler_url function. - The prefix is a valid handler url the handler name is appended to it. + Returns a prefix for use by the Javascript handler_url function. + + The prefix is a valid handler url after the handler name is slash-appended + to it. + """ return handler_url(course_id, block, '').rstrip('/') @@ -86,10 +109,11 @@ class LmsHandlerUrls(object): This must be mixed in to a runtime that already accepts and stores a course_id """ - - def handler_url(self, block, handler_name, suffix='', query=''): # pylint: disable=unused-argument + # pylint: disable=unused-argument + # pylint: disable=no-member + def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): """See :method:`xblock.runtime:Runtime.handler_url`""" - return handler_url(self.course_id, block, handler_name, suffix='', query='') # pylint: disable=no-member + return handler_url(self.course_id, block, handler_name, suffix='', query='', thirdparty=thirdparty) class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract-method diff --git a/lms/urls.py b/lms/urls.py index 4aec95d01fc1994d6b8e6f7a69d528dfbec701cd..6de00a741103c9b2d69355ffea27df0ccd672d69 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -178,7 +178,9 @@ if settings.COURSEWARE_ENABLED: url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/xblock/(?P<usage_id>[^/]*)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$', 'courseware.module_render.handle_xblock_callback', name='xblock_handler'), - + url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/xblock/(?P<usage_id>[^/]*)/handler_noauth/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$', + 'courseware.module_render.handle_xblock_callback_noauth', + name='xblock_handler_noauth'), # Software Licenses diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 26d12f2af93f73ca470d6497c520ca03e32ba300..3f00a7c9159da62138d91ee67a2e26389edd164e 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -15,7 +15,7 @@ -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@d6d2fc91#egg=XBlock +-e git+https://github.com/edx/XBlock.git@341d162f353289cfd3974a4f4f9354ce81ab60db#egg=XBlock -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.2.6#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.1.4#egg=js_test_tool