diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py index 9a2b57af34..ea886e7777 100644 --- a/common/djangoapps/util/views.py +++ b/common/djangoapps/util/views.py @@ -19,7 +19,7 @@ import calc from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from edxmako.shortcuts import render_to_response, render_to_string from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -53,6 +53,26 @@ def ensure_valid_course_key(view_func): return inner +def ensure_valid_usage_key(view_func): + """ + This decorator should only be used with views which have argument usage_key_string. + If usage_key_string is not valid raise 404. + """ + @wraps(view_func) + def inner(request, *args, **kwargs): # pylint: disable=missing-docstring + usage_key = kwargs.get('usage_key_string') + if usage_key is not None: + try: + UsageKey.from_string(usage_key) + except InvalidKeyError: + raise Http404 + + response = view_func(request, *args, **kwargs) + return response + + return inner + + def require_global_staff(func): """View decorator that requires that the user have global staff permissions. """ @wraps(func) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 0fde44ae70..4c0ddc7af3 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1992,11 +1992,19 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase): reload_django_url_config() super(TestRenderXBlock, self).setUp() - def get_response(self, url_encoded_params=None): + def test_render_xblock_with_invalid_usage_key(self): + """ + Test XBlockRendering with invalid usage key + """ + response = self.get_response(usage_key='some_invalid_usage_key') + self.assertEqual(response.status_code, 404) + self.assertIn('Page not found', response.content) + + def get_response(self, url_encoded_params=None, usage_key=None): """ Overridable method to get the response from the endpoint that is being tested. """ - url = reverse('render_xblock', kwargs={"usage_key_string": unicode(self.html_block.location)}) + url = reverse('render_xblock', kwargs={'usage_key_string': unicode(usage_key)}) if url_encoded_params: url += '?' + url_encoded_params return self.client.get(url) diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index 42dc9ec80d..e1fd191eea 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -41,12 +41,13 @@ class RenderXBlockTestMixin(object): ] @abstractmethod - def get_response(self, url_encoded_params=None): + def get_response(self, url_encoded_params=None, usage_key=None): """ Abstract method to get the response from the endpoint that is being tested. Arguments: url_encoded_params - URL encoded parameters that should be appended to the requested URL. + usage_key - course usage key. """ pass # pragma: no cover @@ -96,7 +97,7 @@ class RenderXBlockTestMixin(object): """ if url_params: url_params = urlencode(url_params) - response = self.get_response(url_params) + response = self.get_response(url_params, self.html_block.location) if expected_response_code == 200: self.assertContains(response, self.html_block.data, status_code=expected_response_code) for chrome_element in [self.COURSEWARE_CHROME_HTML_ELEMENTS + self.XBLOCK_REMOVED_HTML_ELEMENTS]: diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index b28c2162c5..2a3813aa27 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -88,7 +88,7 @@ from util.date_utils import strftime_localized from util.db import outer_atomic from util.milestones_helpers import get_prerequisite_courses_display from util.views import _record_feedback_in_zendesk -from util.views import ensure_valid_course_key +from util.views import ensure_valid_course_key, ensure_valid_usage_key from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from xmodule.tabs import CourseTabList @@ -1236,12 +1236,14 @@ def _track_successful_certificate_generation(user_id, course_id): # pylint: dis @require_http_methods(["GET", "POST"]) +@ensure_valid_usage_key def render_xblock(request, usage_key_string, check_if_enrolled=True): """ Returns an HttpResponse with HTML content for the xBlock with the given usage_key. The returned HTML is a chromeless rendering of the xBlock (excluding content of the containing courseware). """ usage_key = UsageKey.from_string(usage_key_string) + usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key)) course_key = usage_key.course_key diff --git a/lms/djangoapps/lti_provider/tests/test_views.py b/lms/djangoapps/lti_provider/tests/test_views.py index b66d6bc4a7..4cd2fb145a 100644 --- a/lms/djangoapps/lti_provider/tests/test_views.py +++ b/lms/djangoapps/lti_provider/tests/test_views.py @@ -170,7 +170,7 @@ class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCa This class overrides the get_response method, which is used by the tests defined in RenderXBlockTestMixin. """ - def get_response(self, url_encoded_params=None): + def get_response(self, url_encoded_params=None, usage_key=None): """ Overridable method to get the response from the endpoint that is being tested. """ @@ -178,7 +178,7 @@ class LtiLaunchTestRender(LtiTestMixin, RenderXBlockTestMixin, ModuleStoreTestCa 'lti_provider_launch', kwargs={ 'course_id': unicode(self.course.id), - 'usage_id': unicode(self.html_block.location) + 'usage_id': unicode(usage_key) } ) if url_encoded_params: