From c2bb4b31b5a09391bb745413b12ca7ab28c19b2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Thu, 28 Jan 2021 14:07:39 +0200 Subject: [PATCH] Add rate limiting for Pushover notifications --- CHANGELOG.md | 1 + hc/api/models.py | 7 +++ hc/api/tests/test_notify.py | 29 +------------ hc/api/tests/test_notify_pushover.py | 65 ++++++++++++++++++++++++++++ hc/api/transports.py | 18 +++++--- 5 files changed, 85 insertions(+), 35 deletions(-) create mode 100644 hc/api/tests/test_notify_pushover.py diff --git a/CHANGELOG.md b/CHANGELOG.md index cacec69b..ef2a1a10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file. - Change Zulip onboarding, ask for the zuliprc file (#202) - Add a section in Docs about running self-hosted instances - Add experimental Dockerfile and docker-compose.yml +- Add rate limiting for Pushover notifications (6 notifications / user / minute) ## Bug Fixes - Fix unwanted HTML escaping in SMS and WhatsApp notifications diff --git a/hc/api/models.py b/hc/api/models.py index 0e684001..76d66e29 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -931,6 +931,13 @@ class TokenBucket(models.Model): # 6 messages for a single recipient per minute: return TokenBucket.authorize(value, 6, 60) + @staticmethod + def authorize_pushover(user_key): + salted_encoded = (user_key + settings.SECRET_KEY).encode() + value = "po-%s" % hashlib.sha1(salted_encoded).hexdigest() + # 6 messages for a single user key per minute: + return TokenBucket.authorize(value, 6, 60) + @staticmethod def authorize_sudo_code(user): value = "sudo-%d" % user.id diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 5b079f23..847928e9 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -2,7 +2,7 @@ from datetime import timedelta as td import json -from unittest.mock import patch, Mock +from unittest.mock import patch from django.core import mail from django.utils.timezone import now @@ -492,33 +492,6 @@ class NotifyTestCase(BaseTestCase): n = Notification.objects.first() self.assertEqual(n.error, 'Received status code 403 with a message: "Nice try"') - @patch("hc.api.transports.requests.request") - def test_pushover(self, mock_post): - self._setup_data("po", "123|0") - 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.assertIn("DOWN", payload["title"]) - - @patch("hc.api.transports.requests.request") - def test_pushover_up_priority(self, mock_post): - self._setup_data("po", "123|0|2", status="up") - 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.assertIn("UP", payload["title"]) - self.assertEqual(payload["priority"], 2) - self.assertIn("retry", payload) - self.assertIn("expire", payload) - @patch("hc.api.transports.requests.request") def test_victorops(self, mock_post): self._setup_data("victorops", "123") diff --git a/hc/api/tests/test_notify_pushover.py b/hc/api/tests/test_notify_pushover.py new file mode 100644 index 00000000..b012ea2c --- /dev/null +++ b/hc/api/tests/test_notify_pushover.py @@ -0,0 +1,65 @@ +# coding: utf-8 + +from datetime import timedelta as td +from unittest.mock import patch + +from django.test.utils import override_settings +from django.utils.timezone import now +from hc.api.models import Channel, Check, Notification, TokenBucket +from hc.test import BaseTestCase + + +class NotifyTestCase(BaseTestCase): + def _setup_data(self, value, status="down", email_verified=True): + 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) + self.channel.kind = "po" + self.channel.value = value + self.channel.email_verified = email_verified + self.channel.save() + self.channel.checks.add(self.check) + + @patch("hc.api.transports.requests.request") + def test_pushover(self, mock_post): + self._setup_data("123|0") + 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.assertIn("DOWN", payload["title"]) + + @patch("hc.api.transports.requests.request") + def test_pushover_up_priority(self, mock_post): + self._setup_data("123|0|2", status="up") + 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.assertIn("UP", payload["title"]) + self.assertEqual(payload["priority"], 2) + self.assertIn("retry", payload) + self.assertIn("expire", payload) + + @override_settings(SECRET_KEY="test-secret") + @patch("hc.api.transports.requests.request") + def test_it_obeys_rate_limit(self, mock_post): + self._setup_data("123|0") + + # "c0ca..." is sha1("123test-secret") + obj = TokenBucket(value="po-c0ca2a9774952af32cabf86453f69e442c4ed0eb") + obj.tokens = 0 + obj.save() + + self.channel.notify(self.check) + n = Notification.objects.first() + self.assertEqual(n.error, "Rate limit exceeded") diff --git a/hc/api/transports.py b/hc/api/transports.py index 4b6c3c34..c0db179f 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -358,20 +358,24 @@ class Pushover(HttpTransport): URL = "https://api.pushover.net/1/messages.json" def notify(self, check): - others = self.checks().filter(status="down").exclude(code=check.code) + pieces = self.channel.value.split("|") + user_key, prio = pieces[0], pieces[1] + # The third element, if present, is the priority for "up" events + if len(pieces) == 3 and check.status == "up": + prio = pieces[2] + + from hc.api.models import TokenBucket + if not TokenBucket.authorize_pushover(user_key): + return "Rate limit exceeded" + + others = self.checks().filter(status="down").exclude(code=check.code) # list() executes the query, to avoid DB access while # rendering a template ctx = {"check": check, "down_checks": list(others)} text = tmpl("pushover_message.html", **ctx) title = tmpl("pushover_title.html", **ctx) - pieces = self.channel.value.split("|") - user_key, prio = pieces[0], pieces[1] - # The third element, if present, is the priority for "up" events - if len(pieces) == 3 and check.status == "up": - prio = pieces[2] - payload = { "token": settings.PUSHOVER_API_TOKEN, "user": user_key,