From 349cee0ce634891927f19c910db84c553d600ebe Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 13 Mar 2026 17:55:17 +0100 Subject: [PATCH] refactor: review remarks --- CODE_GUIDELINES.md | 6 +- DESIGN_GUIDELINES.md | 2 +- assets/css/app.css | 6 +- config/config.exs | 3 + .../user/validations/registration_enabled.ex | 6 +- lib/membership/join_notifier.ex | 13 +++ .../changes/regenerate_confirmation_token.ex | 11 +- lib/membership/membership.ex | 104 ++++++++++++------ lib/membership/settings_cache.ex | 85 ++++++++++++++ lib/mv/application.ex | 37 ++++--- lib/mv_web/components/layouts.ex | 16 ++- .../controllers/join_confirm_controller.ex | 9 +- .../join_confirm_html/confirm.html.heex | 24 +--- lib/mv_web/join_notifier_impl.ex | 25 +++++ lib/mv_web/live/join_live.ex | 29 ++++- priv/gettext/de/LC_MESSAGES/default.po | 6 +- priv/gettext/default.pot | 6 +- priv/gettext/en/LC_MESSAGES/default.po | 6 +- test/mv_web/live/join_live_test.exs | 6 +- 19 files changed, 300 insertions(+), 100 deletions(-) create mode 100644 lib/membership/join_notifier.ex create mode 100644 lib/membership/settings_cache.ex create mode 100644 lib/mv_web/join_notifier_impl.ex diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 898fdd2..8d53484 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -90,6 +90,8 @@ lib/ │ ├── custom_field.ex # Custom field (definition) resource │ ├── custom_field_value.ex # Custom field value resource │ ├── setting.ex # Global settings (singleton resource; incl. join form config) +│ ├── settings_cache.ex # Process cache for get_settings (TTL; invalidate on update; not started in test) +│ ├── join_notifier.ex # Behaviour for join emails (confirmation, already member, already pending) │ ├── setting/ # Setting changes (NormalizeJoinFormSettings, etc.) │ ├── group.ex # Group resource │ ├── member_group.ex # MemberGroup join table resource @@ -1275,6 +1277,8 @@ mix hex.outdated - 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). - **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. - Sender identity is also configurable via ENV (`MAIL_FROM_NAME`, `MAIL_FROM_EMAIL`) or Settings (`smtp_from_name`, `smtp_from_email`). - `SMTP_PASSWORD_FILE`: path to a file containing the password (Docker Secrets / Kubernetes secrets pattern); overridden by `SMTP_PASSWORD` when both are set. - `SMTP_SSL` values: `tls` (default, port 587), `ssl` (port 465), `none` (port 25). @@ -1292,7 +1296,7 @@ mix hex.outdated **Join confirmation email:** -- `MvWeb.Emails.JoinConfirmationEmail` uses `Mailer.deliver(email, Mailer.smtp_config())` so it uses the same SMTP configuration as the test mail (Settings or boot ENV). On delivery failure, `Mv.Membership.submit_join_request/2` returns `{:error, :email_delivery_failed}` (and logs via `Logger.error`); the JoinLive shows an error message and no success UI. +- Join emails are sent via `Mv.Membership.JoinNotifier` (default impl: `MvWeb.JoinNotifierImpl` calling `JoinConfirmationEmail`, etc.). `MvWeb.Emails.JoinConfirmationEmail` uses `Mailer.deliver(email, Mailer.smtp_config())` so it uses the same SMTP configuration as the test mail (Settings or boot ENV). On delivery failure, `Mv.Membership.submit_join_request/2` returns `{:error, :email_delivery_failed}` (and logs via `Logger.error`); the JoinLive shows an error message and no success UI. **Unified layout (transactional emails):** diff --git a/DESIGN_GUIDELINES.md b/DESIGN_GUIDELINES.md index 9a01f9d..0ad562e 100644 --- a/DESIGN_GUIDELINES.md +++ b/DESIGN_GUIDELINES.md @@ -89,7 +89,7 @@ Pages that do not require authentication (e.g. `/join`, `/sign-in`, `/confirm_jo - **Implementation:** - **Sign-in** (`SignInLive`): Uses `use Phoenix.LiveView` (not `use MvWeb, :live_view`) so AshAuthentication’s sign_in_route live_session on_mount chain is not mixed with LiveHelpers hooks. Renders `` with the SignIn component inside a hero. Displays a locale-aware `

` title (“Anmelden” / “Registrieren”) above the AshAuthentication component (the library’s Banner is hidden via `show_banner: false`). - **Join** (`JoinLive`): Uses `use MvWeb, :live_view` and wraps content in `` with a hero for the form. - - **Join Confirm** (controller): Uses `JoinConfirmHTML` with a template that repeats the same header markup and a hero block for the result (no component call from controller templates). + - **Join Confirm** (controller): Uses `JoinConfirmHTML` with a template that wraps content in `` and a hero block for the result, so the confirm page shares the same header and chrome as Join and Sign-in. ## 3) Typography (system) diff --git a/assets/css/app.css b/assets/css/app.css index e79b4b6..d7f873c 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -156,9 +156,9 @@ /* WCAG 2.2 AA (4.5:1 for normal text): Form labels. DaisyUI .label uses 60% opacity, which fails contrast. Override to 85% of base-content so labels stay slightly - de‑emphasised vs body text but meet the minimum ratio. */ -[data-theme="light"] .label, -[data-theme="dark"] .label { + de‑emphasised vs body text but meet the minimum ratio. Match .label directly + so the override applies even when data-theme is not yet set (e.g. initial load). */ +.label { color: color-mix(in oklab, var(--color-base-content) 85%, transparent); } diff --git a/config/config.exs b/config/config.exs index 35e4160..037fd49 100644 --- a/config/config.exs +++ b/config/config.exs @@ -104,6 +104,9 @@ config :mv, :mail_from, {"Mila", "noreply@example.com"} # Join form rate limiting (Hammer). scale_ms: window in ms, limit: max submits per window per IP. config :mv, :join_rate_limit, scale_ms: 60_000, limit: 10 +# Join emails: notifier implementation (domain → web abstraction). Override in test to inject a mock. +config :mv, :join_notifier, MvWeb.JoinNotifierImpl + # Configure esbuild (the version is required) config :esbuild, version: "0.17.11", diff --git a/lib/accounts/user/validations/registration_enabled.ex b/lib/accounts/user/validations/registration_enabled.ex index 71cc7b1..f2342b7 100644 --- a/lib/accounts/user/validations/registration_enabled.ex +++ b/lib/accounts/user/validations/registration_enabled.ex @@ -21,7 +21,11 @@ defmodule Mv.Accounts.User.Validations.RegistrationEnabled do {:error, field: :base, message: - "Registration is disabled. Please use the join form or contact an administrator."} + Gettext.dgettext( + MvWeb.Gettext, + "default", + "Registration is disabled. Please use the join form or contact an administrator." + )} end end end diff --git a/lib/membership/join_notifier.ex b/lib/membership/join_notifier.ex new file mode 100644 index 0000000..daec4c1 --- /dev/null +++ b/lib/membership/join_notifier.ex @@ -0,0 +1,13 @@ +defmodule Mv.Membership.JoinNotifier do + @moduledoc """ + Behaviour for sending join-related emails (confirmation, already member, already pending). + + The domain calls this module instead of MvWeb.Emails directly, so the domain layer + does not depend on the web layer. The default implementation is set in config + (`config :mv, :join_notifier, MvWeb.JoinNotifierImpl`). Tests can override with a mock. + """ + @callback send_confirmation(email :: String.t(), token :: String.t(), opts :: keyword()) :: + {:ok, term()} | {:error, term()} + @callback send_already_member(email :: String.t()) :: {:ok, term()} | {:error, term()} + @callback send_already_pending(email :: String.t()) :: {:ok, term()} | {:error, term()} +end diff --git a/lib/membership/join_request/changes/regenerate_confirmation_token.ex b/lib/membership/join_request/changes/regenerate_confirmation_token.ex index a3206a2..c8055d2 100644 --- a/lib/membership/join_request/changes/regenerate_confirmation_token.ex +++ b/lib/membership/join_request/changes/regenerate_confirmation_token.ex @@ -16,13 +16,16 @@ defmodule Mv.Membership.JoinRequest.Changes.RegenerateConfirmationToken do token = Ash.Changeset.get_argument(changeset, :confirmation_token) if is_binary(token) and token != "" do - hash = JoinRequest.hash_confirmation_token(token) - expires_at = DateTime.utc_now() |> DateTime.add(@confirmation_validity_hours, :hour) + now = DateTime.utc_now() + expires_at = DateTime.add(now, @confirmation_validity_hours, :hour) changeset - |> Ash.Changeset.force_change_attribute(:confirmation_token_hash, hash) + |> Ash.Changeset.force_change_attribute( + :confirmation_token_hash, + JoinRequest.hash_confirmation_token(token) + ) |> Ash.Changeset.force_change_attribute(:confirmation_token_expires_at, expires_at) - |> Ash.Changeset.force_change_attribute(:confirmation_sent_at, DateTime.utc_now()) + |> Ash.Changeset.force_change_attribute(:confirmation_sent_at, now) else changeset end diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 8812d99..7fa35dc 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -32,9 +32,7 @@ defmodule Mv.Membership do alias Mv.Helpers.SystemActor alias Mv.Membership.JoinRequest alias Mv.Membership.Member - alias MvWeb.Emails.JoinAlreadyMemberEmail - alias MvWeb.Emails.JoinAlreadyPendingEmail - alias MvWeb.Emails.JoinConfirmationEmail + alias Mv.Membership.SettingsCache require Logger admin do @@ -118,10 +116,16 @@ defmodule Mv.Membership do """ def get_settings do - # Try to get the first (and only) settings record + case Process.whereis(SettingsCache) do + nil -> get_settings_uncached() + _pid -> SettingsCache.get() + end + end + + @doc false + def get_settings_uncached do case Ash.read_one(Mv.Membership.Setting, domain: __MODULE__) do {:ok, nil} -> - # No settings exist - create as fallback (should normally be created via seed script) default_club_name = System.get_env("ASSOCIATION_NAME") || "Club Name" Mv.Membership.Setting @@ -162,9 +166,16 @@ defmodule Mv.Membership do """ def update_settings(settings, attrs) do - settings - |> Ash.Changeset.for_update(:update, attrs) - |> Ash.update(domain: __MODULE__) + case settings + |> Ash.Changeset.for_update(:update, attrs) + |> Ash.update(domain: __MODULE__) do + {:ok, _updated} = result -> + SettingsCache.invalidate() + result + + error -> + error + end end @doc """ @@ -228,11 +239,18 @@ defmodule Mv.Membership do """ def update_member_field_visibility(settings, visibility_config) do - settings - |> Ash.Changeset.for_update(:update_member_field_visibility, %{ - member_field_visibility: visibility_config - }) - |> Ash.update(domain: __MODULE__) + case settings + |> Ash.Changeset.for_update(:update_member_field_visibility, %{ + member_field_visibility: visibility_config + }) + |> Ash.update(domain: __MODULE__) do + {:ok, _} = result -> + SettingsCache.invalidate() + result + + error -> + error + end end @doc """ @@ -265,12 +283,19 @@ defmodule Mv.Membership do field: field, show_in_overview: show_in_overview ) do - settings - |> Ash.Changeset.new() - |> Ash.Changeset.set_argument(:field, field) - |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) - |> Ash.Changeset.for_update(:update_single_member_field_visibility, %{}) - |> Ash.update(domain: __MODULE__) + case settings + |> Ash.Changeset.new() + |> Ash.Changeset.set_argument(:field, field) + |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) + |> Ash.Changeset.for_update(:update_single_member_field_visibility, %{}) + |> Ash.update(domain: __MODULE__) do + {:ok, _} = result -> + SettingsCache.invalidate() + result + + error -> + error + end end @doc """ @@ -304,13 +329,20 @@ defmodule Mv.Membership do show_in_overview: show_in_overview, required: required ) do - settings - |> Ash.Changeset.new() - |> Ash.Changeset.set_argument(:field, field) - |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) - |> Ash.Changeset.set_argument(:required, required) - |> Ash.Changeset.for_update(:update_single_member_field, %{}) - |> Ash.update(domain: __MODULE__) + case settings + |> Ash.Changeset.new() + |> Ash.Changeset.set_argument(:field, field) + |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) + |> Ash.Changeset.set_argument(:required, required) + |> Ash.Changeset.for_update(:update_single_member_field, %{}) + |> Ash.update(domain: __MODULE__) do + {:ok, _} = result -> + SettingsCache.invalidate() + result + + error -> + error + end end @doc """ @@ -427,12 +459,12 @@ defmodule Mv.Membership do defp pending_join_request_with_email(_), do: nil - defp apply_anti_enumeration_delay do - Process.sleep(100 + :rand.uniform(200)) + defp join_notifier do + Application.get_env(:mv, :join_notifier, MvWeb.JoinNotifierImpl) end defp send_already_member_and_return(email) do - case JoinAlreadyMemberEmail.send(email) do + case join_notifier().send_already_member(email) do {:ok, _} -> :ok @@ -440,7 +472,7 @@ defmodule Mv.Membership do Logger.error("Join already-member email failed for #{email}: #{inspect(reason)}") end - apply_anti_enumeration_delay() + # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. {:ok, :notified_already_member} end @@ -461,7 +493,7 @@ defmodule Mv.Membership do }) |> Ash.update(domain: __MODULE__, authorize?: false) do {:ok, _updated} -> - case JoinConfirmationEmail.send(email, new_token, resend: true) do + case join_notifier().send_confirmation(email, new_token, resend: true) do {:ok, _} -> :ok @@ -469,7 +501,7 @@ defmodule Mv.Membership do Logger.error("Join resend confirmation email failed for #{email}: #{inspect(reason)}") end - apply_anti_enumeration_delay() + # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. {:ok, :notified_already_pending} {:error, _} -> @@ -479,7 +511,7 @@ defmodule Mv.Membership do end defp send_already_pending_and_return(email) do - case JoinAlreadyPendingEmail.send(email) do + case join_notifier().send_already_pending(email) do {:ok, _} -> :ok @@ -487,7 +519,7 @@ defmodule Mv.Membership do Logger.error("Join already-pending email failed for #{email}: #{inspect(reason)}") end - apply_anti_enumeration_delay() + # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. {:ok, :notified_already_pending} end @@ -501,9 +533,9 @@ defmodule Mv.Membership do domain: __MODULE__ ) do {:ok, request} -> - case JoinConfirmationEmail.send(request.email, token) do + case join_notifier().send_confirmation(request.email, token, []) do {:ok, _email} -> - apply_anti_enumeration_delay() + # Delay is applied by the caller (e.g. JoinLive) to avoid blocking the process. {:ok, request} {:error, reason} -> diff --git a/lib/membership/settings_cache.ex b/lib/membership/settings_cache.ex new file mode 100644 index 0000000..d8581d6 --- /dev/null +++ b/lib/membership/settings_cache.ex @@ -0,0 +1,85 @@ +defmodule Mv.Membership.SettingsCache do + @moduledoc """ + Process-based cache for global settings to avoid repeated DB reads on hot paths + (e.g. RegistrationEnabled validation, Layouts.public_page, Plugs). + + Uses a short TTL (default 60 seconds). Cache is invalidated on every settings + update so that changes take effect quickly. If no settings process exists + (e.g. in tests), get/1 falls back to direct read. + """ + use GenServer + + @default_ttl_seconds 60 + + def start_link(opts \\ []) do + GenServer.start_link(__MODULE__, opts, name: __MODULE__) + end + + @doc """ + Returns cached settings or fetches and caches them. Uses TTL; invalidate on update. + """ + def get do + case Process.whereis(__MODULE__) do + nil -> + # No cache process (e.g. test) – read directly + do_fetch() + + _pid -> + GenServer.call(__MODULE__, :get, 10_000) + end + end + + @doc """ + Invalidates the cache so the next get/0 will refetch from the database. + Call after update_settings and any other path that mutates settings. + """ + def invalidate do + case Process.whereis(__MODULE__) do + nil -> :ok + _pid -> GenServer.cast(__MODULE__, :invalidate) + end + end + + @impl true + def init(opts) do + ttl = Keyword.get(opts, :ttl_seconds, @default_ttl_seconds) + state = %{ttl_seconds: ttl, cached: nil, expires_at: nil} + {:ok, state} + end + + @impl true + def handle_call(:get, _from, state) do + now = System.monotonic_time(:second) + expired? = state.expires_at == nil or state.expires_at <= now + + {result, new_state} = + if expired? do + fetch_and_cache(now, state) + else + {{:ok, state.cached}, state} + end + + {:reply, result, new_state} + end + + defp fetch_and_cache(now, state) do + case do_fetch() do + {:ok, settings} = ok -> + expires = now + state.ttl_seconds + {ok, %{state | cached: settings, expires_at: expires}} + + err -> + result = if state.cached, do: {:ok, state.cached}, else: err + {result, state} + end + end + + @impl true + def handle_cast(:invalidate, state) do + {:noreply, %{state | cached: nil, expires_at: nil}} + end + + defp do_fetch do + Mv.Membership.get_settings_uncached() + end +end diff --git a/lib/mv/application.ex b/lib/mv/application.ex index 6b4a10b..1b6014e 100644 --- a/lib/mv/application.ex +++ b/lib/mv/application.ex @@ -6,6 +6,7 @@ defmodule Mv.Application do use Application alias Mv.Helpers.SystemActor + alias Mv.Membership.SettingsCache alias Mv.Repo alias Mv.Vereinfacht.SyncFlash alias MvWeb.Endpoint @@ -16,20 +17,28 @@ defmodule Mv.Application do def start(_type, _args) do SyncFlash.create_table!() - children = [ - Telemetry, - Repo, - {JoinRateLimit, [clean_period: :timer.minutes(1)]}, - {Task.Supervisor, name: Mv.TaskSupervisor}, - {DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore}, - {Phoenix.PubSub, name: Mv.PubSub}, - {AshAuthentication.Supervisor, otp_app: :my}, - SystemActor, - # Start a worker by calling: Mv.Worker.start_link(arg) - # {Mv.Worker, arg}, - # Start to serve requests, typically the last entry - Endpoint - ] + # SettingsCache not started in test so get_settings runs in the test process (Ecto Sandbox). + cache_children = + if Application.get_env(:mv, :environment) == :test, do: [], else: [SettingsCache] + + children = + [ + Telemetry, + Repo + ] ++ + cache_children ++ + [ + {JoinRateLimit, [clean_period: :timer.minutes(1)]}, + {Task.Supervisor, name: Mv.TaskSupervisor}, + {DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore}, + {Phoenix.PubSub, name: Mv.PubSub}, + {AshAuthentication.Supervisor, otp_app: :my}, + SystemActor, + # Start a worker by calling: Mv.Worker.start_link(arg) + # {Mv.Worker, arg}, + # Start to serve requests, typically the last entry + Endpoint + ] # See https://hexdocs.pm/elixir/Supervisor.html # for other strategies and supported options diff --git a/lib/mv_web/components/layouts.ex b/lib/mv_web/components/layouts.ex index 5258ab9..29f5b8e 100644 --- a/lib/mv_web/components/layouts.ex +++ b/lib/mv_web/components/layouts.ex @@ -17,16 +17,24 @@ defmodule MvWeb.Layouts do Renders the public (unauthenticated) page layout: header with logo + "Mitgliederverwaltung" left, club name centered, language selector right; plus main content and flash group. Use for sign-in, join, and join-confirm pages so they share the same chrome without the sidebar or authenticated layout logic. + + Pass optional `:club_name` from the parent (e.g. LiveView mount) to avoid a settings read in the component. """ attr :flash, :map, required: true, doc: "the map of flash messages" + + attr :club_name, :string, + default: nil, + doc: "optional; if set, avoids get_settings() in the component" + slot :inner_block, required: true def public_page(assigns) do club_name = - case Mv.Membership.get_settings() do - {:ok, s} -> s.club_name || "Mitgliederverwaltung" - _ -> "Mitgliederverwaltung" - end + assigns[:club_name] || + case Mv.Membership.get_settings() do + {:ok, s} -> s.club_name || "Mitgliederverwaltung" + _ -> "Mitgliederverwaltung" + end assigns = assign(assigns, :club_name, club_name) diff --git a/lib/mv_web/controllers/join_confirm_controller.ex b/lib/mv_web/controllers/join_confirm_controller.ex index 38a3263..b304b0c 100644 --- a/lib/mv_web/controllers/join_confirm_controller.ex +++ b/lib/mv_web/controllers/join_confirm_controller.ex @@ -48,15 +48,8 @@ defmodule MvWeb.JoinConfirmController do end defp assign_confirm_assigns(conn, result) do - club_name = - case Mv.Membership.get_settings() do - {:ok, settings} -> settings.club_name || "Mitgliederverwaltung" - _ -> "Mitgliederverwaltung" - end - conn |> assign(:result, result) - |> assign(:club_name, club_name) - |> assign(:csrf_token, Plug.CSRFProtection.get_csrf_token()) + |> assign(:flash, conn.assigns[:flash] || conn.flash || %{}) end end diff --git a/lib/mv_web/controllers/join_confirm_html/confirm.html.heex b/lib/mv_web/controllers/join_confirm_html/confirm.html.heex index 8789607..68fb6d3 100644 --- a/lib/mv_web/controllers/join_confirm_html/confirm.html.heex +++ b/lib/mv_web/controllers/join_confirm_html/confirm.html.heex @@ -1,24 +1,4 @@ -<%!-- Public header (same structure as Layouts.app unauthenticated branch) --%> -
- Mila Logo - - {@club_name} - -
- - -
-
- -
+
@@ -62,4 +42,4 @@
-
+ diff --git a/lib/mv_web/join_notifier_impl.ex b/lib/mv_web/join_notifier_impl.ex new file mode 100644 index 0000000..2c29147 --- /dev/null +++ b/lib/mv_web/join_notifier_impl.ex @@ -0,0 +1,25 @@ +defmodule MvWeb.JoinNotifierImpl do + @moduledoc """ + Default implementation of Mv.Membership.JoinNotifier that delegates to MvWeb.Emails. + """ + @behaviour Mv.Membership.JoinNotifier + + alias MvWeb.Emails.JoinAlreadyMemberEmail + alias MvWeb.Emails.JoinAlreadyPendingEmail + alias MvWeb.Emails.JoinConfirmationEmail + + @impl true + def send_confirmation(email, token, opts \\ []) do + JoinConfirmationEmail.send(email, token, opts) + end + + @impl true + def send_already_member(email) do + JoinAlreadyMemberEmail.send(email) + end + + @impl true + def send_already_pending(email) do + JoinAlreadyPendingEmail.send(email) + end +end diff --git a/lib/mv_web/live/join_live.ex b/lib/mv_web/live/join_live.ex index e83031c..ed0e6e6 100644 --- a/lib/mv_web/live/join_live.ex +++ b/lib/mv_web/live/join_live.ex @@ -12,12 +12,22 @@ defmodule MvWeb.JoinLive do # Honeypot field name (legitimate-sounding to avoid bot detection) @honeypot_field "website" + # Anti-enumeration: delay before showing success (ms). Applied in LiveView so the process is not blocked. + @anti_enumeration_delay_ms_min 100 + @anti_enumeration_delay_ms_rand 200 + @impl true def mount(_params, _session, socket) do allowlist = Membership.get_join_form_allowlist() join_fields = build_join_fields_with_labels(allowlist) client_ip = client_ip_from_socket(socket) + club_name = + case Membership.get_settings() do + {:ok, s} -> s.club_name || "Mitgliederverwaltung" + _ -> "Mitgliederverwaltung" + end + socket = socket |> assign(:join_fields, join_fields) @@ -25,6 +35,7 @@ defmodule MvWeb.JoinLive do |> assign(:rate_limit_error, nil) |> assign(:client_ip, client_ip) |> assign(:honeypot_field, @honeypot_field) + |> assign(:club_name, club_name) |> assign(:form, to_form(initial_form_params(join_fields))) {:ok, socket} @@ -33,7 +44,7 @@ defmodule MvWeb.JoinLive do @impl true def render(assigns) do ~H""" - +
@@ -149,7 +160,11 @@ defmodule MvWeb.JoinLive do {:ok, attrs} -> case Membership.submit_join_request(attrs, actor: nil) do {:ok, _} -> - {:noreply, assign(socket, :submitted, true)} + delay_ms = + @anti_enumeration_delay_ms_min + :rand.uniform(@anti_enumeration_delay_ms_rand) + + Process.send_after(self(), :show_join_success, delay_ms) + {:noreply, socket} {:error, :email_delivery_failed} -> {:noreply, @@ -181,6 +196,16 @@ defmodule MvWeb.JoinLive do |> assign(:form, to_form(params, as: "join"))} end + @impl true + def handle_info(:show_join_success, socket) do + {:noreply, assign(socket, :submitted, true)} + end + + # Swoosh (e.g. in test) may send {:email, email} to the LiveView process; ignore. + def handle_info(_msg, socket) do + {:noreply, socket} + end + defp rate_limited_reply(socket, params) do {:noreply, socket diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index d5d3c33..79bd2dc 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2897,7 +2897,6 @@ msgstr "Intervall auswählen" #: lib/mv_web/components/layouts.ex #: lib/mv_web/components/layouts/sidebar.ex -#: lib/mv_web/controllers/join_confirm_html/confirm.html.heex #, elixir-autogen, elixir-format msgid "Select language" msgstr "Sprache auswählen" @@ -3892,3 +3891,8 @@ msgstr "Einstellung konnte nicht gespeichert werden." #, elixir-autogen, elixir-format msgid "If disabled, users cannot sign up via /register; sign-in and the join form remain available." msgstr "Wenn deaktiviert, können sich Nutzer*innen nicht über /register anmelden; Anmeldung und Beitrittsformular bleiben verfügbar." + +#: lib/accounts/user/validations/registration_enabled.ex +#, elixir-autogen, elixir-format +msgid "Registration is disabled. Please use the join form or contact an administrator." +msgstr "Die Registrierung ist deaktiviert. Bitte nutze das Beitrittsformular oder wende dich an eine*n Administrator*in." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 53acf03..a27bdbe 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2898,7 +2898,6 @@ msgstr "" #: lib/mv_web/components/layouts.ex #: lib/mv_web/components/layouts/sidebar.ex -#: lib/mv_web/controllers/join_confirm_html/confirm.html.heex #, elixir-autogen, elixir-format msgid "Select language" msgstr "" @@ -3892,3 +3891,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "If disabled, users cannot sign up via /register; sign-in and the join form remain available." msgstr "" + +#: lib/accounts/user/validations/registration_enabled.ex +#, elixir-autogen, elixir-format +msgid "Registration is disabled. Please use the join form or contact an administrator." +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index eed38d4..69062c2 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2898,7 +2898,6 @@ msgstr "" #: lib/mv_web/components/layouts.ex #: lib/mv_web/components/layouts/sidebar.ex -#: lib/mv_web/controllers/join_confirm_html/confirm.html.heex #, elixir-autogen, elixir-format, fuzzy msgid "Select language" msgstr "" @@ -3892,3 +3891,8 @@ msgstr "Failed to update setting." #, elixir-autogen, elixir-format msgid "If disabled, users cannot sign up via /register; sign-in and the join form remain available." msgstr "If disabled, users cannot sign up via /register; sign-in and the join form remain available." + +#: lib/accounts/user/validations/registration_enabled.ex +#, elixir-autogen, elixir-format +msgid "Registration is disabled. Please use the join form or contact an administrator." +msgstr "" diff --git a/test/mv_web/live/join_live_test.exs b/test/mv_web/live/join_live_test.exs index 1458973..4b6c24a 100644 --- a/test/mv_web/live/join_live_test.exs +++ b/test/mv_web/live/join_live_test.exs @@ -9,7 +9,8 @@ defmodule MvWeb.JoinLiveTest do Honeypot: form param `"website"` (legit-sounding name per best practice; not "honeypot"). Field is hidden via CSS class in app.css (off-screen, no inline styles), type="text". """ - use MvWeb.ConnCase, async: true + # async: false so LiveView and test share sandbox (submit creates JoinRequest in LiveView process). + use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest import Ecto.Query @@ -53,6 +54,9 @@ defmodule MvWeb.JoinLiveTest do }) |> render_submit() + # Anti-enumeration delay is applied in LiveView via send_after (100–300 ms); wait for success UI. + Process.sleep(400) + assert count_join_requests() == count_before + 1 assert view |> element("[data-testid='join-success-message']") |> has_element?() assert render(view) =~ "saved your details"