From eadf90b5fc9e8a8bbc862a47af1ed08338bfc175 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 10 Mar 2026 17:18:14 +0100 Subject: [PATCH 1/3] test: add tests for join request page --- docs/page-permission-route-coverage.md | 4 +- test/membership/join_request_test.exs | 26 ++++ .../join_confirm_controller_test.exs | 11 +- test/mv_web/live/join_live_test.exs | 130 ++++++++++++++++++ .../plugs/check_page_permission_test.exs | 6 + 5 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 test/mv_web/live/join_live_test.exs diff --git a/docs/page-permission-route-coverage.md b/docs/page-permission-route-coverage.md index f91ee0c..38625e6 100644 --- a/docs/page-permission-route-coverage.md +++ b/docs/page-permission-route-coverage.md @@ -36,7 +36,9 @@ This document lists all protected routes, which permission set may access them, ## Public Paths (no permission check) -- `/auth*`, `/register`, `/reset`, `/sign-in`, `/sign-out`, `/confirm*`, `/password-reset*`, `/set_locale` +- `/auth*`, `/register`, `/reset`, `/sign-in`, `/sign-out`, `/confirm*`, `/password-reset*`, `/set_locale`, **`/join`** + +The public join page `GET /join` is explicitly public (Subtask 4); unauthenticated access returns 200 when join form is enabled, 404 when disabled. Unit test: `test/mv_web/plugs/check_page_permission_test.exs` (plug allows /join); integration: `test/mv_web/live/join_live_test.exs`. The join confirmation route `GET /confirm_join/:token` is public (matched by `/confirm*`). Unit tests: `test/mv_web/controllers/join_confirm_controller_test.exs` (stubbed callback, no integration). diff --git a/test/membership/join_request_test.exs b/test/membership/join_request_test.exs index f40c9ec..6c39d4e 100644 --- a/test/membership/join_request_test.exs +++ b/test/membership/join_request_test.exs @@ -121,6 +121,32 @@ defmodule Mv.Membership.JoinRequestTest do end end + describe "allowlist (server-side field filter)" do + test "submit with non-allowlisted form_data keys does not persist those keys" do + # Allowlist restricts which fields are accepted; extra keys must not be stored. + {:ok, settings} = Membership.get_settings() + Mv.Membership.update_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => true, "first_name" => false} + }) + + attrs = %{ + email: "allowlist#{System.unique_integer([:positive])}@example.com", + first_name: "Allowed", + confirmation_token: "tok-#{System.unique_integer([:positive])}", + form_data: %{"city" => "Berlin", "internal_or_secret" => "must not persist"}, + schema_version: 1 + } + + assert {:ok, request} = Membership.submit_join_request(attrs, actor: nil) + assert request.email == attrs.email + assert request.first_name == attrs.first_name + refute Map.has_key?(request.form_data || %{}, "internal_or_secret") + assert (request.form_data || %{})["city"] == "Berlin" + end + end + defp error_message(errors, field) do errors |> Enum.filter(fn err -> Map.get(err, :field) == field end) diff --git a/test/mv_web/controllers/join_confirm_controller_test.exs b/test/mv_web/controllers/join_confirm_controller_test.exs index d1e9117..a85cde5 100644 --- a/test/mv_web/controllers/join_confirm_controller_test.exs +++ b/test/mv_web/controllers/join_confirm_controller_test.exs @@ -60,13 +60,18 @@ defmodule MvWeb.JoinConfirmControllerTest do end @tag role: :unauthenticated - test "expired token returns 200 with expired message", %{conn: conn} do + test "expired token returns 200 with expired message and instructs to submit form again", %{ + conn: conn + } do Application.put_env(:mv, :join_confirm_callback, JoinConfirmExpiredStub) conn = get(conn, "/confirm_join/expired-token") + body = response(conn, 200) - assert response(conn, 200) =~ "expired" - assert response(conn, 200) =~ "submit" + assert body =~ "expired" + assert body =~ "submit" + # Concept §2.5: clear message + "submit form again" + assert body =~ "form" or body =~ "again" end @tag role: :unauthenticated diff --git a/test/mv_web/live/join_live_test.exs b/test/mv_web/live/join_live_test.exs new file mode 100644 index 0000000..cc4e5db --- /dev/null +++ b/test/mv_web/live/join_live_test.exs @@ -0,0 +1,130 @@ +defmodule MvWeb.JoinLiveTest do + @moduledoc """ + Tests for the public join page (Subtask 4: Public join page and anti-abuse). + + Covers: public path /join (unauthenticated 200), 404 when join disabled, + submit creates JoinRequest and shows success copy, honeypot prevents create, + rate limiting rejects excess submits. Uses unauthenticated conn; no User/Member. + """ + use MvWeb.ConnCase, async: true + import Phoenix.LiveViewTest + import Ecto.Query + + alias Mv.Membership + alias Mv.Repo + + describe "GET /join" do + @tag role: :unauthenticated + test "unauthenticated GET /join returns 200 when join form is enabled", %{conn: conn} do + enable_join_form(true) + conn = get(conn, "/join") + assert conn.status == 200 + end + + @tag role: :unauthenticated + test "unauthenticated GET /join returns 404 when join form is disabled", %{conn: conn} do + enable_join_form(false) + conn = get(conn, "/join") + assert conn.status == 404 + end + end + + describe "submit join form" do + setup :enable_join_form_for_test + + @tag role: :unauthenticated + test "submit with valid allowlist data creates one JoinRequest and shows success copy", %{ + conn: conn + } do + count_before = count_join_requests() + {:ok, view, _html} = live(conn, "/join") + + view + |> form("#join-form", %{ + "email" => "newuser#{System.unique_integer([:positive])}@example.com", + "first_name" => "Jane", + "last_name" => "Doe", + "honeypot" => "" + }) + |> render_submit() + + assert count_join_requests() == count_before + 1 + assert view |> element("[data-testid='join-success-message']") |> has_element?() + assert render(view) =~ "saved your details" + assert render(view) =~ "click the link" + end + + @tag role: :unauthenticated + test "submit with honeypot filled does not create JoinRequest but shows same success copy", %{ + conn: conn + } do + count_before = count_join_requests() + {:ok, view, _html} = live(conn, "/join") + + view + |> form("#join-form", %{ + "email" => "bot#{System.unique_integer([:positive])}@example.com", + "first_name" => "Bot", + "last_name" => "User", + "honeypot" => "filled-by-bot" + }) + |> render_submit() + + assert count_join_requests() == count_before + assert view |> element("[data-testid='join-success-message']") |> has_element?() + end + + @tag role: :unauthenticated + @tag :slow + test "after rate limit exceeded submit returns 429 or error and no new JoinRequest", %{ + conn: conn + } do + enable_join_form(true) + # Rely on test config: join rate limit low (e.g. 2 per window) + base_email = "ratelimit#{System.unique_integer([:positive])}@example.com" + count_before = count_join_requests() + + {:ok, view, _html} = live(conn, "/join") + + # Exhaust limit with valid submits + for i <- 0..1 do + view + |> form("#join-form", %{ + "email" => "#{i}-#{base_email}", + "first_name" => "User", + "last_name" => "Test", + "honeypot" => "" + }) + |> render_submit() + end + + # Next submit should be rate limited + result = + view + |> form("#join-form", %{ + "email" => "third-#{base_email}", + "first_name" => "Third", + "last_name" => "User", + "honeypot" => "" + }) + |> render_submit() + + assert count_join_requests() == count_before + 2 + assert result =~ "rate limit" or result =~ "too many" or result =~ "429" + end + end + + defp enable_join_form(enabled) do + {:ok, settings} = Membership.get_settings() + {:ok, _} = Membership.update_settings(settings, %{join_form_enabled: enabled}) + end + + defp enable_join_form_for_test(_context) do + enable_join_form(true) + :ok + end + + defp count_join_requests do + Repo.one(from j in "join_requests", select: count(j.id)) || 0 + end +end diff --git a/test/mv_web/plugs/check_page_permission_test.exs b/test/mv_web/plugs/check_page_permission_test.exs index 1b3f827..80aa95e 100644 --- a/test/mv_web/plugs/check_page_permission_test.exs +++ b/test/mv_web/plugs/check_page_permission_test.exs @@ -204,6 +204,12 @@ defmodule MvWeb.Plugs.CheckPagePermissionTest do refute conn.halted end + + test "unauthenticated user can access /join (public join page, no redirect)" do + conn = conn_without_user("/join") |> CheckPagePermission.call([]) + + refute conn.halted + end end describe "error handling" do -- 2.47.2 From f1d052620957717e6ea0716c39f4604b7ca0da89 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 10 Mar 2026 18:25:17 +0100 Subject: [PATCH 2/3] feat: add join form --- assets/css/app.css | 13 ++ config/config.exs | 3 + config/test.exs | 3 + lib/mv/application.ex | 2 + lib/mv_web/components/layouts.ex | 20 +- lib/mv_web/endpoint.ex | 4 +- lib/mv_web/join_rate_limit.ex | 27 +++ lib/mv_web/live/join_live.ex | 239 ++++++++++++++++++++++ lib/mv_web/live/member_live/index.ex | 3 + lib/mv_web/plugs/check_page_permission.ex | 2 +- lib/mv_web/plugs/join_form_enabled.ex | 33 +++ lib/mv_web/router.ex | 7 + mix.exs | 3 +- mix.lock | 1 + priv/gettext/de/LC_MESSAGES/default.po | 47 +++++ priv/gettext/default.pot | 48 +++++ priv/gettext/en/LC_MESSAGES/default.po | 47 +++++ test/membership/join_request_test.exs | 1 + test/mv_web/live/join_live_test.exs | 59 +++++- 19 files changed, 547 insertions(+), 15 deletions(-) create mode 100644 lib/mv_web/join_rate_limit.ex create mode 100644 lib/mv_web/live/join_live.ex create mode 100644 lib/mv_web/plugs/join_form_enabled.ex diff --git a/assets/css/app.css b/assets/css/app.css index 28ea24b..e3c6e83 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -99,6 +99,19 @@ /* Make LiveView wrapper divs transparent for layout */ [data-phx-session] { display: contents } +/* Honeypot: off-screen and minimal size so bots fill it, humans never see it (best practice) */ +.join-form-helper { + position: absolute; + left: -9999px; + width: 1px; + height: 1px; + overflow: hidden; +} +.join-form-helper .join-form-helper-input { + position: absolute; + left: -9999px; +} + /* WCAG 1.4.12 Text Spacing: allow user stylesheets to adjust text spacing in popovers. Popover content (e.g. from DaisyUI dropdown) must not rely on non-overridable inline spacing; use inherited values so custom stylesheets can override. */ diff --git a/config/config.exs b/config/config.exs index 323f5cd..ab55f2a 100644 --- a/config/config.exs +++ b/config/config.exs @@ -93,6 +93,9 @@ config :mv, Mv.Mailer, adapter: Swoosh.Adapters.Local # user confirmation, password reset). Override in config/runtime.exs from ENV. 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 + # Configure esbuild (the version is required) config :esbuild, version: "0.17.11", diff --git a/config/test.exs b/config/test.exs index 864222f..84ccd70 100644 --- a/config/test.exs +++ b/config/test.exs @@ -55,3 +55,6 @@ config :mv, :default_locale, "en" # Enable SQL Sandbox for async LiveView tests # This flag controls sync vs async behavior in CycleGenerator after_action hooks config :mv, :sql_sandbox, true + +# Join form rate limit: low limit so tests can trigger rate limiting (e.g. 2 per minute) +config :mv, :join_rate_limit, scale_ms: 60_000, limit: 2 diff --git a/lib/mv/application.ex b/lib/mv/application.ex index 835652f..6b4a10b 100644 --- a/lib/mv/application.ex +++ b/lib/mv/application.ex @@ -9,6 +9,7 @@ defmodule Mv.Application do alias Mv.Repo alias Mv.Vereinfacht.SyncFlash alias MvWeb.Endpoint + alias MvWeb.JoinRateLimit alias MvWeb.Telemetry @impl true @@ -18,6 +19,7 @@ defmodule Mv.Application do 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}, diff --git a/lib/mv_web/components/layouts.ex b/lib/mv_web/components/layouts.ex index 79983c5..17fca11 100644 --- a/lib/mv_web/components/layouts.ex +++ b/lib/mv_web/components/layouts.ex @@ -82,7 +82,25 @@ defmodule MvWeb.Layouts do <% else %> - + +
+ Mila Logo + + {@club_name} + +
+ + +
+
{render_slot(@inner_block)} diff --git a/lib/mv_web/endpoint.ex b/lib/mv_web/endpoint.ex index d1b4247..dfa01ad 100644 --- a/lib/mv_web/endpoint.ex +++ b/lib/mv_web/endpoint.ex @@ -12,8 +12,8 @@ defmodule MvWeb.Endpoint do ] socket "/live", Phoenix.LiveView.Socket, - websocket: [connect_info: [session: @session_options]], - longpoll: [connect_info: [session: @session_options]] + websocket: [connect_info: [:peer_data, session: @session_options]], + longpoll: [connect_info: [:peer_data, session: @session_options]] # Serve at "/" the static files from "priv/static" directory. # diff --git a/lib/mv_web/join_rate_limit.ex b/lib/mv_web/join_rate_limit.ex new file mode 100644 index 0000000..6bb1198 --- /dev/null +++ b/lib/mv_web/join_rate_limit.ex @@ -0,0 +1,27 @@ +defmodule MvWeb.JoinRateLimit do + @moduledoc """ + Rate limiting for the public join form (submit action). + + Uses Hammer with ETS backend. Key is derived from client IP so each IP + is limited independently. Config from :mv :join_rate_limit (scale_ms, limit). + """ + use Hammer, backend: :ets + + @doc """ + Checks if the given key (e.g. client IP) is within rate limit for join form submit. + + Returns: + - `:allow` - submission allowed + - `{:deny, _retry_after_ms}` - rate limit exceeded + """ + def check(key) when is_binary(key) do + config = Application.get_env(:mv, :join_rate_limit, []) + scale_ms = Keyword.get(config, :scale_ms, 60_000) + limit = Keyword.get(config, :limit, 10) + + case hit(key, scale_ms, limit) do + {:allow, _count} -> :allow + {:deny, retry_after} -> {:deny, retry_after} + end + end +end diff --git a/lib/mv_web/live/join_live.ex b/lib/mv_web/live/join_live.ex new file mode 100644 index 0000000..fc7038a --- /dev/null +++ b/lib/mv_web/live/join_live.ex @@ -0,0 +1,239 @@ +defmodule MvWeb.JoinLive do + @moduledoc """ + Public join page (unauthenticated). Renders form from allowlist, handles submit + with honeypot and rate limiting; shows success copy after submit. + """ + use MvWeb, :live_view + + alias Mv.Membership + alias MvWeb.JoinRateLimit + alias MvWeb.Translations.MemberFields + + # Honeypot field name (legitimate-sounding to avoid bot detection) + @honeypot_field "website" + + @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) + + socket = + socket + |> assign(:join_fields, join_fields) + |> assign(:submitted, false) + |> assign(:rate_limit_error, nil) + |> assign(:client_ip, client_ip) + |> assign(:honeypot_field, @honeypot_field) + |> assign(:form, to_form(initial_form_params(join_fields))) + + {:ok, socket} + end + + @impl true + def render(assigns) do + ~H""" + +
+ <.header> + {gettext("Become a member")} + + +

+ {gettext("Please enter your details for the membership application here.")} +

+ + <%= if @submitted do %> +
+

+ {gettext( + "We have saved your details. To complete your request, please click the link we sent to your email." + )} +

+
+ <% else %> + <.form + for={@form} + id="join-form" + phx-submit="submit" + class="space-y-4" + > + <%= if @rate_limit_error do %> +
+ {@rate_limit_error} +
+ <% end %> + + <%= for field <- @join_fields do %> +
+ + +
+ <% end %> + + <%!-- + Honeypot (best practice): legit field name "website", type="text", no inline CSS, + hidden via class in app.css (off-screen + 1px). tabindex=-1, autocomplete=off, + aria-hidden so screen readers skip. If filled → silent failure (same success UI). + --%> + + +

+ {gettext( + "By submitting your application you will receive an email with a confirmation link. Once you have confirmed your email address, your application will be reviewed." + )} +

+ +

+ {gettext( + "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." + )} +

+ +
+ +
+ + <% end %> +
+ + """ + end + + @impl true + def handle_event("submit", params, socket) do + # Honeypot: if filled, treat as bot – show same success UI, do not create (silent failure) + honeypot_value = String.trim(params[@honeypot_field] || "") + + if honeypot_value != "" do + {:noreply, assign(socket, :submitted, true)} + else + key = "join:#{socket.assigns.client_ip}" + + case JoinRateLimit.check(key) do + :allow -> do_submit(socket, params) + {:deny, _retry_after} -> rate_limited_reply(socket, params) + end + end + end + + defp do_submit(socket, params) do + case build_submit_attrs(params, socket.assigns.join_fields) do + {:ok, attrs} -> + case Membership.submit_join_request(attrs, actor: nil) do + {:ok, _} -> {:noreply, assign(socket, :submitted, true)} + {:error, _} -> validation_error_reply(socket, params) + end + + {:error, message} -> + {:noreply, + socket + |> put_flash(:error, message) + |> assign(:form, to_form(params, as: "join"))} + end + end + + defp validation_error_reply(socket, params) do + {:noreply, + socket + |> put_flash(:error, gettext("Please check your entries. Email is required.")) + |> assign(:form, to_form(params, as: "join"))} + end + + defp rate_limited_reply(socket, params) do + {:noreply, + socket + |> assign(:rate_limit_error, gettext("Too many requests. Please try again later.")) + |> assign(:form, to_form(params, as: "join"))} + end + + defp build_join_fields_with_labels(allowlist) do + member_field_strings = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + Enum.map(allowlist, fn %{id: id, required: required} -> + label = + if id in member_field_strings do + MemberFields.label(String.to_existing_atom(id)) + else + gettext("Field") + end + + %{id: id, label: label, required: required} + end) + end + + defp initial_form_params(join_fields) do + join_fields + |> Enum.map(fn f -> {f.id, ""} end) + |> Map.new() + |> Map.put(@honeypot_field, "") + end + + defp input_type("email"), do: "email" + defp input_type(_), do: "text" + + defp build_submit_attrs(params, join_fields) do + allowlist_ids = MapSet.new(Enum.map(join_fields, & &1.id)) + typed = ["email", "first_name", "last_name"] + + email = String.trim(params["email"] || "") + + if email == "" do + {:error, gettext("Email is required.")} + else + attrs = %{ + email: email, + first_name: optional_param(params, "first_name"), + last_name: optional_param(params, "last_name"), + form_data: %{}, + schema_version: 1 + } + + form_data = + params + |> Enum.filter(fn {key, _} -> key in allowlist_ids and key not in typed end) + |> Map.new() + |> Enum.map(fn {k, v} -> {k, String.trim(to_string(v))} end) + |> Map.new() + + attrs = %{attrs | form_data: form_data} + {:ok, attrs} + end + end + + defp optional_param(params, key) do + v = params[key] + if is_binary(v), do: String.trim(v), else: nil + end + + defp client_ip_from_socket(socket) do + case get_connect_info(socket, :peer_data) do + %{address: address} when is_tuple(address) -> + address |> Tuple.to_list() |> Enum.join(".") + + _ -> + "unknown" + end + end +end diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index e2e037d..c8f32a0 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -107,6 +107,9 @@ defmodule MvWeb.MemberLive.Index do {:error, _} -> %{member_field_visibility: %{}} end + # Ensure nested module is loaded (can be missing after code reload in dev if load order changes) + Code.ensure_loaded!(FieldSelection) + # Load user field selection from session session_selection = FieldSelection.get_from_session(session) diff --git a/lib/mv_web/plugs/check_page_permission.ex b/lib/mv_web/plugs/check_page_permission.ex index 3aa6561..ff6d47d 100644 --- a/lib/mv_web/plugs/check_page_permission.ex +++ b/lib/mv_web/plugs/check_page_permission.ex @@ -80,7 +80,7 @@ defmodule MvWeb.Plugs.CheckPagePermission do Used by LiveView hook to skip redirect on sign-in etc. """ def public_path?(path) when is_binary(path) do - path in ["/register", "/reset", "/set_locale", "/sign-in", "/sign-out"] or + path in ["/register", "/reset", "/set_locale", "/sign-in", "/sign-out", "/join"] or String.starts_with?(path, "/auth") or String.starts_with?(path, "/confirm") or String.starts_with?(path, "/password-reset") diff --git a/lib/mv_web/plugs/join_form_enabled.ex b/lib/mv_web/plugs/join_form_enabled.ex new file mode 100644 index 0000000..f6865c4 --- /dev/null +++ b/lib/mv_web/plugs/join_form_enabled.ex @@ -0,0 +1,33 @@ +defmodule MvWeb.Plugs.JoinFormEnabled do + @moduledoc """ + For GET /join: returns 404 when the join form is disabled in settings. + No-op for other paths. + """ + import Plug.Conn + + alias Mv.Membership + + def init(opts), do: opts + + def call(conn, _opts) do + if join_path?(conn), do: maybe_404(conn), else: conn + end + + defp join_path?(conn) do + conn.request_path == "/join" and conn.method == "GET" + end + + defp maybe_404(conn) do + case Membership.get_settings() do + {:ok, %{join_form_enabled: true}} -> conn + _ -> send_404(conn) + end + end + + defp send_404(conn) do + conn + |> put_resp_content_type("text/html") + |> send_resp(404, "Not Found") + |> halt() + end +end diff --git a/lib/mv_web/router.ex b/lib/mv_web/router.ex index 3ab264f..74fcd22 100644 --- a/lib/mv_web/router.ex +++ b/lib/mv_web/router.ex @@ -15,6 +15,7 @@ defmodule MvWeb.Router do plug :load_from_session plug :set_locale plug MvWeb.Plugs.CheckPagePermission + plug MvWeb.Plugs.JoinFormEnabled end pipeline :api do @@ -126,6 +127,12 @@ defmodule MvWeb.Router do overrides: [MvWeb.AuthOverrides, AshAuthentication.Phoenix.Overrides.DaisyUI], gettext_backend: {MvWeb.Gettext, "auth"} + # Public join page (no auth required) + live_session :public_join, + on_mount: [{MvWeb.LiveUserAuth, :live_user_optional}] do + live "/join", JoinLive, :index + end + # Public join confirmation (double opt-in); /confirm* is already public in CheckPagePermission get "/confirm_join/:token", JoinConfirmController, :confirm diff --git a/mix.exs b/mix.exs index 0d1d4f1..56e7dde 100644 --- a/mix.exs +++ b/mix.exs @@ -82,7 +82,8 @@ defmodule Mv.MixProject do {:ecto_commons, "~> 0.3"}, {:slugify, "~> 1.3"}, {:nimble_csv, "~> 1.0"}, - {:imprintor, "~> 0.5.0"} + {:imprintor, "~> 0.5.0"}, + {:hammer, "~> 7.0"} ] end diff --git a/mix.lock b/mix.lock index 849dfd5..8ac995a 100644 --- a/mix.lock +++ b/mix.lock @@ -37,6 +37,7 @@ "fine": {:hex, :fine, "0.1.4", "b19a89c1476c7c57afb5f9314aed5960b5bc95d5277de4cb5ee8e1d1616ce379", [:mix], [], "hexpm", "be3324cc454a42d80951cf6023b9954e9ff27c6daa255483b3e8d608670303f5"}, "gettext": {:hex, :gettext, "1.0.2", "5457e1fd3f4abe47b0e13ff85086aabae760497a3497909b8473e0acee57673b", [:mix], [{:expo, "~> 0.5.1 or ~> 1.0", [hex: :expo, repo: "hexpm", optional: false]}], "hexpm", "eab805501886802071ad290714515c8c4a17196ea76e5afc9d06ca85fb1bfeb3"}, "glob_ex": {:hex, :glob_ex, "0.1.11", "cb50d3f1ef53f6ca04d6252c7fde09fd7a1cf63387714fe96f340a1349e62c93", [:mix], [], "hexpm", "342729363056e3145e61766b416769984c329e4378f1d558b63e341020525de4"}, + "hammer": {:hex, :hammer, "7.2.0", "73113eca87f0fd20a6d3679c1182e8c4c1778266f61de4e9dc8c589dee156c30", [:mix], [], "hexpm", "c50fa865ddfe7b3d4f8a6941f56940679e02a9a1465b00668a95d140b101d828"}, "heroicons": {:git, "https://github.com/tailwindlabs/heroicons.git", "0435d4ca364a608cc75e2f8683d374e55abbae26", [tag: "v2.2.0", sparse: "optimized", depth: 1]}, "hpax": {:hex, :hpax, "1.0.3", "ed67ef51ad4df91e75cc6a1494f851850c0bd98ebc0be6e81b026e765ee535aa", [:mix], [], "hexpm", "8eab6e1cfa8d5918c2ce4ba43588e894af35dbd8e91e6e55c817bca5847df34a"}, "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"}, diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 90bedc5..15ade4b 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -1792,6 +1792,7 @@ msgid "Email is invalid." msgstr "E-Mail ist ungültig." #: lib/mv/membership/import/member_csv.ex +#: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "Email is required." msgstr "E-Mail ist erforderlich." @@ -3348,6 +3349,7 @@ msgid "Could not save join form settings." msgstr "Beitrittsformular-Einstellungen konnten nicht gespeichert werden." #: lib/mv_web/live/global_settings_live.ex +#: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "Field" msgstr "Feld" @@ -3401,3 +3403,48 @@ msgstr "Umordnen" #, elixir-autogen, elixir-format msgid "The order of rows determines the field order in the join form." msgstr "Die Reihenfolge der Zeilen bestimmt die Reihenfolge der Felder im Beitrittsformular." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Become a member" +msgstr "Mitglied werden" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Please check your entries. Email is required." +msgstr "Bitte prüfe deine Angaben. E-Mail ist erforderlich." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Submit request" +msgstr "Antrag absenden" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Too many requests. Please try again later." +msgstr "Zu viele Anfragen. Bitte versuche es später erneut." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "We have saved your details. To complete your request, please click the link we sent to your email." +msgstr "Wir haben deine Angaben gespeichert. Um deinen Antrag abzuschließen, klicke bitte auf den Link in der E-Mail, die wir dir geschickt haben." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Website" +msgstr "Webseite" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "By submitting your application you will receive an email with a confirmation link. Once you have confirmed your email address, your application will be reviewed." +msgstr "Mit Absenden deines Antrags erhältst du eine Mail mit einem Bestätigungslink. Sobald du deine Mail-Adresse bestätigt hast, wird dein Antrag geprüft." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Please enter your details for the membership application here." +msgstr "Bitte gib hier die Daten für deinen Mitgliedsantrag an." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." +msgstr "Deine Angaben werden nur zur Bearbeitung deines Mitgliedsantrags und zur Kontaktaufnahme genutzt. Zur Absicherung gegen Missbrauch verarbeiten wir zusätzlich technische Daten (z. B. IP-Adresse) nur im dafür nötigen Umfang." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 70bf233..913d5cc 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -514,6 +514,7 @@ msgstr "" msgid "Back to users list" msgstr "" +#: lib/mv_web/components/layouts.ex #: lib/mv_web/components/layouts/sidebar.ex #, elixir-autogen, elixir-format msgid "Select language" @@ -1793,6 +1794,7 @@ msgid "Email is invalid." msgstr "" #: lib/mv/membership/import/member_csv.ex +#: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "Email is required." msgstr "" @@ -3348,6 +3350,7 @@ msgid "Could not save join form settings." msgstr "" #: lib/mv_web/live/global_settings_live.ex +#: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "Field" msgstr "" @@ -3401,3 +3404,48 @@ msgstr "" #, elixir-autogen, elixir-format msgid "The order of rows determines the field order in the join form." msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Become a member" +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Please check your entries. Email is required." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Submit request" +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Too many requests. Please try again later." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "We have saved your details. To complete your request, please click the link we sent to your email." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Website (leave empty)" +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "By submitting your application you will receive an email with a confirmation link. Once you have confirmed your email address, your application will be reviewed." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Please enter your details for the membership application here." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 02160f9..e91e3be 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -1793,6 +1793,7 @@ msgid "Email is invalid." msgstr "" #: lib/mv/membership/import/member_csv.ex +#: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "Email is required." msgstr "" @@ -3348,6 +3349,7 @@ msgid "Could not save join form settings." msgstr "" #: lib/mv_web/live/global_settings_live.ex +#: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "Field" msgstr "" @@ -3401,3 +3403,48 @@ msgstr "Reorder" #, elixir-autogen, elixir-format msgid "The order of rows determines the field order in the join form." msgstr "The order of rows determines the field order in the join form." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Become a member" +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Please check your entries. Email is required." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Submit request" +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Too many requests. Please try again later." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "We have saved your details. To complete your request, please click the link we sent to your email." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Website (leave empty)" +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "By submitting your application you will receive an email with a confirmation link. Once you have confirmed your email address, your application will be reviewed." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Please enter your details for the membership application here." +msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." +msgstr "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." diff --git a/test/membership/join_request_test.exs b/test/membership/join_request_test.exs index 6c39d4e..ea509fa 100644 --- a/test/membership/join_request_test.exs +++ b/test/membership/join_request_test.exs @@ -125,6 +125,7 @@ defmodule Mv.Membership.JoinRequestTest do test "submit with non-allowlisted form_data keys does not persist those keys" do # Allowlist restricts which fields are accepted; extra keys must not be stored. {:ok, settings} = Membership.get_settings() + Mv.Membership.update_settings(settings, %{ join_form_enabled: true, join_form_field_ids: ["email", "first_name"], diff --git a/test/mv_web/live/join_live_test.exs b/test/mv_web/live/join_live_test.exs index cc4e5db..bd133cd 100644 --- a/test/mv_web/live/join_live_test.exs +++ b/test/mv_web/live/join_live_test.exs @@ -5,6 +5,9 @@ defmodule MvWeb.JoinLiveTest do Covers: public path /join (unauthenticated 200), 404 when join disabled, submit creates JoinRequest and shows success copy, honeypot prevents create, rate limiting rejects excess submits. Uses unauthenticated conn; no User/Member. + + 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 import Phoenix.LiveViewTest @@ -44,7 +47,7 @@ defmodule MvWeb.JoinLiveTest do "email" => "newuser#{System.unique_integer([:positive])}@example.com", "first_name" => "Jane", "last_name" => "Doe", - "honeypot" => "" + "website" => "" }) |> render_submit() @@ -66,7 +69,7 @@ defmodule MvWeb.JoinLiveTest do "email" => "bot#{System.unique_integer([:positive])}@example.com", "first_name" => "Bot", "last_name" => "User", - "honeypot" => "filled-by-bot" + "website" => "filled-by-bot" }) |> render_submit() @@ -79,38 +82,66 @@ defmodule MvWeb.JoinLiveTest do test "after rate limit exceeded submit returns 429 or error and no new JoinRequest", %{ conn: conn } do + # Reset rate limit state so this test is independent of others (same key in test) + try do + :ets.delete_all_objects(MvWeb.JoinRateLimit) + rescue + ArgumentError -> :ok + end + enable_join_form(true) + # Set allowlist so form has email, first_name, last_name + {:ok, settings} = Membership.get_settings() + + Membership.update_settings(settings, %{ + join_form_field_ids: ["email", "first_name", "last_name"], + join_form_field_required: %{"email" => true, "first_name" => false, "last_name" => false} + }) + # Rely on test config: join rate limit low (e.g. 2 per window) base_email = "ratelimit#{System.unique_integer([:positive])}@example.com" count_before = count_join_requests() + sandbox = conn.private[:ecto_sandbox] - {:ok, view, _html} = live(conn, "/join") - - # Exhaust limit with valid submits + # Exhaust limit with 2 valid submits (each needs a fresh session because form disappears after submit) for i <- 0..1 do + c = + build_conn() + |> Phoenix.ConnTest.init_test_session(%{}) + |> Plug.Conn.put_private(:ecto_sandbox, sandbox) + + {:ok, view, _} = live(c, "/join") + view |> form("#join-form", %{ "email" => "#{i}-#{base_email}", "first_name" => "User", "last_name" => "Test", - "honeypot" => "" + "website" => "" }) |> render_submit() end - # Next submit should be rate limited + # Next submit (new session) should be rate limited + c = + build_conn() + |> Phoenix.ConnTest.init_test_session(%{}) + |> Plug.Conn.put_private(:ecto_sandbox, sandbox) + + {:ok, view, _} = live(c, "/join") + result = view |> form("#join-form", %{ "email" => "third-#{base_email}", "first_name" => "Third", "last_name" => "User", - "honeypot" => "" + "website" => "" }) |> render_submit() assert count_join_requests() == count_before + 2 - assert result =~ "rate limit" or result =~ "too many" or result =~ "429" + assert result =~ "rate limit" or String.downcase(result) =~ "too many" or result =~ "429" end end @@ -120,7 +151,15 @@ defmodule MvWeb.JoinLiveTest do end defp enable_join_form_for_test(_context) do - enable_join_form(true) + {:ok, settings} = Membership.get_settings() + + {:ok, _} = + Membership.update_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name", "last_name"], + join_form_field_required: %{"email" => true, "first_name" => false, "last_name" => false} + }) + :ok end -- 2.47.2 From 021b709e6a5ce3bb2c8e65eb43269cd3955f6999 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 10 Mar 2026 22:54:41 +0100 Subject: [PATCH 3/3] refactor: address review comments for join view --- CODE_GUIDELINES.md | 2 +- docs/onboarding-join-concept.md | 4 +- lib/membership/join_request.ex | 3 ++ .../changes/filter_form_data_by_allowlist.ex | 38 ++++++++++++++++++ lib/mv_web/endpoint.ex | 4 +- lib/mv_web/join_rate_limit.ex | 1 + lib/mv_web/live/join_live.ex | 40 ++++++++++++++----- lib/mv_web/plugs/join_form_enabled.ex | 1 + priv/gettext/de/LC_MESSAGES/default.po | 11 ++--- priv/gettext/default.pot | 10 ++--- priv/gettext/en/LC_MESSAGES/default.po | 11 ++--- test/membership/join_request_test.exs | 19 ++++++++- 12 files changed, 113 insertions(+), 31 deletions(-) create mode 100644 lib/membership/join_request/changes/filter_form_data_by_allowlist.ex diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 47f589d..b789088 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -86,7 +86,7 @@ lib/ │ ├── membership.ex # Domain definition │ ├── member.ex # Member resource │ ├── join_request.ex # JoinRequest (public join form, double opt-in) -│ ├── join_request/ # JoinRequest changes (SetConfirmationToken, ConfirmRequest) +│ ├── join_request/ # JoinRequest changes (SetConfirmationToken, FilterFormDataByAllowlist, ConfirmRequest) │ ├── 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) diff --git a/docs/onboarding-join-concept.md b/docs/onboarding-join-concept.md index 01c7ea4..680799d 100644 --- a/docs/onboarding-join-concept.md +++ b/docs/onboarding-join-concept.md @@ -42,7 +42,7 @@ ### 2.3 Data Flow -- **Input:** Only data explicitly allowed for the public form; field set is admin-configured (see §2.6). No internal or sensitive fields. **Server-side allowlist:** The set of accepted fields must be enforced on the server from the join-form settings (allowlist), not only in the UI, to prevent field injection or extra attributes from being stored. +- **Input:** Only data explicitly allowed for the public form; field set is admin-configured (see §2.6). No internal or sensitive fields. **Server-side allowlist:** The set of accepted fields is enforced in the LiveView (`build_submit_attrs`) and in the resource via **`JoinRequest.Changes.FilterFormDataByAllowlist`** so that even direct API/submit_join_request calls only persist allowlisted form_data keys. - **On form submit:** **Create** a JoinRequest with status `pending_confirmation`, store confirmation token **hash** in the DB (raw token only in the email link), set `confirmation_token_expires_at` (e.g. 24h), store all allowlisted form data (see §2.3.2), then send confirmation email. - **On confirmation link click:** **Update** the JoinRequest (find by token hash): set status to `submitted`, set `submitted_at`, clear/invalidate token fields. If the record is already `submitted`, return success without changing it (idempotent). - **No creation** of Member or User in Prio 1; promotion to Member/User happens in a later step (e.g. after approval). @@ -73,7 +73,7 @@ - **Public paths:** `/join` and the confirmation route must be public (unauthenticated access returns 200). - **Explicit public path for `/join`:** Add **`/join`** (and if needed `/join/*`) to the page-permission plug’s **`public_path?/1`** so that the join page is reachable without login. Do not rely on the confirm path alone. - **Confirmation route:** Use **`/confirm_join/:token`** so that the existing whitelist (e.g. `String.starts_with?(path, "/confirm")`) already covers it; no extra plug change for confirm. -- **Abuse:** **Honeypot** (MVP) plus **rate limiting** (MVP). Use Phoenix/Elixir standard options (e.g. **Hammer** with **Hammer.Plug**, ETS backend), scoped to the join flow (e.g. by IP). Verify library version and multi-node behaviour before or during implementation. +- **Abuse:** **Honeypot** (MVP) plus **rate limiting** (MVP). Use Phoenix/Elixir standard options (e.g. **Hammer** with **Hammer.Plug**, ETS backend), scoped to the join flow (e.g. by IP). Client IP for rate limiting: prefer **X-Forwarded-For** / **X-Real-IP** when behind a reverse proxy (see Endpoint `connect_info: [:x_headers]` and `JoinLive.client_ip_from_socket/1`); fallback to peer_data with correct IPv4/IPv6 string via `:inet.ntoa/1`. Verify library version and multi-node behaviour before or during implementation. - **Data:** Minimal PII; no sensitive data on the public form; consider DSGVO when extending. If abuse signals are stored: only hashed or aggregated values; document classification and retention. - **Approval-only:** No automatic User/Member creation from the join form; approval (Step 2) or other trusted path creates identity. - **Ash policies and actor:** JoinRequest has **explicit public actions** allowed with `actor: nil` (e.g. `submit` for create, `confirm` for update). Model via **policies** that permit these actions when actor is nil; do **not** use `authorize?: false` unless documented and clearly not a privilege-escalation path. diff --git a/lib/membership/join_request.ex b/lib/membership/join_request.ex index 2519089..cf220a0 100644 --- a/lib/membership/join_request.ex +++ b/lib/membership/join_request.ex @@ -37,6 +37,7 @@ defmodule Mv.Membership.JoinRequest do accept [:email, :first_name, :last_name, :form_data, :schema_version] change Mv.Membership.JoinRequest.Changes.SetConfirmationToken + change Mv.Membership.JoinRequest.Changes.FilterFormDataByAllowlist end read :get_by_confirmation_token_hash do @@ -77,6 +78,8 @@ defmodule Mv.Membership.JoinRequest do end validations do + # Format/formatting of email is not validated here; invalid addresses may fail at send time + # or can be enforced via an Ash change if needed. validate present(:email), on: [:create] end diff --git a/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex b/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex new file mode 100644 index 0000000..5de15c8 --- /dev/null +++ b/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex @@ -0,0 +1,38 @@ +defmodule Mv.Membership.JoinRequest.Changes.FilterFormDataByAllowlist do + @moduledoc """ + Filters form_data to only keys that are in the join form allowlist (server-side). + + Ensures that even when submit_join_request/2 is called directly (e.g. from tests or API), + only allowlisted custom fields are persisted. Typed fields (email, first_name, last_name) + are not part of form_data; allowlist is join_form_field_ids minus those. + """ + use Ash.Resource.Change + + alias Mv.Membership + + @typed_fields ["email", "first_name", "last_name"] + + @spec change(Ash.Changeset.t(), keyword(), Ash.Resource.Change.context()) :: Ash.Changeset.t() + def change(changeset, _opts, _context) do + form_data = Ash.Changeset.get_attribute(changeset, :form_data) || %{} + + allowlist_ids = + case Membership.get_join_form_allowlist() do + list when is_list(list) -> + list + |> Enum.map(fn item -> item.id end) + |> MapSet.new() + |> MapSet.difference(MapSet.new(@typed_fields)) + + _ -> + MapSet.new() + end + + filtered = + form_data + |> Enum.filter(fn {key, _} -> MapSet.member?(allowlist_ids, to_string(key)) end) + |> Map.new() + + Ash.Changeset.force_change_attribute(changeset, :form_data, filtered) + end +end diff --git a/lib/mv_web/endpoint.ex b/lib/mv_web/endpoint.ex index dfa01ad..a591fb9 100644 --- a/lib/mv_web/endpoint.ex +++ b/lib/mv_web/endpoint.ex @@ -12,8 +12,8 @@ defmodule MvWeb.Endpoint do ] socket "/live", Phoenix.LiveView.Socket, - websocket: [connect_info: [:peer_data, session: @session_options]], - longpoll: [connect_info: [:peer_data, session: @session_options]] + websocket: [connect_info: [:peer_data, :x_headers, session: @session_options]], + longpoll: [connect_info: [:peer_data, :x_headers, session: @session_options]] # Serve at "/" the static files from "priv/static" directory. # diff --git a/lib/mv_web/join_rate_limit.ex b/lib/mv_web/join_rate_limit.ex index 6bb1198..878e1ee 100644 --- a/lib/mv_web/join_rate_limit.ex +++ b/lib/mv_web/join_rate_limit.ex @@ -15,6 +15,7 @@ defmodule MvWeb.JoinRateLimit do - `{:deny, _retry_after_ms}` - rate limit exceeded """ def check(key) when is_binary(key) do + # Read at runtime so config can be changed without restart (e.g. in tests). config = Application.get_env(:mv, :join_rate_limit, []) scale_ms = Keyword.get(config, :scale_ms, 60_000) limit = Keyword.get(config, :limit, 10) diff --git a/lib/mv_web/live/join_live.ex b/lib/mv_web/live/join_live.ex index fc7038a..99a7df9 100644 --- a/lib/mv_web/live/join_live.ex +++ b/lib/mv_web/live/join_live.ex @@ -213,9 +213,7 @@ defmodule MvWeb.JoinLive do form_data = params |> Enum.filter(fn {key, _} -> key in allowlist_ids and key not in typed end) - |> Map.new() - |> Enum.map(fn {k, v} -> {k, String.trim(to_string(v))} end) - |> Map.new() + |> Map.new(fn {k, v} -> {k, String.trim(to_string(v))} end) attrs = %{attrs | form_data: form_data} {:ok, attrs} @@ -227,13 +225,37 @@ defmodule MvWeb.JoinLive do if is_binary(v), do: String.trim(v), else: nil end + # Prefer X-Forwarded-For / X-Real-IP when behind a reverse proxy; fall back to peer_data. + # Uses :inet.ntoa/1 for correct IPv4 and IPv6 string representation. defp client_ip_from_socket(socket) do - case get_connect_info(socket, :peer_data) do - %{address: address} when is_tuple(address) -> - address |> Tuple.to_list() |> Enum.join(".") - - _ -> - "unknown" + with nil <- client_ip_from_headers(socket), + %{address: address} when is_tuple(address) <- get_connect_info(socket, :peer_data) do + address |> :inet.ntoa() |> to_string() + else + ip when is_binary(ip) -> ip + _ -> "unknown" end end + + defp client_ip_from_headers(socket) do + headers = get_connect_info(socket, :x_headers) || [] + real_ip = header_value(headers, "x-real-ip") + forwarded = header_value(headers, "x-forwarded-for") + + cond do + real_ip != nil -> real_ip + forwarded != nil -> String.split(forwarded, ~r/,\s*/) |> List.first() |> String.trim() + true -> nil + end + end + + defp header_value(headers, name) do + name_lower = String.downcase(name) + + headers + |> Enum.find_value(fn + {h, v} when is_binary(h) -> if String.downcase(h) == name_lower, do: String.trim(v) + _ -> nil + end) + end end diff --git a/lib/mv_web/plugs/join_form_enabled.ex b/lib/mv_web/plugs/join_form_enabled.ex index f6865c4..854a7cf 100644 --- a/lib/mv_web/plugs/join_form_enabled.ex +++ b/lib/mv_web/plugs/join_form_enabled.ex @@ -24,6 +24,7 @@ defmodule MvWeb.Plugs.JoinFormEnabled do end end + # Same body as default ErrorHTML 404 (no custom error templates in this app). defp send_404(conn) do conn |> put_resp_content_type("text/html") diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 15ade4b..96b8c07 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -513,6 +513,7 @@ msgstr "Zurück zur Mitgliederliste" msgid "Back to users list" msgstr "Zurück zur Benutzer*innen-Liste" +#: lib/mv_web/components/layouts.ex #: lib/mv_web/components/layouts/sidebar.ex #, elixir-autogen, elixir-format msgid "Select language" @@ -3429,11 +3430,6 @@ msgstr "Zu viele Anfragen. Bitte versuche es später erneut." msgid "We have saved your details. To complete your request, please click the link we sent to your email." msgstr "Wir haben deine Angaben gespeichert. Um deinen Antrag abzuschließen, klicke bitte auf den Link in der E-Mail, die wir dir geschickt haben." -#: lib/mv_web/live/join_live.ex -#, elixir-autogen, elixir-format -msgid "Website" -msgstr "Webseite" - #: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "By submitting your application you will receive an email with a confirmation link. Once you have confirmed your email address, your application will be reviewed." @@ -3448,3 +3444,8 @@ msgstr "Bitte gib hier die Daten für deinen Mitgliedsantrag an." #, elixir-autogen, elixir-format msgid "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." msgstr "Deine Angaben werden nur zur Bearbeitung deines Mitgliedsantrags und zur Kontaktaufnahme genutzt. Zur Absicherung gegen Missbrauch verarbeiten wir zusätzlich technische Daten (z. B. IP-Adresse) nur im dafür nötigen Umfang." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Website" +msgstr "Webseite" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 913d5cc..65197e1 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -3430,11 +3430,6 @@ msgstr "" msgid "We have saved your details. To complete your request, please click the link we sent to your email." msgstr "" -#: lib/mv_web/live/join_live.ex -#, elixir-autogen, elixir-format -msgid "Website (leave empty)" -msgstr "" - #: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "By submitting your application you will receive an email with a confirmation link. Once you have confirmed your email address, your application will be reviewed." @@ -3449,3 +3444,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." msgstr "" + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Website" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index e91e3be..4ebce69 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -514,6 +514,7 @@ msgstr "" msgid "Back to users list" msgstr "" +#: lib/mv_web/components/layouts.ex #: lib/mv_web/components/layouts/sidebar.ex #, elixir-autogen, elixir-format, fuzzy msgid "Select language" @@ -3429,11 +3430,6 @@ msgstr "" msgid "We have saved your details. To complete your request, please click the link we sent to your email." msgstr "" -#: lib/mv_web/live/join_live.ex -#, elixir-autogen, elixir-format -msgid "Website (leave empty)" -msgstr "" - #: lib/mv_web/live/join_live.ex #, elixir-autogen, elixir-format msgid "By submitting your application you will receive an email with a confirmation link. Once you have confirmed your email address, your application will be reviewed." @@ -3448,3 +3444,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." msgstr "Your details are only used to process your membership application and to contact you. To prevent abuse we also process technical data (e.g. IP address) only as necessary." + +#: lib/mv_web/live/join_live.ex +#, elixir-autogen, elixir-format +msgid "Website" +msgstr "" diff --git a/test/membership/join_request_test.exs b/test/membership/join_request_test.exs index ea509fa..1992993 100644 --- a/test/membership/join_request_test.exs +++ b/test/membership/join_request_test.exs @@ -38,6 +38,21 @@ defmodule Mv.Membership.JoinRequestTest do end test "persists first_name, last_name and form_data when provided" do + # Allowlist must include custom fields so FilterFormDataByAllowlist persists them + {:ok, settings} = Membership.get_settings() + + Mv.Membership.update_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name", "last_name", "city", "notes"], + join_form_field_required: %{ + "email" => true, + "first_name" => false, + "last_name" => false, + "city" => false, + "notes" => false + } + }) + attrs = @valid_submit_attrs |> Map.put(:confirmation_token, "token-#{System.unique_integer([:positive])}") @@ -128,8 +143,8 @@ defmodule Mv.Membership.JoinRequestTest do Mv.Membership.update_settings(settings, %{ join_form_enabled: true, - join_form_field_ids: ["email", "first_name"], - join_form_field_required: %{"email" => true, "first_name" => false} + join_form_field_ids: ["email", "first_name", "city"], + join_form_field_required: %{"email" => true, "first_name" => false, "city" => false} }) attrs = %{ -- 2.47.2