diff --git a/contentcuration/contentcuration/tests/test_exportchannel.py b/contentcuration/contentcuration/tests/test_exportchannel.py index 6516117a3e..397c7f86ed 100644 --- a/contentcuration/contentcuration/tests/test_exportchannel.py +++ b/contentcuration/contentcuration/tests/test_exportchannel.py @@ -39,6 +39,7 @@ from .testdata import thumbnail_bytes from .testdata import tree from .utils.qti.test_validation import _item_xml +from .utils.qti.test_validation import ENTITY_CHOICE_ITEM from .utils.qti.test_validation import VALID_CHOICE_ITEM from .utils.restricted_filesystemstorage import RestrictedFileSystemStorage from contentcuration import models as cc @@ -332,6 +333,28 @@ def setUp(self): randomize=False, ) + native_qti_entity_exercise = create_node( + { + "kind_id": "exercise", + "title": "Native QTI Entity Exercise", + "extra_fields": qti_extra_fields, + } + ) + native_qti_entity_exercise.complete = True + native_qti_entity_exercise.parent = current_exercise.parent + native_qti_entity_exercise.save() + cc.AssessmentItem.objects.create( + contentnode=native_qti_entity_exercise, + assessment_id=uuid.uuid4().hex, + type=exercises.QTI, + question="", + answers="[]", + hints="[]", + raw_data=ENTITY_CHOICE_ITEM, + order=1, + randomize=False, + ) + # Only legacy structured-field items, no perseus_question -> must now route to QTI (was Perseus) legacy_no_perseus_exercise = create_node( { @@ -1015,6 +1038,14 @@ def test_native_qti_unsupported_interaction_publishes_qti_only(self): self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists()) + def test_native_qti_entity_item_is_excluded_from_qti_only_package(self): + node = cc.ContentNode.objects.get(title="Native QTI Entity Exercise") + self.assertFalse(node.files.filter(preset_id=format_presets.EXERCISE).exists()) + qti_file = node.files.get(preset_id=format_presets.QTI_ZIP) + with qti_file.file_on_disk.open("rb") as file_handle: + names = zipfile.ZipFile(file_handle).namelist() + self.assertEqual([name for name in names if name.startswith("items/")], []) + def test_legacy_items_without_perseus_question_route_to_qti_packaging(self): node = cc.ContentNode.objects.get(title="Legacy No Perseus Exercise") self.assertTrue(node.files.filter(preset_id=format_presets.QTI_ZIP).exists()) diff --git a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py b/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py index 7cd6e58f91..6ff262fad0 100644 --- a/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py +++ b/contentcuration/contentcuration/tests/utils/qti/test_perseus_derive.py @@ -4,6 +4,7 @@ from le_utils.constants import exercises from contentcuration.tests.utils.qti.test_validation import _item_xml +from contentcuration.tests.utils.qti.test_validation import ENTITY_CHOICE_ITEM from contentcuration.tests.utils.qti.test_validation import HINTED_EDITOR_ITEM from contentcuration.utils.assessment.qti.perseus_derive import derive_perseus_item from contentcuration.utils.assessment.qti.perseus_derive import is_perseus_derivable @@ -279,6 +280,7 @@ def test_response_identifier_mismatch_degrades_to_qti_only(): pytest.param(TWO_INTERACTION_ITEM, id="two_interactions"), pytest.param(EXTENDED_TEXT_ITEM, id="extended_text"), pytest.param(MALFORMED_XML, id="malformed_xml"), + pytest.param(ENTITY_CHOICE_ITEM, id="doctype_entity"), ], ) def test_not_derivable(raw_data): diff --git a/contentcuration/contentcuration/tests/utils/qti/test_validation.py b/contentcuration/contentcuration/tests/utils/qti/test_validation.py index 02cd2abc06..fe22bdd917 100644 --- a/contentcuration/contentcuration/tests/utils/qti/test_validation.py +++ b/contentcuration/contentcuration/tests/utils/qti/test_validation.py @@ -47,6 +47,14 @@ def _item_xml(identifier, title, response_declaration, item_body): "", ) +_XML_DECLARATION = '' + +_ENTITY_DOCTYPE = ']>' + +ENTITY_CHOICE_ITEM = VALID_CHOICE_ITEM.replace( + _XML_DECLARATION, _XML_DECLARATION + _ENTITY_DOCTYPE +).replace("Option A", "Option &e;") + class ValidateQTIItemTests(unittest.TestCase): def test_accepts_valid_item(self): @@ -96,6 +104,46 @@ def test_does_not_resolve_external_entities(self): serialized = " ".join(e.message for e in result.errors) self.assertNotIn("super-secret-value", serialized) + def test_rejects_entity_reference_without_raising(self): + result = validate_qti_item(ENTITY_CHOICE_ITEM) + self.assertFalse(result.is_valid) + self.assertEqual(len(result.errors), 1) + + def test_rejects_external_entity_reference_in_body(self): + with tempfile.NamedTemporaryFile(mode="w", suffix=".txt", delete=False) as f: + f.write("super-secret-value") + secret_path = f.name + self.addCleanup(os.remove, secret_path) + xml = VALID_CHOICE_ITEM.replace( + _XML_DECLARATION, + _XML_DECLARATION + + ']>' + % secret_path, + ).replace("Option A", "Option &e;") + result = validate_qti_item(xml) + self.assertFalse(result.is_valid) + self.assertNotIn( + "super-secret-value", " ".join(e.message for e in result.errors) + ) + + def test_accepts_entity_in_attribute_value(self): + xml = VALID_CHOICE_ITEM.replace( + _XML_DECLARATION, _XML_DECLARATION + _ENTITY_DOCTYPE + ).replace('title="Sample Item"', 'title="&e;"') + self.assertTrue(validate_qti_item(xml).is_valid) + + def test_accepts_unused_entity_declaration(self): + xml = VALID_CHOICE_ITEM.replace( + _XML_DECLARATION, _XML_DECLARATION + _ENTITY_DOCTYPE + ) + self.assertTrue(validate_qti_item(xml).is_valid) + + def test_accepts_bare_doctype(self): + xml = VALID_CHOICE_ITEM.replace( + _XML_DECLARATION, _XML_DECLARATION + "" + ) + self.assertTrue(validate_qti_item(xml).is_valid) + MATCH_INTERACTION_ITEM = _item_xml( "item_match", diff --git a/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py b/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py index ee76130ae8..0ac5377d9f 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py +++ b/contentcuration/contentcuration/tests/viewsets/test_assessmentitem.py @@ -9,6 +9,7 @@ from contentcuration.tests import testdata from contentcuration.tests.base import StudioAPITestCase from contentcuration.tests.utils.qti.test_validation import _item_xml +from contentcuration.tests.utils.qti.test_validation import ENTITY_CHOICE_ITEM from contentcuration.tests.utils.qti.test_validation import VALID_CHOICE_ITEM from contentcuration.tests.viewsets.base import generate_create_event from contentcuration.tests.viewsets.base import generate_delete_event @@ -1087,6 +1088,27 @@ def test_invalid_qti_xml_syntax_assessmentitem(self): assessment_id=assessmentitem["assessment_id"] ) + def test_qti_entity_reference_assessmentitem(self): + self.client.force_authenticate(user=self.user) + assessmentitem = self.assessmentitem_metadata + assessmentitem["type"] = "QTI" + assessmentitem["raw_data"] = ENTITY_CHOICE_ITEM + response = self.sync_changes( + [ + generate_create_event( + [assessmentitem["contentnode"], assessmentitem["assessment_id"]], + ASSESSMENTITEM, + assessmentitem, + channel_id=self.channel.id, + ), + ], + ) + self.assertTrue(response.json()["errors"][0]["errors"]["raw_data"]) + with self.assertRaises(models.AssessmentItem.DoesNotExist): + models.AssessmentItem.objects.get( + assessment_id=assessmentitem["assessment_id"] + ) + def test_valid_answers_assessmentitem(self): self.client.force_authenticate(user=self.user) assessmentitem = self.assessmentitem_metadata diff --git a/contentcuration/contentcuration/utils/assessment/qti/validation.py b/contentcuration/contentcuration/utils/assessment/qti/validation.py index b2f2243dbe..8579320f7a 100644 --- a/contentcuration/contentcuration/utils/assessment/qti/validation.py +++ b/contentcuration/contentcuration/utils/assessment/qti/validation.py @@ -55,7 +55,14 @@ def secure_parser() -> etree.XMLParser: def parse_qti_xml(xml: bytes) -> etree._Element: - return etree.parse(BytesIO(xml), parser=secure_parser()) + doc = etree.parse(BytesIO(xml), parser=secure_parser()) + # Unresolved entity nodes crash schema validation and derivation. + entity = next(doc.iter(etree.Entity), None) + if entity is not None: + raise etree.XMLSyntaxError( + "entity references are not supported", 0, entity.sourceline or 0, 1 + ) + return doc def validate_qti_item(xml: Union[str, bytes]) -> QTIValidationResult: