From 3d46ba655f6ada5ba3ade196f37b984355e07280 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 14:34:24 +0100 Subject: [PATCH 01/79] Add Actor.permission_set_name/1 and admin?/1 for consistent capability checks - Actor.permission_set_name(actor) returns role's permission set (supports nil role load). - Actor.admin?(actor) returns true for system user or admin permission set. - ActorIsAdmin policy check delegates to Actor.admin?/1. --- lib/mv/authorization/actor.ex | 51 +++++++++++++++++-- lib/mv/authorization/checks/actor_is_admin.ex | 13 ++--- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/lib/mv/authorization/actor.ex b/lib/mv/authorization/actor.ex index 3482043..bfc99ed 100644 --- a/lib/mv/authorization/actor.ex +++ b/lib/mv/authorization/actor.ex @@ -1,6 +1,7 @@ defmodule Mv.Authorization.Actor do @moduledoc """ - Helper functions for ensuring User actors have required data loaded. + Helper functions for ensuring User actors have required data loaded + and for querying actor capabilities (e.g. admin, permission set). ## Actor Invariant @@ -27,8 +28,11 @@ defmodule Mv.Authorization.Actor do assign(socket, :current_user, user) end - # In tests - user = Actor.ensure_loaded(user) + # Check if actor is admin (policy checks, validations) + if Actor.admin?(actor), do: ... + + # Get permission set name (string or nil) + ps_name = Actor.permission_set_name(actor) ## Security Note @@ -47,6 +51,8 @@ defmodule Mv.Authorization.Actor do require Logger + alias Mv.Helpers.SystemActor + @doc """ Ensures the actor (User) has their `:role` relationship loaded. @@ -96,4 +102,43 @@ defmodule Mv.Authorization.Actor do actor end end + + @doc """ + Returns the actor's permission set name (string or atom) from their role, or nil. + + Ensures role is loaded (including when role is nil). Supports both atom and + string keys for session/socket assigns. Use for capability checks consistent + with `ActorIsAdmin` and `HasPermission`. + """ + @spec permission_set_name(Mv.Accounts.User.t() | map() | nil) :: String.t() | atom() | nil + def permission_set_name(nil), do: nil + + def permission_set_name(actor) do + actor = actor |> ensure_loaded() |> maybe_load_role() + + get_in(actor, [Access.key(:role), Access.key(:permission_set_name)]) || + get_in(actor, [Access.key("role"), Access.key("permission_set_name")]) + end + + @doc """ + Returns true if the actor is the system user or has the admin permission set. + + Use for validations and policy checks that require admin capability (e.g. + changing a linked member's email). Consistent with `ActorIsAdmin` policy check. + """ + @spec admin?(Mv.Accounts.User.t() | map() | nil) :: boolean() + def admin?(nil), do: false + + def admin?(actor) do + SystemActor.system_user?(actor) or permission_set_name(actor) in ["admin", :admin] + end + + defp maybe_load_role(%Mv.Accounts.User{role: nil} = user) do + case Ash.load(user, :role, domain: Mv.Accounts, authorize?: false) do + {:ok, loaded} -> loaded + _ -> user + end + end + + defp maybe_load_role(actor), do: actor end diff --git a/lib/mv/authorization/checks/actor_is_admin.ex b/lib/mv/authorization/checks/actor_is_admin.ex index 2328876..8ab038a 100644 --- a/lib/mv/authorization/checks/actor_is_admin.ex +++ b/lib/mv/authorization/checks/actor_is_admin.ex @@ -3,20 +3,15 @@ defmodule Mv.Authorization.Checks.ActorIsAdmin do Policy check: true when the actor's role has permission_set_name "admin". Used to restrict actions (e.g. User.update_user for member link/unlink) to admins only. + Delegates to `Mv.Authorization.Actor.admin?/1` for consistency. """ use Ash.Policy.SimpleCheck + alias Mv.Authorization.Actor + @impl true def describe(_opts), do: "actor has admin permission set" @impl true - def match?(nil, _context, _opts), do: false - - def match?(actor, _context, _opts) do - ps_name = - get_in(actor, [Access.key(:role), Access.key(:permission_set_name)]) || - get_in(actor, [Access.key("role"), Access.key("permission_set_name")]) - - ps_name == "admin" - end + def match?(actor, _context, _opts), do: Actor.admin?(actor) end From ad02f8914f2193cefb6f2fe9fda5abc24e6e1665 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 14:35:08 +0100 Subject: [PATCH 02/79] Use EmailSync.Loader.get_linked_user in EmailNotUsedByOtherUser Remove duplicate get_linked_user_id; reuse Loader for linked user lookup. --- .../validations/email_not_used_by_other_user.ex | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex index f9fba1b..1ee8ab0 100644 --- a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex +++ b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex @@ -8,6 +8,8 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do This allows creating members with the same email as unlinked users. """ use Ash.Resource.Validation + + alias Mv.EmailSync.Loader alias Mv.Helpers require Logger @@ -32,7 +34,8 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do def validate(changeset, _opts, _context) do email_changing? = Ash.Changeset.changing_attribute?(changeset, :email) - linked_user_id = get_linked_user_id(changeset.data) + linked_user = Loader.get_linked_user(changeset.data) + linked_user_id = if linked_user, do: linked_user.id, else: nil is_linked? = not is_nil(linked_user_id) # Only validate if member is already linked AND email is changing @@ -76,16 +79,4 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do defp maybe_exclude_id(query, nil), do: query defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) - - defp get_linked_user_id(member_data) do - alias Mv.Helpers.SystemActor - - system_actor = SystemActor.get_system_actor() - opts = Helpers.ash_actor_opts(system_actor) - - case Ash.load(member_data, :user, opts) do - {:ok, %{user: %{id: id}}} -> id - _ -> nil - end - end end From 4ea31f0f37098ece2f10d7a1cf2d89d03bc71492 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 14:35:32 +0100 Subject: [PATCH 03/79] Add email-change permission validation for linked members Only admins or the linked user may change a linked member's email. - New validation EmailChangePermission (uses Actor.admin?, Loader.get_linked_user). - Register on Member update_member; docs and gettext. --- docs/email-sync.md | 1 + lib/membership/member.ex | 4 + .../validations/email_change_permission.ex | 69 +++++ priv/gettext/de/LC_MESSAGES/default.po | 18 +- priv/gettext/default.pot | 5 + priv/gettext/en/LC_MESSAGES/default.po | 18 +- .../member_email_validation_test.exs | 237 ++++++++++++++++++ 7 files changed, 324 insertions(+), 28 deletions(-) create mode 100644 lib/mv/membership/member/validations/email_change_permission.ex create mode 100644 test/mv/membership/member_email_validation_test.exs diff --git a/docs/email-sync.md b/docs/email-sync.md index c191ff4..5675145 100644 --- a/docs/email-sync.md +++ b/docs/email-sync.md @@ -4,6 +4,7 @@ 2. **DB constraints** - Prevent duplicates within same table (users.email, members.email) 3. **Custom validations** - Prevent cross-table conflicts only for linked entities 4. **Sync is bidirectional**: User ↔ Member (but User always wins on link) +5. **Linked member email change** - When a member is linked, only administrators or the linked user may change that member's email (Member resource validation `EmailChangePermission`). This keeps email sync under control and prevents non-admins from changing another user's linked member email. --- diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 7b49c86..8213ecb 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -25,6 +25,7 @@ defmodule Mv.Membership.Member do - Postal code format: exactly 5 digits (German format) - Date validations: join_date not in future, exit_date after join_date - Email uniqueness: prevents conflicts with unlinked users + - Linked member email change: only admins or the linked user may change a linked member's email (see `Mv.Membership.Member.Validations.EmailChangePermission`) ## Full-Text Search Members have a `search_vector` attribute (tsvector) that is automatically @@ -381,6 +382,9 @@ defmodule Mv.Membership.Member do # Validates that member email is not already used by another (unlinked) user validate Mv.Membership.Member.Validations.EmailNotUsedByOtherUser + # Only admins or the linked user may change a linked member's email (prevents breaking sync) + validate Mv.Membership.Member.Validations.EmailChangePermission, on: [:update] + # Prevent linking to a user that already has a member # This validation prevents "stealing" users from other members by checking # if the target user is already linked to a different member diff --git a/lib/mv/membership/member/validations/email_change_permission.ex b/lib/mv/membership/member/validations/email_change_permission.ex new file mode 100644 index 0000000..0a53de1 --- /dev/null +++ b/lib/mv/membership/member/validations/email_change_permission.ex @@ -0,0 +1,69 @@ +defmodule Mv.Membership.Member.Validations.EmailChangePermission do + @moduledoc """ + Validates that only admins or the linked user may change a linked member's email. + + This validation runs on member update when the email attribute is changing. + It allows the change only if: + - The member is not linked to a user, or + - The actor has the admin permission set (via `Mv.Authorization.Actor.admin?/1`), or + - The actor is the user linked to this member (actor.member_id == member.id). + + This prevents non-admins from changing another user's linked member email, + which would sync to that user's account and break email synchronization. + + No system-actor fallback: missing actor is treated as not allowed. + """ + use Ash.Resource.Validation + use Gettext, backend: MvWeb.Gettext, otp_app: :mv + + alias Mv.Authorization.Actor + alias Mv.EmailSync.Loader + + @doc """ + Validates that the actor may change the member's email when the member is linked. + + Only runs when the email attribute is changing (checked inside). Skips when + member is not linked. Allows when actor is admin or owns the linked member. + """ + @impl true + def validate(changeset, _opts, context) do + if Ash.Changeset.changing_attribute?(changeset, :email) do + validate_linked_member_email_change(changeset, context) + else + :ok + end + end + + defp validate_linked_member_email_change(changeset, context) do + linked_user = Loader.get_linked_user(changeset.data) + + if is_nil(linked_user) do + :ok + else + actor = resolve_actor(changeset, context) + member_id = changeset.data.id + + if Actor.admin?(actor) or actor_owns_member?(actor, member_id) do + :ok + else + msg = + dgettext("default", "Only administrators can change email for members linked to users") + + {:error, field: :email, message: msg} + end + end + end + + # Ash stores actor in changeset.context.private.actor; validation context also has .actor + defp resolve_actor(changeset, context) do + get_in(changeset.context || %{}, [:private, :actor]) || + (context && Map.get(context, :actor)) + end + + defp actor_owns_member?(nil, _member_id), do: false + + defp actor_owns_member?(actor, member_id) do + actor_member_id = Map.get(actor, :member_id) || Map.get(actor, "member_id") + actor_member_id == member_id + end +end diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 041507b..3f71644 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2298,17 +2298,7 @@ msgstr "Verwende die Namen der Datenfelder als Spaltennamen in der CSV Datei. Da msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing." msgstr "Unbekannte Spalte '%{header}' wird ignoriert. Falls dies ein Datenfeld ist, erstellen Sie es in Mila vor dem Import." -#~ #: lib/mv_web/live/global_settings_live.ex -#~ #, elixir-autogen, elixir-format, fuzzy -#~ msgid "Custom Fields in CSV Import" -#~ msgstr "Benutzerdefinierte Felder" - -#~ #: lib/mv_web/live/global_settings_live.ex -#~ #, elixir-autogen, elixir-format, fuzzy -#~ msgid "Individual data fields must be created in Mila before importing. Use the field name as the CSV column header. Unknown custom field columns will be ignored with a warning." -#~ msgstr "Individuelle Datenfelder müssen in Mila erstellt werden, bevor sie importiert werden können. Verwenden Sie den Namen des Datenfeldes als CSV-Spaltenüberschrift. Unbekannte Spaltenüberschriften werden mit einer Warnung ignoriert." - -#~ #: lib/mv_web/live/global_settings_live.ex -#~ #, elixir-autogen, elixir-format -#~ msgid "Manage Custom Fields" -#~ msgstr "Benutzerdefinierte Felder verwalten" +#: lib/mv/membership/member/validations/email_change_permission.ex +#, elixir-autogen, elixir-format +msgid "Only administrators can change email for members linked to users" +msgstr "Nur Administrator*innen können die E-Mail von Mitgliedern ändern, die mit Benutzer*innen verknüpft sind." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 2861f2d..7418c9b 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2298,3 +2298,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing." msgstr "" + +#: lib/mv/membership/member/validations/email_change_permission.ex +#, elixir-autogen, elixir-format +msgid "Only administrators can change email for members linked to users" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 3fe9ce3..db00450 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2299,17 +2299,7 @@ msgstr "" msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing." msgstr "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing." -#~ #: lib/mv_web/live/global_settings_live.ex -#~ #, elixir-autogen, elixir-format, fuzzy -#~ msgid "Custom Fields in CSV Import" -#~ msgstr "" - -#~ #: lib/mv_web/live/global_settings_live.ex -#~ #, elixir-autogen, elixir-format, fuzzy -#~ msgid "Individual data fields must be created in Mila before importing. Use the field name as the CSV column header. Unknown custom field columns will be ignored with a warning." -#~ msgstr "" - -#~ #: lib/mv_web/live/global_settings_live.ex -#~ #, elixir-autogen, elixir-format -#~ msgid "Manage Custom Fields" -#~ msgstr "" +#: lib/mv/membership/member/validations/email_change_permission.ex +#, elixir-autogen, elixir-format +msgid "Only administrators can change email for members linked to users" +msgstr "Only administrators can change email for members linked to users" diff --git a/test/mv/membership/member_email_validation_test.exs b/test/mv/membership/member_email_validation_test.exs new file mode 100644 index 0000000..3d2ef68 --- /dev/null +++ b/test/mv/membership/member_email_validation_test.exs @@ -0,0 +1,237 @@ +defmodule Mv.Membership.MemberEmailValidationTest do + @moduledoc """ + Tests for Member email-change permission validation. + + When a member is linked to a user, only admins or the linked user may change + that member's email. Unlinked members and non-email updates are unaffected. + """ + use Mv.DataCase, async: false + + alias Mv.Accounts + alias Mv.Authorization + alias Mv.Helpers.SystemActor + alias Mv.Membership + + setup do + system_actor = SystemActor.get_system_actor() + %{actor: system_actor} + end + + defp create_role_with_permission_set(permission_set_name, actor) do + role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}" + + case Authorization.create_role( + %{ + name: role_name, + description: "Test role for #{permission_set_name}", + permission_set_name: permission_set_name + }, + actor: actor + ) do + {:ok, role} -> role + {:error, error} -> raise "Failed to create role: #{inspect(error)}" + end + end + + defp create_user_with_permission_set(permission_set_name, actor) do + role = create_role_with_permission_set(permission_set_name, actor) + + {:ok, user} = + Accounts.User + |> Ash.Changeset.for_create(:register_with_password, %{ + email: "user#{System.unique_integer([:positive])}@example.com", + password: "testpassword123" + }) + |> Ash.create(actor: actor) + + {:ok, user} = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove) + |> Ash.update(actor: actor) + + {:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts, actor: actor) + user_with_role + end + + defp create_admin_user(actor) do + create_user_with_permission_set("admin", actor) + end + + defp create_linked_member_for_user(user, actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Membership.create_member( + %{ + first_name: "Linked", + last_name: "Member", + email: "linked#{System.unique_integer([:positive])}@example.com" + }, + actor: admin + ) + + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.force_change_attribute(:member_id, member.id) + |> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false) + + member + end + + defp create_unlinked_member(actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Membership.create_member( + %{ + first_name: "Unlinked", + last_name: "Member", + email: "unlinked#{System.unique_integer([:positive])}@example.com" + }, + actor: admin + ) + + member + end + + describe "unlinked member" do + test "normal_user can update email of unlinked member", %{actor: actor} do + normal_user = create_user_with_permission_set("normal_user", actor) + unlinked_member = create_unlinked_member(actor) + + new_email = "new#{System.unique_integer([:positive])}@example.com" + + assert {:ok, updated} = + Membership.update_member(unlinked_member, %{email: new_email}, actor: normal_user) + + assert updated.email == new_email + end + + test "validation does not block when member has no linked user", %{actor: actor} do + normal_user = create_user_with_permission_set("normal_user", actor) + unlinked_member = create_unlinked_member(actor) + + new_email = "other#{System.unique_integer([:positive])}@example.com" + + assert {:ok, _} = + Membership.update_member(unlinked_member, %{email: new_email}, actor: normal_user) + end + end + + describe "linked member – another user's member" do + test "normal_user cannot update email of another user's linked member", %{actor: actor} do + user_a = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(user_a, actor) + + normal_user_b = create_user_with_permission_set("normal_user", actor) + new_email = "other#{System.unique_integer([:positive])}@example.com" + + assert {:error, %Ash.Error.Invalid{} = error} = + Membership.update_member(linked_member, %{email: new_email}, actor: normal_user_b) + + error_str = Exception.message(error) + assert error_str =~ "administrators" + assert error_str =~ "linked to users" + end + + test "admin can update email of linked member", %{actor: actor} do + user_a = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(user_a, actor) + admin = create_admin_user(actor) + + new_email = "admin_changed#{System.unique_integer([:positive])}@example.com" + + assert {:ok, updated} = + Membership.update_member(linked_member, %{email: new_email}, actor: admin) + + assert updated.email == new_email + end + end + + describe "linked member – own member" do + test "own_data user can update email of their own linked member", %{actor: actor} do + own_data_user = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(own_data_user, actor) + + {:ok, own_data_user} = + Ash.get(Accounts.User, own_data_user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + {:ok, own_data_user} = + Ash.load(own_data_user, :member, domain: Mv.Accounts, actor: actor) + + new_email = "own_updated#{System.unique_integer([:positive])}@example.com" + + assert {:ok, updated} = + Membership.update_member(linked_member, %{email: new_email}, actor: own_data_user) + + assert updated.email == new_email + end + + test "normal_user with linked member can update email of that same member", %{actor: actor} do + normal_user = create_user_with_permission_set("normal_user", actor) + linked_member = create_linked_member_for_user(normal_user, actor) + + {:ok, normal_user} = + Ash.get(Accounts.User, normal_user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + {:ok, normal_user} = Ash.load(normal_user, :member, domain: Mv.Accounts, actor: actor) + + new_email = "normal_own#{System.unique_integer([:positive])}@example.com" + + assert {:ok, updated} = + Membership.update_member(linked_member, %{email: new_email}, actor: normal_user) + + assert updated.email == new_email + end + end + + describe "no-op / other fields" do + test "updating only other attributes on linked member as normal_user does not trigger validation error", + %{actor: actor} do + user_a = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(user_a, actor) + normal_user_b = create_user_with_permission_set("normal_user", actor) + + assert {:ok, updated} = + Membership.update_member(linked_member, %{first_name: "UpdatedName"}, + actor: normal_user_b + ) + + assert updated.first_name == "UpdatedName" + assert updated.email == linked_member.email + end + + test "updating email of linked member as admin succeeds", %{actor: actor} do + user_a = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(user_a, actor) + admin = create_admin_user(actor) + + new_email = "admin_ok#{System.unique_integer([:positive])}@example.com" + + assert {:ok, updated} = + Membership.update_member(linked_member, %{email: new_email}, actor: admin) + + assert updated.email == new_email + end + end + + describe "read_only" do + test "read_only cannot update any member (policy rejects before validation)", %{actor: actor} do + read_only_user = create_user_with_permission_set("read_only", actor) + linked_member = create_linked_member_for_user(read_only_user, actor) + + {:ok, read_only_user} = + Ash.get(Accounts.User, read_only_user.id, + domain: Mv.Accounts, + load: [:role], + actor: actor + ) + + assert {:error, %Ash.Error.Forbidden{}} = + Membership.update_member(linked_member, %{email: "changed@example.com"}, + actor: read_only_user + ) + end + end +end From 4e6b7305b6a92520681793e8d8938e4a300d691a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 15:00:14 +0100 Subject: [PATCH 04/79] Doc: Loader auth-independent for link checks; email-sync rule rationale --- docs/email-sync.md | 2 +- lib/mv/email_sync/loader.ex | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/email-sync.md b/docs/email-sync.md index 5675145..2f765f0 100644 --- a/docs/email-sync.md +++ b/docs/email-sync.md @@ -4,7 +4,7 @@ 2. **DB constraints** - Prevent duplicates within same table (users.email, members.email) 3. **Custom validations** - Prevent cross-table conflicts only for linked entities 4. **Sync is bidirectional**: User ↔ Member (but User always wins on link) -5. **Linked member email change** - When a member is linked, only administrators or the linked user may change that member's email (Member resource validation `EmailChangePermission`). This keeps email sync under control and prevents non-admins from changing another user's linked member email. +5. **Linked member email change** - When a member is linked, only administrators or the linked user may change that member's email (Member resource validation `EmailChangePermission`). Because User.email wins on link and changes sync Member → User, allowing anyone to change a linked member's email would overwrite that user's account email; this rule keeps sync under control. --- diff --git a/lib/mv/email_sync/loader.ex b/lib/mv/email_sync/loader.ex index 98f85df..31e0468 100644 --- a/lib/mv/email_sync/loader.ex +++ b/lib/mv/email_sync/loader.ex @@ -3,13 +3,15 @@ defmodule Mv.EmailSync.Loader do Helper functions for loading linked records in email synchronization. Centralizes the logic for retrieving related User/Member entities. - ## Authorization + ## Authorization-independent link checks - This module runs systemically and uses the system actor for all operations. - This ensures that email synchronization always works, regardless of user permissions. - - All functions use `Mv.Helpers.SystemActor.get_system_actor/0` to bypass - user permission checks, as email sync is a mandatory side effect. + All functions use the **system actor** for the load. Link existence + (linked vs not linked) is therefore determined **independently of the + current request actor**. This is required so that validations (e.g. + `EmailChangePermission`, `EmailNotUsedByOtherUser`) can correctly decide + "member is linked" even when the current user would not have read permission + on the related User. Using the request actor would otherwise allow + treating a linked member as unlinked and bypass the permission rule. """ alias Mv.Helpers alias Mv.Helpers.SystemActor From 60a418125518d8bbac15c783e746650c958c1a71 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 15:00:20 +0100 Subject: [PATCH 05/79] Validation: error message admin or linked user; resolve_actor fallback --- .../member/validations/email_change_permission.ex | 14 ++++++++++---- priv/gettext/de/LC_MESSAGES/default.po | 6 +++--- priv/gettext/default.pot | 2 +- priv/gettext/en/LC_MESSAGES/default.po | 6 +++--- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/mv/membership/member/validations/email_change_permission.ex b/lib/mv/membership/member/validations/email_change_permission.ex index 0a53de1..2b1c041 100644 --- a/lib/mv/membership/member/validations/email_change_permission.ex +++ b/lib/mv/membership/member/validations/email_change_permission.ex @@ -11,7 +11,7 @@ defmodule Mv.Membership.Member.Validations.EmailChangePermission do This prevents non-admins from changing another user's linked member email, which would sync to that user's account and break email synchronization. - No system-actor fallback: missing actor is treated as not allowed. + Missing actor is not allowed; the system actor counts as admin (via `Actor.admin?/1`). """ use Ash.Resource.Validation use Gettext, backend: MvWeb.Gettext, otp_app: :mv @@ -47,16 +47,22 @@ defmodule Mv.Membership.Member.Validations.EmailChangePermission do :ok else msg = - dgettext("default", "Only administrators can change email for members linked to users") + dgettext( + "default", + "Only administrators or the linked user can change the email for members linked to users" + ) {:error, field: :email, message: msg} end end end - # Ash stores actor in changeset.context.private.actor; validation context also has .actor + # Ash stores actor in changeset.context.private.actor; validation context has .actor; some callsites use context.actor defp resolve_actor(changeset, context) do - get_in(changeset.context || %{}, [:private, :actor]) || + ctx = changeset.context || %{} + + get_in(ctx, [:private, :actor]) || + Map.get(ctx, :actor) || (context && Map.get(context, :actor)) end diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 3f71644..c4fd57d 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2299,6 +2299,6 @@ msgid "Unknown column '%{header}' will be ignored. If this is a custom field, cr msgstr "Unbekannte Spalte '%{header}' wird ignoriert. Falls dies ein Datenfeld ist, erstellen Sie es in Mila vor dem Import." #: lib/mv/membership/member/validations/email_change_permission.ex -#, elixir-autogen, elixir-format -msgid "Only administrators can change email for members linked to users" -msgstr "Nur Administrator*innen können die E-Mail von Mitgliedern ändern, die mit Benutzer*innen verknüpft sind." +#, elixir-autogen, elixir-format, fuzzy +msgid "Only administrators or the linked user can change the email for members linked to users" +msgstr "Nur Administrator*innen oder die verknüpfte Benutzer*in können die E-Mail von Mitgliedern ändern, die mit Benutzer*innen verknüpft sind." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 7418c9b..0908fd8 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2301,5 +2301,5 @@ msgstr "" #: lib/mv/membership/member/validations/email_change_permission.ex #, elixir-autogen, elixir-format -msgid "Only administrators can change email for members linked to users" +msgid "Only administrators or the linked user can change the email for members linked to users" msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index db00450..6faa102 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2300,6 +2300,6 @@ msgid "Unknown column '%{header}' will be ignored. If this is a custom field, cr msgstr "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing." #: lib/mv/membership/member/validations/email_change_permission.ex -#, elixir-autogen, elixir-format -msgid "Only administrators can change email for members linked to users" -msgstr "Only administrators can change email for members linked to users" +#, elixir-autogen, elixir-format, fuzzy +msgid "Only administrators or the linked user can change the email for members linked to users" +msgstr "Only administrators or the linked user can change the email for members linked to users" From 47b6a16177c50e593a71c619a8204f1fb11311a0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 15:00:24 +0100 Subject: [PATCH 06/79] Doc: Actor maybe_load_role comment; ActorIsAdmin system user = admin --- lib/mv/authorization/actor.ex | 2 ++ lib/mv/authorization/checks/actor_is_admin.ex | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/mv/authorization/actor.ex b/lib/mv/authorization/actor.ex index bfc99ed..edc6b8b 100644 --- a/lib/mv/authorization/actor.ex +++ b/lib/mv/authorization/actor.ex @@ -133,6 +133,8 @@ defmodule Mv.Authorization.Actor do SystemActor.system_user?(actor) or permission_set_name(actor) in ["admin", :admin] end + # Load role only when it is nil (e.g. actor from session without role). ensure_loaded/1 + # already handles %Ash.NotLoaded{}, so we do not double-load in the normal Ash path. defp maybe_load_role(%Mv.Accounts.User{role: nil} = user) do case Ash.load(user, :role, domain: Mv.Accounts, authorize?: false) do {:ok, loaded} -> loaded diff --git a/lib/mv/authorization/checks/actor_is_admin.ex b/lib/mv/authorization/checks/actor_is_admin.ex index 8ab038a..413c6c7 100644 --- a/lib/mv/authorization/checks/actor_is_admin.ex +++ b/lib/mv/authorization/checks/actor_is_admin.ex @@ -1,9 +1,10 @@ defmodule Mv.Authorization.Checks.ActorIsAdmin do @moduledoc """ - Policy check: true when the actor's role has permission_set_name "admin". + Policy check: true when the actor is the system user or has permission_set_name "admin". Used to restrict actions (e.g. User.update_user for member link/unlink) to admins only. - Delegates to `Mv.Authorization.Actor.admin?/1` for consistency. + Delegates to `Mv.Authorization.Actor.admin?/1`, which returns true for the system actor + or for a user whose role has permission_set_name "admin". """ use Ash.Policy.SimpleCheck From 131904f1720a2f82e9bd824bf5ea4ddd2d03e486 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 15:00:27 +0100 Subject: [PATCH 07/79] Test: assert on error field :email instead of message string --- test/mv/membership/member_email_validation_test.exs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/mv/membership/member_email_validation_test.exs b/test/mv/membership/member_email_validation_test.exs index 3d2ef68..d1b5a10 100644 --- a/test/mv/membership/member_email_validation_test.exs +++ b/test/mv/membership/member_email_validation_test.exs @@ -130,9 +130,8 @@ defmodule Mv.Membership.MemberEmailValidationTest do assert {:error, %Ash.Error.Invalid{} = error} = Membership.update_member(linked_member, %{email: new_email}, actor: normal_user_b) - error_str = Exception.message(error) - assert error_str =~ "administrators" - assert error_str =~ "linked to users" + assert Enum.any?(error.errors, &(&1.field == :email)), + "expected an error for field :email, got: #{inspect(error.errors)}" end test "admin can update email of linked member", %{actor: actor} do From 505e31653a64a8bfb17fcec090ab00ec8582afdf Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 16:35:29 +0100 Subject: [PATCH 08/79] Apply UI authorization to Member LiveViews (Index and Show) Gate New Member button, Edit and Delete links with can?/3. Edit button on Member Show visible only when user can update the member. --- lib/mv_web/live/member_live/index.html.heex | 26 +++++++++++++-------- lib/mv_web/live/member_live/show.ex | 8 ++++--- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/mv_web/live/member_live/index.html.heex b/lib/mv_web/live/member_live/index.html.heex index 394db2c..c44f3a3 100644 --- a/lib/mv_web/live/member_live/index.html.heex +++ b/lib/mv_web/live/member_live/index.html.heex @@ -23,9 +23,11 @@ <.icon name="hero-envelope" /> {gettext("Open in email program")} - <.button variant="primary" navigate={~p"/members/new"}> - <.icon name="hero-plus" /> {gettext("New Member")} - + <%= if can?(@current_user, :create, Mv.Membership.Member) do %> + <.button variant="primary" navigate={~p"/members/new"}> + <.icon name="hero-plus" /> {gettext("New Member")} + + <% end %> @@ -297,16 +299,20 @@ <.link navigate={~p"/members/#{member}"}>{gettext("Show")} - <.link navigate={~p"/members/#{member}/edit"}>{gettext("Edit")} + <%= if can?(@current_user, :update, member) do %> + <.link navigate={~p"/members/#{member}/edit"}>{gettext("Edit")} + <% end %> <:action :let={member}> - <.link - phx-click={JS.push("delete", value: %{id: member.id}) |> hide("#row-#{member.id}")} - data-confirm={gettext("Are you sure?")} - > - {gettext("Delete")} - + <%= if can?(@current_user, :destroy, member) do %> + <.link + phx-click={JS.push("delete", value: %{id: member.id}) |> hide("#row-#{member.id}")} + data-confirm={gettext("Are you sure?")} + > + {gettext("Delete")} + + <% end %> diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index d484672..9ac1fc8 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -39,9 +39,11 @@ defmodule MvWeb.MemberLive.Show do {MvWeb.Helpers.MemberHelpers.display_name(@member)} - <.button variant="primary" navigate={~p"/members/#{@member}/edit?return_to=show"}> - {gettext("Edit Member")} - + <%= if can?(@current_user, :update, @member) do %> + <.button variant="primary" navigate={~p"/members/#{@member}/edit?return_to=show"}> + {gettext("Edit Member")} + + <% end %> <%!-- Tab Navigation --%> From 5e361ba4006f2d9cf776eb9ab7992a68b211fdc6 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 16:35:30 +0100 Subject: [PATCH 09/79] Add Member LiveView authorization tests Covers read_only, normal_user, admin, own_data for Index and Show. Asserts New Member / Edit / Delete visibility and redirect for Mitglied. --- .../live/member_live_authorization_test.exs | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 test/mv_web/live/member_live_authorization_test.exs diff --git a/test/mv_web/live/member_live_authorization_test.exs b/test/mv_web/live/member_live_authorization_test.exs new file mode 100644 index 0000000..c8d02b8 --- /dev/null +++ b/test/mv_web/live/member_live_authorization_test.exs @@ -0,0 +1,106 @@ +defmodule MvWeb.MemberLiveAuthorizationTest do + @moduledoc """ + Tests for UI authorization on Member LiveViews (Index and Show). + """ + use MvWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + + alias Mv.Fixtures + + # Use literal strings for button/link text (matches default Gettext locale) + @new_member_text "New Member" + @edit_member_text "Edit Member" + + describe "Member Index - Vorstand (read_only)" do + @tag role: :read_only + test "sees member list but not New Member button", %{conn: conn} do + _member = Fixtures.member_fixture() + + {:ok, _view, html} = live(conn, "/members") + + refute html =~ @new_member_text + end + + @tag role: :read_only + test "does not see Edit or Delete buttons in table", %{conn: conn} do + member = Fixtures.member_fixture() + + {:ok, view, _html} = live(conn, "/members") + + refute has_element?(view, "a[href=\"/members/#{member.id}/edit\"]") + refute has_element?(view, "a[phx-click*='delete']") + end + end + + describe "Member Index - Kassenwart (normal_user)" do + @tag role: :normal_user + test "sees New Member and Edit buttons", %{conn: conn} do + member = Fixtures.member_fixture() + + {:ok, view, html} = live(conn, "/members") + + assert html =~ @new_member_text + assert has_element?(view, "a[href=\"/members/#{member.id}/edit\"]") + end + + @tag role: :normal_user + test "does not see Delete button", %{conn: conn} do + _member = Fixtures.member_fixture() + + {:ok, view, _html} = live(conn, "/members") + + refute has_element?(view, "a[phx-click*='delete']") + end + end + + describe "Member Index - Admin" do + @tag role: :admin + test "sees New Member, Edit and Delete buttons", %{conn: conn} do + member = Fixtures.member_fixture() + + {:ok, view, html} = live(conn, "/members") + + assert html =~ @new_member_text + assert has_element?(view, "a[href=\"/members/#{member.id}/edit\"]") + assert has_element?(view, "a[phx-click*='delete']") + end + end + + describe "Member Index - Mitglied (own_data)" do + @tag role: :member + test "is redirected when accessing /members", %{conn: conn, current_user: user} do + assert {:error, {:redirect, %{to: to}}} = live(conn, "/members") + assert to == "/users/#{user.id}" + end + end + + describe "Member Show - Edit button visibility" do + @tag role: :admin + test "admin sees Edit button", %{conn: conn} do + member = Fixtures.member_fixture() + + {:ok, _view, html} = live(conn, "/members/#{member.id}") + + assert html =~ @edit_member_text + end + + @tag role: :read_only + test "read_only does not see Edit button", %{conn: conn} do + member = Fixtures.member_fixture() + + {:ok, _view, html} = live(conn, "/members/#{member.id}") + + refute html =~ @edit_member_text + end + + @tag role: :normal_user + test "normal_user sees Edit button", %{conn: conn} do + member = Fixtures.member_fixture() + + {:ok, _view, html} = live(conn, "/members/#{member.id}") + + assert html =~ @edit_member_text + end + end +end From 2f67c7099d0b6dfb5a9d5443ddf76644683a7b4d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 16:35:32 +0100 Subject: [PATCH 10/79] Apply UI authorization to User LiveViews (Index and Show) Gate New User button, Edit and Delete links with can?/3. Edit button on User Show visible only when user can update the user. --- lib/mv_web/live/user_live/index.html.heex | 26 ++++++++++++++--------- lib/mv_web/live/user_live/show.ex | 8 ++++--- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/mv_web/live/user_live/index.html.heex b/lib/mv_web/live/user_live/index.html.heex index 9314f1e..dcb2e83 100644 --- a/lib/mv_web/live/user_live/index.html.heex +++ b/lib/mv_web/live/user_live/index.html.heex @@ -2,9 +2,11 @@ <.header> {gettext("Listing Users")} <:actions> - <.button variant="primary" navigate={~p"/users/new"}> - <.icon name="hero-plus" /> {gettext("New User")} - + <%= if can?(@current_user, :create, Mv.Accounts.User) do %> + <.button variant="primary" navigate={~p"/users/new"}> + <.icon name="hero-plus" /> {gettext("New User")} + + <% end %> @@ -62,16 +64,20 @@ <.link navigate={~p"/users/#{user}"}>{gettext("Show")} - <.link navigate={~p"/users/#{user}/edit"}>{gettext("Edit")} + <%= if can?(@current_user, :update, user) do %> + <.link navigate={~p"/users/#{user}/edit"}>{gettext("Edit")} + <% end %> <:action :let={user}> - <.link - phx-click={JS.push("delete", value: %{id: user.id}) |> hide("#row-#{user.id}")} - data-confirm={gettext("Are you sure?")} - > - {gettext("Delete")} - + <%= if can?(@current_user, :destroy, user) do %> + <.link + phx-click={JS.push("delete", value: %{id: user.id}) |> hide("#row-#{user.id}")} + data-confirm={gettext("Are you sure?")} + > + {gettext("Delete")} + + <% end %> diff --git a/lib/mv_web/live/user_live/show.ex b/lib/mv_web/live/user_live/show.ex index e961d84..fa4f186 100644 --- a/lib/mv_web/live/user_live/show.ex +++ b/lib/mv_web/live/user_live/show.ex @@ -41,9 +41,11 @@ defmodule MvWeb.UserLive.Show do <.icon name="hero-arrow-left" /> {gettext("Back to users list")} - <.button variant="primary" navigate={~p"/users/#{@user}/edit?return_to=show"}> - <.icon name="hero-pencil-square" /> {gettext("Edit User")} - + <%= if can?(@current_user, :update, @user) do %> + <.button variant="primary" navigate={~p"/users/#{@user}/edit?return_to=show"}> + <.icon name="hero-pencil-square" /> {gettext("Edit User")} + + <% end %> From cc9e530d8049505b26724704db5350e44ea1cb01 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 16:35:33 +0100 Subject: [PATCH 11/79] Add User LiveView authorization tests Covers admin, read_only, member, normal_user for Index and Show. Asserts New User / Edit / Delete visibility and redirect for non-admin. --- .../live/user_live_authorization_test.exs | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 test/mv_web/live/user_live_authorization_test.exs diff --git a/test/mv_web/live/user_live_authorization_test.exs b/test/mv_web/live/user_live_authorization_test.exs new file mode 100644 index 0000000..9c35d87 --- /dev/null +++ b/test/mv_web/live/user_live_authorization_test.exs @@ -0,0 +1,84 @@ +defmodule MvWeb.UserLiveAuthorizationTest do + @moduledoc """ + Tests for UI authorization on User LiveViews (Index and Show). + """ + use MvWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + + alias Mv.Fixtures + + @new_user_text "New User" + @edit_user_text "Edit User" + + describe "User Index - Admin" do + @tag role: :admin + test "sees New User, Edit and Delete buttons", %{conn: conn} do + user = Fixtures.user_with_role_fixture("admin") + + {:ok, view, html} = live(conn, "/users") + + assert html =~ @new_user_text + assert has_element?(view, "a[href=\"/users/#{user.id}/edit\"]") + assert has_element?(view, "a[phx-click*='delete']") + end + end + + describe "User Index - Non-Admin is redirected" do + @tag role: :read_only + test "read_only is redirected when accessing /users", %{conn: conn, current_user: user} do + assert {:error, {:redirect, %{to: to}}} = live(conn, "/users") + assert to == "/users/#{user.id}" + end + + @tag role: :member + test "member is redirected when accessing /users", %{conn: conn, current_user: user} do + assert {:error, {:redirect, %{to: to}}} = live(conn, "/users") + assert to == "/users/#{user.id}" + end + + @tag role: :normal_user + test "normal_user is redirected when accessing /users", %{conn: conn, current_user: user} do + assert {:error, {:redirect, %{to: to}}} = live(conn, "/users") + assert to == "/users/#{user.id}" + end + end + + describe "User Show - own profile" do + @tag role: :member + test "member sees Edit button on own profile", %{conn: conn, current_user: user} do + {:ok, _view, html} = live(conn, "/users/#{user.id}") + + assert html =~ @edit_user_text + end + + @tag role: :read_only + test "read_only sees Edit button on own profile", %{conn: conn, current_user: user} do + {:ok, _view, html} = live(conn, "/users/#{user.id}") + + assert html =~ @edit_user_text + end + + @tag role: :admin + test "admin sees Edit button on user show", %{conn: conn} do + user = Fixtures.user_with_role_fixture("read_only") + + {:ok, _view, html} = live(conn, "/users/#{user.id}") + + assert html =~ @edit_user_text + end + end + + describe "User Show - other user (non-admin redirected)" do + @tag role: :member + test "member is redirected when accessing other user's profile", %{ + conn: conn, + current_user: current_user + } do + other_user = Fixtures.user_with_role_fixture("admin") + + assert {:error, {:redirect, %{to: to}}} = live(conn, "/users/#{other_user.id}") + assert to == "/users/#{current_user.id}" + end + end +end From f779fd61e054f2862453e87049e373377ead1979 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 16:35:35 +0100 Subject: [PATCH 12/79] Gate sidebar menu items by can_access_page? Members, Fee Types and Administration subitems only shown when user has page permission. Add admin_menu_visible? helper. Sidebar test uses admin user so menu items render. --- lib/mv_web/components/layouts/sidebar.ex | 67 +++++++++++++------ .../components/layouts/sidebar_test.exs | 7 +- 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/lib/mv_web/components/layouts/sidebar.ex b/lib/mv_web/components/layouts/sidebar.ex index 1d564c1..19f5547 100644 --- a/lib/mv_web/components/layouts/sidebar.ex +++ b/lib/mv_web/components/layouts/sidebar.ex @@ -70,33 +70,56 @@ defmodule MvWeb.Layouts.Sidebar do defp sidebar_menu(assigns) do ~H""" """ end + defp admin_menu_visible?(user) do + Enum.any?(admin_page_paths(), &can_access_page?(user, &1)) + end + + defp admin_page_paths do + ["/users", "/groups", "/admin/roles", "/membership_fee_settings", "/settings"] + end + attr :href, :string, required: true, doc: "Navigation path" attr :icon, :string, required: true, doc: "Heroicon name" attr :label, :string, required: true, doc: "Menu item label" diff --git a/test/mv_web/components/layouts/sidebar_test.exs b/test/mv_web/components/layouts/sidebar_test.exs index 75727e3..0975b8f 100644 --- a/test/mv_web/components/layouts/sidebar_test.exs +++ b/test/mv_web/components/layouts/sidebar_test.exs @@ -22,9 +22,14 @@ defmodule MvWeb.Layouts.SidebarTest do # ============================================================================= # Returns assigns for an authenticated user with all required attributes. + # User has admin role so can_access_page? returns true for all sidebar links. defp authenticated_assigns(mobile \\ false) do %{ - current_user: %{id: "user-123", email: "test@example.com"}, + current_user: %{ + id: "user-123", + email: "test@example.com", + role: %{permission_set_name: "admin"} + }, club_name: "Test Club", mobile: mobile } From 1426ef1d38575b385454e210fc182c8d58f4b05f Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 16:35:36 +0100 Subject: [PATCH 13/79] Add sidebar authorization tests Assert menu visibility per role: admin, read_only, normal_user, own_data, nil user, user without role. --- .../components/sidebar_authorization_test.exs | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 test/mv_web/components/sidebar_authorization_test.exs diff --git a/test/mv_web/components/sidebar_authorization_test.exs b/test/mv_web/components/sidebar_authorization_test.exs new file mode 100644 index 0000000..234f7cb --- /dev/null +++ b/test/mv_web/components/sidebar_authorization_test.exs @@ -0,0 +1,120 @@ +defmodule MvWeb.SidebarAuthorizationTest do + @moduledoc """ + Tests for sidebar menu visibility based on user permissions (can_access_page?). + """ + use MvWeb.ConnCase, async: false + + import Phoenix.LiveViewTest + import MvWeb.Layouts.Sidebar + + alias Mv.Fixtures + + defp render_sidebar(assigns) do + render_component(&sidebar/1, assigns) + end + + defp sidebar_assigns(current_user, opts \\ []) do + mobile = Keyword.get(opts, :mobile, false) + club_name = Keyword.get(opts, :club_name, "Test Club") + + %{ + current_user: current_user, + club_name: club_name, + mobile: mobile + } + end + + describe "sidebar menu with admin user" do + test "shows Members, Fee Types and Administration with all subitems" do + user = Fixtures.user_with_role_fixture("admin") + html = render_sidebar(sidebar_assigns(user)) + + assert html =~ ~s(href="/members") + assert html =~ ~s(href="/membership_fee_types") + assert html =~ ~s(aria-label="Administration") + assert html =~ ~s(href="/users") + assert html =~ ~s(href="/groups") + assert html =~ ~s(href="/admin/roles") + assert html =~ ~s(href="/membership_fee_settings") + assert html =~ ~s(href="/settings") + end + end + + describe "sidebar menu with read_only user (Vorstand/Buchhaltung)" do + test "shows Members and Groups (from Administration)" do + user = Fixtures.user_with_role_fixture("read_only") + html = render_sidebar(sidebar_assigns(user)) + + assert html =~ ~s(href="/members") + assert html =~ ~s(href="/groups") + end + + test "does not show Fee Types, Users, Roles or Settings" do + user = Fixtures.user_with_role_fixture("read_only") + html = render_sidebar(sidebar_assigns(user)) + + refute html =~ ~s(href="/membership_fee_types") + refute html =~ ~s(href="/users") + refute html =~ ~s(href="/admin/roles") + refute html =~ ~s(href="/settings") + end + end + + describe "sidebar menu with normal_user (Kassenwart)" do + test "shows Members and Groups" do + user = Fixtures.user_with_role_fixture("normal_user") + html = render_sidebar(sidebar_assigns(user)) + + assert html =~ ~s(href="/members") + assert html =~ ~s(href="/groups") + end + + test "does not show Fee Types, Users, Roles or Settings" do + user = Fixtures.user_with_role_fixture("normal_user") + html = render_sidebar(sidebar_assigns(user)) + + refute html =~ ~s(href="/membership_fee_types") + refute html =~ ~s(href="/users") + refute html =~ ~s(href="/admin/roles") + refute html =~ ~s(href="/settings") + end + end + + describe "sidebar menu with own_data user (Mitglied)" do + test "does not show Members link (no /members page access)" do + user = Fixtures.user_with_role_fixture("own_data") + html = render_sidebar(sidebar_assigns(user)) + + refute html =~ ~s(href="/members") + end + + test "does not show Fee Types or Administration" do + user = Fixtures.user_with_role_fixture("own_data") + html = render_sidebar(sidebar_assigns(user)) + + refute html =~ ~s(href="/membership_fee_types") + refute html =~ ~s(href="/users") + refute html =~ ~s(aria-label="Administration") + end + end + + describe "sidebar with nil current_user" do + test "does not render menu items (only header and footer when present)" do + html = render_sidebar(sidebar_assigns(nil)) + + refute html =~ ~s(role="menubar") + refute html =~ ~s(href="/members") + end + end + + describe "sidebar with user without role" do + test "does not show any navigation links" do + user = %{id: "user-no-role", email: "noreply@test.com", role: nil} + html = render_sidebar(sidebar_assigns(user)) + + refute html =~ ~s(href="/members") + refute html =~ ~s(href="/membership_fee_types") + refute html =~ ~s(href="/users") + end + end +end From 9e8910344e0389fb2fa090ba792d77d0853e1fe9 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 17:16:07 +0100 Subject: [PATCH 14/79] Add MvWeb.PagePaths for central sidebar/page paths Single source for path strings used by Sidebar and can_access_page?. Keep in sync with router when routes change. --- lib/mv_web/page_paths.ex | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 lib/mv_web/page_paths.ex diff --git a/lib/mv_web/page_paths.ex b/lib/mv_web/page_paths.ex new file mode 100644 index 0000000..5606c76 --- /dev/null +++ b/lib/mv_web/page_paths.ex @@ -0,0 +1,42 @@ +defmodule MvWeb.PagePaths do + @moduledoc """ + Central path strings for UI authorization and sidebar menu. + + Keep in sync with `MvWeb.Router`. Used by Sidebar and `can_access_page?/2` + so route changes (prefix, rename) are updated in one place. + """ + + # Sidebar top-level menu paths + @members "/members" + @membership_fee_types "/membership_fee_types" + + # Administration submenu paths (all must match router) + @users "/users" + @groups "/groups" + @admin_roles "/admin/roles" + @membership_fee_settings "/membership_fee_settings" + @settings "/settings" + + @admin_page_paths [ + @users, + @groups, + @admin_roles, + @membership_fee_settings, + @settings + ] + + @doc "Path for Members index (sidebar and page permission check)." + def members, do: @members + + @doc "Path for Membership Fee Types index (sidebar and page permission check)." + def membership_fee_types, do: @membership_fee_types + + @doc "Paths for Administration menu; show group if user can access any of these." + def admin_menu_paths, do: @admin_page_paths + + def users, do: @users + def groups, do: @groups + def admin_roles, do: @admin_roles + def membership_fee_settings, do: @membership_fee_settings + def settings, do: @settings +end From 2ddd22078dbcc93aabe3e7212d1958b8c0dbb841 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Feb 2026 17:16:08 +0100 Subject: [PATCH 15/79] Sidebar: use PagePaths, add testid for Administration Gate menu items via PagePaths; add data-testid=sidebar-administration for stable tests. menu_group accepts optional testid attr. --- lib/mv_web/components/layouts/sidebar.ex | 33 +++++++++++++----------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/mv_web/components/layouts/sidebar.ex b/lib/mv_web/components/layouts/sidebar.ex index 19f5547..26c0d7a 100644 --- a/lib/mv_web/components/layouts/sidebar.ex +++ b/lib/mv_web/components/layouts/sidebar.ex @@ -4,6 +4,8 @@ defmodule MvWeb.Layouts.Sidebar do """ use MvWeb, :html + alias MvWeb.PagePaths + attr :current_user, :map, default: nil, doc: "The current user" attr :club_name, :string, required: true, doc: "The name of the club" attr :mobile, :boolean, default: false, doc: "Whether this is mobile view" @@ -70,7 +72,7 @@ defmodule MvWeb.Layouts.Sidebar do defp sidebar_menu(assigns) do ~H"""