Merge pull request #2661 from johannaengland/invalid-alert-address-error

Handle invalid alert address in alert sending

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:

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
@@ -406,6 +409,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
@@ -417,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,
@@ -429,7 +438,7 @@ def send(self, alert, subscription):
  self.id,
  )
 
- return True
+ raise InvalidAlertAddressError
 
  if self.type.is_blacklisted():
  _logger.debug(
@@ -505,7 +514,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 +525,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')
@@ -1358,7 +1367,7 @@ def send(self):
 
  super(AccountAlertQueue, self).delete()
  return False
- except FatalDispatcherException:
+ except (FatalDispatcherException, InvalidAlertAddressError):
  self.delete()
  return False
 

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)

tests/integration/models/alertsubscription_test.py → 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,11 +94,20 @@ 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
  )
  alert.save()
  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()