From ef23c19bf4195f9c70032cc13c2d88a408ad699c Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 23 Sep 2014 15:02:27 -0400 Subject: [PATCH] Prevent queries into split using deprecated CourseLocators, BlockUsageLocators, or DefinitionLocators --- .../xmodule/modulestore/split_mongo/split.py | 40 +++++++++++++++++-- .../tests/test_mixed_modulestore.py | 3 +- .../xmodule/modulestore/xml_exporter.py | 14 +++---- .../xmodule/modulestore/xml_importer.py | 2 + requirements/edx/github.txt | 2 +- 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 94d8380e23..544415da04 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -773,7 +773,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): ''' Gets the course descriptor for the course identified by the locator ''' - if not isinstance(course_id, CourseLocator): + if not isinstance(course_id, CourseLocator) or course_id.deprecated: # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. raise ItemNotFoundError(course_id) @@ -791,7 +791,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): Note: we return the course_id instead of a boolean here since the found course may have a different id than the given course_id when ignore_case is True. ''' - if not isinstance(course_id, CourseLocator): + if not isinstance(course_id, CourseLocator) or course_id.deprecated: # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. return False @@ -804,6 +804,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): the course or the block w/in the course do not exist for the given version. raises InsufficientSpecificationError if the usage_key does not id a block """ + if not isinstance(usage_key, BlockUsageLocator) or usage_key.deprecated: + # The supplied UsageKey is of the wrong type, so it can't possibly be stored in this modulestore. + return False + if usage_key.block_id is None: raise InsufficientSpecificationError(usage_key) try: @@ -823,7 +827,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): descendants. raises InsufficientSpecificationError or ItemNotFoundError """ - if not isinstance(usage_key, BlockUsageLocator): + if not isinstance(usage_key, BlockUsageLocator) or usage_key.deprecated: # The supplied UsageKey is of the wrong type, so it can't possibly be stored in this modulestore. raise ItemNotFoundError(usage_key) @@ -856,6 +860,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): For split, you can search by ``edited_by``, ``edited_on`` providing a function testing limits. """ + if not isinstance(course_locator, CourseLocator) or course_locator.deprecated: + # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + return [] + course = self._lookup_course(course_locator) items = [] qualifiers = qualifiers.copy() if qualifiers else {} # copy the qualifiers (destructively manipulated here) @@ -910,6 +918,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): :param locator: BlockUsageLocator restricting search scope ''' + if not isinstance(locator, BlockUsageLocator) or locator.deprecated: + # The supplied locator is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(locator) + course = self._lookup_course(locator.course_key) parent_id = self._get_parent_from_structure(BlockKey.from_usage_key(locator), course.structure) if parent_id is None: @@ -924,6 +936,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ Return an array of all of the orphans in the course. """ + if not isinstance(course_key, CourseLocator) or course_key.deprecated: + # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(course_key) + detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")] course = self._lookup_course(course_key) items = set(course.structure['blocks'].keys()) @@ -952,6 +968,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): 'edited_on': when the course was originally created } """ + if not isinstance(course_key, CourseLocator) or course_key.deprecated: + # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(course_key) + if not (course_key.course and course_key.run and course_key.org): return None index = self.get_course_index(course_key) @@ -969,6 +989,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): 'edited_on': when the change was made } """ + if not isinstance(course_key, CourseLocator) or course_key.deprecated: + # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(course_key) + course = self._lookup_course(course_key).structure return { 'original_version': course['original_version'], @@ -987,6 +1011,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): 'edited_on': when the change was made } """ + if not isinstance(definition_locator, DefinitionLocator) or definition_locator.deprecated: + # The supplied locator is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(definition_locator) + definition = self.db_connection.get_definition(definition_locator.definition_id) if definition is None: return None @@ -999,6 +1027,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): next versions, these do include those created for other courses. :param course_locator: ''' + if not isinstance(course_locator, CourseLocator) or course_locator.deprecated: + # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(course_locator) + if version_history_depth < 1: return None if course_locator.version_guid is None: @@ -1882,7 +1914,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): change to this item, it raises a VersionConflictError unless force is True. In the force case, it forks the course but leaves the head pointer where it is (this change will not be in the course head). """ - if not isinstance(usage_locator, BlockUsageLocator): + if not isinstance(usage_locator, BlockUsageLocator) or usage_locator.deprecated: # The supplied UsageKey is of the wrong type, so it can't possibly be stored in this modulestore. raise ItemNotFoundError(usage_locator) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 7dc26c4b8c..d3b1469519 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -245,7 +245,8 @@ class TestMixedModuleStore(CourseComparisonTest): for course_id, course_key in self.course_locations.iteritems() # pylint: disable=maybe-no-member } - self.fake_location = self.course_locations[self.MONGO_COURSEID].course_key.make_usage_key('vertical', 'fake') + mongo_course_key = self.course_locations[self.MONGO_COURSEID].course_key + self.fake_location = self.store.make_course_key(mongo_course_key.org, mongo_course_key.course, mongo_course_key.run).make_usage_key('vertical', 'fake') self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace( category='chapter', name='Overview' diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 180d840781..6b1bb275a4 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -85,16 +85,16 @@ def export_to_xml(modulestore, contentstore, course_key, root_dir, course_dir): course_image_file.write(course_image.data) # export the static tabs - export_extra_content(export_fs, modulestore, xml_centric_course_key, 'static_tab', 'tabs', '.html') + export_extra_content(export_fs, modulestore, course_key, xml_centric_course_key, 'static_tab', 'tabs', '.html') # export the custom tags - export_extra_content(export_fs, modulestore, xml_centric_course_key, 'custom_tag_template', 'custom_tags') + export_extra_content(export_fs, modulestore, course_key, xml_centric_course_key, 'custom_tag_template', 'custom_tags') # export the course updates - export_extra_content(export_fs, modulestore, xml_centric_course_key, 'course_info', 'info', '.html') + export_extra_content(export_fs, modulestore, course_key, xml_centric_course_key, 'course_info', 'info', '.html') # export the 'about' data (e.g. overview, etc.) - export_extra_content(export_fs, modulestore, xml_centric_course_key, 'about', 'about', '.html') + export_extra_content(export_fs, modulestore, course_key, xml_centric_course_key, 'about', 'about', '.html') # export the grading policy course_run_policy_dir = policies_dir.makeopendir(course.location.name) @@ -183,13 +183,13 @@ def _export_field_content(xblock_item, item_dir): field_content_file.write(dumps(module_data.get(field_name, {}), cls=EdxJSONEncoder, sort_keys=True, indent=4)) -def export_extra_content(export_fs, modulestore, course_key, category_type, dirname, file_suffix=''): - items = modulestore.get_items(course_key, qualifiers={'category': category_type}) +def export_extra_content(export_fs, modulestore, source_course_key, dest_course_key, category_type, dirname, file_suffix=''): + items = modulestore.get_items(source_course_key, qualifiers={'category': category_type}) if len(items) > 0: item_dir = export_fs.makeopendir(dirname) for item in items: - adapt_references(item, course_key, export_fs) + adapt_references(item, dest_course_key, export_fs) with item_dir.open(item.location.name + file_suffix, 'w') as item_file: item_file.write(item.data.encode('utf8')) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 092fb08696..79304da535 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -42,6 +42,7 @@ from xmodule.modulestore.django import ASSET_IGNORE_REGEX from xmodule.modulestore.exceptions import DuplicateCourseError from xmodule.modulestore.mongo.base import MongoRevisionKey from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.exceptions import ItemNotFoundError log = logging.getLogger(__name__) @@ -588,6 +589,7 @@ def _import_course_draft( # IMPORTANT: Be sure to update the sequential in the NEW namespace seq_location = seq_location.map_into_course(target_course_id) + sequential = store.get_item(seq_location, depth=0) non_draft_location = module.location.map_into_course(target_course_id) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 7977dd679d..6fc67a9c07 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -29,7 +29,7 @@ -e git+https://github.com/edx-solutions/django-splash.git@7579d052afcf474ece1239153cffe1c89935bc4f#egg=django-splash -e git+https://github.com/edx/acid-block.git@459aff7b63db8f2c5decd1755706c1a64fb4ebb1#egg=acid-xblock -e git+https://github.com/edx/edx-ora2.git@release-2014-09-18T16.00#egg=edx-ora2 --e git+https://github.com/edx/opaque-keys.git@013e30b4c4909b55e03c409a90ec6ef805903e04#egg=opaque-keys +-e git+https://github.com/edx/opaque-keys.git@295d93170b2f6e57e3a2b9ba0a52087a4e8712c5#egg=opaque-keys -e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease -e git+https://github.com/edx/i18n-tools.git@56f048af9b6868613c14aeae760548834c495011#egg=i18n-tools -e git+https://github.com/edx/edx-oauth2-provider.git@0.2.1#egg=oauth2-provider