Skip to content

Commit

Permalink
improve tests and change config key name
Browse files Browse the repository at this point in the history
  • Loading branch information
dpgaspar committed Aug 31, 2022
1 parent 7206339 commit 499e1ae
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 25 deletions.
2 changes: 1 addition & 1 deletion docs/docs/installation/alerts-reports.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 19 additions & 20 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
22 changes: 20 additions & 2 deletions 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,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")
Expand Down

0 comments on commit 499e1ae

Please sign in to comment.