From 9dc476f39317679fa03df741bf5fc8166d93aae6 Mon Sep 17 00:00:00 2001 From: sayali Date: Thu, 15 Oct 2020 10:44:46 -0700 Subject: [PATCH 1/8] Use cab_compliant option instead of authority name list --- docs/administration.rst | 13 ++++--------- lemur/authorities/models.py | 13 +++++++++++++ lemur/authorities/schemas.py | 8 +++++--- lemur/certificates/models.py | 14 -------------- .../certificates/certificate/certificate.js | 4 ++-- .../certificates/certificate/tracking.tpl.html | 2 +- lemur/static/app/angular/certificates/services.js | 4 ++-- .../app/angular/pending_certificates/services.js | 4 ++-- 8 files changed, 29 insertions(+), 33 deletions(-) diff --git a/docs/administration.rst b/docs/administration.rst index 00da0c8a..6e53c826 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -155,17 +155,12 @@ Specifying the `SQLALCHEMY_MAX_OVERFLOW` to 0 will enforce limit to not create c LEMUR_ENCRYPTION_KEYS = ['1YeftooSbxCiX2zo8m1lXtpvQjy27smZcUUaGmffhMY=', 'LAfQt6yrkLqOK5lwpvQcT4jf2zdeTQJV1uYeh9coT5s='] -.. data:: PUBLIC_CA_AUTHORITY_NAMES - :noindex: - A list of public issuers which would be checked against to determine whether limit of max validity of 397 days - should be applied to the certificate. Configure public CA authority names in this list to enforce validity check. - This is an optional setting. Using this will allow the sanity check as mentioned. The name check is a case-insensitive - string comparision. .. data:: PUBLIC_CA_MAX_VALIDITY_DAYS :noindex: - Use this config to override the limit of 397 days of validity for certificates issued by public issuers configured - using PUBLIC_CA_AUTHORITY_NAMES. Below example overrides the default validity of 397 days and sets it to 365 days. + Use this config to override the limit of 397 days of validity for certificates issued by CA/Browser compliant authorities. + The authorities with cab_compliant option set to true will use this config. Below example overrides the default validity + of 397 days and sets it to 365 days. :: @@ -175,7 +170,7 @@ Specifying the `SQLALCHEMY_MAX_OVERFLOW` to 0 will enforce limit to not create c .. data:: DEFAULT_VALIDITY_DAYS :noindex: Use this config to override the default validity of 365 days for certificates offered through Lemur UI. Any CA which - is not listed in PUBLIC_CA_AUTHORITY_NAMES will be using this value as default validity to be displayed on UI. Please + is not CA/Browser Forum compliant will be using this value as default validity to be displayed on UI. Please note that this config is used for cert issuance only through Lemur UI. Below example overrides the default validity of 365 days and sets it to 1095 days (3 years). diff --git a/lemur/authorities/models.py b/lemur/authorities/models.py index d1b41a21..f042f773 100644 --- a/lemur/authorities/models.py +++ b/lemur/authorities/models.py @@ -8,6 +8,7 @@ """ import json +from flask import current_app from sqlalchemy.orm import relationship from sqlalchemy import ( Column, @@ -98,5 +99,17 @@ class Authority(db.Model): return None + @property + def max_issuance_days(self): + if self.is_cab_compliant: + return current_app.config.get("PUBLIC_CA_MAX_VALIDITY_DAYS", 397) + + @property + def default_validity_days(self): + if self.is_cab_compliant: + return current_app.config.get("PUBLIC_CA_MAX_VALIDITY_DAYS", 397) + + return current_app.config.get("DEFAULT_VALIDITY_DAYS", 365) # 1 year default + def __repr__(self): return "Authority(name={name})".format(name=self.name) diff --git a/lemur/authorities/schemas.py b/lemur/authorities/schemas.py index 6c48a183..555ba931 100644 --- a/lemur/authorities/schemas.py +++ b/lemur/authorities/schemas.py @@ -111,8 +111,6 @@ class RootAuthorityCertificateOutputSchema(LemurOutputSchema): cn = fields.String() not_after = fields.DateTime() not_before = fields.DateTime() - max_issuance_days = fields.Integer() - default_validity_days = fields.Integer() owner = fields.Email() status = fields.Boolean() user = fields.Nested(UserNestedOutputSchema) @@ -127,6 +125,8 @@ class AuthorityOutputSchema(LemurOutputSchema): active = fields.Boolean() options = fields.Dict() roles = fields.List(fields.Nested(AssociatedRoleSchema)) + max_issuance_days = fields.Integer() + default_validity_days = fields.Integer() authority_certificate = fields.Nested(RootAuthorityCertificateOutputSchema) @@ -138,8 +138,10 @@ class AuthorityNestedOutputSchema(LemurOutputSchema): owner = fields.Email() plugin = fields.Nested(PluginOutputSchema) active = fields.Boolean() - authority_certificate = fields.Nested(RootAuthorityCertificateOutputSchema, only=["max_issuance_days", "default_validity_days"]) + authority_certificate = fields.Nested(RootAuthorityCertificateOutputSchema, only=["not_after", "not_before"]) is_cab_compliant = fields.Boolean() + max_issuance_days = fields.Integer() + default_validity_days = fields.Integer() authority_update_schema = AuthorityUpdateSchema() diff --git a/lemur/certificates/models.py b/lemur/certificates/models.py index 60442de2..f6562b3f 100644 --- a/lemur/certificates/models.py +++ b/lemur/certificates/models.py @@ -317,20 +317,6 @@ class Certificate(db.Model): def validity_range(self): return self.not_after - self.not_before - @property - def max_issuance_days(self): - public_CA = current_app.config.get("PUBLIC_CA_AUTHORITY_NAMES", []) - if self.name.lower() in [ca.lower() for ca in public_CA]: - return current_app.config.get("PUBLIC_CA_MAX_VALIDITY_DAYS", 397) - - @property - def default_validity_days(self): - public_CA = current_app.config.get("PUBLIC_CA_AUTHORITY_NAMES", []) - if self.name.lower() in [ca.lower() for ca in public_CA]: - return current_app.config.get("PUBLIC_CA_MAX_VALIDITY_DAYS", 397) - - return current_app.config.get("DEFAULT_VALIDITY_DAYS", 365) # 1 year default - @property def subject(self): return self.parsed_cert.subject diff --git a/lemur/static/app/angular/certificates/certificate/certificate.js b/lemur/static/app/angular/certificates/certificate/certificate.js index 4bdbf60e..41e04d55 100644 --- a/lemur/static/app/angular/certificates/certificate/certificate.js +++ b/lemur/static/app/angular/certificates/certificate/certificate.js @@ -190,7 +190,7 @@ angular.module('lemur') function populateValidityDateAsPerDefault(certificate) { // calculate start and end date as per default validity let startDate = new Date(), endDate = new Date(); - endDate.setDate(startDate.getDate() + certificate.authority.authorityCertificate.defaultValidityDays); + endDate.setDate(startDate.getDate() + certificate.authority.defaultValidityDays); certificate.validityStart = startDate; certificate.validityEnd = endDate; } @@ -359,7 +359,7 @@ angular.module('lemur') function populateValidityDateAsPerDefault(certificate) { // calculate start and end date as per default validity let startDate = new Date(), endDate = new Date(); - endDate.setDate(startDate.getDate() + certificate.authority.authorityCertificate.defaultValidityDays); + endDate.setDate(startDate.getDate() + certificate.authority.defaultValidityDays); certificate.validityStart = startDate; certificate.validityEnd = endDate; } diff --git a/lemur/static/app/angular/certificates/certificate/tracking.tpl.html b/lemur/static/app/angular/certificates/certificate/tracking.tpl.html index d60a1a6a..c50d40ba 100644 --- a/lemur/static/app/angular/certificates/certificate/tracking.tpl.html +++ b/lemur/static/app/angular/certificates/certificate/tracking.tpl.html @@ -139,7 +139,7 @@
+ Default ({{certificate.authority.defaultValidityDays}} days)
diff --git a/lemur/static/app/angular/certificates/services.js b/lemur/static/app/angular/certificates/services.js index 280d6078..be19bafd 100644 --- a/lemur/static/app/angular/certificates/services.js +++ b/lemur/static/app/angular/certificates/services.js @@ -172,12 +172,12 @@ angular.module('lemur') // Minimum end date will be same as selected start date this.authority.authorityCertificate.minValidityEnd = value; - if(!this.authority.authorityCertificate || !this.authority.authorityCertificate.maxIssuanceDays) { + if(!this.authority.maxIssuanceDays) { this.authority.authorityCertificate.maxValidityEnd = this.authority.authorityCertificate.notAfter; } else { // Move max end date by maxIssuanceDays let endDate = new Date(value); - endDate.setDate(endDate.getDate() + this.authority.authorityCertificate.maxIssuanceDays); + endDate.setDate(endDate.getDate() + this.authority.maxIssuanceDays); this.authority.authorityCertificate.maxValidityEnd = endDate; } } diff --git a/lemur/static/app/angular/pending_certificates/services.js b/lemur/static/app/angular/pending_certificates/services.js index 9b32c1d3..7f20355b 100644 --- a/lemur/static/app/angular/pending_certificates/services.js +++ b/lemur/static/app/angular/pending_certificates/services.js @@ -152,12 +152,12 @@ angular.module('lemur') // Minimum end date will be same as selected start date this.authority.authorityCertificate.minValidityEnd = value; - if(!this.authority.authorityCertificate || !this.authority.authorityCertificate.maxIssuanceDays) { + if(!this.authority.maxIssuanceDays) { this.authority.authorityCertificate.maxValidityEnd = this.authority.authorityCertificate.notAfter; } else { // Move max end date by maxIssuanceDays let endDate = new Date(value); - endDate.setDate(endDate.getDate() + this.authority.authorityCertificate.maxIssuanceDays); + endDate.setDate(endDate.getDate() + this.authority.maxIssuanceDays); this.authority.authorityCertificate.maxValidityEnd = endDate; } } From 29f3dd43f2e1779866e61ba96547ca1b561d093f Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 15 Oct 2020 15:18:04 -0700 Subject: [PATCH 2/8] Update administration.rst language --- docs/administration.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/administration.rst b/docs/administration.rst index 6e53c826..c2f20362 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -159,7 +159,7 @@ Specifying the `SQLALCHEMY_MAX_OVERFLOW` to 0 will enforce limit to not create c .. data:: PUBLIC_CA_MAX_VALIDITY_DAYS :noindex: Use this config to override the limit of 397 days of validity for certificates issued by CA/Browser compliant authorities. - The authorities with cab_compliant option set to true will use this config. Below example overrides the default validity + The authorities with cab_compliant option set to true will use this config. The example below overrides the default validity of 397 days and sets it to 365 days. :: @@ -171,7 +171,7 @@ Specifying the `SQLALCHEMY_MAX_OVERFLOW` to 0 will enforce limit to not create c :noindex: Use this config to override the default validity of 365 days for certificates offered through Lemur UI. Any CA which is not CA/Browser Forum compliant will be using this value as default validity to be displayed on UI. Please - note that this config is used for cert issuance only through Lemur UI. Below example overrides the default validity + note that this config is used for cert issuance only through Lemur UI. The example below overrides the default validity of 365 days and sets it to 1095 days (3 years). :: From 60bb0037f0b3bd774b935dbc7991e32f2f6386fc Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Fri, 16 Oct 2020 15:13:12 -0700 Subject: [PATCH 3/8] Miscellaneous notification fixes and tests --- lemur/notifications/messaging.py | 9 +- lemur/plugins/lemur_email/plugin.py | 9 +- .../lemur_email/templates/rotation.html | 18 ++-- lemur/plugins/lemur_email/tests/test_email.py | 89 ++++++++++++++----- lemur/plugins/lemur_slack/plugin.py | 31 +++---- lemur/plugins/lemur_slack/tests/test_slack.py | 45 ++++++++++ lemur/tests/conf.py | 2 +- lemur/tests/test_messaging.py | 15 +++- 8 files changed, 157 insertions(+), 61 deletions(-) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index 82db7b6e..82a1ff1e 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -101,6 +101,9 @@ def send_notification(event_type, data, targets, notification): notification.plugin.send(event_type, data, targets, notification.options) status = SUCCESS_METRIC_STATUS except Exception as e: + current_app.logger.error( + "Unable to send notification to {}.".format(targets), exc_info=True + ) sentry.captureException() metrics.send( @@ -190,13 +193,13 @@ def send_rotation_notification(certificate, notification_plugin=None): status = FAILURE_METRIC_STATUS if not notification_plugin: notification_plugin = plugins.get( - current_app.config.get("LEMUR_DEFAULT_NOTIFICATION_PLUGIN") + current_app.config.get("LEMUR_DEFAULT_NOTIFICATION_PLUGIN", "email-notification") ) data = certificate_notification_output_schema.dump(certificate).data try: - notification_plugin.send("rotation", data, [data["owner"]]) + notification_plugin.send("rotation", data, [data["owner"]], []) status = SUCCESS_METRIC_STATUS except Exception as e: current_app.logger.error( @@ -290,7 +293,7 @@ def needs_notification(certificate): for notification in certificate.notifications: if not notification.active or not notification.options: - return + continue interval = get_plugin_option("interval", notification.options) unit = get_plugin_option("unit", notification.options) diff --git a/lemur/plugins/lemur_email/plugin.py b/lemur/plugins/lemur_email/plugin.py index 241aa1b0..08332ef1 100644 --- a/lemur/plugins/lemur_email/plugin.py +++ b/lemur/plugins/lemur_email/plugin.py @@ -19,14 +19,16 @@ from lemur.plugins import lemur_email as email from lemur.plugins.lemur_email.templates.config import env -def render_html(template_name, message): +def render_html(template_name, options, certificates): """ Renders the html for our email notification. :param template_name: - :param message: + :param options: + :param certificates: :return: """ + message = {"options": options, "certificates": certificates} template = env.get_template("{}.html".format(template_name)) return template.render( dict(message=message, hostname=current_app.config.get("LEMUR_HOSTNAME")) @@ -100,8 +102,7 @@ class EmailNotificationPlugin(ExpirationNotificationPlugin): subject = "Lemur: {0} Notification".format(notification_type.capitalize()) - data = {"options": options, "certificates": message} - body = render_html(notification_type, data) + body = render_html(notification_type, options, message) s_type = current_app.config.get("LEMUR_EMAIL_SENDER", "ses").lower() diff --git a/lemur/plugins/lemur_email/templates/rotation.html b/lemur/plugins/lemur_email/templates/rotation.html index 521eb327..9ce7ff33 100644 --- a/lemur/plugins/lemur_email/templates/rotation.html +++ b/lemur/plugins/lemur_email/templates/rotation.html @@ -83,12 +83,12 @@ - {{ certificate.name }} + {{ message.certificates.name }}
-
{{ certificate.owner }} -
{{ certificate.validityEnd | time }} - Details +
{{ message.certificates.owner }} +
{{ message.certificates.validityEnd | time }} + Details @@ -110,12 +110,12 @@ - {{ certificate.replacedBy[0].name }} + {{ message.certificates.name }}
-
{{ certificate.replacedBy[0].owner }} -
{{ certificate.replacedBy[0].validityEnd | time }} - Details +
{{ message.certificates.owner }} +
{{ message.certificates.validityEnd | time }} + Details @@ -133,7 +133,7 @@ - {% for endpoint in certificate.endpoints %} + {% for endpoint in message.certificates.endpoints %} diff --git a/lemur/plugins/lemur_email/tests/test_email.py b/lemur/plugins/lemur_email/tests/test_email.py index 43168cab..4f1ea187 100644 --- a/lemur/plugins/lemur_email/tests/test_email.py +++ b/lemur/plugins/lemur_email/tests/test_email.py @@ -1,36 +1,83 @@ import os -from lemur.plugins.lemur_email.templates.config import env +from datetime import timedelta +import arrow +import boto3 +from moto import mock_ses + +from lemur.certificates.schemas import certificate_notification_output_schema +from lemur.plugins.lemur_email.plugin import render_html from lemur.tests.factories import CertificateFactory dir_path = os.path.dirname(os.path.realpath(__file__)) -def test_render(certificate, endpoint): - from lemur.certificates.schemas import certificate_notification_output_schema +@mock_ses +def verify_sender_email(): + ses_client = boto3.client("ses", region_name="us-east-1") + ses_client.verify_email_identity(EmailAddress="lemur@example.com") + + +def get_options(): + return [ + {"name": "interval", "value": 10}, + {"name": "unit", "value": "days"}, + ] + + +def test_render_expiration(certificate, endpoint): new_cert = CertificateFactory() new_cert.replaces.append(certificate) - data = { - "certificates": [certificate_notification_output_schema.dump(certificate).data], - "options": [ - {"name": "interval", "value": 10}, - {"name": "unit", "value": "days"}, - ], - } + assert render_html("expiration", get_options(), [certificate_notification_output_schema.dump(certificate).data]) - template = env.get_template("{}.html".format("expiration")) - - body = template.render(dict(message=data, hostname="lemur.test.example.com")) - - template = env.get_template("{}.html".format("rotation")) +def test_render_rotation(certificate, endpoint): certificate.endpoints.append(endpoint) - body = template.render( - dict( - certificate=certificate_notification_output_schema.dump(certificate).data, - hostname="lemur.test.example.com", - ) - ) + assert render_html("rotation", get_options(), certificate_notification_output_schema.dump(certificate).data) + + +def test_render_rotation_failure(pending_certificate): + assert render_html("failed", get_options(), certificate_notification_output_schema.dump(pending_certificate).data) + + +@mock_ses +def test_send_expiration_notification(): + from lemur.notifications.messaging import send_expiration_notifications + from lemur.tests.factories import CertificateFactory + from lemur.tests.factories import NotificationFactory + + now = arrow.utcnow() + in_ten_days = now + timedelta(days=10, hours=1) # a bit more than 10 days since we'll check in the future + certificate = CertificateFactory() + notification = NotificationFactory(plugin_name="email-notification") + + certificate.not_after = in_ten_days + certificate.notifications.append(notification) + certificate.notifications[0].options = get_options() + + verify_sender_email() + assert send_expiration_notifications([]) == (2, 0) + + +@mock_ses +def test_send_rotation_notification(endpoint, source_plugin): + from lemur.notifications.messaging import send_rotation_notification + from lemur.deployment.service import rotate_certificate + + new_certificate = CertificateFactory() + rotate_certificate(endpoint, new_certificate) + assert endpoint.certificate == new_certificate + + verify_sender_email() + assert send_rotation_notification(new_certificate) + + +@mock_ses +def test_send_pending_failure_notification(user, pending_certificate, async_issuer_plugin): + from lemur.notifications.messaging import send_pending_failure_notification + + verify_sender_email() + assert send_pending_failure_notification(pending_certificate) diff --git a/lemur/plugins/lemur_slack/plugin.py b/lemur/plugins/lemur_slack/plugin.py index 7569d295..67c3fd84 100644 --- a/lemur/plugins/lemur_slack/plugin.py +++ b/lemur/plugins/lemur_slack/plugin.py @@ -58,26 +58,19 @@ def create_rotation_attachments(certificate): "title": certificate["name"], "title_link": create_certificate_url(certificate["name"]), "fields": [ + {"title": "Owner", "value": certificate["owner"], "short": True}, { - {"title": "Owner", "value": certificate["owner"], "short": True}, - { - "title": "Expires", - "value": arrow.get(certificate["validityEnd"]).format( - "dddd, MMMM D, YYYY" - ), - "short": True, - }, - { - "title": "Replaced By", - "value": len(certificate["replaced"][0]["name"]), - "short": True, - }, - { - "title": "Endpoints Rotated", - "value": len(certificate["endpoints"]), - "short": True, - }, - } + "title": "Expires", + "value": arrow.get(certificate["validityEnd"]).format( + "dddd, MMMM D, YYYY" + ), + "short": True, + }, + { + "title": "Endpoints Rotated", + "value": len(certificate["endpoints"]), + "short": True, + }, ], } diff --git a/lemur/plugins/lemur_slack/tests/test_slack.py b/lemur/plugins/lemur_slack/tests/test_slack.py index 86add25f..77abd542 100644 --- a/lemur/plugins/lemur_slack/tests/test_slack.py +++ b/lemur/plugins/lemur_slack/tests/test_slack.py @@ -21,3 +21,48 @@ def test_formatting(certificate): } assert attachment == create_expiration_attachments(data)[0] + + +def get_options(): + return [ + {"name": "interval", "value": 10}, + {"name": "unit", "value": "days"}, + ] + + +# Currently disabled as we have no good way to mock Slack webhooks +# def test_send_expiration_notification(): +# from lemur.notifications.messaging import send_expiration_notifications +# from lemur.tests.factories import CertificateFactory +# +# now = arrow.utcnow() +# in_ten_days = now + timedelta(days=10, hours=1) # a bit more than 10 days since we'll check in the future +# certificate = CertificateFactory() +# notification = NotificationFactory(plugin_name="slack-notification") +# +# certificate.not_after = in_ten_days +# certificate.notifications.append(notification) +# certificate.notifications[0].options = get_options() +# +# assert send_expiration_notifications([]) == (2, 0) + + +# Currently disabled as we have no good way to mock Slack webhooks +# def test_send_rotation_notification(endpoint, source_plugin): +# from lemur.notifications.messaging import send_rotation_notification +# from lemur.deployment.service import rotate_certificate +# +# notification = NotificationFactory(plugin_name="slack-notification") +# +# new_certificate = CertificateFactory() +# rotate_certificate(endpoint, new_certificate) +# assert endpoint.certificate == new_certificate +# +# assert send_rotation_notification(new_certificate, notification_plugin=notification.plugin) + + +# Currently disabled as the Slack plugin doesn't support this type of notification +# def test_send_pending_failure_notification(user, pending_certificate, async_issuer_plugin): +# from lemur.notifications.messaging import send_pending_failure_notification +# +# assert send_pending_failure_notification(pending_certificate, notification_plugin=plugins.get("slack-notification")) diff --git a/lemur/tests/conf.py b/lemur/tests/conf.py index 38b8bade..3dfb5621 100644 --- a/lemur/tests/conf.py +++ b/lemur/tests/conf.py @@ -46,7 +46,7 @@ LEMUR_ALLOWED_DOMAINS = [ # Lemur currently only supports SES for sending email, this address # needs to be verified -LEMUR_EMAIL = "" +LEMUR_EMAIL = "lemur@example.com" LEMUR_SECURITY_TEAM_EMAIL = ["security@example.com"] LEMUR_HOSTNAME = "lemur.example.com" diff --git a/lemur/tests/test_messaging.py b/lemur/tests/test_messaging.py index 98e9ebf3..dd8f339f 100644 --- a/lemur/tests/test_messaging.py +++ b/lemur/tests/test_messaging.py @@ -1,8 +1,8 @@ +from datetime import timedelta + +import arrow import pytest from freezegun import freeze_time - -from datetime import timedelta -import arrow from moto import mock_ses @@ -105,4 +105,11 @@ def test_send_expiration_notification_with_no_notifications( def test_send_rotation_notification(notification_plugin, certificate): from lemur.notifications.messaging import send_rotation_notification - send_rotation_notification(certificate, notification_plugin=notification_plugin) + assert send_rotation_notification(certificate, notification_plugin=notification_plugin) + + +@mock_ses +def test_send_pending_failure_notification(notification_plugin, async_issuer_plugin, pending_certificate): + from lemur.notifications.messaging import send_pending_failure_notification + + assert send_pending_failure_notification(pending_certificate, notification_plugin=notification_plugin) From 072b337f37487a5750e339ffdf05100aec24162c Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Fri, 16 Oct 2020 16:21:43 -0700 Subject: [PATCH 4/8] Restructure log messages --- lemur/notifications/messaging.py | 54 ++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index 82a1ff1e..78809dbc 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -8,6 +8,7 @@ .. moduleauthor:: Kevin Glisson """ +import sys from collections import defaultdict from datetime import timedelta from itertools import groupby @@ -96,14 +97,20 @@ def send_notification(event_type, data, targets, notification): :param notification: :return: """ + function = f"{__name__}.{sys._getframe().f_code.co_name}" + log_data = { + "function": function, + "message": "Sending expiration notification for to targets {}".format(targets), + "notification_type": "rotation", + "targets": targets, + } status = FAILURE_METRIC_STATUS try: notification.plugin.send(event_type, data, targets, notification.options) status = SUCCESS_METRIC_STATUS except Exception as e: - current_app.logger.error( - "Unable to send notification to {}.".format(targets), exc_info=True - ) + log_data["message"] = "Unable to send expiration notification to targets {}".format(targets) + current_app.logger.error(log_data, exc_info=True) sentry.captureException() metrics.send( @@ -190,6 +197,14 @@ def send_rotation_notification(certificate, notification_plugin=None): :param notification_plugin: :return: """ + function = f"{__name__}.{sys._getframe().f_code.co_name}" + log_data = { + "function": function, + "message": "Sending rotation notification for certificate {}".format(certificate.name), + "notification_type": "rotation", + "name": certificate.name, + "owner": certificate.owner, + } status = FAILURE_METRIC_STATUS if not notification_plugin: notification_plugin = plugins.get( @@ -202,9 +217,9 @@ def send_rotation_notification(certificate, notification_plugin=None): notification_plugin.send("rotation", data, [data["owner"]], []) status = SUCCESS_METRIC_STATUS except Exception as e: - current_app.logger.error( - "Unable to send notification to {}.".format(data["owner"]), exc_info=True - ) + log_data["message"] = "Unable to send rotation notification for certificate {0} to ownner {1}" \ + .format(certificate.name, data["owner"]) + current_app.logger.error(log_data) sentry.captureException() metrics.send( @@ -228,6 +243,14 @@ def send_pending_failure_notification( :param notification_plugin: :return: """ + function = f"{__name__}.{sys._getframe().f_code.co_name}" + log_data = { + "function": function, + "message": "Sending pending failure notification for pending certificate {}".format(pending_cert.name), + "notification_type": "rotation", + "name": pending_cert.name, + "owner": pending_cert.owner, + } status = FAILURE_METRIC_STATUS if not notification_plugin: @@ -245,12 +268,10 @@ def send_pending_failure_notification( notification_plugin.send("failed", data, [data["owner"]], pending_cert) status = SUCCESS_METRIC_STATUS except Exception as e: - current_app.logger.error( - "Unable to send pending failure notification to {}.".format( - data["owner"] - ), - exc_info=True, - ) + log_data["recipient"] = data["owner"] + log_data["message"] = "Unable to send pending failure notification for certificate {0} to owner {1}" \ + .format(pending_cert.name, pending_cert.owner) + current_app.logger.error(log_data, exc_info=True) sentry.captureException() if notify_security: @@ -260,11 +281,10 @@ def send_pending_failure_notification( ) status = SUCCESS_METRIC_STATUS except Exception as e: - current_app.logger.error( - "Unable to send pending failure notification to " - "{}.".format(data["security_email"]), - exc_info=True, - ) + log_data["recipient"] = data["security_email"] + log_data["message"] = "Unable to send pending failure notification for certificate {0} to security email " \ + "{1}".format(pending_cert.name, pending_cert.owner) + current_app.logger.error(log_data, exc_info=True) sentry.captureException() metrics.send( From 6a1889787dc2d5df5836a9f19ca2b503c80515b5 Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Fri, 16 Oct 2020 16:30:21 -0700 Subject: [PATCH 5/8] Correct log attributes --- lemur/notifications/messaging.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index 78809dbc..5452f4fc 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -101,8 +101,8 @@ def send_notification(event_type, data, targets, notification): log_data = { "function": function, "message": "Sending expiration notification for to targets {}".format(targets), - "notification_type": "rotation", - "targets": targets, + "notification_type": "expiration", + "certificate_targets": targets, } status = FAILURE_METRIC_STATUS try: @@ -202,8 +202,8 @@ def send_rotation_notification(certificate, notification_plugin=None): "function": function, "message": "Sending rotation notification for certificate {}".format(certificate.name), "notification_type": "rotation", - "name": certificate.name, - "owner": certificate.owner, + "certificate_name": certificate.name, + "certificate_owner": certificate.owner, } status = FAILURE_METRIC_STATUS if not notification_plugin: @@ -247,9 +247,9 @@ def send_pending_failure_notification( log_data = { "function": function, "message": "Sending pending failure notification for pending certificate {}".format(pending_cert.name), - "notification_type": "rotation", - "name": pending_cert.name, - "owner": pending_cert.owner, + "notification_type": "failed", + "certificate_name": pending_cert.name, + "certificate_owner": pending_cert.owner, } status = FAILURE_METRIC_STATUS From e90b08b3633c7a277da84ad19cde45324ca9bf5a Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Fri, 16 Oct 2020 17:08:44 -0700 Subject: [PATCH 6/8] Correct typo and enable Slack notification test --- lemur/notifications/messaging.py | 4 +- lemur/plugins/lemur_slack/tests/test_slack.py | 40 +++++++++++-------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index 5452f4fc..aa85123d 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -217,9 +217,9 @@ def send_rotation_notification(certificate, notification_plugin=None): notification_plugin.send("rotation", data, [data["owner"]], []) status = SUCCESS_METRIC_STATUS except Exception as e: - log_data["message"] = "Unable to send rotation notification for certificate {0} to ownner {1}" \ + log_data["message"] = "Unable to send rotation notification for certificate {0} to owner {1}" \ .format(certificate.name, data["owner"]) - current_app.logger.error(log_data) + current_app.logger.error(log_data, exc_info=True) sentry.captureException() metrics.send( diff --git a/lemur/plugins/lemur_slack/tests/test_slack.py b/lemur/plugins/lemur_slack/tests/test_slack.py index 77abd542..da232d61 100644 --- a/lemur/plugins/lemur_slack/tests/test_slack.py +++ b/lemur/plugins/lemur_slack/tests/test_slack.py @@ -1,3 +1,10 @@ +from datetime import timedelta + +import arrow + +from lemur.tests.factories import NotificationFactory, CertificateFactory + + def test_formatting(certificate): from lemur.plugins.lemur_slack.plugin import create_expiration_attachments from lemur.certificates.schemas import certificate_notification_output_schema @@ -27,32 +34,33 @@ def get_options(): return [ {"name": "interval", "value": 10}, {"name": "unit", "value": "days"}, + {"name": "webhook", "value": "https://slack.com/api/api.test"}, ] -# Currently disabled as we have no good way to mock Slack webhooks -# def test_send_expiration_notification(): -# from lemur.notifications.messaging import send_expiration_notifications -# from lemur.tests.factories import CertificateFactory -# -# now = arrow.utcnow() -# in_ten_days = now + timedelta(days=10, hours=1) # a bit more than 10 days since we'll check in the future -# certificate = CertificateFactory() -# notification = NotificationFactory(plugin_name="slack-notification") -# -# certificate.not_after = in_ten_days -# certificate.notifications.append(notification) -# certificate.notifications[0].options = get_options() -# -# assert send_expiration_notifications([]) == (2, 0) +def test_send_expiration_notification(): + from lemur.notifications.messaging import send_expiration_notifications + + notification = NotificationFactory(plugin_name="slack-notification") + notification.options = get_options() + + now = arrow.utcnow() + in_ten_days = now + timedelta(days=10, hours=1) # a bit more than 10 days since we'll check in the future + + certificate = CertificateFactory() + certificate.not_after = in_ten_days + certificate.notifications.append(notification) + + assert send_expiration_notifications([]) == (2, 0) -# Currently disabled as we have no good way to mock Slack webhooks +# Currently disabled as the Slack plugin doesn't support this type of notification # def test_send_rotation_notification(endpoint, source_plugin): # from lemur.notifications.messaging import send_rotation_notification # from lemur.deployment.service import rotate_certificate # # notification = NotificationFactory(plugin_name="slack-notification") +# notification.options = get_options() # # new_certificate = CertificateFactory() # rotate_certificate(endpoint, new_certificate) From ecd4d6ebe3489cb9f2ad7988eb3eb373fb053714 Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Mon, 19 Oct 2020 15:12:48 -0700 Subject: [PATCH 7/8] Change string formatting pattern --- lemur/notifications/messaging.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index aa85123d..6c8599aa 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -100,7 +100,7 @@ def send_notification(event_type, data, targets, notification): function = f"{__name__}.{sys._getframe().f_code.co_name}" log_data = { "function": function, - "message": "Sending expiration notification for to targets {}".format(targets), + "message": f"Sending expiration notification for to targets {targets}", "notification_type": "expiration", "certificate_targets": targets, } @@ -109,7 +109,7 @@ def send_notification(event_type, data, targets, notification): notification.plugin.send(event_type, data, targets, notification.options) status = SUCCESS_METRIC_STATUS except Exception as e: - log_data["message"] = "Unable to send expiration notification to targets {}".format(targets) + log_data["message"] = f"Unable to send expiration notification to targets {targets}" current_app.logger.error(log_data, exc_info=True) sentry.captureException() @@ -200,7 +200,7 @@ def send_rotation_notification(certificate, notification_plugin=None): function = f"{__name__}.{sys._getframe().f_code.co_name}" log_data = { "function": function, - "message": "Sending rotation notification for certificate {}".format(certificate.name), + "message": f"Sending rotation notification for certificate {certificate.name}", "notification_type": "rotation", "certificate_name": certificate.name, "certificate_owner": certificate.owner, @@ -217,8 +217,8 @@ def send_rotation_notification(certificate, notification_plugin=None): notification_plugin.send("rotation", data, [data["owner"]], []) status = SUCCESS_METRIC_STATUS except Exception as e: - log_data["message"] = "Unable to send rotation notification for certificate {0} to owner {1}" \ - .format(certificate.name, data["owner"]) + log_data["message"] = f"Unable to send rotation notification for certificate {certificate.name} " \ + f"to owner {data['owner']}" current_app.logger.error(log_data, exc_info=True) sentry.captureException() @@ -246,7 +246,7 @@ def send_pending_failure_notification( function = f"{__name__}.{sys._getframe().f_code.co_name}" log_data = { "function": function, - "message": "Sending pending failure notification for pending certificate {}".format(pending_cert.name), + "message": f"Sending pending failure notification for pending certificate {pending_cert}" "notification_type": "failed", "certificate_name": pending_cert.name, "certificate_owner": pending_cert.owner, @@ -269,8 +269,8 @@ def send_pending_failure_notification( status = SUCCESS_METRIC_STATUS except Exception as e: log_data["recipient"] = data["owner"] - log_data["message"] = "Unable to send pending failure notification for certificate {0} to owner {1}" \ - .format(pending_cert.name, pending_cert.owner) + log_data["message"] = f"Unable to send pending failure notification for certificate {pending_cert.name} " \ + f"to owner {pending_cert.owner}" current_app.logger.error(log_data, exc_info=True) sentry.captureException() @@ -282,8 +282,8 @@ def send_pending_failure_notification( status = SUCCESS_METRIC_STATUS except Exception as e: log_data["recipient"] = data["security_email"] - log_data["message"] = "Unable to send pending failure notification for certificate {0} to security email " \ - "{1}".format(pending_cert.name, pending_cert.owner) + log_data["message"] = f"Unable to send pending failure notification for certificate {pending_cert.name} " \ + f"to security email {pending_cert.owner}" current_app.logger.error(log_data, exc_info=True) sentry.captureException() @@ -291,7 +291,7 @@ def send_pending_failure_notification( "notification", "counter", 1, - metric_tags={"status": status, "event_type": "rotation"}, + metric_tags={"status": status, "event_type": "failed"}, ) if status == SUCCESS_METRIC_STATUS: @@ -329,7 +329,7 @@ def needs_notification(certificate): else: raise Exception( - "Invalid base unit for expiration interval: {0}".format(unit) + f"Invalid base unit for expiration interval: {unit}" ) if days == interval: From b5f0fc5a195f464885a3f808edcfe4f95b054f4d Mon Sep 17 00:00:00 2001 From: Jasmine Schladen Date: Mon, 19 Oct 2020 15:21:34 -0700 Subject: [PATCH 8/8] Fix syntax error --- lemur/notifications/messaging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/notifications/messaging.py b/lemur/notifications/messaging.py index 6c8599aa..ca955b69 100644 --- a/lemur/notifications/messaging.py +++ b/lemur/notifications/messaging.py @@ -246,7 +246,7 @@ def send_pending_failure_notification( function = f"{__name__}.{sys._getframe().f_code.co_name}" log_data = { "function": function, - "message": f"Sending pending failure notification for pending certificate {pending_cert}" + "message": f"Sending pending failure notification for pending certificate {pending_cert}", "notification_type": "failed", "certificate_name": pending_cert.name, "certificate_owner": pending_cert.owner,