From bbb0bc77d969d896b112b62f19eee4ac845603cb Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Wed, 2 Mar 2022 14:01:42 -0500 Subject: [PATCH] revert: reverts 53041a2, causing email issues [MICROBA-1666] This reverts commit 53041a2d34906eafaeff7452dc37f733ad2b2395 after course team started reporting issues of images in emails not respecting dimensions set with the email editor. After a brief investigation we found unexpected attributes (like `width` and `height` of an image) being stripped from the HTML. --- lms/djangoapps/instructor/tests/test_api.py | 35 --------------------- lms/djangoapps/instructor/views/api.py | 8 +---- lms/envs/common.py | 9 ------ 3 files changed, 1 insertion(+), 51 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 4ed9e179d4..77bef6407a 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -10,7 +10,6 @@ import shutil import tempfile from unittest.mock import Mock, NonCallableMock, patch -import bleach import ddt import pytest from boto.exception import BotoServerError @@ -3484,40 +3483,6 @@ class TestInstructorSendEmail(SiteMixin, SharedModuleStoreTestCase, LoginEnrollm html_message=self.full_test_message['message'], template_name=org_template, from_addr=org_email).count() - def test_send_email_and_sanitize_content(self): - test_subject = 'sanitization test subject' - test_message = """ -

Welcome to course101!

-

We are going to do all the learning together.

- -
-
-

- -
- """ - message = { - 'send_to': '["myself", "staff"]', - 'subject': test_subject, - 'message': test_message, - } - sanitized_subject = bleach.clean(test_subject, tags=settings.BULK_COURSE_EMAIL_ALLOWED_HTML_TAGS) - sanitized_message = bleach.clean(test_message, tags=settings.BULK_COURSE_EMAIL_ALLOWED_HTML_TAGS) - - url = reverse('send_email', kwargs={'course_id': str(self.course.id)}) - response = self.client.post(url, message) - - email = CourseEmail.objects.filter(course_id=self.course.id, sender=self.instructor) - - assert response.status_code == 200 - assert email[0].subject == sanitized_subject - assert email[0].html_message == sanitized_message - - # deeper verification, confirm `h1` element hasn't been stripped from message content - assert "

" in email[0].html_message - # deeper verification, confirm `script` element has been stripped from message content - assert "<script>Content inside script tag</script>" in email[0].html_message - class MockCompletionInfo: """Mock for get_task_completion_info""" diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 1b58c0fc61..67cc72b68b 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -13,7 +13,6 @@ import string import random import re -import bleach import edx_api_doc_tools as apidocs from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user @@ -2735,16 +2734,11 @@ def send_email(request, course_id): # any transaction that has been pending up to this point will also be # committed. try: - # sanitize the email content before storing in the database - sanitized_subject = bleach.clean(subject, tags=settings.BULK_COURSE_EMAIL_ALLOWED_HTML_TAGS) - sanitized_message = bleach.clean(message, tags=settings.BULK_COURSE_EMAIL_ALLOWED_HTML_TAGS) - email = CourseEmail.create( course_id, request.user, targets, - sanitized_subject, - sanitized_message, + subject, message, template_name=template_name, from_addr=from_addr ) diff --git a/lms/envs/common.py b/lms/envs/common.py index f167c1860d..44d22a5965 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4964,12 +4964,3 @@ CUSTOM_PAGES_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-a # The expected value is an Integer representing the cutoff point (in months) for inclusion to the message. Example: # a value of `3` would include learners who have logged in within the past 3 months. BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD = None -# HTML tags allowed within the body of a message authored via the Bulk Course Email Tool -BULK_COURSE_EMAIL_ALLOWED_HTML_TAGS = [ - "a", "abbr", "address", "area", "article", "aside", "audio", "b", "bdi", "bdo", "blockquote", "br", "caption", - "cite", "code", "col", "colgroup", "data", "dd", "del", "dfn", "div", "dl", "dt", "em", "embed", "figcaption", - "figure", "footer", "h1", "h2", "h3", "h4", "h5", "h6", "header", "hr", "i", "img", "ins", "kbd", "li", "link", - "main", "map", "mark", "meta", "menu", "nav", "object", "ol", "p", "param", "picture", "pre", "q", "rp", "rt", - "ruby", "s", "samp", "section", "small", "source", "span", "strong", "style", "sub", "sup", "table", "tbody", "td", - "tfoot", "th", "thead", "time", "tr", "track", "u", "ul", "var", "video", "wbr", -]