From d9ce3162e017acf2881fa26c7460386dbacb5c8b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 10 Jun 2013 11:47:37 -0400 Subject: [PATCH 1/2] partial fix for STUD-145, catch invalid location errors and return a HTTP 400 error to client --- .../contentstore/tests/test_contentstore.py | 11 +++++++++ .../contentstore/views/component.py | 24 +++++++++++++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 771871e9bc..a6b4d1f57c 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -108,6 +108,17 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): for expected in expected_types: self.assertIn(expected, resp.content) + def test_malformed_edit_unit_request(self): + store = modulestore('direct') + import_from_xml(store, 'common/test/data/', ['simple']) + + # just pick one vertical + descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0] + location = descriptor.location._replace(name='.' + descriptor.location.name) + + resp = self.client.get(reverse('edit_unit', kwargs={'location': location.url()})) + self.assertEqual(resp.status_code, 400) + def test_advanced_components_in_edit_unit(self): # This could be made better, but for now let's just assert that we see the advanced modules mentioned in the page # response HTML diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 8120e08107..039deb2740 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -7,7 +7,7 @@ from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django_future.csrf import ensure_csrf_cookie from django.conf import settings - +from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from mitxmako.shortcuts import render_to_response from xmodule.modulestore import Location @@ -50,11 +50,18 @@ ADVANCED_COMPONENT_POLICY_KEY = 'advanced_modules' @login_required def edit_subsection(request, location): # check that we have permissions to edit this item - course = get_course_for_item(location) + try: + course = get_course_for_item(location) + except InvalidLocationError: + return HttpResponseBadRequest() + if not has_access(request.user, course.location): raise PermissionDenied() - item = modulestore().get_item(location, depth=1) + try: + item = modulestore().get_item(location, depth=1) + except ItemNotFoundError: + return HttpResponseBadRequest() lms_link = get_lms_link_for_item(location, course_id=course.location.course_id) preview_link = get_lms_link_for_item(location, course_id=course.location.course_id, preview=True) @@ -113,11 +120,18 @@ def edit_unit(request, location): id: A Location URL """ - course = get_course_for_item(location) + try: + course = get_course_for_item(location) + except InvalidLocationError: + return HttpResponseBadRequest() + if not has_access(request.user, course.location): raise PermissionDenied() - item = modulestore().get_item(location, depth=1) + try: + item = modulestore().get_item(location, depth=1) + except ItemNotFoundError: + return HttpResponseBadRequest() lms_link = get_lms_link_for_item(item.location, course_id=course.location.course_id) From 0c6991822b89c8e1c17c8b956725e61310bc7a4d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 10 Jun 2013 12:46:03 -0400 Subject: [PATCH 2/2] move test so it doesn't split the tests from a helper method --- .../contentstore/tests/test_contentstore.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index a6b4d1f57c..92bed5e187 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -108,17 +108,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): for expected in expected_types: self.assertIn(expected, resp.content) - def test_malformed_edit_unit_request(self): - store = modulestore('direct') - import_from_xml(store, 'common/test/data/', ['simple']) - - # just pick one vertical - descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0] - location = descriptor.location._replace(name='.' + descriptor.location.name) - - resp = self.client.get(reverse('edit_unit', kwargs={'location': location.url()})) - self.assertEqual(resp.status_code, 400) - def test_advanced_components_in_edit_unit(self): # This could be made better, but for now let's just assert that we see the advanced modules mentioned in the page # response HTML @@ -131,6 +120,17 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_advanced_components_require_two_clicks(self): self.check_components_on_page(['videoalpha'], ['Video Alpha']) + def test_malformed_edit_unit_request(self): + store = modulestore('direct') + import_from_xml(store, 'common/test/data/', ['simple']) + + # just pick one vertical + descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0] + location = descriptor.location._replace(name='.' + descriptor.location.name) + + resp = self.client.get(reverse('edit_unit', kwargs={'location': location.url()})) + self.assertEqual(resp.status_code, 400) + def check_edit_unit(self, test_course_name): import_from_xml(modulestore('direct'), 'common/test/data/', [test_course_name])