diff --git a/lti_consumer/lti_consumer.py b/lti_consumer/lti_consumer.py index 9afcff06e812dd48a95ef2fbbe8236fc68045905..6474341582d700995777ae5a26ace30ce2f71fc7 100644 --- a/lti_consumer/lti_consumer.py +++ b/lti_consumer/lti_consumer.py @@ -574,8 +574,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): try: lti_id, key, secret = [i.strip() for i in lti_passport.split(':')] except ValueError: - msg = self.ugettext('Could not parse LTI passport: {lti_passport}. Should be "id:key:secret" string.').\ - format(lti_passport='{0!r}'.format(lti_passport)) + msg = 'Could not parse LTI passport: {lti_passport!r}. Should be "id:key:secret" string.' + msg = self.ugettext(msg).format(lti_passport=lti_passport) raise LtiError(msg) if lti_id == self.lti_id.strip(): @@ -694,9 +694,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): param_name, param_value = [p.strip() for p in custom_parameter.split('=', 1)] except ValueError: _ = self.runtime.service(self, "i18n").ugettext - # pylint: disable=line-too-long - msg = self.ugettext('Could not parse custom parameter: {custom_parameter}. Should be "x=y" string.').\ - format(custom_parameter="{0!r}".format(custom_parameter)) + msg = 'Could not parse custom parameter: {custom_parameter!r}. Should be "x=y" string.' + msg = self.ugettext(msg).format(custom_parameter=custom_parameter) raise LtiError(msg) # LTI specs: 'custom_' should be prepended before each custom parameter, as pointed in link above. @@ -923,17 +922,12 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): dict: Context variables for templates """ - # use bleach defaults. see https://github.com/jsocol/bleach/blob/master/bleach/__init__.py - # ALLOWED_TAGS are - # ['a', 'abbr', 'acronym', 'b', 'blockquote', 'code', 'em', 'i', 'li', 'ol', 'strong', 'ul'] - # - # ALLOWED_ATTRIBUTES are - # 'a': ['href', 'title'], - # 'abbr': ['title'], - # 'acronym': ['title'], - # + # For more context on ALLOWED_TAGS and ALLOWED_ATTRIBUTES + # Look into this documentation URL see https://bleach.readthedocs.io/en/latest/clean.html#allowed-tags-tags # This lets all plaintext through. - sanitized_comment = bleach.clean(self.score_comment) + allowed_tags = bleach.sanitizer.ALLOWED_TAGS + ['img'] + allowed_attributes = dict(bleach.sanitizer.ALLOWED_ATTRIBUTES, **{'img': ['src', 'alt']}) + sanitized_comment = bleach.clean(self.score_comment, tags=allowed_tags, attributes=allowed_attributes) return { 'launch_url': self.launch_url.strip(), diff --git a/lti_consumer/tests/unit/test_lti_consumer.py b/lti_consumer/tests/unit/test_lti_consumer.py index 6bae920cf28b4b8233b9466da4a2ae15b068a807..5767ee66a5d0bdb6215079d967c80790e0d64f44 100644 --- a/lti_consumer/tests/unit/test_lti_consumer.py +++ b/lti_consumer/tests/unit/test_lti_consumer.py @@ -303,6 +303,7 @@ class TestEditableFields(TestLtiConsumerXBlock): """ Unit tests for LtiConsumerXBlock.editable_fields """ + def get_mock_lti_configuration(self, editable): """ Returns a mock object of lti-configuration service @@ -805,6 +806,7 @@ class TestParseSuffix(TestLtiConsumerXBlock): self.assertEqual(parsed, FAKE_USER_ID) +@ddt.ddt class TestGetContext(TestLtiConsumerXBlock): """ Unit tests for LtiConsumerXBlock._get_context_for_template() @@ -825,6 +827,27 @@ class TestGetContext(TestLtiConsumerXBlock): for key in context_keys: self.assertIn(key, context) + @ddt.data('a', 'abbr', 'acronym', 'b', 'blockquote', 'code', 'em', 'i', 'li', 'ol', 'strong', 'ul', 'img') + def test_comment_allowed_tags(self, tag): + """ + Test that allowed tags are not escaped in context['comment'] + """ + comment = u'<{0}>This is a comment</{0}>!'.format(tag) + self.xblock.set_user_module_score(Mock(), 0.92, 1.0, comment) + context = self.xblock._get_context_for_template() # pylint: disable=protected-access + + self.assertIn('<{}>'.format(tag), context['comment']) + + def test_comment_retains_image_src(self): + """ + Test that image tag has src and other attrs are sanitized + """ + comment = u'<img src="example.com/image.jpeg" onerror="myFunction()">' + self.xblock.set_user_module_score(Mock(), 0.92, 1.0, comment) + context = self.xblock._get_context_for_template() # pylint: disable=protected-access + + self.assertIn(u'<img src="example.com/image.jpeg">', context['comment']) + @ddt.ddt class TestProcessorSettings(TestLtiConsumerXBlock): diff --git a/setup.py b/setup.py index f623e7ef803f022aa948852868c3c2e5caa92331..99d1da9d1d581cf4fc622ddbbe9268f9f9f00b78 100644 --- a/setup.py +++ b/setup.py @@ -25,7 +25,7 @@ def package_data(pkg, roots): setup( name='lti_consumer-xblock', - version='1.2.3', + version='1.2.4', description='This XBlock implements the consumer side of the LTI specification.', packages=[ 'lti_consumer',