diff --git a/.env.example b/.env.example index a57adc7..21796b1 100644 --- a/.env.example +++ b/.env.example @@ -40,6 +40,16 @@ OIDC_CLIENT_SECRET= # redirect address registered at the provider is this plus /auth/callback. PUBLIC_BASE_URL=https://portal.example.test +# Required. The mail server and mailbox the portal sends e-mail through, always over TLS. +# SMTP_SECURITY is starttls (default, port 587) or ssl (port 465); SMTP_PORT overrides the port. +# MAIL_FROM is the sender address, optionally with a display name: Portal +SMTP_HOST= +# SMTP_PORT=587 +# SMTP_SECURITY=starttls +SMTP_USERNAME= +SMTP_PASSWORD= +MAIL_FROM= + # Optional. Directory for the database. Default /data (the container volume). # DATA_DIR=/data diff --git a/Dockerfile b/Dockerfile index 1ee8413..9204a39 100644 --- a/Dockerfile +++ b/Dockerfile @@ -27,6 +27,7 @@ RUN SECRET_KEY=build-time-only-value-not-used-at-runtime-000000 \ LDAP_CA_FILE=/app/README.md LDAP_PUBLISHER_GROUP_DN=build LDAP_ADMIN_GROUP_DN=build \ OIDC_ISSUER=https://build.invalid/ OIDC_CLIENT_ID=build OIDC_CLIENT_SECRET=build \ PUBLIC_BASE_URL=https://build.invalid \ + SMTP_HOST=build.invalid SMTP_USERNAME=build SMTP_PASSWORD=build MAIL_FROM=build@build.invalid \ sh -c '/app/.venv/bin/python manage.py compilemessages && /app/.venv/bin/python manage.py collectstatic --noinput' # Not part of the runtime image: `docker compose --profile test run --rm live-test` diff --git a/README.md b/README.md index e321135..360f46c 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,12 @@ All configuration comes from environment variables, validated at startup. If any | `OIDC_CLIENT_ID` | yes | | Client ID of the provider's application | | `OIDC_CLIENT_SECRET` | yes | | Client secret of the provider's application | | `PUBLIC_BASE_URL` | yes | | Address users reach the portal at, scheme and host only. The provider's redirect address must be this plus `/auth/callback` | +| `SMTP_HOST` | yes | | Host name of the mail server (no port, no URL) | +| `SMTP_SECURITY` | no | `starttls` | `starttls` (port 587) or `ssl` (port 465); mail is never sent unencrypted and the server certificate is verified | +| `SMTP_PORT` | no | 587 or 465 | Overrides the port | +| `SMTP_USERNAME` | yes | | Mailbox user name | +| `SMTP_PASSWORD` | yes | | Mailbox password | +| `MAIL_FROM` | yes | | Sender address of every e-mail, optionally with a display name | | `LOG_LEVEL` | no | `INFO` | `DEBUG`, `INFO`, `WARNING`, `ERROR`, `CRITICAL` | | `LDAP_SERVER` | yes | | Directory server host name (optionally `:port`); LDAPS on 636 is always used and the name must be in the server's certificate | | `LDAP_USER_OU_DN` | yes | | Distinguished name of the OU whose users the portal may read and change | @@ -61,6 +67,18 @@ Text is German (formal and informal) and English. The English text is the messag Theme, language and form of address are stored per device in the cookies `pm_theme`, `pm_lang` and `pm_form`; nothing is stored on the server. +### Scheduler and e-mail + +One scheduler thread runs inside the single gunicorn worker and ticks every 30 seconds (`docker/gunicorn.conf.py`). Each tick runs the jobs in `portal/scheduler/services.py`; every run is recorded in `JobRun`. Other code never talks to the mail server: it calls `portal.mailer.services.enqueue(template, recipient, context)` inside its own transaction, and the `mail.flush` job sends from the outbox with retries (1, 5, 15 and 60 minutes between attempts, 5 attempts), at most 20 mails a minute. A mail that was being sent when the process stopped is marked failed and is not sent again. + +Mail templates live in `portal/templates/mail//subject.txt` and `body.txt`. They are rendered when queued, in the portal-wide language (`MAIL_LANGUAGE`, German formal), and the text is removed from the database once the mail is sent. + +To check the SMTP settings, queue and send a test mail from the running container: + +```sh +docker compose exec portal python manage.py send_test_mail someone@example.test +``` + ### Directory adapter against the test OU `uv sync` builds `python-ldap`, which needs the OpenLDAP and SASL development libraries (the Docker image installs them). The automated tests use an in-memory fake and a stand-in for the `ldap` module, so they run anywhere. diff --git a/_bmad-output/initiative-ping-mee-portal/epic-platform-and-sign-in/story-scheduler-and-mail-outbox-plan.md b/_bmad-output/initiative-ping-mee-portal/epic-platform-and-sign-in/story-scheduler-and-mail-outbox-plan.md new file mode 100644 index 0000000..4d00fe7 --- /dev/null +++ b/_bmad-output/initiative-ping-mee-portal/epic-platform-and-sign-in/story-scheduler-and-mail-outbox-plan.md @@ -0,0 +1,36 @@ +--- +title: 'Scheduler and mail outbox' +type: 'feature' +ticket: '7' +created: '2026-10-18' +status: 'built' +baseline_revision: '0dc079a' +route: 'full' +route_source: 'auto' +risk: 'medium' +review: 'quick' +review_source: 'pinned' +context: + - '{project-root}/_bmad-output/architecture/architecture-ping-mee-self-service-platform-2026-10-15/ARCHITECTURE-SPINE.md' +--- + +## Intent + +**Problem:** Later epics must send e-mail (password change notices, confirmation links, announcements) without blocking requests, losing mail on restart or exceeding the provider's limits, and need a place to run repeating jobs. + +**Approach:** `adapters/mail` is the only code that talks SMTP (TLS always, certificate verified), with an in-memory fake. `mailer` owns the outbox; other code calls `enqueue(template, recipient, context)` in its own transaction. `scheduler` owns `JobRun` and one 30-second tick that runs idempotent jobs, started in the gunicorn worker by `docker/gunicorn.conf.py` (AD-11, AD-12, AD-19). + +## Decisions (made by the build; the spine left them open) + +- Retries: 5 attempts, waits of 1, 5, 15 and 60 minutes. Rate limit: 20 mails a minute. Named constants until the settings registry exists; the IONOS limit is still unknown (spec open question). +- Mails are rendered when queued, in German formal (`MAIL_LANGUAGE`), as plain text only. HTML parts wait for the sanitizer (AD-16). +- The body is removed from the database once the mail is sent or has failed, because it may hold a one-time link. +- Restart safety: a row is claimed (`queued` to `sending`) before sending. A row still `sending` after 10 minutes is marked failed with an unknown outcome and is not sent again: a lost notice is better than a duplicate. +- A refused login is treated as temporary, so a corrected password works on the next retry. A dead server stops the run after three consecutive outages. +- `manage.py send_test_mail
` queues and sends a test mail at once, for the done check. +- Only the `mail.flush` job exists; later entries add theirs to `JOBS`. + +## Implementation Notes + +- `pytest` 319 passed, 6 skipped; `ruff check` clean. +- Not verified: a real SMTP server and the scheduler thread inside the container (needs the Linux host). diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 2a37191..d333bce 100644 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -4,6 +4,7 @@ set -eu python manage.py migrate --noinput exec gunicorn portal.config.wsgi:application \ + --config /app/docker/gunicorn.conf.py \ --bind 0.0.0.0:8000 \ --workers 1 \ --threads 4 \ diff --git a/docker/gunicorn.conf.py b/docker/gunicorn.conf.py new file mode 100644 index 0000000..4ce1729 --- /dev/null +++ b/docker/gunicorn.conf.py @@ -0,0 +1,7 @@ +"""gunicorn settings that need code. The scheduler runs inside the single worker (AD-12).""" + + +def post_worker_init(worker): + from portal.scheduler.runner import runner + + runner.start() diff --git a/portal/adapters/mail/__init__.py b/portal/adapters/mail/__init__.py new file mode 100644 index 0000000..d871f73 --- /dev/null +++ b/portal/adapters/mail/__init__.py @@ -0,0 +1,15 @@ +"""Mail adapter: the only code that talks SMTP (AD-1).""" + +from .base import Mailer, MailError, MailMessage, MailRejected, MailUnavailable +from .fake import FakeMailer +from .smtp import SmtpMailer + +__all__ = [ + "FakeMailer", + "MailError", + "MailMessage", + "MailRejected", + "MailUnavailable", + "Mailer", + "SmtpMailer", +] diff --git a/portal/adapters/mail/base.py b/portal/adapters/mail/base.py new file mode 100644 index 0000000..a8c40f7 --- /dev/null +++ b/portal/adapters/mail/base.py @@ -0,0 +1,32 @@ +"""The mail interface the outbox uses (AD-1, AD-11).""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Protocol + + +class MailError(Exception): + """Base class. Messages never contain the recipient or the text of the e-mail.""" + + +class MailUnavailable(MailError): + """The server could not take the message right now; trying again later may work.""" + + +class MailRejected(MailError): + """The server refused the message for good (for example an unknown recipient).""" + + +@dataclass(frozen=True) +class MailMessage: + sender: str + recipient: str + subject: str + body_text: str + + +class Mailer(Protocol): + def send(self, message: MailMessage) -> None: + """Hand the message to the mail server, or raise MailUnavailable or MailRejected.""" + ... diff --git a/portal/adapters/mail/fake.py b/portal/adapters/mail/fake.py new file mode 100644 index 0000000..5b96e3d --- /dev/null +++ b/portal/adapters/mail/fake.py @@ -0,0 +1,24 @@ +"""In-memory mail server for tests (AD-1).""" + +from __future__ import annotations + +from .base import MailMessage, MailRejected, MailUnavailable + + +class FakeMailer: + """Records what was sent. `unavailable` makes every send fail temporarily; recipients in + `rejected` are refused for good.""" + + def __init__(self) -> None: + self.sent: list[MailMessage] = [] + self.unavailable = False + self.rejected: set[str] = set() + self.attempts = 0 + + def send(self, message: MailMessage) -> None: + self.attempts += 1 + if self.unavailable: + raise MailUnavailable("The mail server is not available") + if message.recipient in self.rejected: + raise MailRejected("The mail server refused the message (550)") + self.sent.append(message) diff --git a/portal/adapters/mail/smtp.py b/portal/adapters/mail/smtp.py new file mode 100644 index 0000000..e72ad26 --- /dev/null +++ b/portal/adapters/mail/smtp.py @@ -0,0 +1,89 @@ +"""Sends e-mail through an SMTP server over TLS. The only module that talks SMTP (AD-1).""" + +from __future__ import annotations + +import logging +import smtplib +import ssl +from email.message import EmailMessage +from email.utils import formatdate, make_msgid + +from portal.config.env import SmtpConfig + +from .base import MailMessage, MailRejected, MailUnavailable + +logger = logging.getLogger(__name__) + +TIMEOUT_SECONDS = 20 + + +class SmtpMailer: + def __init__(self, config: SmtpConfig, smtp_module=smtplib) -> None: + self._config = config + self._smtp = smtp_module + + def _build(self, message: MailMessage) -> EmailMessage: + email = EmailMessage() + # Setting a header that holds a line break raises ValueError, so no header injection. + email["From"] = message.sender + email["To"] = message.recipient + email["Subject"] = message.subject + email["Date"] = formatdate(localtime=False) + email["Message-ID"] = make_msgid(domain=self._config.host) + email.set_content(message.body_text, charset="utf-8") + return email + + def _connect(self): + config = self._config + context = ssl.create_default_context() # verifies the certificate and the host name + if config.security == "ssl": + connection = self._smtp.SMTP_SSL( + config.host, config.port, timeout=TIMEOUT_SECONDS, context=context + ) + else: + connection = self._smtp.SMTP(config.host, config.port, timeout=TIMEOUT_SECONDS) + try: + connection.ehlo() + if config.security == "starttls": + connection.starttls(context=context) + connection.ehlo() + connection.login(config.username, config.password) + except BaseException: + connection.close() + raise + return connection + + def send(self, message: MailMessage) -> None: + try: + email = self._build(message) + except ValueError as exc: + raise MailRejected("The message could not be built") from exc + try: + connection = self._connect() + try: + connection.send_message(email) + finally: + try: + connection.quit() + except (self._smtp.SMTPException, OSError): + connection.close() + except self._smtp.SMTPAuthenticationError as exc: + # A configuration problem: the Admin fixes it and the retry works. + logger.error("SMTP login refused (code %s)", exc.smtp_code) + raise MailUnavailable(f"The mail server refused the login ({exc.smtp_code})") from exc + except self._smtp.SMTPRecipientsRefused as exc: + codes = [code for code, _ in exc.recipients.values()] + raise _by_code(max(codes, default=0)) from exc + except self._smtp.SMTPResponseException as exc: + raise _by_code(exc.smtp_code) from exc + except (self._smtp.SMTPException, OSError, ssl.SSLError) as exc: + logger.warning("SMTP request failed: %s", type(exc).__name__) + name = type(exc).__name__ + raise MailUnavailable(f"The mail server is not available ({name})") from exc + + +def _by_code(code: int): + """Codes 5xx are permanent; everything else may work later.""" + if 500 <= code < 600: + return MailRejected(f"The mail server refused the message ({code})") + return MailUnavailable(f"The mail server could not take the message ({code})") diff --git a/portal/adapters/registry.py b/portal/adapters/registry.py index 824c978..1ff2998 100644 --- a/portal/adapters/registry.py +++ b/portal/adapters/registry.py @@ -6,9 +6,11 @@ from django.conf import settings from .directory import Directory, LdapDirectory from .idp import Idp, OidcIdp +from .mail import Mailer, SmtpMailer _directory: Directory | None = None _idp: Idp | None = None +_mailer: Mailer | None = None def get_directory() -> Directory: @@ -25,7 +27,16 @@ def get_idp() -> Idp: return _idp -def install(directory: Directory | None = None, idp: Idp | None = None) -> None: +def get_mailer() -> Mailer: + global _mailer + if _mailer is None: + _mailer = SmtpMailer(settings.SMTP) + return _mailer + + +def install( + directory: Directory | None = None, idp: Idp | None = None, mailer: Mailer | None = None +) -> None: """Replace the adapters (tests). Passing None for one leaves it to be built on first use.""" - global _directory, _idp - _directory, _idp = directory, idp + global _directory, _idp, _mailer + _directory, _idp, _mailer = directory, idp, mailer diff --git a/portal/config/env.py b/portal/config/env.py index 77bb236..51b2293 100644 --- a/portal/config/env.py +++ b/portal/config/env.py @@ -5,10 +5,13 @@ from __future__ import annotations import ipaddress from collections.abc import Mapping from dataclasses import dataclass, field +from email.utils import parseaddr from pathlib import Path from urllib.parse import urlsplit from zoneinfo import ZoneInfo, ZoneInfoNotFoundError +SMTP_SECURITY_MODES = ("starttls", "ssl") +SMTP_DEFAULT_PORTS = {"starttls": 587, "ssl": 465} LOG_LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL") MIN_SECRET_KEY_LENGTH = 32 _TRUE = {"1", "true", "yes", "on"} @@ -55,6 +58,16 @@ class OidcConfig: return f"{self.public_base_url}/auth/callback" +@dataclass(frozen=True) +class SmtpConfig: + host: str + port: int + security: str # "starttls" or "ssl"; mail is never sent unencrypted + username: str + password: str = field(repr=False) + sender: str # the From address of every e-mail + + @dataclass(frozen=True) class Config: secret_key: str @@ -66,6 +79,7 @@ class Config: default_timezone: str directory: DirectoryConfig oidc: OidcConfig + smtp: SmtpConfig def load_config(environ: Mapping[str, str]) -> Config: @@ -162,6 +176,32 @@ def load_config(environ: Mapping[str, str]) -> Config: if parts.scheme not in allowed or not parts.netloc or parts.path or parts.query: problems.append("PUBLIC_BASE_URL: must be an https origin such as https://host") + smtp_host = required("SMTP_HOST", "host name of the mail server") + if "://" in smtp_host or "/" in smtp_host or ":" in smtp_host: + problems.append("SMTP_HOST: must be a host name only; set the port in SMTP_PORT") + smtp_security = (raw("SMTP_SECURITY") or "starttls").lower() + if smtp_security not in SMTP_SECURITY_MODES: + problems.append(f"SMTP_SECURITY: must be one of {', '.join(SMTP_SECURITY_MODES)}") + smtp_security = "starttls" + smtp_port = SMTP_DEFAULT_PORTS[smtp_security] + port_raw = raw("SMTP_PORT") + if port_raw is not None: + if port_raw.isdigit() and 0 < int(port_raw) < 65536: + smtp_port = int(port_raw) + else: + problems.append("SMTP_PORT: must be a port number from 1 to 65535") + smtp_username = required("SMTP_USERNAME", "user name of the mailbox") + smtp_password = required("SMTP_PASSWORD", "password of the mailbox") + sender = required("MAIL_FROM", "sender address of the portal's e-mails") + address = parseaddr(sender)[1] + if sender and ( + address.count("@") != 1 + or any(c in sender for c in chr(13) + chr(10)) + or address.startswith("@") + or address.endswith("@") + ): + problems.append("MAIL_FROM: must be an e-mail address, optionally with a display name") + if problems: raise ConfigError(problems) @@ -188,4 +228,12 @@ def load_config(environ: Mapping[str, str]) -> Config: client_secret=client_secret, public_base_url=base_url, ), + smtp=SmtpConfig( + host=smtp_host, + port=smtp_port, + security=smtp_security, + username=smtp_username, + password=smtp_password, + sender=sender, + ), ) diff --git a/portal/config/settings.py b/portal/config/settings.py index 502b0d3..2a71c90 100644 --- a/portal/config/settings.py +++ b/portal/config/settings.py @@ -25,6 +25,7 @@ DATA_DIR = _config.data_dir DISPLAY_TIME_ZONE = _config.default_timezone DIRECTORY = _config.directory OIDC = _config.oidc +SMTP = _config.smtp INSTALLED_APPS = [ "django.contrib.contenttypes", @@ -34,6 +35,8 @@ INSTALLED_APPS = [ "portal.audit", "portal.accounts", "portal.shell", + "portal.mailer", + "portal.scheduler", ] MIDDLEWARE = [ @@ -90,6 +93,10 @@ LANGUAGE_CODE = "en" LANGUAGES = [("en", "English"), ("de", "Deutsch")] LOCALE_PATHS = [BASE_DIR / "portal" / "locale"] +# E-mails use the portal-wide language and form of address (a per-device choice cannot reach +# server-sent mail). Becomes a runtime setting with the settings registry. +MAIL_LANGUAGE = "de" + # Static files served by whitenoise; assets are built into the image. STATIC_URL = "static/" STATIC_ROOT = BASE_DIR / "staticfiles" diff --git a/portal/locale/de/LC_MESSAGES/django.po b/portal/locale/de/LC_MESSAGES/django.po index cfa938f..48a6ab1 100644 --- a/portal/locale/de/LC_MESSAGES/django.po +++ b/portal/locale/de/LC_MESSAGES/django.po @@ -107,3 +107,9 @@ msgstr "Protokoll" msgid "Status" msgstr "Status" + +msgid "Test e-mail from the portal" +msgstr "Test-E-Mail vom Portal" + +msgid "This is a test e-mail from the portal. If you can read it, sending e-mail works." +msgstr "Das ist eine Test-E-Mail vom Portal. Wenn Sie sie lesen können, funktioniert der E-Mail-Versand." diff --git a/portal/locale/de_DU/LC_MESSAGES/django.po b/portal/locale/de_DU/LC_MESSAGES/django.po index d985de7..5b3d32d 100644 --- a/portal/locale/de_DU/LC_MESSAGES/django.po +++ b/portal/locale/de_DU/LC_MESSAGES/django.po @@ -26,3 +26,6 @@ msgstr "Du wurdest abgemeldet." msgid "This is not possible right now. Please try again later." msgstr "Das ist gerade nicht möglich. Bitte versuche es später erneut." + +msgid "This is a test e-mail from the portal. If you can read it, sending e-mail works." +msgstr "Das ist eine Test-E-Mail vom Portal. Wenn du sie lesen kannst, funktioniert der E-Mail-Versand." diff --git a/portal/mailer/__init__.py b/portal/mailer/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/portal/mailer/apps.py b/portal/mailer/apps.py new file mode 100644 index 0000000..84014ba --- /dev/null +++ b/portal/mailer/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class MailerConfig(AppConfig): + name = "portal.mailer" + label = "mailer" diff --git a/portal/mailer/management/__init__.py b/portal/mailer/management/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/portal/mailer/management/commands/__init__.py b/portal/mailer/management/commands/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/portal/mailer/management/commands/send_test_mail.py b/portal/mailer/management/commands/send_test_mail.py new file mode 100644 index 0000000..b332b55 --- /dev/null +++ b/portal/mailer/management/commands/send_test_mail.py @@ -0,0 +1,28 @@ +from django.core.management.base import BaseCommand, CommandError + +from portal.mailer import services +from portal.mailer.models import OutboxMail + + +class Command(BaseCommand): + help = "Queue a test e-mail and send it now, to check the SMTP settings." + + def add_arguments(self, parser): + parser.add_argument("recipient", help="E-mail address that should receive the test mail") + + def handle(self, *args, recipient, **options): + try: + mail = services.enqueue("test", recipient) + except ValueError as exc: + raise CommandError(str(exc)) from exc + result = services.flush() + mail.refresh_from_db() + self.stdout.write(f"Outbox entry {mail.pk}: {mail.status} ({result})") + if mail.status == OutboxMail.Status.SENT: + self.stdout.write("The mail server accepted the message.") + elif mail.status == OutboxMail.Status.QUEUED: + self.stdout.write(f"Not sent yet, will be retried: {mail.last_error}") + raise SystemExit(1) + else: + self.stdout.write(f"Failed: {mail.last_error}") + raise SystemExit(1) diff --git a/portal/mailer/migrations/0001_initial.py b/portal/mailer/migrations/0001_initial.py new file mode 100644 index 0000000..8ee6966 --- /dev/null +++ b/portal/mailer/migrations/0001_initial.py @@ -0,0 +1,35 @@ +# Generated by Django 5.2.18 on 2026-10-18 07:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='OutboxMail', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('template', models.CharField(max_length=100)), + ('recipient', models.CharField(max_length=254)), + ('subject', models.CharField(max_length=300)), + ('body_text', models.TextField(blank=True)), + ('status', models.CharField(choices=[('queued', 'Queued'), ('sending', 'Sending'), ('sent', 'Sent'), ('failed', 'Failed')], default='queued', max_length=10)), + ('attempts', models.PositiveSmallIntegerField(default=0)), + ('created_at', models.DateTimeField()), + ('next_attempt_at', models.DateTimeField()), + ('claimed_at', models.DateTimeField(blank=True, null=True)), + ('sent_at', models.DateTimeField(blank=True, null=True)), + ('last_error', models.CharField(blank=True, max_length=200)), + ], + options={ + 'ordering': ['id'], + 'indexes': [models.Index(fields=['status', 'next_attempt_at'], name='mailer_outb_status_d5af6e_idx')], + }, + ), + ] diff --git a/portal/mailer/migrations/__init__.py b/portal/mailer/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/portal/mailer/models.py b/portal/mailer/models.py new file mode 100644 index 0000000..9cf6ecf --- /dev/null +++ b/portal/mailer/models.py @@ -0,0 +1,29 @@ +from django.db import models + + +class OutboxMail(models.Model): + """One e-mail waiting to be sent, being sent, sent or failed. Owned by `mailer` (AD-19).""" + + class Status(models.TextChoices): + QUEUED = "queued" + SENDING = "sending" + SENT = "sent" + FAILED = "failed" + + template = models.CharField(max_length=100) + recipient = models.CharField(max_length=254) + subject = models.CharField(max_length=300) + # Kept only until the mail is sent: it may hold a one-time link. + body_text = models.TextField(blank=True) + status = models.CharField(max_length=10, choices=Status.choices, default=Status.QUEUED) + attempts = models.PositiveSmallIntegerField(default=0) + created_at = models.DateTimeField() + next_attempt_at = models.DateTimeField() + claimed_at = models.DateTimeField(null=True, blank=True) + sent_at = models.DateTimeField(null=True, blank=True) + # What went wrong, in words without the recipient or the text of the mail. + last_error = models.CharField(max_length=200, blank=True) + + class Meta: + indexes = [models.Index(fields=["status", "next_attempt_at"])] + ordering = ["id"] diff --git a/portal/mailer/services.py b/portal/mailer/services.py new file mode 100644 index 0000000..b490ede --- /dev/null +++ b/portal/mailer/services.py @@ -0,0 +1,151 @@ +"""The outbox: other modules call `enqueue`, the scheduler calls `flush` (AD-11). + +A mail is claimed (queued -> sending) before it is handed to the server, so two runs never +send the same row. If the process dies in between, the row stays `sending`; later it is marked +failed with an unknown outcome and is not sent again, because a lost mail is better than a +duplicate one for a password or sign-in notice. +""" + +from __future__ import annotations + +import logging +import re +from dataclasses import dataclass +from datetime import datetime, timedelta + +from django.conf import settings +from django.db.models import F +from django.template.loader import render_to_string +from django.utils import timezone, translation + +from portal.adapters.mail import MailMessage, MailRejected, MailUnavailable +from portal.adapters.registry import get_mailer + +from .models import OutboxMail + +logger = logging.getLogger(__name__) + +MAX_ATTEMPTS = 5 +BACKOFF_SECONDS = (60, 5 * 60, 15 * 60, 60 * 60) # wait after the 1st, 2nd, ... failed attempt +RATE_PER_MINUTE = 20 +LEASE_SECONDS = 10 * 60 # a row that has been `sending` this long is given up on +STOP_AFTER_CONSECUTIVE_OUTAGES = 3 + +_ADDRESS = re.compile(r"^[^@\s<>,;\"()\[\]\\]+@[^@\s<>,;\"()\[\]\\]+\.[^@\s<>,;\"()\[\]\\]+$") + + +@dataclass(frozen=True) +class FlushResult: + sent: int = 0 + retried: int = 0 + failed: int = 0 + + def __str__(self) -> str: + return f"sent={self.sent} retried={self.retried} failed={self.failed}" + + +def is_valid_address(value: str) -> bool: + return len(value) <= 254 and bool(_ADDRESS.match(value)) + + +def enqueue(template: str, recipient: str, context: dict | None = None) -> OutboxMail: + """Render the e-mail in the portal-wide language and queue it. + + Call it inside the transaction of the change that causes the mail, so a rolled-back change + queues nothing. + """ + if not is_valid_address(recipient): + raise ValueError("Not a usable e-mail address") + with translation.override(settings.MAIL_LANGUAGE): + subject = render_to_string(f"mail/{template}/subject.txt", context or {}) + body = render_to_string(f"mail/{template}/body.txt", context or {}) + subject = " ".join(subject.split()) # one line, whatever the template did + now = timezone.now() + return OutboxMail.objects.create( + template=template, + recipient=recipient, + subject=subject[:300], + body_text=body.strip() + "\n", + created_at=now, + next_attempt_at=now, + ) + + +def _give_up_on_stale(now: datetime) -> None: + stale = OutboxMail.objects.filter( + status=OutboxMail.Status.SENDING, claimed_at__lt=now - timedelta(seconds=LEASE_SECONDS) + ) + count = stale.update( + status=OutboxMail.Status.FAILED, + body_text="", + last_error="Outcome unknown after a restart; not sent again", + ) + if count: + logger.error("Gave up on %s mails that were being sent when the process stopped", count) + + +def _backoff(attempts: int) -> timedelta: + return timedelta(seconds=BACKOFF_SECONDS[min(attempts, len(BACKOFF_SECONDS)) - 1]) + + +def flush(now: datetime | None = None) -> FlushResult: + """Send what is due, within the rate limit. Safe to call repeatedly.""" + now = now or timezone.now() + _give_up_on_stale(now) + recent = OutboxMail.objects.filter( + status=OutboxMail.Status.SENT, sent_at__gt=now - timedelta(minutes=1) + ).count() + budget = max(RATE_PER_MINUTE - recent, 0) + due = list( + OutboxMail.objects.filter(status=OutboxMail.Status.QUEUED, next_attempt_at__lte=now) + .order_by("id") + .values_list("pk", flat=True)[:budget] + ) + sent = retried = failed = outages = 0 + mailer = get_mailer() + for pk in due: + claimed = OutboxMail.objects.filter(pk=pk, status=OutboxMail.Status.QUEUED).update( + status=OutboxMail.Status.SENDING, claimed_at=now, attempts=F("attempts") + 1 + ) + if not claimed: # another run took it + continue + mail = OutboxMail.objects.get(pk=pk) + message = MailMessage( + sender=settings.SMTP.sender, + recipient=mail.recipient, + subject=mail.subject, + body_text=mail.body_text, + ) + try: + mailer.send(message) + except MailUnavailable as exc: + outages += 1 + if mail.attempts >= MAX_ATTEMPTS: + _finish(mail, OutboxMail.Status.FAILED, str(exc)) + failed += 1 + else: + mail.status = OutboxMail.Status.QUEUED + mail.next_attempt_at = now + _backoff(mail.attempts) + mail.last_error = str(exc)[:200] + mail.save(update_fields=["status", "next_attempt_at", "last_error"]) + retried += 1 + logger.warning("Mail %s not sent (attempt %s)", pk, mail.attempts) + if outages >= STOP_AFTER_CONSECUTIVE_OUTAGES: + break + except MailRejected as exc: + _finish(mail, OutboxMail.Status.FAILED, str(exc)) + failed += 1 + logger.warning("Mail %s was refused", pk) + else: + outages = 0 + _finish(mail, OutboxMail.Status.SENT, "", sent_at=now) + sent += 1 + return FlushResult(sent, retried, failed) + + +def _finish(mail: OutboxMail, status: str, error: str, sent_at: datetime | None = None) -> None: + mail.status = status + mail.last_error = error[:200] + mail.body_text = "" + mail.sent_at = sent_at + mail.save(update_fields=["status", "last_error", "body_text", "sent_at"]) diff --git a/portal/scheduler/__init__.py b/portal/scheduler/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/portal/scheduler/apps.py b/portal/scheduler/apps.py new file mode 100644 index 0000000..80fd91c --- /dev/null +++ b/portal/scheduler/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class SchedulerConfig(AppConfig): + name = "portal.scheduler" + label = "scheduler" diff --git a/portal/scheduler/migrations/0001_initial.py b/portal/scheduler/migrations/0001_initial.py new file mode 100644 index 0000000..05a4d20 --- /dev/null +++ b/portal/scheduler/migrations/0001_initial.py @@ -0,0 +1,29 @@ +# Generated by Django 5.2.18 on 2026-10-18 07:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='JobRun', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('job', models.CharField(max_length=60)), + ('started_at', models.DateTimeField()), + ('finished_at', models.DateTimeField(blank=True, null=True)), + ('status', models.CharField(choices=[('running', 'Running'), ('ok', 'Ok'), ('error', 'Error')], default='running', max_length=10)), + ('detail', models.CharField(blank=True, max_length=200)), + ], + options={ + 'ordering': ['-started_at', '-id'], + 'indexes': [models.Index(fields=['job', '-started_at'], name='scheduler_j_job_2c7e36_idx')], + }, + ), + ] diff --git a/portal/scheduler/migrations/__init__.py b/portal/scheduler/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/portal/scheduler/models.py b/portal/scheduler/models.py new file mode 100644 index 0000000..dbc485f --- /dev/null +++ b/portal/scheduler/models.py @@ -0,0 +1,21 @@ +from django.db import models + + +class JobRun(models.Model): + """The record of one run of one job. Owned by `scheduler` (AD-19).""" + + class Status(models.TextChoices): + RUNNING = "running" + OK = "ok" + ERROR = "error" + + job = models.CharField(max_length=60) + started_at = models.DateTimeField() + finished_at = models.DateTimeField(null=True, blank=True) + status = models.CharField(max_length=10, choices=Status.choices, default=Status.RUNNING) + # A short summary, or the name of the error. Never personal content. + detail = models.CharField(max_length=200, blank=True) + + class Meta: + indexes = [models.Index(fields=["job", "-started_at"])] + ordering = ["-started_at", "-id"] diff --git a/portal/scheduler/runner.py b/portal/scheduler/runner.py new file mode 100644 index 0000000..8105b16 --- /dev/null +++ b/portal/scheduler/runner.py @@ -0,0 +1,51 @@ +"""The scheduler thread: one per process, one tick every 30 seconds (AD-12).""" + +from __future__ import annotations + +import logging +import threading +from collections.abc import Callable + +from django.db import close_old_connections + +from . import services + +logger = logging.getLogger(__name__) + +TICK_SECONDS = 30 + + +class Runner: + def __init__(self, tick: Callable[[], None] = services.tick, interval: float = TICK_SECONDS): + self._tick = tick + self._interval = interval + self._stop = threading.Event() + self._thread: threading.Thread | None = None + + def start(self) -> None: + """Start the thread; calling it again while it runs does nothing.""" + if self._thread is not None and self._thread.is_alive(): + return + self._stop.clear() + self._thread = threading.Thread(target=self._loop, name="scheduler", daemon=True) + self._thread.start() + + def stop(self, timeout: float = 5) -> None: + self._stop.set() + if self._thread is not None: + self._thread.join(timeout) + + def _loop(self) -> None: + logger.info("Scheduler started") + while not self._stop.is_set(): + try: + self._tick() + except Exception: + logger.exception("Scheduler tick failed") + finally: + close_old_connections() + self._stop.wait(self._interval) + logger.info("Scheduler stopped") + + +runner = Runner() diff --git a/portal/scheduler/services.py b/portal/scheduler/services.py new file mode 100644 index 0000000..f5c3304 --- /dev/null +++ b/portal/scheduler/services.py @@ -0,0 +1,48 @@ +"""Jobs and one tick that runs them (AD-12). + +Every job reads the database for what is due and is safe to repeat, so a missed or doubled +tick does no harm. Each run is recorded for the Admin status page. +""" + +from __future__ import annotations + +import logging +from collections.abc import Callable +from dataclasses import dataclass +from datetime import datetime, timedelta + +from django.utils import timezone + +from portal.mailer import services as mailer + +from .models import JobRun + +logger = logging.getLogger(__name__) + +KEEP_RUNS_DAYS = 7 + + +@dataclass(frozen=True) +class Job: + name: str + run: Callable[[datetime], object] # returns something printable as the run's summary + + +# The order is the order they run in. Later entries add jobs here. +JOBS: tuple[Job, ...] = (Job("mail.flush", lambda now: mailer.flush(now)),) + + +def tick(now: datetime | None = None, jobs: tuple[Job, ...] | None = None) -> None: + """Run every job once. A failing job is recorded and does not stop the others.""" + now = now or timezone.now() + for job in JOBS if jobs is None else jobs: + run = JobRun.objects.create(job=job.name, started_at=now) + try: + detail = str(job.run(now)) + status = JobRun.Status.OK + except Exception as exc: + logger.exception("Job %s failed", job.name) + detail, status = type(exc).__name__, JobRun.Status.ERROR + run.status, run.detail, run.finished_at = status, detail[:200], timezone.now() + run.save(update_fields=["status", "detail", "finished_at"]) + JobRun.objects.filter(started_at__lt=now - timedelta(days=KEEP_RUNS_DAYS)).delete() diff --git a/portal/templates/mail/test/body.txt b/portal/templates/mail/test/body.txt new file mode 100644 index 0000000..df7d6f2 --- /dev/null +++ b/portal/templates/mail/test/body.txt @@ -0,0 +1 @@ +{% load i18n %}{% autoescape off %}{% trans "This is a test e-mail from the portal. If you can read it, sending e-mail works." %}{% endautoescape %} diff --git a/portal/templates/mail/test/subject.txt b/portal/templates/mail/test/subject.txt new file mode 100644 index 0000000..84b2cea --- /dev/null +++ b/portal/templates/mail/test/subject.txt @@ -0,0 +1 @@ +{% load i18n %}{% autoescape off %}{% trans "Test e-mail from the portal" %}{% endautoescape %} diff --git a/portal/tests/conftest.py b/portal/tests/conftest.py index c3e8690..ebd4da3 100644 --- a/portal/tests/conftest.py +++ b/portal/tests/conftest.py @@ -7,6 +7,7 @@ import pytest from portal.adapters import registry from portal.adapters.directory import FakeDirectory from portal.adapters.idp import FakeIdp +from portal.adapters.mail import FakeMailer @pytest.fixture @@ -28,14 +29,18 @@ def valid_env(tmp_path): "OIDC_CLIENT_ID": "portal-client", "OIDC_CLIENT_SECRET": "client-secret", "PUBLIC_BASE_URL": "https://portal.example.test", + "SMTP_HOST": "mail.example.test", + "SMTP_USERNAME": "portal@example.test", + "SMTP_PASSWORD": "mail-password", + "MAIL_FROM": "Portal ", } @pytest.fixture(autouse=True) def adapters(): """Fakes for every adapter, so no test can reach a real directory or provider.""" - fakes = SimpleNamespace(directory=FakeDirectory(), idp=FakeIdp()) - registry.install(fakes.directory, fakes.idp) + fakes = SimpleNamespace(directory=FakeDirectory(), idp=FakeIdp(), mailer=FakeMailer()) + registry.install(fakes.directory, fakes.idp, fakes.mailer) yield fakes registry.install() diff --git a/portal/tests/test_config.py b/portal/tests/test_config.py index 4177845..0977a2f 100644 --- a/portal/tests/test_config.py +++ b/portal/tests/test_config.py @@ -140,3 +140,41 @@ def test_oidc_addresses_must_be_https_origins(valid_env, name, value): def test_a_plain_http_origin_is_allowed_only_in_debug(valid_env): env = {**valid_env, "PUBLIC_BASE_URL": "http://localhost:8000"} assert load_config({**env, "DEBUG": "true"}).oidc.public_base_url == "http://localhost:8000" + + +def test_smtp_settings_default_to_starttls_on_587_and_hide_the_password(valid_env): + smtp = load_config(valid_env).smtp + assert (smtp.host, smtp.port, smtp.security) == ("mail.example.test", 587, "starttls") + assert smtp.sender == "Portal " + assert "mail-password" not in repr(smtp) + + +def test_smtp_ssl_defaults_to_port_465_and_the_port_can_be_set(valid_env): + assert load_config({**valid_env, "SMTP_SECURITY": "SSL"}).smtp.port == 465 + assert load_config({**valid_env, "SMTP_PORT": "2525"}).smtp.port == 2525 + + +@pytest.mark.parametrize( + ("name", "value"), + [ + ("SMTP_HOST", "smtp://mail.example.test"), + ("SMTP_HOST", "mail.example.test:587"), + ("SMTP_SECURITY", "plain"), + ("SMTP_PORT", "abc"), + ("SMTP_PORT", "0"), + ("SMTP_PORT", "70000"), + ("MAIL_FROM", "not-an-address"), + ("MAIL_FROM", "a@b.test\r\nBcc: x@evil.test"), + ("MAIL_FROM", "@example.test"), + ], +) +def test_invalid_smtp_settings_are_refused(valid_env, name, value): + with pytest.raises(ConfigError, match=name): + load_config({**valid_env, name: value}) + + +@pytest.mark.parametrize("name", ["SMTP_HOST", "SMTP_USERNAME", "SMTP_PASSWORD", "MAIL_FROM"]) +def test_the_mail_settings_are_required(valid_env, name): + del valid_env[name] + with pytest.raises(ConfigError, match=name): + load_config(valid_env) diff --git a/portal/tests/test_mailer.py b/portal/tests/test_mailer.py new file mode 100644 index 0000000..6b9b4aa --- /dev/null +++ b/portal/tests/test_mailer.py @@ -0,0 +1,263 @@ +"""The outbox: queue, send, retry, rate limit and restart safety (AD-11).""" + +import logging +from datetime import timedelta + +import pytest +from django.core.management import call_command +from django.db import transaction +from django.utils import timezone + +from portal.mailer import services +from portal.mailer.models import OutboxMail + +pytestmark = pytest.mark.django_db(transaction=True) + +Status = OutboxMail.Status + + +def queue(recipient="lena@example.test", n=1): + return [services.enqueue("test", recipient) for _ in range(n)][-1] + + +# -- queueing ----------------------------------------- + + +def test_enqueue_renders_the_mail_in_formal_german(): + mail = queue() + assert mail.status == Status.QUEUED and mail.attempts == 0 + assert mail.subject == "Test-E-Mail vom Portal" + assert "Wenn Sie sie lesen können" in mail.body_text + + +def test_the_language_of_the_request_does_not_change_the_mail(): + from django.utils import translation + + with translation.override("en"): + assert queue().subject == "Test-E-Mail vom Portal" + + +@pytest.mark.parametrize( + "address", + ["", "nobody", "a@b", "a b@example.test", "a@example.test\r\nBcc: x@evil.test", ""], +) +def test_an_unusable_address_is_refused_and_nothing_is_queued(address): + with pytest.raises(ValueError, match="e-mail address"): + services.enqueue("test", address) + assert OutboxMail.objects.count() == 0 + + +def test_a_rolled_back_change_queues_nothing(): + with pytest.raises(RuntimeError), transaction.atomic(): + queue() + raise RuntimeError("the change failed") + assert OutboxMail.objects.count() == 0 + + +# -- sending ----------------------------------------- + + +def test_flush_sends_a_queued_mail_and_removes_its_text(adapters): + mail = queue() + result = services.flush() + mail.refresh_from_db() + assert (result.sent, result.retried, result.failed) == (1, 0, 0) + assert mail.status == Status.SENT and mail.sent_at and mail.attempts == 1 + assert mail.body_text == "" # a one-time link must not stay in the database + sent = adapters.mailer.sent[0] + assert sent.recipient == "lena@example.test" and sent.sender == "Portal " + assert "Test-E-Mail" in sent.subject and "Test-E-Mail" in sent.body_text + + +def test_a_mail_is_never_sent_twice(adapters): + queue() + services.flush() + services.flush() + services.flush(timezone.now() + timedelta(days=1)) + assert len(adapters.mailer.sent) == 1 + + +def test_a_mail_that_is_not_due_yet_waits(adapters): + mail = queue() + later = timezone.now() + timedelta(hours=1) + OutboxMail.objects.filter(pk=mail.pk).update(next_attempt_at=later) + services.flush() + assert adapters.mailer.sent == [] + + +def test_mails_are_sent_in_the_order_they_were_queued(adapters): + for name in ("a", "b", "c"): + queue(f"{name}@example.test") + services.flush() + assert [m.recipient for m in adapters.mailer.sent] == [f"{n}@example.test" for n in "abc"] + + +# -- failures ----------------------------------------- + + +def test_a_temporary_failure_is_retried_later_with_a_growing_wait(adapters): + mail = queue() + adapters.mailer.unavailable = True + now = timezone.now() + waits = [] + for attempt in range(1, 5): + result = services.flush(now) + mail.refresh_from_db() + assert result.retried == 1 and mail.status == Status.QUEUED and mail.attempts == attempt + assert mail.last_error + waits.append(mail.next_attempt_at - now) + now = mail.next_attempt_at + assert waits == [timedelta(seconds=s) for s in services.BACKOFF_SECONDS] + assert waits == sorted(waits) + + +def test_a_retry_before_the_wait_is_over_does_nothing(adapters): + mail = queue() + adapters.mailer.unavailable = True + now = timezone.now() + services.flush(now) + attempts = adapters.mailer.attempts + services.flush(now + timedelta(seconds=30)) + assert adapters.mailer.attempts == attempts + mail.refresh_from_db() + assert mail.attempts == 1 + + +def test_a_mail_that_works_on_a_retry_is_sent(adapters): + mail = queue() + adapters.mailer.unavailable = True + now = timezone.now() + services.flush(now) + adapters.mailer.unavailable = False + mail.refresh_from_db() + result = services.flush(mail.next_attempt_at) + mail.refresh_from_db() + assert result.sent == 1 and mail.status == Status.SENT and mail.attempts == 2 + + +def test_after_the_last_attempt_the_mail_is_failed_and_kept_for_the_admin(adapters): + mail = queue() + adapters.mailer.unavailable = True + now = timezone.now() + for _ in range(services.MAX_ATTEMPTS): + services.flush(now) + mail.refresh_from_db() + now = mail.next_attempt_at + assert mail.status == Status.FAILED and mail.attempts == services.MAX_ATTEMPTS + assert mail.recipient == "lena@example.test" and mail.last_error and mail.body_text == "" + attempts = adapters.mailer.attempts + services.flush(now + timedelta(days=1)) + assert adapters.mailer.attempts == attempts # no further tries + + +def test_a_permanent_refusal_fails_at_once_without_retries(adapters): + mail = queue("gone@example.test") + adapters.mailer.rejected.add("gone@example.test") + result = services.flush() + mail.refresh_from_db() + assert result.failed == 1 and mail.status == Status.FAILED and mail.attempts == 1 + assert "550" in mail.last_error + + +def test_one_refused_mail_does_not_block_the_others(adapters): + queue("gone@example.test") + queue("lena@example.test") + adapters.mailer.rejected.add("gone@example.test") + result = services.flush() + assert (result.sent, result.failed) == (1, 1) + + +def test_a_dead_server_is_not_tried_for_every_mail_in_the_queue(adapters): + for i in range(10): + queue(f"u{i}@example.test") + adapters.mailer.unavailable = True + services.flush() + assert adapters.mailer.attempts == services.STOP_AFTER_CONSECUTIVE_OUTAGES + + +# -- rate limit ----------------------------------------- + + +def test_no_more_than_the_rate_limit_is_sent_per_minute(adapters): + for i in range(services.RATE_PER_MINUTE + 5): + queue(f"u{i}@example.test") + now = timezone.now() + assert services.flush(now).sent == services.RATE_PER_MINUTE + assert services.flush(now + timedelta(seconds=30)).sent == 0 + assert services.flush(now + timedelta(seconds=61)).sent == 5 + assert len(adapters.mailer.sent) == services.RATE_PER_MINUTE + 5 + + +# -- restarts ----------------------------------------- + + +def test_a_mail_cut_off_while_sending_is_given_up_not_sent_again(adapters): + mail = queue() + now = timezone.now() + OutboxMail.objects.filter(pk=mail.pk).update( + status=Status.SENDING, claimed_at=now - timedelta(seconds=services.LEASE_SECONDS + 1) + ) + result = services.flush(now) + mail.refresh_from_db() + assert mail.status == Status.FAILED and "unknown" in mail.last_error + assert result.sent == 0 and adapters.mailer.sent == [] + + +def test_a_mail_being_sent_right_now_is_left_alone(adapters): + mail = queue() + now = timezone.now() + OutboxMail.objects.filter(pk=mail.pk).update(status=Status.SENDING, claimed_at=now) + services.flush(now) + mail.refresh_from_db() + assert mail.status == Status.SENDING and adapters.mailer.sent == [] + + +def test_a_row_taken_by_another_run_is_skipped(adapters, monkeypatch): + mail = queue() + real = OutboxMail.objects.filter + + def take_first(*args, **kwargs): + # The moment the claim is attempted, another run has already taken the row. + if kwargs.get("pk") == mail.pk: + real(pk=mail.pk).update(status=Status.SENDING) + return real(*args, **kwargs) + + monkeypatch.setattr(OutboxMail.objects, "filter", take_first) + assert services.flush().sent == 0 + assert adapters.mailer.sent == [] + + +# -- privacy ----------------------------------------- + + +def test_logs_name_the_row_but_not_the_recipient_or_text(adapters, caplog): + queue("secret.person@example.test") + adapters.mailer.unavailable = True + with caplog.at_level(logging.DEBUG): + services.flush() + assert "not sent" in caplog.text + assert "secret.person" not in caplog.text and "Test-E-Mail" not in caplog.text + + +# -- the test command ----------------------------------------- + + +def test_the_test_command_sends_a_mail_now(adapters, capsys): + call_command("send_test_mail", "lena@example.test") + assert "sent" in capsys.readouterr().out + assert [m.recipient for m in adapters.mailer.sent] == ["lena@example.test"] + + +def test_the_test_command_reports_a_failure(adapters, capsys): + adapters.mailer.unavailable = True + with pytest.raises(SystemExit) as stop: + call_command("send_test_mail", "lena@example.test") + assert stop.value.code == 1 + assert "retried" in capsys.readouterr().out + + +def test_the_test_command_refuses_a_bad_address(adapters): + from django.core.management.base import CommandError + + with pytest.raises(CommandError): + call_command("send_test_mail", "not-an-address") diff --git a/portal/tests/test_scheduler.py b/portal/tests/test_scheduler.py new file mode 100644 index 0000000..d9a37ef --- /dev/null +++ b/portal/tests/test_scheduler.py @@ -0,0 +1,104 @@ +"""The scheduler: one tick runs the jobs and records each run (AD-12).""" + +import threading +from datetime import timedelta + +import pytest +from django.utils import timezone + +from portal.mailer import services as mailer +from portal.scheduler import services +from portal.scheduler.models import JobRun +from portal.scheduler.runner import Runner + +pytestmark = pytest.mark.django_db(transaction=True) + + +def test_a_tick_runs_the_mail_job_and_records_the_run(adapters): + mailer.enqueue("test", "lena@example.test") + services.tick() + assert len(adapters.mailer.sent) == 1 + run = JobRun.objects.get(job="mail.flush") + assert run.status == JobRun.Status.OK and run.finished_at + assert run.detail == "sent=1 retried=0 failed=0" + + +def test_an_idle_tick_is_recorded_too_and_repeating_it_is_harmless(adapters): + services.tick() + services.tick() + assert JobRun.objects.filter(job="mail.flush").count() == 2 + assert adapters.mailer.sent == [] + + +def test_a_restart_sends_nothing_twice(adapters): + mailer.enqueue("test", "lena@example.test") + services.tick() + services.tick(timezone.now() + timedelta(minutes=5)) # "after a restart" + assert len(adapters.mailer.sent) == 1 + + +def test_a_failing_job_is_recorded_and_the_others_still_run(): + ran = [] + + def broken(now): + raise RuntimeError("secret detail that must not be stored") + + jobs = ( + services.Job("first", lambda now: ran.append("first") or "fine"), + services.Job("broken", broken), + services.Job("last", lambda now: ran.append("last") or "also fine"), + ) + services.tick(jobs=jobs) + assert ran == ["first", "last"] + broken_run = JobRun.objects.get(job="broken") + assert broken_run.status == JobRun.Status.ERROR and broken_run.detail == "RuntimeError" + assert JobRun.objects.get(job="last").status == JobRun.Status.OK + + +def test_old_run_records_are_removed(): + now = timezone.now() + JobRun.objects.create(job="x", started_at=now - timedelta(days=services.KEEP_RUNS_DAYS + 1)) + JobRun.objects.create(job="x", started_at=now - timedelta(days=1)) + services.tick(now, jobs=()) + assert JobRun.objects.filter(job="x").count() == 1 + + +# -- the thread ----------------------------------------- + + +def test_the_thread_ticks_repeatedly_and_stops(): + ticked = threading.Semaphore(0) + runner = Runner(tick=ticked.release, interval=0.01) + runner.start() + assert ticked.acquire(timeout=2) and ticked.acquire(timeout=2) + runner.stop() + assert not runner._thread.is_alive() + + +def test_a_failing_tick_does_not_stop_the_thread(): + calls = threading.Semaphore(0) + + def tick(): + calls.release() + raise RuntimeError("boom") + + runner = Runner(tick=tick, interval=0.01) + runner.start() + assert calls.acquire(timeout=2) and calls.acquire(timeout=2) + runner.stop() + + +def test_starting_twice_runs_only_one_thread(): + runner = Runner(tick=lambda: None, interval=0.05) + runner.start() + first = runner._thread + runner.start() + assert runner._thread is first + runner.stop() + assert sum(1 for t in threading.enumerate() if t.name == "scheduler") == 0 + + +def test_the_default_interval_is_thirty_seconds(): + from portal.scheduler.runner import TICK_SECONDS, runner + + assert TICK_SECONDS == 30 and runner._interval == 30 diff --git a/portal/tests/test_smtp_mailer.py b/portal/tests/test_smtp_mailer.py new file mode 100644 index 0000000..bc0c5f2 --- /dev/null +++ b/portal/tests/test_smtp_mailer.py @@ -0,0 +1,180 @@ +"""The SMTP adapter against a stand-in for `smtplib`; no network.""" + +import smtplib +import ssl + +import pytest + +from portal.adapters.mail import MailMessage, MailRejected, MailUnavailable, SmtpMailer +from portal.config.env import SmtpConfig + +MESSAGE = MailMessage( + sender="Portal ", + recipient="lena@example.test", + subject="Hallo Lena", + body_text="Guten Tag, schöne Grüße\n", +) + + +def config(security="starttls", port=587): + return SmtpConfig("mail.example.test", port, security, "box@example.test", "pw", "a@b.test") + + +class FakeSmtpModule: + """Looks like the `smtplib` module and records what the adapter does.""" + + SMTPException = smtplib.SMTPException + SMTPAuthenticationError = smtplib.SMTPAuthenticationError + SMTPRecipientsRefused = smtplib.SMTPRecipientsRefused + SMTPResponseException = smtplib.SMTPResponseException + + def __init__(self): + self.calls = [] + self.fail_at = {} # step name -> exception to raise + self.sent = [] + self.connect_error = None + + def _connection(self, kind, args, kwargs): + if self.connect_error: + raise self.connect_error + module = self + module.calls.append((kind, args, kwargs)) + + class Connection: + def _step(self, name, *a, **k): + module.calls.append((name, a, k)) + if name in module.fail_at: + raise module.fail_at[name] + + def ehlo(self): + self._step("ehlo") + + def starttls(self, context=None): + self._step("starttls", context=context) + + def login(self, user, password): + self._step("login", user, password) + + def send_message(self, message): + self._step("send_message") + module.sent.append(message) + + def quit(self): + self._step("quit") + + def close(self): + module.calls.append(("close", (), {})) + + return Connection() + + def SMTP(self, *args, **kwargs): # noqa: N802 + return self._connection("SMTP", args, kwargs) + + def SMTP_SSL(self, *args, **kwargs): # noqa: N802 + return self._connection("SMTP_SSL", args, kwargs) + + def names(self): + return [c[0] for c in self.calls] + + +@pytest.fixture +def smtp(): + return FakeSmtpModule() + + +def test_starttls_encrypts_before_the_login(smtp): + SmtpMailer(config("starttls"), smtp).send(MESSAGE) + assert smtp.names() == ["SMTP", "ehlo", "starttls", "ehlo", "login", "send_message", "quit"] + context = [c for c in smtp.calls if c[0] == "starttls"][0][2]["context"] + assert context.verify_mode == ssl.CERT_REQUIRED and context.check_hostname + + +def test_ssl_mode_uses_an_encrypted_connection_from_the_start(smtp): + SmtpMailer(config("ssl", 465), smtp).send(MESSAGE) + assert smtp.names() == ["SMTP_SSL", "ehlo", "login", "send_message", "quit"] + kind, args, kwargs = smtp.calls[0] + assert args == ("mail.example.test", 465) and kwargs["timeout"] > 0 + assert kwargs["context"].verify_mode == ssl.CERT_REQUIRED + + +def test_the_login_uses_the_configured_mailbox(smtp): + SmtpMailer(config(), smtp).send(MESSAGE) + assert ("login", ("box@example.test", "pw"), {}) in smtp.calls + + +def test_the_message_is_a_utf8_plain_text_mail_with_the_usual_headers(smtp): + SmtpMailer(config(), smtp).send(MESSAGE) + email = smtp.sent[0] + assert email["From"] == "Portal " + assert email["To"] == "lena@example.test" + assert email["Subject"] == "Hallo Lena" + assert email["Date"] and email["Message-ID"].endswith("@mail.example.test>") + assert email.get_content_type() == "text/plain" + assert "schöne Grüße" in email.get_content() + + +@pytest.mark.parametrize( + "bad", + [ + {"subject": "Hi\r\nBcc: x@evil.test"}, + {"recipient": "a@example.test\r\nBcc: x@evil.test"}, + {"recipient": "a@example.test\nBcc: x@evil.test"}, + ], +) +def test_a_line_break_in_a_header_cannot_add_headers(smtp, bad): + message = MailMessage(**{**MESSAGE.__dict__, **bad}) + with pytest.raises(MailRejected): + SmtpMailer(config(), smtp).send(message) + assert smtp.sent == [] + + +@pytest.mark.parametrize( + ("error", "expected"), + [ + (smtplib.SMTPResponseException(421, b"busy"), MailUnavailable), + (smtplib.SMTPResponseException(450, b"try later"), MailUnavailable), + (smtplib.SMTPResponseException(550, b"no such user"), MailRejected), + (smtplib.SMTPResponseException(554, b"refused"), MailRejected), + (smtplib.SMTPServerDisconnected("gone"), MailUnavailable), + (TimeoutError("slow"), MailUnavailable), + (ssl.SSLError("bad certificate"), MailUnavailable), + ( + smtplib.SMTPRecipientsRefused({"lena@example.test": (550, b"unknown")}), + MailRejected, + ), + ( + smtplib.SMTPRecipientsRefused({"lena@example.test": (452, b"mailbox full")}), + MailUnavailable, + ), + ], +) +def test_server_answers_are_mapped_to_temporary_or_permanent(smtp, error, expected): + smtp.fail_at["send_message"] = error + with pytest.raises(expected): + SmtpMailer(config(), smtp).send(MESSAGE) + + +def test_a_refused_login_is_temporary_so_a_fixed_password_works_on_the_retry(smtp): + smtp.fail_at["login"] = smtplib.SMTPAuthenticationError(535, b"bad credentials") + with pytest.raises(MailUnavailable): + SmtpMailer(config(), smtp).send(MESSAGE) + assert "close" in smtp.names() and "send_message" not in smtp.names() + + +def test_an_unreachable_server_is_temporary(smtp): + smtp.connect_error = ConnectionRefusedError("no") + with pytest.raises(MailUnavailable): + SmtpMailer(config(), smtp).send(MESSAGE) + + +def test_a_failing_quit_does_not_hide_a_successful_send(smtp): + smtp.fail_at["quit"] = smtplib.SMTPServerDisconnected("closed") + SmtpMailer(config(), smtp).send(MESSAGE) + assert len(smtp.sent) == 1 + + +def test_errors_never_contain_the_recipient_or_the_server_text(smtp): + smtp.fail_at["send_message"] = smtplib.SMTPResponseException(550, b"lena@example.test unknown") + with pytest.raises(MailRejected) as caught: + SmtpMailer(config(), smtp).send(MESSAGE) + assert "lena" not in str(caught.value) and "unknown" not in str(caught.value) diff --git a/portal/tests/test_translations.py b/portal/tests/test_translations.py index 239b41c..be477b8 100644 --- a/portal/tests/test_translations.py +++ b/portal/tests/test_translations.py @@ -17,7 +17,8 @@ PY_CALL = re.compile(r"""(? set[str]: found = set() - for template in (PORTAL / "templates").rglob("*.html"): + templates = [*(PORTAL / "templates").rglob("*.html"), *(PORTAL / "templates").rglob("*.txt")] + for template in templates: text = template.read_text(encoding="utf-8") found.update(a or b for a, b in TRANS_TAG.findall(text)) found.update(VARIABLE.sub(r"%(\1)s", body).strip() for body in BLOCKTRANS.findall(text)) diff --git a/pyproject.toml b/pyproject.toml index 5683b9a..362f674 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,6 +44,10 @@ env = [ "OIDC_CLIENT_ID=portal-client", "OIDC_CLIENT_SECRET=test-only-client-secret", "PUBLIC_BASE_URL=https://portal.example.test", + "SMTP_HOST=mail.example.test", + "SMTP_USERNAME=portal@example.test", + "SMTP_PASSWORD=test-only-mail-password", + "MAIL_FROM=Portal ", ] [tool.ruff]