From 9fd752057eb261b0e5db87636836fd30579ffce6 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Thu, 1 Sep 2022 10:51:34 +0100 Subject: [PATCH] feat: adds TLS certificate validation option for SMTP (#21272) --- docs/docs/installation/alerts-reports.mdx | 1 + superset/config.py | 4 ++- superset/utils/core.py | 34 +++++++++++++---------- tests/integration_tests/email_tests.py | 29 ++++++++++++++++++- 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/docs/docs/installation/alerts-reports.mdx b/docs/docs/installation/alerts-reports.mdx index d382b95f8ef81..d8f04817e8724 100644 --- a/docs/docs/installation/alerts-reports.mdx +++ b/docs/docs/installation/alerts-reports.mdx @@ -126,6 +126,7 @@ SLACK_API_TOKEN = "xoxb-" # Email configuration SMTP_HOST = "smtp.sendgrid.net" #change to your host SMTP_STARTTLS = True +SMTP_SSL_SERVER_AUTH = True # If your using an SMTP server with a valid certificate SMTP_SSL = False SMTP_USER = "your_user" SMTP_PORT = 2525 # your port eg. 587 diff --git a/superset/config.py b/superset/config.py index ffc7528fa45ea..e5e9f506ccba1 100644 --- a/superset/config.py +++ b/superset/config.py @@ -987,7 +987,9 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name SMTP_PORT = 25 SMTP_PASSWORD = "superset" SMTP_MAIL_FROM = "superset@superset.com" - +# If True creates a default SSL context with ssl.Purpose.CLIENT_AUTH using the +# default system root CA certificates. +SMTP_SSL_SERVER_AUTH = False ENABLE_CHUNK_ENCODING = False # Whether to bump the logging level to ERROR on the flask_appbuilder package diff --git a/superset/utils/core.py b/superset/utils/core.py index 46318dd50ea03..73f70e7ae7450 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -27,6 +27,7 @@ import re import signal import smtplib +import ssl import tempfile import threading import traceback @@ -994,23 +995,28 @@ def send_mime_email( smtp_password = config["SMTP_PASSWORD"] smtp_starttls = config["SMTP_STARTTLS"] smtp_ssl = config["SMTP_SSL"] + smpt_ssl_server_auth = config["SMTP_SSL_SERVER_AUTH"] - if not dryrun: - smtp = ( - smtplib.SMTP_SSL(smtp_host, smtp_port) - if smtp_ssl - else smtplib.SMTP(smtp_host, smtp_port) - ) - if smtp_starttls: - smtp.starttls() - if smtp_user and smtp_password: - smtp.login(smtp_user, smtp_password) - logger.debug("Sent an email to %s", str(e_to)) - smtp.sendmail(e_from, e_to, mime_msg.as_string()) - smtp.quit() - else: + if dryrun: logger.info("Dryrun enabled, email notification content is below:") logger.info(mime_msg.as_string()) + return + + # Default ssl context is SERVER_AUTH using the default system + # root CA certificates + ssl_context = ssl.create_default_context() if smpt_ssl_server_auth else None + smtp = ( + smtplib.SMTP_SSL(smtp_host, smtp_port, context=ssl_context) + if smtp_ssl + else smtplib.SMTP(smtp_host, smtp_port) + ) + if smtp_starttls: + smtp.starttls(context=ssl_context) + if smtp_user and smtp_password: + smtp.login(smtp_user, smtp_password) + logger.debug("Sent an email to %s", str(e_to)) + smtp.sendmail(e_from, e_to, mime_msg.as_string()) + smtp.quit() def get_email_address_list(address_string: str) -> List[str]: diff --git a/tests/integration_tests/email_tests.py b/tests/integration_tests/email_tests.py index 4b546fea0eb8c..381b8cda1b771 100644 --- a/tests/integration_tests/email_tests.py +++ b/tests/integration_tests/email_tests.py @@ -17,6 +17,7 @@ # under the License. """Unit tests for email service in Superset""" import logging +import ssl import tempfile import unittest from email.mime.application import MIMEApplication @@ -175,9 +176,35 @@ def test_send_mime_ssl(self, mock_smtp, mock_smtp_ssl): utils.send_mime_email("from", "to", MIMEMultipart(), app.config, dryrun=False) assert not mock_smtp.called mock_smtp_ssl.assert_called_with( - app.config["SMTP_HOST"], app.config["SMTP_PORT"] + app.config["SMTP_HOST"], app.config["SMTP_PORT"], context=None ) + @mock.patch("smtplib.SMTP_SSL") + @mock.patch("smtplib.SMTP") + def test_send_mime_ssl_server_auth(self, mock_smtp, mock_smtp_ssl): + app.config["SMTP_SSL"] = True + app.config["SMTP_SSL_SERVER_AUTH"] = True + mock_smtp.return_value = mock.Mock() + mock_smtp_ssl.return_value = mock.Mock() + utils.send_mime_email("from", "to", MIMEMultipart(), app.config, dryrun=False) + assert not mock_smtp.called + mock_smtp_ssl.assert_called_with( + app.config["SMTP_HOST"], app.config["SMTP_PORT"], context=mock.ANY + ) + called_context = mock_smtp_ssl.call_args.kwargs["context"] + self.assertEqual(called_context.verify_mode, ssl.CERT_REQUIRED) + + @mock.patch("smtplib.SMTP") + def test_send_mime_tls_server_auth(self, mock_smtp): + app.config["SMTP_STARTTLS"] = True + app.config["SMTP_SSL_SERVER_AUTH"] = True + mock_smtp.return_value = mock.Mock() + mock_smtp.return_value.starttls.return_value = mock.Mock() + utils.send_mime_email("from", "to", MIMEMultipart(), app.config, dryrun=False) + mock_smtp.return_value.starttls.assert_called_with(context=mock.ANY) + called_context = mock_smtp.return_value.starttls.call_args.kwargs["context"] + self.assertEqual(called_context.verify_mode, ssl.CERT_REQUIRED) + @mock.patch("smtplib.SMTP_SSL") @mock.patch("smtplib.SMTP") def test_send_mime_noauth(self, mock_smtp, mock_smtp_ssl):