From 72063391dd5af6ca576e7072d4413b9406479bde Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 31 Aug 2022 16:05:33 +0100 Subject: [PATCH 1/2] feat: adds TLS certificate validation option for SMTP --- docs/docs/installation/alerts-reports.mdx | 1 + superset/config.py | 4 +++- superset/utils/core.py | 9 ++++++++- tests/integration_tests/email_tests.py | 9 +++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/docs/docs/installation/alerts-reports.mdx b/docs/docs/installation/alerts-reports.mdx index d382b95f8ef81..4ac31826cc32c 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_TLS_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..6ac6ca03bc417 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. Only used when SMTP_STARTTLS is True +SMTP_TLS_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..54ed9610382e5 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,6 +995,7 @@ def send_mime_email( smtp_password = config["SMTP_PASSWORD"] smtp_starttls = config["SMTP_STARTTLS"] smtp_ssl = config["SMTP_SSL"] + smtp_tls_server_auth = config["SMTP_TLS_SERVER_AUTH"] if not dryrun: smtp = ( @@ -1002,7 +1004,12 @@ def send_mime_email( else smtplib.SMTP(smtp_host, smtp_port) ) if smtp_starttls: - smtp.starttls() + ssl_context = None + if smtp_tls_server_auth: + # Default ssl context is SERVER_AUTH using the default system + # root CA certificates + ssl_context = ssl.create_default_context() + 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)) diff --git a/tests/integration_tests/email_tests.py b/tests/integration_tests/email_tests.py index 4b546fea0eb8c..b47974777fde2 100644 --- a/tests/integration_tests/email_tests.py +++ b/tests/integration_tests/email_tests.py @@ -178,6 +178,15 @@ def test_send_mime_ssl(self, mock_smtp, mock_smtp_ssl): app.config["SMTP_HOST"], app.config["SMTP_PORT"] ) + @mock.patch("smtplib.SMTP") + def test_send_mime_tls_server_auth(self, mock_smtp): + app.config["SMTP_STARTTLS"] = True + app.config["SMTP_TLS_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) + @mock.patch("smtplib.SMTP_SSL") @mock.patch("smtplib.SMTP") def test_send_mime_noauth(self, mock_smtp, mock_smtp_ssl): From 499e1aed2c26bb0dc5198ab3e99280b01d39fafd Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 31 Aug 2022 16:35:37 +0100 Subject: [PATCH 2/2] improve tests and change config key name --- docs/docs/installation/alerts-reports.mdx | 2 +- superset/config.py | 4 +-- superset/utils/core.py | 39 +++++++++++------------ tests/integration_tests/email_tests.py | 22 +++++++++++-- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/docs/docs/installation/alerts-reports.mdx b/docs/docs/installation/alerts-reports.mdx index 4ac31826cc32c..d8f04817e8724 100644 --- a/docs/docs/installation/alerts-reports.mdx +++ b/docs/docs/installation/alerts-reports.mdx @@ -126,7 +126,7 @@ SLACK_API_TOKEN = "xoxb-" # Email configuration SMTP_HOST = "smtp.sendgrid.net" #change to your host SMTP_STARTTLS = True -SMTP_TLS_SERVER_AUTH = True # If your using an SMTP server with a valid certificate +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 6ac6ca03bc417..e5e9f506ccba1 100644 --- a/superset/config.py +++ b/superset/config.py @@ -988,8 +988,8 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name 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. Only used when SMTP_STARTTLS is True -SMTP_TLS_SERVER_AUTH = False +# 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 54ed9610382e5..73f70e7ae7450 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -995,29 +995,28 @@ def send_mime_email( smtp_password = config["SMTP_PASSWORD"] smtp_starttls = config["SMTP_STARTTLS"] smtp_ssl = config["SMTP_SSL"] - smtp_tls_server_auth = config["SMTP_TLS_SERVER_AUTH"] + 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: - ssl_context = None - if smtp_tls_server_auth: - # Default ssl context is SERVER_AUTH using the default system - # root CA certificates - ssl_context = ssl.create_default_context() - 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() - 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 b47974777fde2..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,17 +176,34 @@ 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_TLS_SERVER_AUTH"] = 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")