Add fix for SMTP server overriding and #1306 (#1330)

* Fix a potential issue where config.py SMTP settings might not have been overrideable from the Admin Panel
* Closes #1306 by using `email.message.EmailMessage` in Python 3. Python 2 will use the old `sendmail` behavior.

CTFd/utils/config/__init__.py

@@ -53,14 +53,14 @@ def can_send_mail():
 
 
 def get_mail_provider():
-    if app.config.get("MAIL_SERVER") and app.config.get("MAIL_PORT"):
-        return "smtp"
     if get_config("mail_server") and get_config("mail_port"):
         return "smtp"
-    if app.config.get("MAILGUN_API_KEY") and app.config.get("MAILGUN_BASE_URL"):
-        return "mailgun"
     if get_config("mailgun_api_key") and get_config("mailgun_base_url"):
         return "mailgun"
+    if app.config.get("MAIL_SERVER") and app.config.get("MAIL_PORT"):
+        return "smtp"
+    if app.config.get("MAILGUN_API_KEY") and app.config.get("MAILGUN_BASE_URL"):
+        return "mailgun"
 
 
 def mailgun():

CTFd/utils/email/smtp.py

@@ -1,5 +1,9 @@
+import six
 import smtplib
 from email.mime.text import MIMEText
+
+if six.PY3:
+    from email.message import EmailMessage
 from socket import timeout
 
 from CTFd.utils import get_app_config, get_config
@@ -47,12 +51,22 @@ def sendmail(addr, text, subject):
 
     try:
         smtp = get_smtp(**data)
-        msg = MIMEText(text)
+
+        if six.PY2:
+            msg = MIMEText(text)
+        else:
+            msg = EmailMessage()
+            msg.set_content(text)
+
         msg["Subject"] = subject
         msg["From"] = mailfrom_addr
         msg["To"] = addr
 
-        smtp.sendmail(msg["From"], [msg["To"]], msg.as_string())
+        if six.PY2:
+            smtp.sendmail(msg["From"], [msg["To"]], msg.as_string())
+        else:
+            smtp.send_message(msg)
+
         smtp.quit()
         return True, "Email sent"
     except smtplib.SMTPException as e:

Makefile

@@ -8,7 +8,7 @@ format:
 	prettier --write 'CTFd/themes/**/assets/**/*'
 
 test:
-	pytest --cov=CTFd --cov-context=test --ignore=node_modules/ --disable-warnings -n auto
+	pytest -rf --cov=CTFd --cov-context=test --ignore=node_modules/ --disable-warnings -n auto
 	bandit -r CTFd -x CTFd/uploads
 	yarn verify
 

tests/users/test_auth.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
+import six
 from freezegun import freeze_time
 from mock import patch
 
@@ -304,8 +305,11 @@ def test_user_can_confirm_email(mock_smtp):
         r = client.get("http://localhost/confirm")
         assert "Need to resend the confirmation email?" in r.get_data(as_text=True)
 
-        # smtp.sendmail was called
+        # smtp send message function was called
-        mock_smtp.return_value.sendmail.assert_called()
+        if six.PY2:
+            mock_smtp.return_value.sendmail.assert_called()
+        else:
+            mock_smtp.return_value.send_message.assert_called()
 
         with client.session_transaction() as sess:
             data = {"nonce": sess.get("nonce")}
@@ -334,6 +338,9 @@ def test_user_can_reset_password(mock_smtp):
     """Test that a user is capable of resetting their password"""
     from email.mime.text import MIMEText
 
+    if six.PY3:
+        from email.message import EmailMessage
+
     app = create_ctfd()
     with app.app_context(), freeze_time("2012-01-14 03:21:34"):
         # Set CTFd to send emails
@@ -369,7 +376,13 @@ def test_user_can_reset_password(mock_smtp):
                 "http://localhost/reset_password/InVzZXJAdXNlci5jb20i.TxD0vg.28dY_Gzqb1TH9nrcE_H7W8YFM-U"
             )
             ctf_name = get_config("ctf_name")
-            email_msg = MIMEText(msg)
+
+            if six.PY2:
+                email_msg = MIMEText(msg)
+            else:
+                email_msg = EmailMessage()
+                email_msg.set_content(msg)
+
             email_msg["Subject"] = "Password Reset Request from {ctf_name}".format(
                 ctf_name=ctf_name
             )
@@ -377,9 +390,15 @@ def test_user_can_reset_password(mock_smtp):
             email_msg["To"] = to_addr
 
             # Make sure that the reset password email is sent
-            mock_smtp.return_value.sendmail.assert_called_with(
+            if six.PY2:
-                from_addr, [to_addr], email_msg.as_string()
+                mock_smtp.return_value.sendmail.assert_called_with(
-            )
+                    from_addr, [to_addr], email_msg.as_string()
+                )
+            else:
+                mock_smtp.return_value.send_message.assert_called()
+                assert str(mock_smtp.return_value.send_message.call_args[0][0]) == str(
+                    email_msg
+                )
 
             # Get user's original password
             user = Users.query.filter_by(email="user@user.com").first()

tests/utils/test_email.py

@@ -1,5 +1,9 @@
+import six
 from email.mime.text import MIMEText
 
+if six.PY3:
+    from email.message import EmailMessage
+
 import requests
 from freezegun import freeze_time
 from mock import Mock, patch
@@ -34,14 +38,25 @@ def test_sendmail_with_smtp_from_config_file(mock_smtp):
         sendmail(to_addr, msg)
 
         ctf_name = get_config("ctf_name")
-        email_msg = MIMEText(msg)
+        if six.PY2:
+            email_msg = MIMEText(msg)
+        else:
+            email_msg = EmailMessage()
+            email_msg.set_content(msg)
+
         email_msg["Subject"] = "Message from {0}".format(ctf_name)
         email_msg["From"] = from_addr
         email_msg["To"] = to_addr
 
-        mock_smtp.return_value.sendmail.assert_called_once_with(
+        if six.PY2:
-            from_addr, [to_addr], email_msg.as_string()
+            mock_smtp.return_value.sendmail.assert_called_with(
-        )
+                from_addr, [to_addr], email_msg.as_string()
+            )
+        else:
+            mock_smtp.return_value.send_message.assert_called()
+            assert str(mock_smtp.return_value.send_message.call_args[0][0]) == str(
+                email_msg
+            )
     destroy_ctfd(app)
 
 
@@ -66,14 +81,24 @@ def test_sendmail_with_smtp_from_db_config(mock_smtp):
         sendmail(to_addr, msg)
 
         ctf_name = get_config("ctf_name")
-        email_msg = MIMEText(msg)
+        if six.PY2:
+            email_msg = MIMEText(msg)
+        else:
+            email_msg = EmailMessage()
+            email_msg.set_content(msg)
         email_msg["Subject"] = "Message from {0}".format(ctf_name)
         email_msg["From"] = from_addr
         email_msg["To"] = to_addr
 
-        mock_smtp.return_value.sendmail.assert_called_once_with(
+        if six.PY2:
-            from_addr, [to_addr], email_msg.as_string()
+            mock_smtp.return_value.sendmail.assert_called_with(
-        )
+                from_addr, [to_addr], email_msg.as_string()
+            )
+        else:
+            mock_smtp.return_value.send_message.assert_called()
+            assert str(mock_smtp.return_value.send_message.call_args[0][0]) == str(
+                email_msg
+            )
     destroy_ctfd(app)
 
 
@@ -85,18 +110,11 @@ def test_sendmail_with_mailgun_from_config_file(fake_post_request):
         app.config["MAILGUN_API_KEY"] = "key-1234567890-file-config"
         app.config["MAILGUN_BASE_URL"] = "https://api.mailgun.net/v3/file.faked.com"
 
-        from_addr = get_config("mailfrom_addr") or app.config.get("MAILFROM_ADDR")
         to_addr = "user@user.com"
         msg = "this is a test"
 
         sendmail(to_addr, msg)
 
-        ctf_name = get_config("ctf_name")
-        email_msg = MIMEText(msg)
-        email_msg["Subject"] = "Message from {0}".format(ctf_name)
-        email_msg["From"] = from_addr
-        email_msg["To"] = to_addr
-
         fake_response = Mock()
         fake_post_request.return_value = fake_response
         fake_response.status_code = 200
@@ -132,18 +150,11 @@ def test_sendmail_with_mailgun_from_db_config(fake_post_request):
         set_config("mailgun_api_key", "key-1234567890-db-config")
         set_config("mailgun_base_url", "https://api.mailgun.net/v3/db.faked.com")
 
-        from_addr = get_config("mailfrom_addr") or app.config.get("MAILFROM_ADDR")
         to_addr = "user@user.com"
         msg = "this is a test"
 
         sendmail(to_addr, msg)
 
-        ctf_name = get_config("ctf_name")
-        email_msg = MIMEText(msg)
-        email_msg["Subject"] = "Message from {0}".format(ctf_name)
-        email_msg["From"] = from_addr
-        email_msg["To"] = to_addr
-
         fake_response = Mock()
         fake_post_request.return_value = fake_response
         fake_response.status_code = 200
@@ -196,18 +207,26 @@ def test_verify_email(mock_smtp):
         )
 
         ctf_name = get_config("ctf_name")
-        email_msg = MIMEText(msg)
+        if six.PY2:
+            email_msg = MIMEText(msg)
+        else:
+            email_msg = EmailMessage()
+            email_msg.set_content(msg)
         email_msg["Subject"] = "Confirm your account for {ctf_name}".format(
             ctf_name=ctf_name
         )
         email_msg["From"] = from_addr
         email_msg["To"] = to_addr
 
-        # Need to freeze time to predict the value of the itsdangerous token.
+        if six.PY2:
-        # For now just assert that sendmail was called.
+            mock_smtp.return_value.sendmail.assert_called_with(
-        mock_smtp.return_value.sendmail.assert_called_with(
+                from_addr, [to_addr], email_msg.as_string()
-            from_addr, [to_addr], email_msg.as_string()
+            )
-        )
+        else:
+            mock_smtp.return_value.send_message.assert_called()
+            assert str(mock_smtp.return_value.send_message.call_args[0][0]) == str(
+                email_msg
+            )
     destroy_ctfd(app)
 
 
@@ -233,16 +252,24 @@ def test_successful_registration_email(mock_smtp):
 
         msg = "You've successfully registered for CTFd!"
 
-        email_msg = MIMEText(msg)
+        if six.PY2:
+            email_msg = MIMEText(msg)
+        else:
+            email_msg = EmailMessage()
+            email_msg.set_content(msg)
         email_msg["Subject"] = "Successfully registered for {ctf_name}".format(
             ctf_name=ctf_name
         )
         email_msg["From"] = from_addr
         email_msg["To"] = to_addr
 
-        # Need to freeze time to predict the value of the itsdangerous token.
+        if six.PY2:
-        # For now just assert that sendmail was called.
+            mock_smtp.return_value.sendmail.assert_called_with(
-        mock_smtp.return_value.sendmail.assert_called_with(
+                from_addr, [to_addr], email_msg.as_string()
-            from_addr, [to_addr], email_msg.as_string()
+            )
-        )
+        else:
+            mock_smtp.return_value.send_message.assert_called()
+            assert str(mock_smtp.return_value.send_message.call_args[0][0]) == str(
+                email_msg
+            )
     destroy_ctfd(app)