Repository: CTFd/CTFd
Pull request #532 (2017-12-19 08:36:53)

Use mailserver before mailgun & test mailgun behavior
Prioritize mailserver over mailgun and test mailgun behavior.

codecov[bot]: # [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/532?src=pr&el=h1) Report
> Merging [#532](https://codecov.io/gh/CTFd/CTFd/pull/532?src=pr&el=desc) into [master](https://codecov.io/gh/CTFd/CTFd/commit/94a29e43e9efbd651fd1e1f6d6b6c70c921a8887?src=pr&el=desc) will **increase** coverage by `0.23%`.
> The diff coverage is `78.57%`.

[![Impacted file tree graph](https://codecov.io/gh/CTFd/CTFd/pull/532/graphs/tree.svg?src=pr&token=IfPqkWkt9A&width=650&height=150)](https://codecov.io/gh/CTFd/CTFd/pull/532?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #532      +/-   ##
==========================================
+ Coverage   71.97%   72.21%   +0.23%     
==========================================
  Files          18       18              
  Lines        2655     2663       +8     
==========================================
+ Hits         1911     1923      +12     
+ Misses        744      740       -4
```


| [Impacted Files](https://codecov.io/gh/CTFd/CTFd/pull/532?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [CTFd/utils.py](https://codecov.io/gh/CTFd/CTFd/pull/532/diff?src=pr&el=tree#diff-Q1RGZC91dGlscy5weQ==) | `82.73% <78.57%> (+0.89%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/CTFd/CTFd/pull/532?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/532?src=pr&el=footer). Last update [94a29e4...e6ec5e5](https://codecov.io/gh/CTFd/CTFd/pull/532?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CTFd/utils.py b/CTFd/utils.py
index 193c3ad8..ca015c27 100644
--- a/CTFd/utils.py
+++ b/CTFd/utils.py
@@ -558,21 +558,21 @@ def set_config(key, value):
 
 @cache.memoize()
 def can_send_mail():
-    return mailgun() or mailserver()
+    return mailserver() or mailgun()
 
 
 @cache.memoize()
 def mailgun():
     if app.config.get('MAILGUN_API_KEY') and app.config.get('MAILGUN_BASE_URL'):
         return True
-    if (get_config('mg_api_key') and get_config('mg_base_url')):
+    if get_config('mg_api_key') and get_config('mg_base_url'):
         return True
     return False
 
 
 @cache.memoize()
 def mailserver():
-    if (get_config('mail_server') and get_config('mail_port')):
+    if get_config('mail_server') and get_config('mail_port'):
         return True
     return False
 
@@ -595,16 +595,28 @@ def sendmail(addr, text):
     ctf_name = get_config('ctf_name')
     mailfrom_addr = get_config('mailfrom_addr') or app.config.get('MAILFROM_ADDR')
     if mailgun():
-        mg_api_key = get_config('mg_api_key') or app.config.get('MAILGUN_API_KEY')
-        mg_base_url = get_config('mg_base_url') or app.config.get('MAILGUN_BASE_URL')
-
-        r = requests.post(
-            mg_base_url + '/messages',
-            auth=("api", mg_api_key),
-            data={"from": "{} Admin <{}>".format(ctf_name, mailfrom_addr),
-                  "to": [addr],
-                  "subject": "Message from {0}".format(ctf_name),
-                  "text": text})
+        if get_config('mg_api_key') and get_config('mg_base_url'):
+            mg_api_key = get_config('mg_api_key')
+            mg_base_url = get_config('mg_base_url')
+        elif app.config.get('MAILGUN_API_KEY') and app.config.get('MAILGUN_BASE_URL'):
+            mg_api_key = app.config.get('MAILGUN_API_KEY')
+            mg_base_url = app.config.get('MAILGUN_BASE_URL')
+        else:
+            return False, "Mailgun settings are missing"
+
+        try:
+            r = requests.post(
+                mg_base_url + '/messages',
+                auth=("api", mg_api_key),
+                data={"from": "{} Admin <{}>".format(ctf_name, mailfrom_addr),
+                      "to": [addr],
+                      "subject": "Message from {0}".format(ctf_name),
+                      "text": text},
+                timeout=1.0
+            )
+        except requests.RequestException as e:
+            return False, "{error} exception occured while handling your request".format(error=type(e).__name__)
+
         if r.status_code == 200:
             return True, "Email sent"
         else:


Test patch:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 50af6b98..6425085a 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -8,6 +8,7 @@ from CTFd.utils import register_plugin_script, register_plugin_stylesheet
 from CTFd.utils import base64encode, base64decode
 from CTFd.utils import check_email_format
 from CTFd.utils import update_check
+from email.mime.text import MIMEText
 from freezegun import freeze_time
 from mock import patch, Mock
 import json
@@ -117,7 +118,6 @@ def test_admin_override_template():
 @patch('smtplib.SMTP')
 def test_sendmail_with_smtp(mock_smtp):
     """Does sendmail work properly with simple SMTP mail servers"""
-    from email.mime.text import MIMEText
     app = create_ctfd()
     with app.app_context():
         set_config('mail_server', 'localhost')
@@ -141,11 +141,92 @@ def test_sendmail_with_smtp(mock_smtp):
     destroy_ctfd(app)
 
 
+@patch.object(requests, 'post')
+def test_sendmail_with_mailgun_from_config_file(fake_post_request):
+    """Does sendmail work properly with Mailgun using file configuration"""
+    app = create_ctfd()
+    with app.app_context():
+        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
+
+        status, message = sendmail(to_addr, msg)
+
+        args, kwargs = fake_post_request.call_args
+        assert args[0] == 'https://api.mailgun.net/v3/file.faked.com/messages'
+        assert kwargs['auth'] == ('api', u'key-1234567890-file-config')
+        assert kwargs['timeout'] == 1.0
+        assert kwargs['data'] == {'to': ['user@user.com'], 'text': 'this is a test',
+                                  'from': 'CTFd Admin <noreply@ctfd.io>', 'subject': 'Message from CTFd'}
+
+        assert fake_response.status_code == 200
+        assert status is True
+        assert message == "Email sent"
+    destroy_ctfd(app)
+
+
+@patch.object(requests, 'post')
+def test_sendmail_with_mailgun_from_db_config(fake_post_request):
+    """Does sendmail work properly with Mailgun using database configuration"""
+    app = create_ctfd()
+    with app.app_context():
+        app.config['MAILGUN_API_KEY'] = 'key-1234567890-file-config'
+        app.config['MAILGUN_BASE_URL'] = 'https://api.mailgun.net/v3/file.faked.com'
+
+        # db values should take precedence over file values
+        set_config('mg_api_key', 'key-1234567890-db-config')
+        set_config('mg_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
+
+        status, message = sendmail(to_addr, msg)
+
+        args, kwargs = fake_post_request.call_args
+        assert args[0] == 'https://api.mailgun.net/v3/db.faked.com/messages'
+        assert kwargs['auth'] == ('api', u'key-1234567890-db-config')
+        assert kwargs['timeout'] == 1.0
+        assert kwargs['data'] == {'to': ['user@user.com'], 'text': 'this is a test',
+                                  'from': 'CTFd Admin <noreply@ctfd.io>', 'subject': 'Message from CTFd'}
+
+        assert fake_response.status_code == 200
+        assert status is True
+        assert message == "Email sent"
+    destroy_ctfd(app)
+
+
 @patch('smtplib.SMTP')
 @freeze_time("2012-01-14 03:21:34")
 def test_verify_email(mock_smtp):
     """Does verify_email send emails"""
-    from email.mime.text import MIMEText
     app = create_ctfd()
     with app.app_context():
         set_config('mail_server', 'localhost')