Skip to content

Commit

Permalink
Disable escaping when sending abuse emails (#21897)
Browse files Browse the repository at this point in the history
* Disable escaping when sending abuse emails
  • Loading branch information
diox committed Feb 20, 2024
1 parent 73500c8 commit 66c2417
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 13 deletions.
28 changes: 24 additions & 4 deletions src/olympia/abuse/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ def _test_reporter_takedown_email(self, subject):
assert f'[ref:ab89/{self.abuse_report_auth.id}]' in mail.outbox[1].body
assert 'After reviewing' not in mail.outbox[0].body
assert 'After reviewing' not in mail.outbox[0].body
assert '"' not in mail.outbox[0].body
assert '"' not in mail.outbox[1].body
assert '<b>' not in mail.outbox[0].body
assert '<b>' not in mail.outbox[1].body

def _test_reporter_ignore_email(self, subject):
assert mail.outbox[0].to == ['email@domain.com']
Expand Down Expand Up @@ -121,6 +125,10 @@ def _test_reporter_ignore_email(self, subject):
)
assert f'[ref:ab89/{self.abuse_report_no_auth.id}]' in mail.outbox[0].body
assert f'[ref:ab89/{self.abuse_report_auth.id}]' in mail.outbox[1].body
assert '"' not in mail.outbox[0].body
assert '"' not in mail.outbox[1].body
assert '<b>' not in mail.outbox[0].body
assert '<b>' not in mail.outbox[1].body

def _test_reporter_appeal_takedown_email(self, subject):
assert mail.outbox[0].to == [self.abuse_report_auth.reporter.email]
Expand All @@ -131,6 +139,8 @@ def _test_reporter_appeal_takedown_email(self, subject):
assert 'right to appeal' not in mail.outbox[0].body
assert f'[ref:ab89/{self.abuse_report_auth.id}]' in mail.outbox[0].body
assert 'After reviewing' in mail.outbox[0].body
assert '"' not in mail.outbox[0].body
assert '<b>' not in mail.outbox[0].body

def _test_reporter_ignore_appeal_email(self, subject):
assert mail.outbox[0].to == [self.abuse_report_auth.reporter.email]
Expand All @@ -141,13 +151,17 @@ def _test_reporter_ignore_appeal_email(self, subject):
assert 'right to appeal' not in mail.outbox[0].body
assert 'was correct' in mail.outbox[0].body
assert f'[ref:ab89/{self.abuse_report_auth.id}]' in mail.outbox[0].body
assert '"' not in mail.outbox[0].body
assert '<b>' not in mail.outbox[0].body

def _check_owner_email(self, mail_item, subject, snippet):
user = getattr(self, 'user', getattr(self, 'author', None))
assert mail_item.to == [user.email]
assert mail_item.subject == subject + ' [ref:ab89]'
assert snippet in mail_item.body
assert '[ref:ab89]' in mail_item.body
assert '"' not in mail_item.body
assert '<b>' not in mail_item.body

def _test_owner_takedown_email(self, subject, snippet):
mail_item = mail.outbox[-1]
Expand All @@ -166,6 +180,8 @@ def _test_owner_takedown_email(self, subject, snippet):
'\n - Parent Policy, specifically Bad policy: This is bad thing\n'
in mail_item.body
)
assert '"' not in mail_item.body
assert '<b>' not in mail_item.body

def _test_owner_affirmation_email(self, subject):
mail_item = mail.outbox[0]
Expand Down Expand Up @@ -222,7 +238,7 @@ class TestCinderActionUser(BaseTestCinderAction, TestCase):

def setUp(self):
super().setUp()
self.user = user_factory()
self.user = user_factory(display_name='<b>Bad Hørse</b>')
self.cinder_job.abusereport_set.update(user=self.user, guid=None)

def _test_ban_user(self):
Expand Down Expand Up @@ -313,7 +329,7 @@ class TestCinderActionAddon(BaseTestCinderAction, TestCase):
def setUp(self):
super().setUp()
self.author = user_factory()
self.addon = addon_factory(users=(self.author,))
self.addon = addon_factory(users=(self.author,), name='<b>Bad Addön</b>')
ActivityLog.objects.all().delete()
self.cinder_job.abusereport_set.update(guid=self.addon.guid)

Expand Down Expand Up @@ -537,7 +553,11 @@ class TestCinderActionCollection(BaseTestCinderAction, TestCase):
def setUp(self):
super().setUp()
self.author = user_factory()
self.collection = collection_factory(author=self.author)
self.collection = collection_factory(
author=self.author,
name='<b>Bad Collectiôn</b>',
slug='bad-collection',
)
self.cinder_job.abusereport_set.update(collection=self.collection, guid=None)

def _test_delete_collection(self):
Expand Down Expand Up @@ -632,7 +652,7 @@ def setUp(self):
super().setUp()
self.author = user_factory()
self.rating = Rating.objects.create(
addon=addon_factory(), user=self.author, body='Saying something bad'
addon=addon_factory(), user=self.author, body='Saying something <b>bad</b>'
)
self.cinder_job.abusereport_set.update(rating=self.rating, guid=None)
ActivityLog.objects.all().delete()
Expand Down
33 changes: 24 additions & 9 deletions src/olympia/abuse/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,24 @@
from django.template import loader
from django.urls import reverse
from django.utils import translation
from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _

from olympia import amo
from olympia.activity import log_create
from olympia.addons.models import Addon
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.utils import send_mail
from olympia.amo.utils import no_jinja_autoescape, send_mail
from olympia.bandwagon.models import Collection
from olympia.ratings.models import Rating
from olympia.users.models import UserProfile


POLICY_DOCUMENT_URL = (
'https://extensionworkshop.com/documentation/publish/add-on-policies/'
)


class CinderAction:
description = 'Action has been taken'
valid_targets = []
Expand Down Expand Up @@ -70,13 +76,18 @@ def notify_owners(self, *, policy_text=None):
owners = self.get_owners()
if not owners:
return
name = self.get_target_name()
with no_jinja_autoescape():
template = loader.get_template(self.owner_template_path)
target_name = self.get_target_name()
reference_id = f'ref:{self.cinder_job.decision_id}'
context_dict = {
'additional_reasoning': self.cinder_job.decision_notes or '',
'is_third_party_initiated': self.is_third_party_initiated,
'name': name,
'policy_document_url': 'https://extensionworkshop.com/documentation/publish/add-on-policies/',
# Auto-escaping is already disabled above as we're dealing with an
# email but the target name could have triggered lazy escaping when
# it was generated so it needs special treatment to avoid it.
'name': mark_safe(target_name),
'policy_document_url': POLICY_DOCUMENT_URL,
'reference_id': reference_id,
'target': self.target,
'target_url': absolutify(self.target.get_url_path()),
Expand All @@ -100,8 +111,7 @@ def notify_owners(self, *, policy_text=None):
)
)

subject = f'Mozilla Add-ons: {name} [{reference_id}]'
template = loader.get_template(self.owner_template_path)
subject = f'Mozilla Add-ons: {target_name} [{reference_id}]'
self.send_mail(
subject,
template.render(context_dict),
Expand All @@ -119,7 +129,8 @@ def notify_reporters(self):
)
if not template:
return
template = loader.get_template(template)
with no_jinja_autoescape():
template = loader.get_template(template)
reporters = (
self.cinder_job.appellants.all()
if self.cinder_job.is_appeal
Expand All @@ -142,8 +153,12 @@ def notify_reporters(self):
target_name, reference_id
)
context_dict = {
'name': target_name,
'policy_document_url': 'https://extensionworkshop.com/documentation/publish/add-on-policies/',
# Auto-escaping is already disabled above as we're dealing
# with an email but the target name could have triggered
# lazy escaping when it was generated so it needs special
# treatment to avoid it.
'name': mark_safe(target_name),
'policy_document_url': POLICY_DOCUMENT_URL,
'reference_id': reference_id,
'target_url': absolutify(self.target.get_url_path()),
'type': self.get_target_type(),
Expand Down

0 comments on commit 66c2417

Please sign in to comment.