Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: adds TLS certificate validation option for SMTP #21272

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/docs/installation/alerts-reports.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 20 additions & 14 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import re
import signal
import smtplib
import ssl
import tempfile
import threading
import traceback
Expand Down Expand Up @@ -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]:
Expand Down
29 changes: 28 additions & 1 deletion tests/integration_tests/email_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down