From db1bc5705fb5221bb34c22ed7eba64f9302e6967 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Tue, 5 Sep 2023 10:29:28 +0200 Subject: [PATCH 1/4] Make load_dispatcher_class a public method --- python/nav/models/profiles.py | 4 ++-- tests/integration/alertengine_test.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/nav/models/profiles.py b/python/nav/models/profiles.py index 362db77ffb..674139f9f2 100644 --- a/python/nav/models/profiles.py +++ b/python/nav/models/profiles.py @@ -505,7 +505,7 @@ def send(self, *args, **kwargs): if not self.supported: raise FatalDispatcherException("{} is not supported".format(self.name)) if self.handler not in self._handlers: - dispatcher_class = self._load_dispatcher_class() + dispatcher_class = self.load_dispatcher_class() dispatcher = dispatcher_class( config=AlertSender.config.get(self.handler, {}) ) @@ -516,7 +516,7 @@ def send(self, *args, **kwargs): # Delegate sending of message return dispatcher.send(*args, **kwargs) - def _load_dispatcher_class(self): + def load_dispatcher_class(self): # Get config if not hasattr(AlertSender, 'config'): AlertSender.config = get_alertengine_config('alertengine.conf') diff --git a/tests/integration/alertengine_test.py b/tests/integration/alertengine_test.py index 7a575c91f5..8ca0eb6fb3 100644 --- a/tests/integration/alertengine_test.py +++ b/tests/integration/alertengine_test.py @@ -4,5 +4,5 @@ def test_all_handlers_should_be_loadable(): for sender in AlertSender.objects.filter(supported=True): - dispatcher = sender._load_dispatcher_class() + dispatcher = sender.load_dispatcher_class() assert issubclass(dispatcher, Dispatcher) From 44bcc84b353291b995dfc41117dba71ab20f44b1 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Mon, 14 Aug 2023 16:54:08 +0200 Subject: [PATCH 2/4] Add has_valid_address method to AlertAddress --- python/nav/models/profiles.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/nav/models/profiles.py b/python/nav/models/profiles.py index 674139f9f2..cb8271bd55 100644 --- a/python/nav/models/profiles.py +++ b/python/nav/models/profiles.py @@ -406,6 +406,12 @@ class Meta(object): def __str__(self): return self.type.scheme() + self.address + def has_valid_address(self): + if not self.type.supported or not self.address: + return False + dispatcher = self.type.load_dispatcher_class() + return dispatcher.is_valid_address(self.address) + @transaction.atomic def send(self, alert, subscription): """Handles sending of alerts to with defined alert notification types From d31567859252a8b33db693d937c6be12d9e999f7 Mon Sep 17 00:00:00 2001 From: Johanna England Date: Mon, 14 Aug 2023 16:54:19 +0200 Subject: [PATCH 3/4] Raise error on invalid alert address in alert send --- python/nav/alertengine/dispatchers/__init__.py | 7 +++++++ python/nav/models/profiles.py | 17 ++++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/python/nav/alertengine/dispatchers/__init__.py b/python/nav/alertengine/dispatchers/__init__.py index 9b998c0bbc..1fdeea94ed 100644 --- a/python/nav/alertengine/dispatchers/__init__.py +++ b/python/nav/alertengine/dispatchers/__init__.py @@ -101,6 +101,13 @@ class FatalDispatcherException(DispatcherException): pass +class InvalidAlertAddressError(Exception): + """Raised when an alert address is invalid, which is determined by the + is_valid_address method of each dispatcher""" + + pass + + def is_valid_email(address): """Validates a string as an e-mail address""" try: diff --git a/python/nav/models/profiles.py b/python/nav/models/profiles.py index cb8271bd55..984aa28a73 100644 --- a/python/nav/models/profiles.py +++ b/python/nav/models/profiles.py @@ -35,8 +35,11 @@ import nav.buildconf import nav.pwhash from nav.config import getconfig as get_alertengine_config -from nav.alertengine.dispatchers import DispatcherException -from nav.alertengine.dispatchers import FatalDispatcherException +from nav.alertengine.dispatchers import ( + DispatcherException, + FatalDispatcherException, + InvalidAlertAddressError, +) from nav.models.event import AlertQueue, AlertType, EventType from nav.models.manage import Arp, Cam, Category, Device, Location @@ -423,10 +426,10 @@ def send(self, alert, subscription): # Determine the right language for the user. lang = self.account.preferences.get(Account.PREFERENCE_KEY_LANGUAGE, 'en') - if not (self.address or '').strip(): + if not self.has_valid_address(): _logger.error( - 'Ignoring alert %d (%s: %s)! Account %s does not have an ' - 'address set for the alertaddress with id %d, this needs ' + 'Ignoring alert %d (%s: %s)! Account %s does not have a ' + 'valid address for the alertaddress with id %d, this needs ' 'to be fixed before the user will recieve any alerts.', alert.id, alert, @@ -435,7 +438,7 @@ def send(self, alert, subscription): self.id, ) - return True + raise InvalidAlertAddressError if self.type.is_blacklisted(): _logger.debug( @@ -1364,7 +1367,7 @@ def send(self): super(AccountAlertQueue, self).delete() return False - except FatalDispatcherException: + except (FatalDispatcherException, InvalidAlertAddressError): self.delete() return False From c666afd8aa2f171cff9e13d5d85bdbcdb02cbffd Mon Sep 17 00:00:00 2001 From: Johanna England Date: Mon, 14 Aug 2023 16:54:28 +0200 Subject: [PATCH 4/4] Add tests for invalid alert address in alert send --- ...lertsubscription_test.py => alert_test.py} | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) rename tests/integration/models/{alertsubscription_test.py => alert_test.py} (54%) diff --git a/tests/integration/models/alertsubscription_test.py b/tests/integration/models/alert_test.py similarity index 54% rename from tests/integration/models/alertsubscription_test.py rename to tests/integration/models/alert_test.py index 160cc12358..c3cf957104 100644 --- a/tests/integration/models/alertsubscription_test.py +++ b/tests/integration/models/alert_test.py @@ -1,4 +1,6 @@ from datetime import datetime + +from nav.alertengine.dispatchers import InvalidAlertAddressError from nav.models.profiles import ( Account, AccountAlertQueue, @@ -10,24 +12,46 @@ TimePeriod, ) from nav.models.event import AlertQueue, Subsystem + import pytest -def test_delete_alert_subscription(db, alert, alertsub): - account_queue = AccountAlertQueue(alert=alert, subscription=alertsub) - account_queue.save() +def test_delete_alert_subscription(db, alert, alertsub, account_alert_queue): alertsub.delete() - assert not AccountAlertQueue.objects.filter(pk=account_queue.pk).exists() + assert not AccountAlertQueue.objects.filter(pk=account_alert_queue.pk).exists() + assert not AlertQueue.objects.filter(pk=alert.pk).exists() + + +def test_sending_alert_to_alert_address_with_empty_address_will_raise_error( + db, alert_address, alert, alertsub +): + with pytest.raises(InvalidAlertAddressError): + alert_address.send(alert, alertsub) + + +def test_sending_alert_to_alert_address_with_invalid_address_will_raise_error( + db, alert_address, alert, alertsub +): + alert_address.address = "abc" + alert_address.save() + with pytest.raises(InvalidAlertAddressError): + alert_address.send(alert, alertsub) + + +def test_sending_alert_to_alert_address_with_invalid_address_will_delete_alert_and_fail( + db, alert, account_alert_queue +): + assert not account_alert_queue.send() assert not AlertQueue.objects.filter(pk=alert.pk).exists() @pytest.fixture -def account(): +def account(db): return Account.objects.get(pk=Account.ADMIN_ACCOUNT) @pytest.fixture -def alert_address(account): +def alert_address(db, account): addr = AlertAddress( account=account, type=AlertSender.objects.get(name=AlertSender.SMS), @@ -39,7 +63,7 @@ def alert_address(account): @pytest.fixture -def alert_profile(account): +def alert_profile(db, account): profile = AlertProfile(account=account) profile.save() yield profile @@ -48,7 +72,7 @@ def alert_profile(account): @pytest.fixture -def time_period(alert_profile): +def time_period(db, alert_profile): time_period = TimePeriod(profile=alert_profile) time_period.save() yield time_period @@ -57,7 +81,7 @@ def time_period(alert_profile): @pytest.fixture -def alertsub(alert_address, time_period): +def alertsub(db, alert_address, time_period): alertsub = AlertSubscription( alert_address=alert_address, time_period=time_period, @@ -70,7 +94,7 @@ def alertsub(alert_address, time_period): @pytest.fixture -def alert(): +def alert(db): alert = AlertQueue( source=Subsystem.objects.first(), time=datetime.now(), value=1, severity=3 ) @@ -78,3 +102,12 @@ def alert(): yield alert if alert.pk: alert.delete() + + +@pytest.fixture +def account_alert_queue(db, alert, alertsub): + account_queue = AccountAlertQueue(alert=alert, subscription=alertsub) + account_queue.save() + yield account_queue + if account_queue.pk: + account_queue.delete()