From f826097c6755d6737c77394f914c6c6779c4d25f Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Tue, 24 Sep 2024 19:36:05 +0200 Subject: [PATCH 1/5] Move up `mail_from` config dependency --- pycroft/lib/mail.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail.py index e5d62f103..a1270746f 100644 --- a/pycroft/lib/mail.py +++ b/pycroft/lib/mail.py @@ -67,10 +67,10 @@ def render(self, **kwargs: t.Any) -> tuple[str, str]: return plain, html -def compose_mail(mail: Mail) -> MIMEMultipart: +def compose_mail(mail: Mail, from_: str) -> MIMEMultipart: msg = MIMEMultipart("alternative", _charset="utf-8") msg["Message-Id"] = make_msgid() - msg["From"] = mail_from + msg["From"] = from_ msg["To"] = str(Header(mail.to_address)) msg["Subject"] = mail.subject msg["Date"] = formatdate(localtime=True) @@ -159,7 +159,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: for mail in mails: try: - mime_mail = compose_mail(mail) + mime_mail = compose_mail(mail, from_=mail_from) assert mail_envelope_from is not None smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, msg=mime_mail.as_string()) From f1d10026797398939a2b6d8807de2d25d2d04bb6 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Tue, 24 Sep 2024 19:41:48 +0200 Subject: [PATCH 2/5] Group mail config context in `MailConfig` --- pycroft/lib/mail.py | 117 ++++++++++++++++++++++++++++++++------------ 1 file changed, 87 insertions(+), 30 deletions(-) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail.py index a1270746f..07bd78f7f 100644 --- a/pycroft/lib/mail.py +++ b/pycroft/lib/mail.py @@ -2,44 +2,38 @@ pycroft.lib.mail ~~~~~~~~~~~~~~~~ """ + +from __future__ import annotations import logging import os import smtplib import ssl import traceback import typing as t -from dataclasses import dataclass +from dataclasses import dataclass, field from email.header import Header from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText from email.utils import make_msgid, formatdate +from functools import lru_cache import jinja2 from pycroft.lib.exc import PycroftLibException -mail_envelope_from = os.environ["PYCROFT_MAIL_ENVELOPE_FROM"] -mail_from = os.environ["PYCROFT_MAIL_FROM"] -mail_reply_to = os.environ["PYCROFT_MAIL_REPLY_TO"] -smtp_host = os.environ["PYCROFT_SMTP_HOST"] -smtp_port = int(os.environ.get('PYCROFT_SMTP_PORT', 465)) -smtp_user = os.environ["PYCROFT_SMTP_USER"] -smtp_password = os.environ["PYCROFT_SMTP_PASSWORD"] -smtp_ssl = os.environ.get('PYCROFT_SMTP_SSL', 'ssl') -template_path_type = os.environ.get('PYCROFT_TEMPLATE_PATH_TYPE', 'filesystem') -template_path = os.environ.get('PYCROFT_TEMPLATE_PATH', 'pycroft/templates') -logger = logging.getLogger('mail') -logger.setLevel(logging.INFO) +# TODO proxy and DI; set at app init +config: MailConfig | None = None -template_loader: jinja2.BaseLoader -if template_path_type == 'filesystem': - template_loader = jinja2.FileSystemLoader(searchpath=f'{template_path}/mail') -else: - template_loader = jinja2.PackageLoader(package_name='pycroft', - package_path=f'{template_path}/mail') -template_env = jinja2.Environment(loader=template_loader) +def set_config(value: MailConfig) -> None: + global config + config = value + return + + +logger = logging.getLogger('mail') +logger.setLevel(logging.INFO) @dataclass @@ -61,13 +55,24 @@ def __init__(self, **kwargs: t.Any) -> None: self.args = kwargs def render(self, **kwargs: t.Any) -> tuple[str, str]: - plain = template_env.get_template(self.template).render(mode='plain', **self.args, **kwargs) - html = template_env.get_template(self.template).render(mode='html', **self.args, **kwargs) + plain = self.jinja_template.render(mode="plain", **self.args, **kwargs) + html = self.jinja_template.render(mode="html", **self.args, **kwargs) return plain, html + @property + def jinja_template(self) -> jinja2.Template: + return _get_template(self.template) + + +@lru_cache(maxsize=None) +def _get_template(template_location: str) -> jinja2.Template: + if config is None: + raise RuntimeError("`mail.config` not set up!") + return config.template_env.get_template(template_location) -def compose_mail(mail: Mail, from_: str) -> MIMEMultipart: + +def compose_mail(mail: Mail, from_: str, default_reply_to: str) -> MIMEMultipart: msg = MIMEMultipart("alternative", _charset="utf-8") msg["Message-Id"] = make_msgid() msg["From"] = from_ @@ -81,7 +86,7 @@ def compose_mail(mail: Mail, from_: str) -> MIMEMultipart: msg.attach(MIMEText(mail.body_html, 'html', _charset='utf-8')) if mail.reply_to is not None or mail.reply_to is not None: - msg['Reply-To'] = mail_reply_to if mail.reply_to is None else mail.reply_to + msg["Reply-To"] = default_reply_to if mail.reply_to is None else mail.reply_to print(msg) @@ -101,11 +106,17 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: :returns: Whether the transmission succeeded """ - - if not smtp_host: - logger.critical("No mailserver config available") - - raise RuntimeError + if config is None: + raise RuntimeError("`mail.config` not set up!") + + mail_envelope_from = config.mail_envelope_from + mail_from = config.mail_from + mail_reply_to = config.mail_reply_to + smtp_host = config.smtp_host + smtp_port = config.smtp_port + smtp_user = config.smtp_user + smtp_password = config.smtp_password + smtp_ssl = config.smtp_ssl use_ssl = smtp_ssl == 'ssl' use_starttls = smtp_ssl == 'starttls' @@ -159,7 +170,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: for mail in mails: try: - mime_mail = compose_mail(mail, from_=mail_from) + mime_mail = compose_mail(mail, from_=mail_from, default_reply_to=mail_reply_to) assert mail_envelope_from is not None smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, msg=mime_mail.as_string()) @@ -248,3 +259,49 @@ def send_template_mails( from pycroft.task import send_mails_async send_mails_async.delay(mails) + + +@dataclass +class MailConfig: + mail_envelope_from: str + mail_from: str + mail_reply_to: str + smtp_host: str + smtp_port: int + smtp_user: str + smtp_password: str + smtp_ssl: str + template_path_type: str + template_path: str + template_env: jinja2.Environment = field(init=False) + + @classmethod + def from_env(cls) -> t.Self: + env = os.environ + return cls( + mail_envelope_from=env["PYCROFT_MAIL_ENVELOPE_FROM"], + mail_from=env["PYCROFT_MAIL_FROM"], + mail_reply_to=env["PYCROFT_MAIL_REPLY_TO"], + smtp_host=env["PYCROFT_SMTP_HOST"], + smtp_port=int(env.get("PYCROFT_SMTP_PORT", 465)), + smtp_user=env["PYCROFT_SMTP_USER"], + smtp_password=env["PYCROFT_SMTP_PASSWORD"], + smtp_ssl=env.get("PYCROFT_SMTP_SSL", "ssl"), + template_path_type=env.get("PYCROFT_TEMPLATE_PATH_TYPE", "filesystem"), + template_path=env.get("PYCROFT_TEMPLATE_PATH", "pycroft/templates"), + ) + + def __post_init__(self) -> None: + template_loader: jinja2.BaseLoader + if self.template_path_type == "filesystem": + template_loader = jinja2.FileSystemLoader(searchpath=f"{self.template_path}/mail") + else: + template_loader = jinja2.PackageLoader( + package_name="pycroft", package_path=f"{self.template_path}/mail" + ) + + self.template_env = jinja2.Environment(loader=template_loader) + + +# TODO do on demand at initialization; replace `config` by proxy to `_config` +set_config(MailConfig.from_env()) From 9820de3583c37c05664dfb711d27b82f17bd8a45 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 10:34:14 +0200 Subject: [PATCH 3/5] lib: use configvar for mail config --- pycroft/lib/mail.py | 55 +++++++++++++++++++++++-------------------- tests/lib/conftest.py | 17 +++++++++++++ 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail.py index 07bd78f7f..b3b61aab8 100644 --- a/pycroft/lib/mail.py +++ b/pycroft/lib/mail.py @@ -10,7 +10,8 @@ import ssl import traceback import typing as t -from dataclasses import dataclass, field +from contextvars import ContextVar +from dataclasses import dataclass, field, InitVar from email.header import Header from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText @@ -18,19 +19,14 @@ from functools import lru_cache import jinja2 +from werkzeug.local import LocalProxy from pycroft.lib.exc import PycroftLibException # TODO proxy and DI; set at app init -config: MailConfig | None = None - - -def set_config(value: MailConfig) -> None: - global config - config = value - return - +_config_var: ContextVar[MailConfig] = ContextVar("config") +config: MailConfig = LocalProxy(_config_var) # type: ignore[assignment] logger = logging.getLogger('mail') logger.setLevel(logging.INFO) @@ -105,6 +101,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: :param mails: A list of mails :returns: Whether the transmission succeeded + :context: config """ if config is None: raise RuntimeError("`mail.config` not set up!") @@ -267,41 +264,47 @@ class MailConfig: mail_from: str mail_reply_to: str smtp_host: str - smtp_port: int smtp_user: str smtp_password: str - smtp_ssl: str - template_path_type: str - template_path: str + smtp_port: int = field(default=465) + smtp_ssl: str = field(default="ssl") + + template_path_type: InitVar[str | None] = None + template_path: InitVar[str | None] = None template_env: jinja2.Environment = field(init=False) @classmethod def from_env(cls) -> t.Self: env = os.environ - return cls( + config = cls( mail_envelope_from=env["PYCROFT_MAIL_ENVELOPE_FROM"], mail_from=env["PYCROFT_MAIL_FROM"], mail_reply_to=env["PYCROFT_MAIL_REPLY_TO"], smtp_host=env["PYCROFT_SMTP_HOST"], - smtp_port=int(env.get("PYCROFT_SMTP_PORT", 465)), smtp_user=env["PYCROFT_SMTP_USER"], smtp_password=env["PYCROFT_SMTP_PASSWORD"], - smtp_ssl=env.get("PYCROFT_SMTP_SSL", "ssl"), - template_path_type=env.get("PYCROFT_TEMPLATE_PATH_TYPE", "filesystem"), - template_path=env.get("PYCROFT_TEMPLATE_PATH", "pycroft/templates"), + template_path_type=env.get("PYCROFT_TEMPLATE_PATH_TYPE"), + template_path=env.get("PYCROFT_TEMPLATE_PATH"), ) + if (smtp_port := env.get("PYCROFT_SMTP_SSL")) is not None: + config.smtp_port = int(smtp_port) + if (smtp_ssl := env.get("PYCROFT_SMTP_SSL")) is not None: + config.smtp_ssl = smtp_ssl - def __post_init__(self) -> None: + return config + + def __post_init__(self, template_path_type: str | None, template_path: str | None) -> None: template_loader: jinja2.BaseLoader - if self.template_path_type == "filesystem": - template_loader = jinja2.FileSystemLoader(searchpath=f"{self.template_path}/mail") + if template_path_type is None: + template_path_type = "filesystem" + if template_path is None: + template_path = "pycroft/templates" + + if template_path_type == "filesystem": + template_loader = jinja2.FileSystemLoader(searchpath=f"{template_path}/mail") else: template_loader = jinja2.PackageLoader( - package_name="pycroft", package_path=f"{self.template_path}/mail" + package_name="pycroft", package_path=f"{template_path}/mail" ) self.template_env = jinja2.Environment(loader=template_loader) - - -# TODO do on demand at initialization; replace `config` by proxy to `_config` -set_config(MailConfig.from_env()) diff --git a/tests/lib/conftest.py b/tests/lib/conftest.py index da0db345a..a7ce75d7a 100644 --- a/tests/lib/conftest.py +++ b/tests/lib/conftest.py @@ -6,6 +6,7 @@ from sqlalchemy.future import select from pycroft.model import _all as m +from pycroft.lib.mail import MailConfig, _config_var from tests import factories @@ -22,3 +23,19 @@ def processor(module_session) -> m.User: @pytest.fixture(scope="module") def config(module_session) -> m.Config: return factories.ConfigFactory.create() + + +@pytest.fixture(scope="module", autouse=True) +def with_mail_config(): + token = _config_var.set( + MailConfig( + mail_envelope_from="noreply@agdsn.de", + mail_from="noreply@agdsn.de", + mail_reply_to="support@agdsn.de", + smtp_host="agdsn.de", + smtp_user="pycroft", + smtp_password="password", + ) + ) + yield + _config_var.reset(token) From 725e2b47f6d02e2bc6a890d252e4b388110773af Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 10:55:17 +0200 Subject: [PATCH 4/5] Fix nullability requirement of `reply_to` This fixes a buggy if-check introduced in 02eb901744f5ffe42fe468d460cc291d06ed3a68. It clarifies the nullability intent of the environment variable, which e.g. is not set in the dev setup (but might be in production). --- pycroft/lib/mail.py | 10 +++++----- scripts/server_run.py | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail.py index b3b61aab8..88b4dcf09 100644 --- a/pycroft/lib/mail.py +++ b/pycroft/lib/mail.py @@ -68,7 +68,7 @@ def _get_template(template_location: str) -> jinja2.Template: return config.template_env.get_template(template_location) -def compose_mail(mail: Mail, from_: str, default_reply_to: str) -> MIMEMultipart: +def compose_mail(mail: Mail, from_: str, default_reply_to: str | None) -> MIMEMultipart: msg = MIMEMultipart("alternative", _charset="utf-8") msg["Message-Id"] = make_msgid() msg["From"] = from_ @@ -81,8 +81,8 @@ def compose_mail(mail: Mail, from_: str, default_reply_to: str) -> MIMEMultipart if mail.body_html is not None: msg.attach(MIMEText(mail.body_html, 'html', _charset='utf-8')) - if mail.reply_to is not None or mail.reply_to is not None: - msg["Reply-To"] = default_reply_to if mail.reply_to is None else mail.reply_to + if reply_to := mail.reply_to or default_reply_to: + msg["Reply-To"] = reply_to print(msg) @@ -262,7 +262,7 @@ def send_template_mails( class MailConfig: mail_envelope_from: str mail_from: str - mail_reply_to: str + mail_reply_to: str | None smtp_host: str smtp_user: str smtp_password: str @@ -279,7 +279,7 @@ def from_env(cls) -> t.Self: config = cls( mail_envelope_from=env["PYCROFT_MAIL_ENVELOPE_FROM"], mail_from=env["PYCROFT_MAIL_FROM"], - mail_reply_to=env["PYCROFT_MAIL_REPLY_TO"], + mail_reply_to=env.get("PYCROFT_MAIL_REPLY_TO"), smtp_host=env["PYCROFT_SMTP_HOST"], smtp_user=env["PYCROFT_SMTP_USER"], smtp_password=env["PYCROFT_SMTP_PASSWORD"], diff --git a/scripts/server_run.py b/scripts/server_run.py index baa3bc9fc..1726791fe 100755 --- a/scripts/server_run.py +++ b/scripts/server_run.py @@ -19,6 +19,7 @@ import pycroft import web from pycroft.helpers.i18n import set_translation_lookup, get_locale +from pycroft.lib.mail import _config_var, MailConfig from pycroft.model.session import set_scoped_session from scripts.connection import get_connection_string from pycroft.model.alembic import determine_schema_state @@ -54,6 +55,8 @@ def prepare_server(echo=False, ensure_schema=False) -> PycroftFlask: ) ) _setup_translations() + _config_var.set(MailConfig.from_env()) + if app.config.get("PROFILE", False): app.wsgi_app = ProfilerMiddleware(app.wsgi_app, restrictions=[30]) return app From 3512c07edeb4620d8afab4bcc06103abab154fa0 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 10:59:42 +0200 Subject: [PATCH 5/5] lib.mail: Move MIMEText transformation to `Mail` class --- pycroft/lib/mail.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail.py index 88b4dcf09..80f121d55 100644 --- a/pycroft/lib/mail.py +++ b/pycroft/lib/mail.py @@ -41,6 +41,16 @@ class Mail: body_html: str | None = None reply_to: str | None = None + @property + def body_plain_mime(self) -> MIMEText: + return MIMEText(self.body_plain, "plain", _charset="utf-8") + + @property + def body_html_mime(self) -> MIMEText | None: + if not self.body_html: + return None + return MIMEText(self.body_html, "html", _charset="utf-8") + class MailTemplate: template: str @@ -75,12 +85,9 @@ def compose_mail(mail: Mail, from_: str, default_reply_to: str | None) -> MIMEMu msg["To"] = str(Header(mail.to_address)) msg["Subject"] = mail.subject msg["Date"] = formatdate(localtime=True) - - msg.attach(MIMEText(mail.body_plain, 'plain', _charset='utf-8')) - - if mail.body_html is not None: - msg.attach(MIMEText(mail.body_html, 'html', _charset='utf-8')) - + msg.attach(mail.body_plain_mime) + if (html := mail.body_html_mime) is not None: + msg.attach(html) if reply_to := mail.reply_to or default_reply_to: msg["Reply-To"] = reply_to