diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 0d478f9..d721a3a 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -1277,7 +1277,8 @@ mix hex.outdated **SMTP configuration:** -- SMTP can be configured via **ENV variables** (`SMTP_HOST`, `SMTP_PORT`, `SMTP_USERNAME`, `SMTP_PASSWORD`, `SMTP_PASSWORD_FILE`, `SMTP_SSL`) or via **Admin Settings** (database: `smtp_host`, `smtp_port`, `smtp_username`, `smtp_password`, `smtp_ssl`). ENV takes priority (same pattern as OIDC/Vereinfacht). +- SMTP can be configured via **ENV variables** (`SMTP_HOST`, `SMTP_PORT`, `SMTP_USERNAME`, `SMTP_PASSWORD`, `SMTP_PASSWORD_FILE`, `SMTP_SSL`) or via **Admin Settings** (database: `smtp_host`, `smtp_port`, `smtp_username`, `smtp_password`, `smtp_ssl`). +- **ENV-only policy:** If `SMTP_HOST` is set, SMTP is treated as environment-managed only. All SMTP fields in Settings are read-only, SMTP save action is hidden, and the UI shows a warning when required ENV values are missing (`SMTP_USERNAME`, and `SMTP_PASSWORD` or `SMTP_PASSWORD_FILE`). - **Sensitive settings in DB:** `smtp_password` and `oidc_client_secret` are excluded from the default read of the Setting resource; they are loaded only via explicit select when needed (e.g. `Mv.Config.smtp_password/0`, `Mv.Config.oidc_client_secret/0`). This avoids exposing secrets through `get_settings()`. - **Settings cache:** `Mv.Membership.get_settings/0` uses `Mv.Membership.SettingsCache` when the cache process is running (not in test). Cache has a short TTL and is invalidated on every settings update. This avoids repeated DB reads on hot paths (e.g. `RegistrationEnabled` validation, `Layouts.public_page`). In test, the cache is not started so all callers use `get_settings_uncached/0` in the test process (Ecto Sandbox). - **Join emails (domain → web):** The domain calls `Mv.Membership.JoinNotifier` (config `:join_notifier`, default `MvWeb.JoinNotifierImpl`) for sending join confirmation, already-member, and already-pending emails. This keeps the domain independent of the web layer; tests can override the notifier. diff --git a/docs/smtp-configuration-concept.md b/docs/smtp-configuration-concept.md index 4ae7760..6668485 100644 --- a/docs/smtp-configuration-concept.md +++ b/docs/smtp-configuration-concept.md @@ -25,7 +25,10 @@ Enable configurable SMTP for sending transactional emails (join confirmation, us | 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"). +When `SMTP_HOST` is set, SMTP runs in **ENV-only mode**: +- all SMTP fields in Settings are read-only, +- saving SMTP settings in the UI is disabled, +- and the UI shows a warning block if required SMTP ENV values are missing. --- @@ -63,6 +66,14 @@ Support **SMTP_PASSWORD_FILE** (path to file containing the password), same patt - Show a warning in the Settings UI. - Delivery attempts silently fall back to the Local adapter (no crash). +### 6.1 Behaviour in ENV-only mode (`SMTP_HOST` set) + +- The SMTP source of truth is environment variables only. +- The UI does not allow editing SMTP fields in this mode. +- The Settings page shows a warning block when required values are missing: + - `SMTP_USERNAME` + - `SMTP_PASSWORD` or `SMTP_PASSWORD_FILE` + --- ## 7. Test Email (Settings UI) diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 7fa35dc..ffe7703 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -178,6 +178,18 @@ defmodule Mv.Membership do end end + @doc """ + Invalidates the global settings cache. + + This should be used by callers that update settings through paths outside of + `update_settings/2` (for example, custom form submit flows) to keep reads via + `get_settings/0` consistent across views. + """ + @spec invalidate_settings_cache() :: :ok + def invalidate_settings_cache do + SettingsCache.invalidate() + end + @doc """ Lists only required custom fields. diff --git a/lib/mv/config.ex b/lib/mv/config.ex index 3494937..33c92cf 100644 --- a/lib/mv/config.ex +++ b/lib/mv/config.ex @@ -143,6 +143,27 @@ defmodule Mv.Config do |> parse_and_validate_integer(default) end + # --------------------------------------------------------------------------- + # Association name + # ENV variable takes priority; fallback to Settings from database. + # --------------------------------------------------------------------------- + + @doc """ + Returns the association name. + + Reads from `ASSOCIATION_NAME` env first, then from Settings. + """ + @spec association_name() :: String.t() | nil + def association_name do + env_or_setting("ASSOCIATION_NAME", :club_name) + end + + @doc """ + Returns true if ASSOCIATION_NAME is set (field is read-only in Settings). + """ + @spec association_name_env_set?() :: boolean() + def association_name_env_set?, do: env_set?("ASSOCIATION_NAME") + # --------------------------------------------------------------------------- # Vereinfacht accounting software integration # ENV variables take priority; fallback to Settings from database. @@ -478,48 +499,61 @@ defmodule Mv.Config do end @doc """ - Returns SMTP port as integer. ENV `SMTP_PORT` (parsed) overrides Settings. - Returns nil when neither ENV nor Settings provide a valid port. + Returns SMTP port as integer. + + Policy: + - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_PORT` + - Settings mode: read from Settings only """ @spec smtp_port() :: non_neg_integer() | nil def smtp_port do - case System.get_env("SMTP_PORT") do - nil -> - get_from_settings_integer(:smtp_port) - - value when is_binary(value) -> - case Integer.parse(String.trim(value)) do - {port, _} when port > 0 -> port - _ -> nil - end + if smtp_env_mode?() do + parse_smtp_port_env(System.get_env("SMTP_PORT")) + else + get_from_settings_integer(:smtp_port) end end @doc """ - Returns SMTP username. ENV `SMTP_USERNAME` overrides Settings. + Returns SMTP username. + + Policy: + - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_USERNAME` + - Settings mode: read from Settings only """ @spec smtp_username() :: String.t() | nil def smtp_username do - smtp_env_or_setting("SMTP_USERNAME", :smtp_username) + if smtp_env_mode?() do + System.get_env("SMTP_USERNAME") |> trim_nil() + else + get_from_settings(:smtp_username) + end end @doc """ Returns SMTP password. - Priority: `SMTP_PASSWORD` ENV > `SMTP_PASSWORD_FILE` (file contents) > Settings. + Policy: + - ENV-only mode (`SMTP_HOST` set): `SMTP_PASSWORD` > `SMTP_PASSWORD_FILE` + - Settings mode: read from Settings only + Strips trailing whitespace/newlines from file contents. """ @spec smtp_password() :: String.t() | nil def smtp_password do - case System.get_env("SMTP_PASSWORD") do - nil -> smtp_password_from_file_or_settings() - value -> trim_nil(value) + if smtp_env_mode?() do + case System.get_env("SMTP_PASSWORD") do + nil -> smtp_password_from_file_or_settings() + value -> trim_nil(value) + end + else + get_smtp_password_from_settings() end end defp smtp_password_from_file_or_settings do case System.get_env("SMTP_PASSWORD_FILE") do - nil -> get_smtp_password_from_settings() + nil -> nil path -> read_smtp_password_file(path) end end @@ -533,11 +567,18 @@ defmodule Mv.Config do @doc """ Returns SMTP TLS/SSL mode string (e.g. 'tls', 'ssl', 'none'). - ENV `SMTP_SSL` overrides Settings. + + Policy: + - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_SSL` + - Settings mode: read from Settings only """ @spec smtp_ssl() :: String.t() | nil def smtp_ssl do - smtp_env_or_setting("SMTP_SSL", :smtp_ssl) + if smtp_env_mode?() do + System.get_env("SMTP_SSL") |> trim_nil() + else + get_from_settings(:smtp_ssl) + end end @doc """ @@ -549,12 +590,39 @@ defmodule Mv.Config do end @doc """ - Returns true when any SMTP ENV variable is set (used in Settings UI for hints). + Returns true when SMTP ENV mode is active. """ @spec smtp_env_configured?() :: boolean() def smtp_env_configured? do - smtp_host_env_set?() or smtp_port_env_set?() or smtp_username_env_set?() or - smtp_password_env_set?() or smtp_ssl_env_set?() + smtp_env_mode?() + end + + @doc """ + Returns true when SMTP is managed by environment variables. + + Policy: if `SMTP_HOST` is set, SMTP is treated as ENV-only. + """ + @spec smtp_env_mode?() :: boolean() + def smtp_env_mode? do + smtp_host_env_set?() + end + + @doc """ + Returns missing required SMTP ENV keys for ENV-only mode warnings. + + Required in ENV-only mode: + - `SMTP_USERNAME` + - one of `SMTP_PASSWORD` or `SMTP_PASSWORD_FILE` + """ + @spec smtp_missing_required_env_keys() :: [String.t()] + def smtp_missing_required_env_keys do + if smtp_env_mode?() do + [] + |> maybe_add_missing("SMTP_USERNAME", smtp_username_env_set?()) + |> maybe_add_missing("SMTP_PASSWORD/SMTP_PASSWORD_FILE", smtp_password_env_set?()) + else + [] + end end @doc "Returns true if SMTP_HOST ENV is set." @@ -618,6 +686,17 @@ defmodule Mv.Config do @spec mail_from_email_env_set?() :: boolean() def mail_from_email_env_set?, do: env_set?("MAIL_FROM_EMAIL") + defp parse_smtp_port_env(nil), do: nil + + defp parse_smtp_port_env(value) when is_binary(value) do + case Integer.parse(String.trim(value)) do + {port, _} when port > 0 -> port + _ -> nil + end + end + + defp parse_smtp_port_env(_), do: nil + # Reads a plain string SMTP setting: ENV first, then Settings. defp smtp_env_or_setting(env_key, setting_key) do case System.get_env(env_key) do @@ -626,6 +705,9 @@ defmodule Mv.Config do end end + defp maybe_add_missing(acc, _label, true), do: acc + defp maybe_add_missing(acc, label, false), do: acc ++ [label] + # Reads an integer setting attribute from Settings. defp get_from_settings_integer(key) do case Mv.Membership.get_settings() do diff --git a/lib/mv_web/components/layouts.ex b/lib/mv_web/components/layouts.ex index 9aff23c..c6e3e01 100644 --- a/lib/mv_web/components/layouts.ex +++ b/lib/mv_web/components/layouts.ex @@ -135,8 +135,11 @@ defmodule MvWeb.Layouts do slot :inner_block, required: true def app(assigns) do - # Single get_settings() for layout; derive club_name and join_form_enabled to avoid duplicate query. - %{club_name: club_name, join_form_enabled: join_form_enabled} = get_layout_settings() + # Single settings read for layout defaults. + # Use an explicitly provided club_name as source of truth to avoid stale + # values from cache reads immediately after a settings update in LiveViews. + %{club_name: fallback_club_name, join_form_enabled: join_form_enabled} = get_layout_settings() + club_name = assigns[:club_name] || fallback_club_name # NOTE: Unprocessed count runs on every page load when join form is enabled; consider # loading only on navigation or caching briefly if performance becomes an issue. diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 43851db..228970a 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -65,6 +65,7 @@ defmodule MvWeb.GlobalSettingsLive do |> assign(:settings, settings) |> assign(:locale, locale) |> assign(:environment, environment) + |> assign(:association_name_env_set, Mv.Config.association_name_env_set?()) |> assign(:vereinfacht_env_configured, Mv.Config.vereinfacht_env_configured?()) |> assign(:vereinfacht_api_url_env_set, Mv.Config.vereinfacht_api_url_env_set?()) |> assign(:vereinfacht_api_key_env_set, Mv.Config.vereinfacht_api_key_env_set?()) @@ -86,6 +87,8 @@ defmodule MvWeb.GlobalSettingsLive do |> assign(:oidc_client_secret_set, Mv.Config.oidc_client_secret_set?()) |> assign(:registration_enabled, settings.registration_enabled != false) |> assign(:smtp_env_configured, Mv.Config.smtp_env_configured?()) + |> assign(:smtp_env_mode, Mv.Config.smtp_env_mode?()) + |> assign(:smtp_missing_required_env_keys, Mv.Config.smtp_missing_required_env_keys()) |> assign(:smtp_host_env_set, Mv.Config.smtp_host_env_set?()) |> assign(:smtp_port_env_set, Mv.Config.smtp_port_env_set?()) |> assign(:smtp_username_env_set, Mv.Config.smtp_username_env_set?()) @@ -123,6 +126,13 @@ defmodule MvWeb.GlobalSettingsLive do
<%!-- Club Settings Section --%> <.form_section title={gettext("Club Settings")}> + <%= if @association_name_env_set do %> +

+ {gettext( + "Association name is set via environment variable ASSOCIATION_NAME. This field is read-only." + )} +

+ <% end %> <.form for={@form} id="settings-form" phx-change="validate" phx-submit="save">
<.input @@ -130,10 +140,18 @@ defmodule MvWeb.GlobalSettingsLive do type="text" label={gettext("Association Name")} required + disabled={@association_name_env_set} + placeholder={ + if(@association_name_env_set, do: gettext("From ASSOCIATION_NAME"), else: nil) + } />
- <.button phx-disable-with={gettext("Saving...")} variant="primary"> + <.button + :if={not @association_name_env_set} + phx-disable-with={gettext("Saving...")} + variant="primary" + > {gettext("Save Name")} @@ -321,12 +339,25 @@ defmodule MvWeb.GlobalSettingsLive do <%!-- SMTP / E-Mail Section --%> <.form_section title={gettext("SMTP / E-Mail")}> - <%= if @smtp_env_configured do %> + <%= if @smtp_env_mode do %>

- {gettext("Some values are set via environment variables. Those fields are read-only.")} + {gettext( + "SMTP is fully managed via environment variables. All SMTP fields are read-only." + )}

<% end %> + <%= if @smtp_env_mode and @smtp_missing_required_env_keys != [] do %> +
+ <.icon name="hero-exclamation-triangle" class="size-5 shrink-0 mt-0.5" /> + + {gettext("SMTP environment configuration appears incomplete. Missing: %{keys}", + keys: Enum.join(@smtp_missing_required_env_keys, ", ") + )} + +
+ <% end %> + <%= if @environment == :prod and not @smtp_configured do %>
<.icon name="hero-exclamation-triangle" class="size-5 shrink-0 mt-0.5" /> @@ -345,7 +376,7 @@ defmodule MvWeb.GlobalSettingsLive do field={@form[:smtp_host]} type="text" label={gettext("Host")} - disabled={@smtp_host_env_set} + disabled={@smtp_env_mode} placeholder={ if(@smtp_host_env_set, do: gettext("From SMTP_HOST"), @@ -357,14 +388,14 @@ defmodule MvWeb.GlobalSettingsLive do field={@form[:smtp_port]} type="number" label={gettext("Port")} - disabled={@smtp_port_env_set} + disabled={@smtp_env_mode} placeholder={if(@smtp_port_env_set, do: gettext("From SMTP_PORT"), else: "587")} /> <.input field={@form[:smtp_ssl]} type="select" label={gettext("TLS/SSL")} - disabled={@smtp_ssl_env_set} + disabled={@smtp_env_mode} options={[ {gettext("TLS (port 587, recommended)"), "tls"}, {gettext("SSL (port 465)"), "ssl"}, @@ -379,7 +410,7 @@ defmodule MvWeb.GlobalSettingsLive do field={@form[:smtp_username]} type="text" label={gettext("Username")} - disabled={@smtp_username_env_set} + disabled={@smtp_env_mode} placeholder={ if(@smtp_username_env_set, do: gettext("From SMTP_USERNAME"), @@ -391,7 +422,7 @@ defmodule MvWeb.GlobalSettingsLive do field={@form[:smtp_password]} type="password" label={gettext("Password")} - disabled={@smtp_password_env_set} + disabled={@smtp_env_mode} placeholder={ if(@smtp_password_env_set, do: gettext("From SMTP_PASSWORD"), @@ -410,7 +441,7 @@ defmodule MvWeb.GlobalSettingsLive do field={@form[:smtp_from_email]} type="email" label={gettext("Sender email (From)")} - disabled={@smtp_from_email_env_set} + disabled={@smtp_env_mode} placeholder={ if(@smtp_from_email_env_set, do: gettext("From MAIL_FROM_EMAIL"), @@ -422,7 +453,7 @@ defmodule MvWeb.GlobalSettingsLive do field={@form[:smtp_from_name]} type="text" label={gettext("Sender name (From)")} - disabled={@smtp_from_name_env_set} + disabled={@smtp_env_mode} placeholder={ if(@smtp_from_name_env_set, do: gettext("From MAIL_FROM_NAME"), else: "Mila") } @@ -436,9 +467,10 @@ defmodule MvWeb.GlobalSettingsLive do

<.button :if={ - not (@smtp_host_env_set and @smtp_port_env_set and @smtp_username_env_set and - @smtp_password_env_set and @smtp_ssl_env_set and @smtp_from_email_env_set and - @smtp_from_name_env_set) + not @smtp_env_mode and + not (@smtp_host_env_set and @smtp_port_env_set and @smtp_username_env_set and + @smtp_password_env_set and @smtp_ssl_env_set and @smtp_from_email_env_set and + @smtp_from_name_env_set) } phx-disable-with={gettext("Saving...")} variant="primary" @@ -903,6 +935,7 @@ defmodule MvWeb.GlobalSettingsLive do # Never send blank API key / client secret / smtp password so we do not overwrite stored secrets setting_params_clean = setting_params + |> drop_env_managed_association_name() |> drop_blank_vereinfacht_api_key() |> drop_blank_oidc_client_secret() |> drop_blank_smtp_password() @@ -911,6 +944,10 @@ defmodule MvWeb.GlobalSettingsLive do case MvWeb.LiveHelpers.submit_form(socket.assigns.form, setting_params_clean, actor) do {:ok, updated_settings} -> + # Keep cross-view reads consistent after settings updates (layouts/sidebar + # read via Membership.get_settings/0). + Membership.invalidate_settings_cache() + # Use the returned record for the form so saved values show immediately; # get_settings() can return cached data without the new attribute until reload. test_result = @@ -1179,10 +1216,19 @@ defmodule MvWeb.GlobalSettingsLive do end end + defp drop_env_managed_association_name(params) when is_map(params) do + if Mv.Config.association_name_env_set?() do + Map.delete(params, "club_name") + else + params + end + end + defp assign_form(%{assigns: %{settings: settings}} = socket) do - # Show ENV values in disabled fields (Vereinfacht, OIDC, SMTP); never expose secrets in form + # Show ENV values in disabled fields (Association Name, Vereinfacht, OIDC, SMTP); never expose secrets in form settings_display = settings + |> merge_association_env_values() |> merge_vereinfacht_env_values() |> merge_oidc_env_values() |> merge_smtp_env_values() @@ -1209,6 +1255,15 @@ defmodule MvWeb.GlobalSettingsLive do defp put_if_env_set(map, _key, false, _value), do: map defp put_if_env_set(map, key, true, value), do: Map.put(map, key, value) + defp merge_association_env_values(s) do + put_if_env_set( + s, + :club_name, + Mv.Config.association_name_env_set?(), + Mv.Config.association_name() + ) + end + defp merge_vereinfacht_env_values(s) do s |> put_if_env_set( diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 52270cc..31830ce 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -3917,3 +3917,23 @@ msgstr "Offen" #, elixir-autogen, elixir-format msgid "join page URL in a new tab" msgstr "Beitrittslink in einem neuen Tab" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "SMTP environment configuration appears incomplete. Missing: %{keys}" +msgstr "Die SMTP-Umgebungs-Konfiguration ist unvollständig. Fehlend: %{keys}" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "SMTP is fully managed via environment variables. All SMTP fields are read-only." +msgstr "SMTP wird vollständig über Umgebungsvariablen verwaltet. Alle SMTP-Felder sind schreibgeschützt." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Association name is set via environment variable ASSOCIATION_NAME. This field is read-only." +msgstr "Der Vereinsname wird über die Umgebungsvariable ASSOCIATION_NAME gesetzt. Dieses Feld ist schreibgeschützt." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "From ASSOCIATION_NAME" +msgstr "Aus ASSOCIATION_NAME" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 5d48691..586d1f4 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -3917,3 +3917,23 @@ msgstr "" #, elixir-autogen, elixir-format msgid "join page URL in a new tab" msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "SMTP environment configuration appears incomplete. Missing: %{keys}" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "SMTP is fully managed via environment variables. All SMTP fields are read-only." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Association name is set via environment variable ASSOCIATION_NAME. This field is read-only." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "From ASSOCIATION_NAME" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index ec6f305..6b8db53 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -3917,3 +3917,23 @@ msgstr "Open" #, elixir-autogen, elixir-format msgid "join page URL in a new tab" msgstr "join page URL in a new tab" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "SMTP environment configuration appears incomplete. Missing: %{keys}" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "SMTP is fully managed via environment variables. All SMTP fields are read-only." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Association name is set via environment variable ASSOCIATION_NAME. This field is read-only." +msgstr "Association name is set via environment variable ASSOCIATION_NAME. This field is read-only." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "From ASSOCIATION_NAME" +msgstr "From ASSOCIATION_NAME" diff --git a/test/mv/config_smtp_test.exs b/test/mv/config_smtp_test.exs index 5359366..7a1b895 100644 --- a/test/mv/config_smtp_test.exs +++ b/test/mv/config_smtp_test.exs @@ -23,7 +23,8 @@ defmodule Mv.ConfigSmtpTest do end describe "smtp_port/0" do - test "returns parsed integer when SMTP_PORT ENV is set" do + test "returns parsed integer when SMTP_PORT ENV is set in ENV-only mode" do + set_smtp_env("SMTP_HOST", "smtp.example.com") set_smtp_env("SMTP_PORT", "587") assert Mv.Config.smtp_port() == 587 after @@ -52,13 +53,21 @@ defmodule Mv.ConfigSmtpTest do end describe "smtp_env_configured?/0" do - test "returns true when any SMTP ENV variable is set" do + test "returns true when SMTP_HOST 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 SMTP_HOST is not set even if other SMTP ENV variables are set" do + set_smtp_env("SMTP_USERNAME", "user@example.com") + set_smtp_env("SMTP_PASSWORD", "secret") + refute Mv.Config.smtp_env_configured?() + 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?() @@ -66,15 +75,17 @@ defmodule Mv.ConfigSmtpTest do end describe "smtp_password/0 and SMTP_PASSWORD_FILE" do - test "returns value from SMTP_PASSWORD when set" do + test "returns value from SMTP_PASSWORD when set in ENV-only mode" do + set_smtp_env("SMTP_HOST", "smtp.example.com") 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 + test "returns content of file when SMTP_PASSWORD_FILE is set in ENV-only mode and SMTP_PASSWORD is not" do clear_smtp_env() + set_smtp_env("SMTP_HOST", "smtp.example.com") 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) @@ -85,7 +96,8 @@ defmodule Mv.ConfigSmtpTest do 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 + test "SMTP_PASSWORD overrides SMTP_PASSWORD_FILE in ENV-only mode when both are set" do + set_smtp_env("SMTP_HOST", "smtp.example.com") 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) diff --git a/test/mv_web/components/layouts_test.exs b/test/mv_web/components/layouts_test.exs new file mode 100644 index 0000000..411750b --- /dev/null +++ b/test/mv_web/components/layouts_test.exs @@ -0,0 +1,26 @@ +defmodule MvWeb.LayoutsTest do + use MvWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + + alias Mv.Membership + alias MvWeb.Layouts + + describe "app/1" do + test "prefers provided club_name over settings fallback" do + {:ok, settings} = Membership.get_settings() + {:ok, _} = Membership.update_settings(settings, %{club_name: "Settings Club Name"}) + + html = + render_component(&Layouts.app/1, %{ + flash: %{}, + current_user: nil, + club_name: "Provided Club Name", + inner_block: [%{inner_block: fn _, _ -> "content" end}] + }) + + assert html =~ "Provided Club Name" + refute html =~ "Settings Club Name" + 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 2edaf74..8d2963c 100644 --- a/test/mv_web/live/global_settings_live_test.exs +++ b/test/mv_web/live/global_settings_live_test.exs @@ -1,8 +1,20 @@ defmodule MvWeb.GlobalSettingsLiveTest do - use MvWeb.ConnCase, async: true + use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest alias Mv.Membership + @smtp_env_keys [ + "SMTP_HOST", + "SMTP_PORT", + "SMTP_USERNAME", + "SMTP_PASSWORD", + "SMTP_PASSWORD_FILE", + "SMTP_SSL", + "MAIL_FROM_NAME", + "MAIL_FROM_EMAIL" + ] + @association_env_key "ASSOCIATION_NAME" + describe "Global Settings LiveView" do setup %{conn: conn} do user = create_test_user(%{email: "admin@example.com"}) @@ -40,6 +52,17 @@ defmodule MvWeb.GlobalSettingsLiveTest do assert render(view) =~ "Updated Club Name" end + test "updated club name is shown after remount", %{conn: conn} do + {:ok, view, _html} = live(conn, ~p"/settings") + + assert view + |> form("#settings-form", %{setting: %{club_name: "Remount Club Name"}}) + |> render_submit() + + {:ok, _view_after_remount, html_after_remount} = live(conn, ~p"/settings") + assert html_after_remount =~ "Remount Club Name" + end + test "shows error when club_name is empty", %{conn: conn} do {:ok, view, _html} = live(conn, ~p"/settings") @@ -79,6 +102,34 @@ defmodule MvWeb.GlobalSettingsLiveTest do "Open" ) end + + @tag :ui + test "disables association name input when ASSOCIATION_NAME is set", %{conn: conn} do + clear_association_name_env() + System.put_env(@association_env_key, "Association Name from ENV") + on_exit(fn -> clear_association_name_env() end) + + {:ok, view, _html} = live(conn, ~p"/settings") + + assert has_element?(view, "#setting_club_name[disabled]") + assert has_element?(view, "#setting_club_name[placeholder='From ASSOCIATION_NAME']") + refute has_element?(view, "#settings-form button", "Save Name") + assert render(view) =~ "Association name is set via environment variable ASSOCIATION_NAME" + end + + @tag :ui + test "keeps association name input editable when ASSOCIATION_NAME is not set", %{conn: conn} do + clear_association_name_env() + on_exit(fn -> clear_association_name_env() end) + + {:ok, view, _html} = live(conn, ~p"/settings") + + refute has_element?(view, "#setting_club_name[disabled]") + assert has_element?(view, "#settings-form button", "Save Name") + + refute render(view) =~ + "Association name is set via environment variable ASSOCIATION_NAME" + end end describe "SMTP / E-Mail section" do @@ -124,6 +175,71 @@ defmodule MvWeb.GlobalSettingsLiveTest do {:ok, _view, html} = live(conn, ~p"/settings") assert html =~ "SMTP" or html =~ "E-Mail" or html =~ "Settings" end + + @tag :ui + test "disables all SMTP inputs when SMTP_HOST is set", %{conn: conn} do + clear_smtp_env() + System.put_env("SMTP_HOST", "smtp.env-only.example") + on_exit(fn -> clear_smtp_env() end) + + {:ok, view, _html} = live(conn, ~p"/settings") + + assert has_element?(view, "#setting_smtp_host[disabled]") + assert has_element?(view, "#setting_smtp_port[disabled]") + assert has_element?(view, "#setting_smtp_ssl[disabled]") + assert has_element?(view, "#setting_smtp_username[disabled]") + assert has_element?(view, "#setting_smtp_password[disabled]") + assert has_element?(view, "#setting_smtp_from_email[disabled]") + assert has_element?(view, "#setting_smtp_from_name[disabled]") + end + + @tag :ui + test "does not render SMTP save action when SMTP_HOST is set", %{conn: conn} do + clear_smtp_env() + System.put_env("SMTP_HOST", "smtp.env-only.example") + on_exit(fn -> clear_smtp_env() end) + + {:ok, view, _html} = live(conn, ~p"/settings") + refute has_element?(view, "#smtp-form button", "Save SMTP Settings") + end + + @tag :ui + test "shows explicit ENV-only mode hint when SMTP_HOST is set", %{conn: conn} do + clear_smtp_env() + System.put_env("SMTP_HOST", "smtp.env-only.example") + on_exit(fn -> clear_smtp_env() end) + + {:ok, _view, html} = live(conn, ~p"/settings") + assert html =~ "SMTP is fully managed via environment variables" + end + + @tag :ui + test "shows warning block for missing required SMTP ENV values in ENV-only mode", %{ + conn: conn + } do + clear_smtp_env() + System.put_env("SMTP_HOST", "smtp.env-only.example") + on_exit(fn -> clear_smtp_env() end) + + {:ok, _view, html} = live(conn, ~p"/settings") + assert html =~ "SMTP environment configuration appears incomplete" + assert html =~ "SMTP_USERNAME" + assert html =~ "SMTP_PASSWORD/SMTP_PASSWORD_FILE" + end + + @tag :ui + test "does not enter ENV-only mode when SMTP_HOST is not set", %{conn: conn} do + clear_smtp_env() + System.put_env("SMTP_USERNAME", "leftover@example.com") + on_exit(fn -> clear_smtp_env() end) + + {:ok, view, html} = live(conn, ~p"/settings") + + refute html =~ "SMTP is fully managed via environment variables" + refute html =~ "SMTP environment configuration appears incomplete" + refute has_element?(view, "#setting_smtp_host[disabled]") + refute has_element?(view, "#setting_smtp_username[disabled]") + end end describe "Authentication section when OIDC-only is enabled" do @@ -190,4 +306,12 @@ defmodule MvWeb.GlobalSettingsLiveTest do end end end + + defp clear_smtp_env do + Enum.each(@smtp_env_keys, &System.delete_env/1) + end + + defp clear_association_name_env do + System.delete_env(@association_env_key) + end end