diff --git a/CHANGELOG.md b/CHANGELOG.md index 706bef88..c7e373fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ All notable changes to this project will be documented in this file. - Improve the crontab snippet in the "Check Details" page (#465) - Add Signal integration (#428) +## Bug Fixes +- Fix unwanted HTML escaping in SMS and WhatsApp notifications + ## v1.18.0 - 2020-12-09 ## Improvements diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index a5679eee..acb730fd 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -607,137 +607,6 @@ class NotifyTestCase(BaseTestCase): n = Notification.objects.first() self.assertEqual(n.error, "Rate limit exceeded") - @patch("hc.api.transports.requests.request") - def test_sms(self, mock_post): - self._setup_data("sms", "+1234567890") - self.check.last_ping = now() - td(hours=2) - - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - - n = Notification.objects.get() - - args, kwargs = mock_post.call_args - payload = kwargs["data"] - self.assertEqual(payload["To"], "+1234567890") - self.assertFalse("\xa0" in payload["Body"]) - - callback_path = f"/api/v1/notifications/{n.code}/status" - self.assertTrue(payload["StatusCallback"].endswith(callback_path)) - - # sent SMS counter should go up - self.profile.refresh_from_db() - self.assertEqual(self.profile.sms_sent, 1) - - @patch("hc.api.transports.requests.request") - def test_sms_handles_json_value(self, mock_post): - value = {"label": "foo", "value": "+1234567890"} - self._setup_data("sms", json.dumps(value)) - self.check.last_ping = now() - td(hours=2) - - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - assert Notification.objects.count() == 1 - - args, kwargs = mock_post.call_args - payload = kwargs["data"] - self.assertEqual(payload["To"], "+1234567890") - - @patch("hc.api.transports.requests.request") - def test_sms_limit(self, mock_post): - # At limit already: - self.profile.last_sms_date = now() - self.profile.sms_sent = 50 - self.profile.save() - - self._setup_data("sms", "+1234567890") - - self.channel.notify(self.check) - self.assertFalse(mock_post.called) - - n = Notification.objects.get() - self.assertTrue("Monthly SMS limit exceeded" in n.error) - - # And email should have been sent - self.assertEqual(len(mail.outbox), 1) - - email = mail.outbox[0] - self.assertEqual(email.to[0], "alice@example.org") - self.assertEqual(email.subject, "Monthly SMS Limit Reached") - - @patch("hc.api.transports.requests.request") - def test_sms_limit_reset(self, mock_post): - # At limit, but also into a new month - self.profile.sms_sent = 50 - self.profile.last_sms_date = now() - td(days=100) - self.profile.save() - - self._setup_data("sms", "+1234567890") - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - self.assertTrue(mock_post.called) - - @patch("hc.api.transports.requests.request") - def test_whatsapp(self, mock_post): - definition = {"value": "+1234567890", "up": True, "down": True} - - self._setup_data("whatsapp", json.dumps(definition)) - self.check.last_ping = now() - td(hours=2) - - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - - args, kwargs = mock_post.call_args - payload = kwargs["data"] - self.assertEqual(payload["To"], "whatsapp:+1234567890") - - n = Notification.objects.get() - callback_path = f"/api/v1/notifications/{n.code}/status" - self.assertTrue(payload["StatusCallback"].endswith(callback_path)) - - # sent SMS counter should go up - self.profile.refresh_from_db() - self.assertEqual(self.profile.sms_sent, 1) - - @patch("hc.api.transports.requests.request") - def test_whatsapp_obeys_up_down_flags(self, mock_post): - definition = {"value": "+1234567890", "up": True, "down": False} - - self._setup_data("whatsapp", json.dumps(definition)) - self.check.last_ping = now() - td(hours=2) - - self.channel.notify(self.check) - self.assertEqual(Notification.objects.count(), 0) - - self.assertFalse(mock_post.called) - - @patch("hc.api.transports.requests.request") - def test_whatsapp_limit(self, mock_post): - # At limit already: - self.profile.last_sms_date = now() - self.profile.sms_sent = 50 - self.profile.save() - - definition = {"value": "+1234567890", "up": True, "down": True} - self._setup_data("whatsapp", json.dumps(definition)) - - self.channel.notify(self.check) - self.assertFalse(mock_post.called) - - n = Notification.objects.get() - self.assertTrue("Monthly message limit exceeded" in n.error) - - # And email should have been sent - self.assertEqual(len(mail.outbox), 1) - - email = mail.outbox[0] - self.assertEqual(email.to[0], "alice@example.org") - self.assertEqual(email.subject, "Monthly WhatsApp Limit Reached") - @patch("hc.api.transports.requests.request") def test_call(self, mock_post): self.profile.call_limit = 1 diff --git a/hc/api/tests/test_notify_sms.py b/hc/api/tests/test_notify_sms.py new file mode 100644 index 00000000..cdfc0590 --- /dev/null +++ b/hc/api/tests/test_notify_sms.py @@ -0,0 +1,109 @@ +# coding: utf-8 + +from datetime import timedelta as td +import json +from unittest.mock import patch + +from django.core import mail +from django.utils.timezone import now +from hc.api.models import Channel, Check, Notification +from hc.test import BaseTestCase + + +class NotifyTestCase(BaseTestCase): + def _setup_data(self, value): + self.check = Check(project=self.project) + self.check.status = "down" + self.check.last_ping = now() - td(minutes=61) + self.check.save() + + self.channel = Channel(project=self.project, kind="sms") + self.channel.value = value + self.channel.save() + self.channel.checks.add(self.check) + + @patch("hc.api.transports.requests.request") + def test_it_works(self, mock_post): + self._setup_data("+1234567890") + self.check.last_ping = now() - td(hours=2) + + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + + args, kwargs = mock_post.call_args + payload = kwargs["data"] + self.assertEqual(payload["To"], "+1234567890") + self.assertFalse("\xa0" in payload["Body"]) + + n = Notification.objects.get() + callback_path = f"/api/v1/notifications/{n.code}/status" + self.assertTrue(payload["StatusCallback"].endswith(callback_path)) + + # sent SMS counter should go up + self.profile.refresh_from_db() + self.assertEqual(self.profile.sms_sent, 1) + + @patch("hc.api.transports.requests.request") + def test_it_handles_json_value(self, mock_post): + value = {"label": "foo", "value": "+1234567890"} + self._setup_data(json.dumps(value)) + self.check.last_ping = now() - td(hours=2) + + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + assert Notification.objects.count() == 1 + + args, kwargs = mock_post.call_args + payload = kwargs["data"] + self.assertEqual(payload["To"], "+1234567890") + + @patch("hc.api.transports.requests.request") + def test_it_enforces_limit(self, mock_post): + # At limit already: + self.profile.last_sms_date = now() + self.profile.sms_sent = 50 + self.profile.save() + + self._setup_data("+1234567890") + + self.channel.notify(self.check) + self.assertFalse(mock_post.called) + + n = Notification.objects.get() + self.assertTrue("Monthly SMS limit exceeded" in n.error) + + # And email should have been sent + self.assertEqual(len(mail.outbox), 1) + + email = mail.outbox[0] + self.assertEqual(email.to[0], "alice@example.org") + self.assertEqual(email.subject, "Monthly SMS Limit Reached") + + @patch("hc.api.transports.requests.request") + def test_it_resets_limit_next_month(self, mock_post): + # At limit, but also into a new month + self.profile.sms_sent = 50 + self.profile.last_sms_date = now() - td(days=100) + self.profile.save() + + self._setup_data("+1234567890") + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + self.assertTrue(mock_post.called) + + @patch("hc.api.transports.requests.request") + def test_it_does_not_escape_special_characters(self, mock_post): + self._setup_data("+1234567890") + self.check.name = "Foo > Bar & Co" + self.check.last_ping = now() - td(hours=2) + + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + + args, kwargs = mock_post.call_args + payload = kwargs["data"] + self.assertIn("Foo > Bar & Co", payload["Body"]) diff --git a/hc/api/tests/test_notify_whatsapp.py b/hc/api/tests/test_notify_whatsapp.py new file mode 100644 index 00000000..357e822b --- /dev/null +++ b/hc/api/tests/test_notify_whatsapp.py @@ -0,0 +1,96 @@ +# coding: utf-8 + +from datetime import timedelta as td +import json +from unittest.mock import patch + +from django.core import mail +from django.utils.timezone import now +from hc.api.models import Channel, Check, Notification +from hc.test import BaseTestCase + + +class NotifyTestCase(BaseTestCase): + def _setup_data(self, value, status="down"): + self.check = Check(project=self.project) + self.check.status = status + self.check.last_ping = now() - td(minutes=61) + self.check.save() + + self.channel = Channel(project=self.project, kind="whatsapp") + self.channel.value = value + self.channel.save() + self.channel.checks.add(self.check) + + @patch("hc.api.transports.requests.request") + def test_it_works(self, mock_post): + definition = {"value": "+1234567890", "up": True, "down": True} + + self._setup_data(json.dumps(definition)) + self.check.last_ping = now() - td(hours=2) + + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + + args, kwargs = mock_post.call_args + payload = kwargs["data"] + self.assertEqual(payload["To"], "whatsapp:+1234567890") + + n = Notification.objects.get() + callback_path = f"/api/v1/notifications/{n.code}/status" + self.assertTrue(payload["StatusCallback"].endswith(callback_path)) + + # sent SMS counter should go up + self.profile.refresh_from_db() + self.assertEqual(self.profile.sms_sent, 1) + + @patch("hc.api.transports.requests.request") + def test_it_obeys_up_down_flags(self, mock_post): + definition = {"value": "+1234567890", "up": True, "down": False} + + self._setup_data(json.dumps(definition)) + self.check.last_ping = now() - td(hours=2) + + self.channel.notify(self.check) + self.assertEqual(Notification.objects.count(), 0) + + self.assertFalse(mock_post.called) + + @patch("hc.api.transports.requests.request") + def test_it_enforces_limit(self, mock_post): + # At limit already: + self.profile.last_sms_date = now() + self.profile.sms_sent = 50 + self.profile.save() + + definition = {"value": "+1234567890", "up": True, "down": True} + self._setup_data(json.dumps(definition)) + + self.channel.notify(self.check) + self.assertFalse(mock_post.called) + + n = Notification.objects.get() + self.assertTrue("Monthly message limit exceeded" in n.error) + + # And email should have been sent + self.assertEqual(len(mail.outbox), 1) + + email = mail.outbox[0] + self.assertEqual(email.to[0], "alice@example.org") + self.assertEqual(email.subject, "Monthly WhatsApp Limit Reached") + + @patch("hc.api.transports.requests.request") + def test_it_does_not_escape_special_characters(self, mock_post): + definition = {"value": "+1234567890", "up": True, "down": True} + + self._setup_data(json.dumps(definition)) + self.check.name = "Foo > Bar & Co" + + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + + args, kwargs = mock_post.call_args + payload = kwargs["data"] + self.assertIn("Foo > Bar & Co", payload["Body"]) diff --git a/templates/integrations/sms_message.html b/templates/integrations/sms_message.html index 1fdb6c4c..2b1f5d4f 100644 --- a/templates/integrations/sms_message.html +++ b/templates/integrations/sms_message.html @@ -1 +1 @@ -{% load humanize %}{{ site_name }}: The check "{{ check.name_then_code }}" is DOWN. Last ping was {{ check.last_ping|naturaltime }}. \ No newline at end of file +{% load humanize %}{{ site_name }}: The check "{{ check.name_then_code|safe }}" is DOWN. Last ping was {{ check.last_ping|naturaltime }}. \ No newline at end of file diff --git a/templates/integrations/whatsapp_message.html b/templates/integrations/whatsapp_message.html index 8d5e3a8e..253c8dac 100644 --- a/templates/integrations/whatsapp_message.html +++ b/templates/integrations/whatsapp_message.html @@ -1,7 +1,7 @@ {% load humanize %}{% spaceless %} {% if check.status == "down" %} - The check “{{ check.name_then_code }}” is DOWN. Last ping was {{ check.last_ping|naturaltime }}. + The check “{{ check.name_then_code|safe }}” is DOWN. Last ping was {{ check.last_ping|naturaltime }}. {% else %} - The check “{{ check.name_then_code }}” is now UP. + The check “{{ check.name_then_code|safe }}” is now UP. {% endif %} {% endspaceless %}