Repository: radiasoft/sirepo

Issue:
add message-id to outgoing smtp
I thought the smtp library or postfix would add the message-id, but it doesn't. The message id should be formatted like `<datetime.random@sending-host>`.


Discussion:
robnagler: @garsuga ping

Patch:
diff --git a/sirepo/smtp.py b/sirepo/smtp.py
index cb59a990a..f8b722f0c 100644
--- a/sirepo/smtp.py
+++ b/sirepo/smtp.py
@@ -10,26 +10,36 @@ from pykern.pkdebug import pkdp, pkdlog
 from pykern import pkconfig
 from pykern.pkcollections import PKDict
 import email
+import email.utils
+import pyisemail
 import smtplib
 
 _DEV_SMTP_SERVER = "dev"
 _SEND = None
-cfg = None
+_FROM_DOMAIN = None
+_cfg = None
 
 
 def send(recipient, subject, body):
-    if cfg.server == _DEV_SMTP_SERVER:
+    if _cfg.server == _DEV_SMTP_SERVER:
         pkdlog("DEV configuration so not sending to {}", recipient)
         return False
     m = email.message.EmailMessage()
-    m["From"] = f"{cfg.from_name} <{cfg.from_email}>"
+    m["From"] = f"{_cfg.from_name} <{_cfg.from_email}>"
     m["To"] = recipient
     m["Subject"] = subject
+    m["Message-Id"] = email.utils.make_msgid(domain=_FROM_DOMAIN)
     m.set_content(body)
     _SEND(m)
     return True
 
 
+def _cfg_from_email(value):
+    if not pyisemail.is_email(value):
+        pkconfig.raise_error(f"invalid from_email={value}")
+    return value.lower()
+
+
 def _mx(msg):
     import dns.resolver
 
@@ -59,19 +69,19 @@ def _send_directly(msg):
 
 
 def _send_with_auth(msg):
-    with smtplib.SMTP(cfg.server, cfg.port) as s:
+    with smtplib.SMTP(_cfg.server, _cfg.port) as s:
         s.starttls()
         s.ehlo()
-        s.login(cfg.user, cfg.password)
+        s.login(_cfg.user, _cfg.password)
         s.send_message(msg)
 
 
 def _init():
-    global cfg, _SEND
-    if cfg:
+    global _cfg, _SEND, _FROM_DOMAIN
+    if _cfg:
         return
-    cfg = pkconfig.init(
-        from_email=("support@sirepo.com", str, "Email address of sender"),
+    _cfg = pkconfig.init(
+        from_email=("support@sirepo.com", _cfg_from_email, "Email address of sender"),
         from_name=("Sirepo Support", str, "Name of email sender"),
         password=(None, str, "SMTP password"),
         port=(587, int, "SMTP Port"),
@@ -79,18 +89,19 @@ def _init():
         server=(None, str, "SMTP TLS server"),
         user=(None, str, "SMTP user"),
     )
-    if cfg.send_directly:
-        cfg.server = "not " + _DEV_SMTP_SERVER
+    _FROM_DOMAIN = _cfg.from_email.split("@")[1]
+    if _cfg.send_directly:
+        _cfg.server = "not " + _DEV_SMTP_SERVER
         _SEND = _send_directly
         return
     _SEND = _send_with_auth
     if pkconfig.in_dev_mode():
-        if cfg.server is None:
-            cfg.server = _DEV_SMTP_SERVER
+        if _cfg.server is None:
+            _cfg.server = _DEV_SMTP_SERVER
         return
-    if cfg.server is None or cfg.user is None or cfg.password is None:
+    if _cfg.server is None or _cfg.user is None or _cfg.password is None:
         pkconfig.raise_error(
-            f"server={cfg.server}, user={cfg.user}, and password={cfg.password} must be defined",
+            f"server={_cfg.server}, user={_cfg.user}, and password={_cfg.password} must be defined",
         )
 
 


Test patch:
diff --git a/tests/auth/guest_deprecated_test.py b/tests/auth/guest_deprecated_test.py
index 96a142960..bdbff0561 100644
--- a/tests/auth/guest_deprecated_test.py
+++ b/tests/auth/guest_deprecated_test.py
@@ -14,7 +14,7 @@ pytestmark = pytest.mark.sirepo_args(
     fc_module=PKDict(
         cfg=PKDict(
             SIREPO_AUTH_DEPRECATED_METHODS="guest",
-            SIREPO_SMTP_FROM_EMAIL="x",
+            SIREPO_SMTP_FROM_EMAIL="x@x.x",
             SIREPO_SMTP_FROM_NAME="x",
             SIREPO_SMTP_PASSWORD="x",
             SIREPO_SMTP_SERVER="dev",
diff --git a/tests/auth_db_test.py b/tests/auth_db_test.py
index 5c78394a2..8b9daa9d4 100644
--- a/tests/auth_db_test.py
+++ b/tests/auth_db_test.py
@@ -12,7 +12,7 @@ import pytest
 _sim_type = "myapp"
 _cfg = {
     "SIREPO_AUTH_DEPRECATED_METHODS": "github",
-    "SIREPO_SMTP_FROM_EMAIL": "x",
+    "SIREPO_SMTP_FROM_EMAIL": "x@x.x",
     "SIREPO_SMTP_FROM_NAME": "x",
     "SIREPO_SMTP_PASSWORD": "x",
     "SIREPO_SMTP_SERVER": "dev",
diff --git a/tests/conftest.py b/tests/conftest.py
index ee663ec86..a8f390380 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -168,7 +168,7 @@ def _auth_client_module(request):
     cfg = PKDict(
         SIREPO_AUTH_BASIC_PASSWORD="pass",
         SIREPO_AUTH_BASIC_UID="dev-no-validate",
-        SIREPO_SMTP_FROM_EMAIL="x",
+        SIREPO_SMTP_FROM_EMAIL="x@x.x",
         SIREPO_SMTP_FROM_NAME="x",
         SIREPO_SMTP_PASSWORD="x",
         SIREPO_SMTP_SERVER="dev",
diff --git a/tests/smtp1_test.py b/tests/smtp1_test.py
index d5c85ffab..1461e3537 100644
--- a/tests/smtp1_test.py
+++ b/tests/smtp1_test.py
@@ -15,19 +15,19 @@ pytestmark = pytest.mark.skipif(
 
 
 def test_send_directly():
-    import pykern.pkconfig
+    from pykern import pkconfig
     from pykern.pkcollections import PKDict
 
-    pykern.pkconfig.reset_state_for_testing(
+    pkconfig.reset_state_for_testing(
         PKDict(
             SIREPO_SMTP_SEND_DIRECTLY="1",
         ),
     )
-    import sirepo.smtp
-    import sirepo.util
 
-    sirepo.smtp.send(
+    from sirepo import smtp, util
+
+    smtp.send(
         os.environ.get("SIREPO_TESTS_SMTP_EMAIL"),
-        f"sirepo.smtp1_test {sirepo.util.random_base62(5)}",
+        f"sirepo.smtp1_test {util.random_base62(5)}",
         "This is a message from sirepo/tests/smtp1_test.py\n",
     )
diff --git a/tests/smtp2_test.py b/tests/smtp2_test.py
index 66a8bc21c..4d818da53 100644
--- a/tests/smtp2_test.py
+++ b/tests/smtp2_test.py
@@ -15,20 +15,19 @@ pytestmark = pytest.mark.skipif(
 
 
 def test_send_directly():
-    import pykern.pkconfig
+    from pykern import pkconfig
     from pykern.pkcollections import PKDict
 
-    pykern.pkconfig.reset_state_for_testing(
+    pkconfig.reset_state_for_testing(
         PKDict(
             SIREPO_SMTP_SEND_DIRECTLY="0",
         ),
     )
-    import sirepo.smtp
-    import sirepo.util
+    from sirepo import smtp, util
 
-    assert sirepo.smtp.cfg.server != sirepo.smtp._DEV_SMTP_SERVER
-    sirepo.smtp.send(
+    assert smtp.cfg.server != smtp._DEV_SMTP_SERVER
+    smtp.send(
         os.environ.get("SIREPO_TESTS_SMTP_EMAIL"),
-        f"sirepo.smtp2_test {sirepo.util.random_base62(5)}",
+        f"sirepo.smtp2_test {util.random_base62(5)}",
         "This is a message from sirepo/tests/smtp2_test.py\n",
     )
diff --git a/tests/smtp3_test.py b/tests/smtp3_test.py
new file mode 100644
index 000000000..b9511f01b
--- /dev/null
+++ b/tests/smtp3_test.py
@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+"""test smtp logic, not sending
+
+:copyright: Copyright (c) 2023 RadiaSoft LLC.  All Rights Reserved.
+:license: http://www.apache.org/licenses/LICENSE-2.0.html
+"""
+import pytest
+
+
+def test_msg():
+    from pykern.pkcollections import PKDict
+    from pykern import pkconfig
+
+    d = "from-domain.com"
+    pkconfig.reset_state_for_testing(
+        PKDict(
+            SIREPO_SMTP_SEND_DIRECTLY="1",
+            SIREPO_SMTP_FROM_EMAIL=f"sender@{d}",
+        ),
+    )
+
+    from sirepo import smtp
+    from pykern import pkunit
+
+    m = None
+
+    def _send(value):
+        nonlocal m
+        m = value
+
+    smtp._SEND = _send
+    smtp.send(
+        "any-user@recipient-domain.com",
+        "any-subject",
+        "any-body",
+    )
+    pkunit.pkre(d, m["Message-Id"])