Merge pull request #3246 from jtschladen/send-single-email

Send a single email to multiple recipients instead of multiple emails
This commit is contained in:
Jasmine Schladen 2020-11-16 17:45:22 -08:00 committed by GitHub
commit 33e34d76b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 55 additions and 58 deletions

View File

@ -103,8 +103,9 @@ def send_plugin_notification(event_type, data, recipients, notification):
function = f"{__name__}.{sys._getframe().f_code.co_name}"
log_data = {
"function": function,
"message": f"Sending expiration notification for to recipients {recipients}",
"notification_type": "expiration",
"message": f"Sending {event_type} notification for to recipients {recipients}",
"notification_type": event_type,
"notification_plugin": notification.plugin.slug,
"certificate_targets": recipients,
}
status = FAILURE_METRIC_STATUS
@ -121,7 +122,7 @@ def send_plugin_notification(event_type, data, recipients, notification):
"notification",
"counter",
1,
metric_tags={"status": status, "event_type": event_type},
metric_tags={"status": status, "event_type": event_type, "plugin": notification.plugin.slug},
)
if status == SUCCESS_METRIC_STATUS:
@ -142,7 +143,6 @@ def send_expiration_notifications(exclude):
for notification_label, certificates in notification_group.items():
notification_data = []
security_data = []
notification = certificates[0][0]
@ -152,33 +152,26 @@ def send_expiration_notifications(exclude):
certificate
).data
notification_data.append(cert_data)
security_data.append(cert_data)
if send_default_notification(
"expiration", notification_data, [owner], notification.options
):
success += 1
else:
failure += 1
recipients = notification.plugin.filter_recipients(notification.options, security_email + [owner])
email_recipients = notification.plugin.get_recipients(notification.options, security_email + [owner])
# Plugin will ONLY use the provided recipients if it's email; any other notification plugin ignores them
if send_plugin_notification(
"expiration",
notification_data,
recipients,
notification,
"expiration", notification_data, email_recipients, notification
):
success += 1
success += len(email_recipients)
else:
failure += 1
if send_default_notification(
"expiration", security_data, security_email, notification.options
):
success += 1
else:
failure += 1
failure += len(email_recipients)
# If we're using an email plugin, we're done,
# since "security_email + [owner]" were added as email_recipients.
# If we're not using an email plugin, we also need to send an email to the security team and owner,
# since the plugin notification didn't send anything to them.
if notification.plugin.slug != "email-notification":
if send_default_notification(
"expiration", notification_data, email_recipients, notification.options
):
success = 1 + len(email_recipients)
else:
failure = 1 + len(email_recipients)
return success, failure
@ -195,15 +188,16 @@ def send_default_notification(notification_type, data, targets, notification_opt
:return:
"""
function = f"{__name__}.{sys._getframe().f_code.co_name}"
log_data = {
"function": function,
"message": f"Sending notification for certificate data {data}",
"notification_type": notification_type,
}
status = FAILURE_METRIC_STATUS
notification_plugin = plugins.get(
current_app.config.get("LEMUR_DEFAULT_NOTIFICATION_PLUGIN", "email-notification")
)
log_data = {
"function": function,
"message": f"Sending {notification_type} notification for certificate data {data} to targets {targets}",
"notification_type": notification_type,
"notification_plugin": notification_plugin.slug,
}
try:
current_app.logger.debug(log_data)
@ -212,7 +206,7 @@ def send_default_notification(notification_type, data, targets, notification_opt
status = SUCCESS_METRIC_STATUS
except Exception as e:
log_data["message"] = f"Unable to send {notification_type} notification for certificate data {data} " \
f"to target {targets}"
f"to targets {targets}"
current_app.logger.error(log_data, exc_info=True)
sentry.captureException()
@ -220,7 +214,7 @@ def send_default_notification(notification_type, data, targets, notification_opt
"notification",
"counter",
1,
metric_tags={"status": status, "event_type": notification_type},
metric_tags={"status": status, "event_type": notification_type, "plugin": notification_plugin.slug},
)
if status == SUCCESS_METRIC_STATUS:
@ -247,15 +241,14 @@ def send_pending_failure_notification(
data = pending_certificate_output_schema.dump(pending_cert).data
data["security_email"] = current_app.config.get("LEMUR_SECURITY_TEAM_EMAIL")
notify_owner_success = False
email_recipients = []
if notify_owner:
notify_owner_success = send_default_notification("failed", data, [data["owner"]], pending_cert)
email_recipients = email_recipients + [data["owner"]]
notify_security_success = False
if notify_security:
notify_security_success = send_default_notification("failed", data, data["security_email"], pending_cert)
email_recipients = email_recipients + data["security_email"]
return notify_owner_success or notify_security_success
return send_default_notification("failed", data, email_recipients, pending_cert)
def needs_notification(certificate):

View File

@ -20,14 +20,14 @@ class NotificationPlugin(Plugin):
def send(self, notification_type, message, targets, options, **kwargs):
raise NotImplementedError
def filter_recipients(self, options, excluded_recipients):
def get_recipients(self, options, additional_recipients):
"""
Given a set of options (which should include configured recipient info), filters out recipients that
we do NOT want to notify.
Given a set of options (which should include configured recipient info), returns the parsed list of recipients
from those options plus the additional recipients specified. The returned value has no duplicates.
For any notification types where recipients can't be dynamically modified, this returns an empty list.
For any notification types where recipients can't be dynamically modified, this returns only the additional recipients.
"""
return []
return additional_recipients
class ExpirationNotificationPlugin(NotificationPlugin):

View File

@ -105,6 +105,8 @@ class EmailNotificationPlugin(ExpirationNotificationPlugin):
@staticmethod
def send(notification_type, message, targets, options, **kwargs):
if not targets:
return
subject = "Lemur: {0} Notification".format(notification_type.capitalize())
@ -119,11 +121,9 @@ class EmailNotificationPlugin(ExpirationNotificationPlugin):
send_via_smtp(subject, body, targets)
@staticmethod
def filter_recipients(options, excluded_recipients, **kwargs):
def get_recipients(options, additional_recipients, **kwargs):
notification_recipients = get_plugin_option("recipients", options)
if notification_recipients:
notification_recipients = notification_recipients.split(",")
# removing owner and security_email from notification_recipient
notification_recipients = [i for i in notification_recipients if i not in excluded_recipients]
return notification_recipients
return list(set(notification_recipients + additional_recipients))

View File

@ -21,7 +21,6 @@ def get_options():
def test_render_expiration(certificate, endpoint):
new_cert = CertificateFactory()
new_cert.replaces.append(certificate)
@ -54,7 +53,7 @@ def test_send_expiration_notification():
certificate.notifications[0].options = get_options()
verify_sender_email()
assert send_expiration_notifications([]) == (3, 0) # owner, recipients (only counted as 1), and security
assert send_expiration_notifications([]) == (4, 0) # owner (1), recipients (2), and security (1)
@mock_ses
@ -76,15 +75,20 @@ def test_send_pending_failure_notification(user, pending_certificate, async_issu
verify_sender_email()
assert send_pending_failure_notification(pending_certificate)
assert send_pending_failure_notification(pending_certificate, True, True)
assert send_pending_failure_notification(pending_certificate, True, False)
assert send_pending_failure_notification(pending_certificate, False, True)
assert send_pending_failure_notification(pending_certificate, False, False)
def test_filter_recipients(certificate, endpoint):
def test_get_recipients(certificate, endpoint):
from lemur.plugins.lemur_email.plugin import EmailNotificationPlugin
options = [{"name": "recipients", "value": "security@example.com,bob@example.com,joe@example.com"}]
assert EmailNotificationPlugin.filter_recipients(options, []) == ["security@example.com", "bob@example.com",
"joe@example.com"]
assert EmailNotificationPlugin.filter_recipients(options, ["security@example.com"]) == ["bob@example.com",
"joe@example.com"]
assert EmailNotificationPlugin.filter_recipients(options, ["security@example.com", "bob@example.com",
"joe@example.com"]) == []
options = [{"name": "recipients", "value": "security@example.com,joe@example.com"}]
two_emails = sorted(["security@example.com", "joe@example.com"])
assert sorted(EmailNotificationPlugin.get_recipients(options, [])) == two_emails
assert sorted(EmailNotificationPlugin.get_recipients(options, ["security@example.com"])) == two_emails
three_emails = sorted(["security@example.com", "bob@example.com", "joe@example.com"])
assert sorted(EmailNotificationPlugin.get_recipients(options, ["bob@example.com"])) == three_emails
assert sorted(EmailNotificationPlugin.get_recipients(options, ["security@example.com", "bob@example.com",
"joe@example.com"])) == three_emails