From c4135308e69be6536c9f17395cba677759ca5666 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 11 Mar 2026 09:18:37 +0100 Subject: [PATCH] test: add tests for smtp mailer config --- docs/feature-roadmap.md | 2 + docs/smtp-configuration-concept.md | 101 ++++++++++++++ lib/mv/config.ex | 38 ++++++ lib/mv/mailer.ex | 11 ++ test/membership/setting_smtp_test.exs | 63 +++++++++ test/mv/config_smtp_test.exs | 129 ++++++++++++++++++ test/mv/mailer_test.exs | 46 +++++++ .../mv_web/live/global_settings_live_test.exs | 48 +++++++ test/mv_web/live/join_live_test.exs | 2 + 9 files changed, 440 insertions(+) create mode 100644 docs/smtp-configuration-concept.md create mode 100644 test/membership/setting_smtp_test.exs create mode 100644 test/mv/config_smtp_test.exs create mode 100644 test/mv/mailer_test.exs diff --git a/docs/feature-roadmap.md b/docs/feature-roadmap.md index 89c2f39..f3b1e27 100644 --- a/docs/feature-roadmap.md +++ b/docs/feature-roadmap.md @@ -271,6 +271,7 @@ - [#186](https://git.local-it.org/local-it/mitgliederverwaltung/issues/186) - Create Architecture docs in Repo (S, Low priority) **Missing Features:** +- ❌ **SMTP configuration** – Configure mail server via ENV and Admin Settings, test email from Settings. See [`docs/smtp-configuration-concept.md`](smtp-configuration-concept.md). - ❌ Email templates configuration - ❌ System health dashboard - ❌ Audit log viewer @@ -287,6 +288,7 @@ - ✅ Swoosh mailer integration - ✅ Email confirmation (via AshAuthentication) - ✅ Password reset emails (via AshAuthentication) +- ⚠️ No SMTP configuration (mailer uses Local/Test adapter; prod not configured) - ⚠️ No member communication features **Missing Features:** diff --git a/docs/smtp-configuration-concept.md b/docs/smtp-configuration-concept.md new file mode 100644 index 0000000..b0ca8cc --- /dev/null +++ b/docs/smtp-configuration-concept.md @@ -0,0 +1,101 @@ +# SMTP Configuration – Concept + +**Status:** Draft +**Last updated:** 2026-03-11 + +--- + +## 1. Goal + +Enable configurable SMTP for sending transactional emails (join confirmation, user confirmation, password reset). Configuration via **environment variables** and **Admin Settings** (database), with the same precedence pattern as OIDC and Vereinfacht: **ENV overrides Settings**. Include a **test email** action in Settings (button + recipient field) with clear success/error feedback. + +--- + +## 2. Scope + +- **In scope:** SMTP server configuration (host, port, credentials, TLS/SSL), test email from Settings UI, warning when SMTP is not configured in production. +- **Out of scope:** Changing how AshAuthentication or existing senders use the mailer; they keep using `Mv.Mailer` and `mail_from/0`. No separate "form_mail" config – the existing **mail_from** (MAIL_FROM_NAME, MAIL_FROM_EMAIL) remains the single sender identity for all transactional mails. + +--- + +## 3. Configuration Sources + +| Source | Priority | Use case | +|----------|----------|-----------------------------------| +| ENV | 1 | Production, Docker, 12-factor | +| Settings | 2 | Admin UI, dev without ENV | + +When an ENV variable is set, the corresponding Settings field is read-only in the UI (with hint "Set by environment"). + +--- + +## 4. SMTP Parameters + +| Parameter | ENV | Settings attribute | Notes | +|------------|------------------------|--------------------|--------------------------------------------| +| Host | `SMTP_HOST` | `smtp_host` | e.g. `smtp.example.com` | +| Port | `SMTP_PORT` | `smtp_port` | Default 587 (TLS), 465 (SSL), 25 (plain) | +| Username | `SMTP_USERNAME` | `smtp_username` | Optional if no auth | +| Password | `SMTP_PASSWORD` | `smtp_password` | Sensitive, not shown when set | +| Password | `SMTP_PASSWORD_FILE` | — | Docker/Secrets: path to file with password | +| TLS/SSL | `SMTP_SSL` or similar | `smtp_ssl` | e.g. `tls` / `ssl` / `none` (default: tls)| + +**Sender (unchanged):** `mail_from` stays separate (`MAIL_FROM_NAME`, `MAIL_FROM_EMAIL` in ENV; no DB fields for from-address). + +--- + +## 5. Password from File + +Support **SMTP_PASSWORD_FILE** (path to file containing the password), same pattern as `OIDC_CLIENT_SECRET_FILE` and `TOKEN_SIGNING_SECRET_FILE` in `runtime.exs`. Read once at runtime when building mailer config; ENV `SMTP_PASSWORD` overrides file if both are set (or define explicit precedence and document it). + +--- + +## 6. Behaviour When SMTP Is Not Configured + +- **Dev/Test:** Keep current adapters (`Swoosh.Adapters.Local`, `Swoosh.Adapters.Test`). No change. +- **Production:** If neither ENV nor Settings provide SMTP (e.g. no host): + - Keep using the default adapter (e.g. Local) or a no-op adapter so the app does not crash. + - **Show a clear warning in the Settings UI** (SMTP section): e.g. "SMTP is not configured. Transactional emails (join confirmation, password reset, etc.) will not be delivered reliably." and optionally list consequences (no join confirmations, no password resets, etc.). + - Log a warning at startup or when sending is attempted if SMTP is not configured in prod. + +--- + +## 7. Test Email (Settings UI) + +- **Location:** SMTP / E-Mail section in Global Settings (same page as OIDC, Vereinfacht). +- **Elements:** + - Input: **recipient email address** (required for sending). + - Button: **"Send test email"** (or similar). +- **Behaviour:** On click, send one simple transactional-style email to the given address (subject and body translatable via Gettext, e.g. "Mila – Test email" / "This is a test."). Use current SMTP config and `mail_from`. +- **Feedback:** Show success message or error (e.g. connection refused, auth failed, invalid address). Reuse the same UI pattern as Vereinfacht "Test Integration" (result assign, small result component with success/error states). +- **Permission:** Reuse existing Settings page authorization (admin); no extra check for the test-email action. + +--- + +## 8. Implementation Hints + +- **Config module:** Extend `Mv.Config` with `smtp_*` helpers (e.g. `smtp_host/0`, `smtp_port/0`, …) using `env_or_setting/2` and, for password, ENV vs `SMTP_PASSWORD_FILE` vs Settings (sensitive). +- **runtime.exs:** When SMTP is configured (e.g. host present), set `config :mv, Mv.Mailer, adapter: Swoosh.Adapters.SMTP, ...` with the merged options. Otherwise leave adapter as in base config (Local in dev, Test in test, and in prod either Local with warning or explicit "not configured" behaviour). +- **Setting resource:** New attributes: `smtp_host`, `smtp_port`, `smtp_username`, `smtp_password` (sensitive), `smtp_ssl` (string or enum). Add to create/update `accept` lists and to seeds if needed. +- **Migration:** Add columns for the new Setting attributes. +- **Test email:** New function (e.g. `Mv.Mailer.send_test_email(to_email)`) returning `{:ok, _}` or `{:error, reason}`; call from LiveView event and render result in the SMTP section. + +--- + +## 9. Documentation and i18n + +- **Gettext:** Use Gettext for test email subject and body and for all new Settings labels/hints (including the "SMTP not configured" warning). +- **Docs:** Update `CODE_GUIDELINES.md` (e.g. §3.11 Email) and deployment/configuration docs to describe ENV and Settings for SMTP and the test email. Add this feature to `docs/feature-roadmap.md` (e.g. under Admin Panel & Configuration or Communication). + +--- + +## 10. Summary Checklist + +- [ ] ENV: `SMTP_HOST`, `SMTP_PORT`, `SMTP_USERNAME`, `SMTP_PASSWORD`, `SMTP_PASSWORD_FILE`, `SMTP_SSL` (or equivalent). +- [ ] Settings: attributes and UI for host, port, username, password, TLS/SSL; ENV-override hints. +- [ ] Password from file: `SMTP_PASSWORD_FILE` supported in runtime config. +- [ ] Mailer: Swoosh SMTP adapter configured from merged ENV + Settings when SMTP is configured. +- [ ] Prod warning: clear message in Settings when SMTP is not configured, with consequences. +- [ ] Test email: button + recipient field, translatable content, success/error display; existing permission sufficient. +- [ ] Gettext for new UI and test email text. +- [ ] Feature roadmap and code guidelines updated. diff --git a/lib/mv/config.ex b/lib/mv/config.ex index 8b8c088..e176b8c 100644 --- a/lib/mv/config.ex +++ b/lib/mv/config.ex @@ -449,4 +449,42 @@ defmodule Mv.Config do def oidc_admin_group_name_env_set?, do: env_set?("OIDC_ADMIN_GROUP_NAME") def oidc_groups_claim_env_set?, do: env_set?("OIDC_GROUPS_CLAIM") def oidc_only_env_set?, do: env_set?("OIDC_ONLY") + + # --------------------------------------------------------------------------- + # SMTP configuration (stubs for TDD – ENV overrides Settings; see docs/smtp-configuration-concept.md) + # --------------------------------------------------------------------------- + + @doc "Returns SMTP host. ENV SMTP_HOST overrides Settings. Stub: always nil until implemented." + @spec smtp_host() :: String.t() | nil + def smtp_host, do: nil + + @doc "Returns SMTP port (e.g. 587). ENV SMTP_PORT overrides Settings. Stub: always nil until implemented." + @spec smtp_port() :: non_neg_integer() | nil + def smtp_port, do: nil + + @doc "Returns SMTP username. ENV SMTP_USERNAME overrides Settings. Stub: always nil until implemented." + @spec smtp_username() :: String.t() | nil + def smtp_username, do: nil + + @doc "Returns SMTP password. ENV SMTP_PASSWORD overrides SMTP_PASSWORD_FILE overrides Settings. Stub: always nil until implemented." + @spec smtp_password() :: String.t() | nil + def smtp_password, do: nil + + @doc "Returns SMTP TLS/SSL mode (e.g. 'tls', 'ssl', 'none'). Stub: always nil until implemented." + @spec smtp_ssl() :: String.t() | nil + def smtp_ssl, do: nil + + @doc "Returns true when SMTP is configured (e.g. host present). Stub: always false until implemented." + @spec smtp_configured?() :: boolean() + def smtp_configured?, do: false + + @doc "Returns true when any SMTP ENV variable is set (for Settings UI hint). Stub: always false until implemented." + @spec smtp_env_configured?() :: boolean() + def smtp_env_configured?, do: false + + def smtp_host_env_set?, do: env_set?("SMTP_HOST") + def smtp_port_env_set?, do: env_set?("SMTP_PORT") + def smtp_username_env_set?, do: env_set?("SMTP_USERNAME") + def smtp_password_env_set?, do: env_set?("SMTP_PASSWORD") or env_set?("SMTP_PASSWORD_FILE") + def smtp_ssl_env_set?, do: env_set?("SMTP_SSL") end diff --git a/lib/mv/mailer.ex b/lib/mv/mailer.ex index 3d83636..e78735b 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -16,4 +16,15 @@ defmodule Mv.Mailer do def mail_from do Application.get_env(:mv, :mail_from, {"Mila", "noreply@example.com"}) end + + @doc """ + Sends a test email to the given address. Used from Global Settings SMTP section. + + Returns `{:ok, email}` on success, `{:error, reason}` on failure (e.g. invalid address, + SMTP not configured, connection error). Stub: always returns error until implemented. + """ + @spec send_test_email(String.t()) :: {:ok, Swoosh.Email.t()} | {:error, term()} + def send_test_email(_to_email) do + {:error, :not_implemented} + end end diff --git a/test/membership/setting_smtp_test.exs b/test/membership/setting_smtp_test.exs new file mode 100644 index 0000000..ea4a954 --- /dev/null +++ b/test/membership/setting_smtp_test.exs @@ -0,0 +1,63 @@ +defmodule Mv.Membership.SettingSmtpTest do + @moduledoc """ + Unit tests for Setting resource SMTP attributes. + + TDD: tests expect smtp_host, smtp_port, smtp_username, smtp_password, smtp_ssl + to be accepted on update and persisted. Password must not be exposed in plaintext + when reading settings (sensitive). Tests will fail until Setting has these attributes. + """ + use Mv.DataCase, async: false + + alias Mv.Helpers.SystemActor + alias Mv.Membership + + setup do + {:ok, settings} = Membership.get_settings() + # Save current SMTP values to restore in on_exit (when attributes exist) + saved = %{ + smtp_host: Map.get(settings, :smtp_host), + smtp_port: Map.get(settings, :smtp_port), + smtp_username: Map.get(settings, :smtp_username), + smtp_ssl: Map.get(settings, :smtp_ssl) + } + + on_exit(fn -> + {:ok, s} = Membership.get_settings() + attrs = Enum.reject(saved, fn {_k, v} -> is_nil(v) end) |> Map.new() + if attrs != %{}, do: Membership.update_settings(s, attrs) + end) + + {:ok, settings: settings, saved: saved} + end + + describe "SMTP attributes update and persistence" do + test "update_settings accepts smtp_host, smtp_port, smtp_username, smtp_ssl and persists", %{ + settings: settings + } do + attrs = %{ + smtp_host: "smtp.example.com", + smtp_port: 587, + smtp_username: "user", + smtp_ssl: "tls" + } + + assert {:ok, updated} = Membership.update_settings(settings, attrs) + assert updated.smtp_host == "smtp.example.com" + assert updated.smtp_port == 587 + assert updated.smtp_username == "user" + assert updated.smtp_ssl == "tls" + end + + test "smtp_password can be set and is not exposed in plaintext when reading settings", %{ + settings: settings + } do + secret = "sensitive-password-#{System.unique_integer([:positive])}" + assert {:ok, _} = Membership.update_settings(settings, %{smtp_password: secret}) + + {:ok, read_back} = Membership.get_settings() + # Sensitive: raw password must not be returned (e.g. nil or redacted) + refute read_back.smtp_password == secret, + "smtp_password must not be returned in plaintext when reading settings" + end + end +end diff --git a/test/mv/config_smtp_test.exs b/test/mv/config_smtp_test.exs new file mode 100644 index 0000000..5359366 --- /dev/null +++ b/test/mv/config_smtp_test.exs @@ -0,0 +1,129 @@ +defmodule Mv.ConfigSmtpTest do + @moduledoc """ + Unit tests for Mv.Config SMTP-related helpers. + + ENV overrides Settings (same pattern as OIDC/Vereinfacht). Uses real ENV and + Settings; no mocking so we test the actual precedence. async: false because + we mutate ENV. + """ + use Mv.DataCase, async: false + + describe "smtp_host/0" do + test "returns ENV value when SMTP_HOST is set" do + set_smtp_env("SMTP_HOST", "smtp.example.com") + assert Mv.Config.smtp_host() == "smtp.example.com" + after + clear_smtp_env() + end + + test "returns nil when SMTP_HOST is not set and Settings have no smtp_host" do + clear_smtp_env() + assert Mv.Config.smtp_host() == nil + end + end + + describe "smtp_port/0" do + test "returns parsed integer when SMTP_PORT ENV is set" do + set_smtp_env("SMTP_PORT", "587") + assert Mv.Config.smtp_port() == 587 + after + clear_smtp_env() + end + + test "returns nil or default when SMTP_PORT is not set" do + clear_smtp_env() + port = Mv.Config.smtp_port() + assert port == nil or (is_integer(port) and port in [25, 465, 587]) + end + end + + describe "smtp_configured?/0" do + test "returns true when smtp_host is present (from ENV or Settings)" do + set_smtp_env("SMTP_HOST", "smtp.example.com") + assert Mv.Config.smtp_configured?() == true + after + clear_smtp_env() + end + + test "returns false when no SMTP host is set" do + clear_smtp_env() + refute Mv.Config.smtp_configured?() + end + end + + describe "smtp_env_configured?/0" do + test "returns true when any SMTP ENV variable is set" do + set_smtp_env("SMTP_HOST", "smtp.example.com") + assert Mv.Config.smtp_env_configured?() == true + after + clear_smtp_env() + end + + test "returns false when no SMTP ENV variables are set" do + clear_smtp_env() + refute Mv.Config.smtp_env_configured?() + end + end + + describe "smtp_password/0 and SMTP_PASSWORD_FILE" do + test "returns value from SMTP_PASSWORD when set" do + set_smtp_env("SMTP_PASSWORD", "env-secret") + assert Mv.Config.smtp_password() == "env-secret" + after + clear_smtp_env() + end + + test "returns content of file when SMTP_PASSWORD_FILE is set and SMTP_PASSWORD is not" do + clear_smtp_env() + path = Path.join(System.tmp_dir!(), "mv_smtp_test_#{System.unique_integer([:positive])}") + File.write!(path, "file-secret\n") + Process.put(:smtp_password_file_path, path) + set_smtp_env("SMTP_PASSWORD_FILE", path) + assert Mv.Config.smtp_password() == "file-secret" + after + clear_smtp_env() + if path = Process.get(:smtp_password_file_path), do: File.rm(path) + end + + test "SMTP_PASSWORD overrides SMTP_PASSWORD_FILE when both are set" do + path = Path.join(System.tmp_dir!(), "mv_smtp_test_#{System.unique_integer([:positive])}") + File.write!(path, "file-secret") + Process.put(:smtp_password_file_path, path) + set_smtp_env("SMTP_PASSWORD_FILE", path) + set_smtp_env("SMTP_PASSWORD", "env-wins") + assert Mv.Config.smtp_password() == "env-wins" + after + clear_smtp_env() + if path = Process.get(:smtp_password_file_path), do: File.rm(path) + end + end + + describe "smtp_*_env_set?/0" do + test "smtp_host_env_set? returns true when SMTP_HOST is set" do + set_smtp_env("SMTP_HOST", "x") + assert Mv.Config.smtp_host_env_set?() == true + after + clear_smtp_env() + end + + test "smtp_password_env_set? returns true when SMTP_PASSWORD or SMTP_PASSWORD_FILE is set" do + set_smtp_env("SMTP_PASSWORD", "x") + assert Mv.Config.smtp_password_env_set?() == true + after + clear_smtp_env() + end + end + + defp set_smtp_env(key, value) do + System.put_env(key, value) + end + + defp clear_smtp_env do + System.delete_env("SMTP_HOST") + System.delete_env("SMTP_PORT") + System.delete_env("SMTP_USERNAME") + System.delete_env("SMTP_PASSWORD") + System.delete_env("SMTP_PASSWORD_FILE") + System.delete_env("SMTP_SSL") + end +end diff --git a/test/mv/mailer_test.exs b/test/mv/mailer_test.exs new file mode 100644 index 0000000..22cc49f --- /dev/null +++ b/test/mv/mailer_test.exs @@ -0,0 +1,46 @@ +defmodule Mv.MailerTest do + @moduledoc """ + Unit tests for Mv.Mailer, in particular send_test_email/1. + + Uses Swoosh.Adapters.Test (configured in test.exs); no real SMTP. Asserts + success/error contract and that one test email is sent on success. + """ + use Mv.DataCase, async: true + + import Swoosh.TestAssertions + + alias Mv.Mailer + + describe "send_test_email/1" do + test "returns {:ok, email} and sends one email with expected subject/body when successful" do + to_email = "test-#{System.unique_integer([:positive])}@example.com" + + assert {:ok, _email} = Mailer.send_test_email(to_email) + + assert_email_sent(fn email -> + to_addresses = Enum.map(email.to, &elem(&1, 1)) + subject = email.subject || "" + body = email.html_body || email.text_body || "" + + to_email in to_addresses and + (String.contains?(subject, "Test") or String.contains?(body, "test")) + end) + end + + test "returns {:error, reason} for invalid email address" do + result = Mailer.send_test_email("not-an-email") + assert {:error, _reason} = result + end + + test "uses mail_from as sender" do + to_email = "recipient-#{System.unique_integer([:positive])}@example.com" + assert {:ok, _} = Mailer.send_test_email(to_email) + + assert_email_sent(fn email -> + {_name, from_email} = Mailer.mail_from() + from_addresses = Enum.map(email.from, &elem(&1, 1)) + from_email in from_addresses + end) + end + end +end diff --git a/test/mv_web/live/global_settings_live_test.exs b/test/mv_web/live/global_settings_live_test.exs index 6a739b5..0cb4ead 100644 --- a/test/mv_web/live/global_settings_live_test.exs +++ b/test/mv_web/live/global_settings_live_test.exs @@ -65,4 +65,52 @@ defmodule MvWeb.GlobalSettingsLiveTest do assert html =~ "must be present" end end + + describe "SMTP / E-Mail section" do + setup %{conn: conn} do + user = create_test_user(%{email: "admin@example.com"}) + conn = conn_with_oidc_user(conn, user) + {:ok, conn: conn, user: user} + end + + test "renders SMTP section with host/port fields and test email area", %{conn: conn} do + {:ok, _view, html} = live(conn, ~p"/settings") + # Section title (Gettext key: SMTP or E-Mail per concept) + assert html =~ "SMTP" or html =~ "E-Mail" + end + + test "shows Send test email button when SMTP is configured", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/settings") + # When Mv.Config.smtp_configured?() is true, button and recipient input should be present + # In test env SMTP is typically not configured; we only assert the section exists + html = render(view) + assert html =~ "SMTP" or html =~ "E-Mail" + end + + test "send test email with valid address shows success or error result", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/settings") + # If test email UI exists: fill recipient, click button, assert result area updates + # Uses data-testid or button text "Send test email" / "Test email" + if has_element?(view, "[data-testid='smtp-test-email-form']") do + view + |> element("[data-testid='smtp-test-email-input']") + |> render_change(%{"to_email" => "test@example.com"}) + view + |> element("[data-testid='smtp-send-test-email']") + |> render_click() + # Result is either success or error message + assert has_element?(view, "[data-testid='smtp-test-result']") + else + # Section not yet implemented: just ensure page still renders + assert render(view) =~ "Settings" + end + end + + test "shows warning when SMTP is not configured in production", %{conn: conn} do + # Concept: in prod, show warning "SMTP is not configured. Transactional emails..." + # In test we only check that the section exists; warning visibility is env-dependent + {:ok, view, html} = live(conn, ~p"/settings") + assert html =~ "SMTP" or html =~ "E-Mail" or html =~ "Settings" + end + end end diff --git a/test/mv_web/live/join_live_test.exs b/test/mv_web/live/join_live_test.exs index bd133cd..1458973 100644 --- a/test/mv_web/live/join_live_test.exs +++ b/test/mv_web/live/join_live_test.exs @@ -39,6 +39,8 @@ defmodule MvWeb.JoinLiveTest do test "submit with valid allowlist data creates one JoinRequest and shows success copy", %{ conn: conn } do + # Re-apply allowlist so this test is robust when run in parallel with others (Settings singleton). + enable_join_form_for_test(%{}) count_before = count_join_requests() {:ok, view, _html} = live(conn, "/join")