diff --git a/docker-compose.yml b/docker-compose.yml index 626f353..4c169b5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -25,7 +25,7 @@ services: rauthy: container_name: rauthy-dev - image: ghcr.io/sebadob/rauthy:0.34.2 + image: ghcr.io/sebadob/rauthy:0.33.4 environment: - LOCAL_TEST=true - SMTP_URL=mailcrab diff --git a/docs/email-sync.md b/docs/email-sync.md index 2f765f0..c191ff4 100644 --- a/docs/email-sync.md +++ b/docs/email-sync.md @@ -4,7 +4,6 @@ 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`). 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/membership/member.ex b/lib/membership/member.ex index 8213ecb..7b49c86 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -25,7 +25,6 @@ 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 @@ -382,9 +381,6 @@ 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/authorization/actor.ex b/lib/mv/authorization/actor.ex index edc6b8b..3482043 100644 --- a/lib/mv/authorization/actor.ex +++ b/lib/mv/authorization/actor.ex @@ -1,7 +1,6 @@ defmodule Mv.Authorization.Actor do @moduledoc """ - Helper functions for ensuring User actors have required data loaded - and for querying actor capabilities (e.g. admin, permission set). + Helper functions for ensuring User actors have required data loaded. ## Actor Invariant @@ -28,11 +27,8 @@ defmodule Mv.Authorization.Actor do assign(socket, :current_user, user) end - # 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) + # In tests + user = Actor.ensure_loaded(user) ## Security Note @@ -51,8 +47,6 @@ defmodule Mv.Authorization.Actor do require Logger - alias Mv.Helpers.SystemActor - @doc """ Ensures the actor (User) has their `:role` relationship loaded. @@ -102,45 +96,4 @@ 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 - - # 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 - _ -> 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 413c6c7..2328876 100644 --- a/lib/mv/authorization/checks/actor_is_admin.ex +++ b/lib/mv/authorization/checks/actor_is_admin.ex @@ -1,18 +1,22 @@ defmodule Mv.Authorization.Checks.ActorIsAdmin do @moduledoc """ - Policy check: true when the actor is the system user or has permission_set_name "admin". + 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`, which returns true for the system actor - or for a user whose role has permission_set_name "admin". """ use Ash.Policy.SimpleCheck - alias Mv.Authorization.Actor - @impl true def describe(_opts), do: "actor has admin permission set" @impl true - def match?(actor, _context, _opts), do: Actor.admin?(actor) + 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 end diff --git a/lib/mv/email_sync/loader.ex b/lib/mv/email_sync/loader.ex index 31e0468..98f85df 100644 --- a/lib/mv/email_sync/loader.ex +++ b/lib/mv/email_sync/loader.ex @@ -3,15 +3,13 @@ defmodule Mv.EmailSync.Loader do Helper functions for loading linked records in email synchronization. Centralizes the logic for retrieving related User/Member entities. - ## Authorization-independent link checks + ## Authorization - 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. + 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. """ alias Mv.Helpers alias Mv.Helpers.SystemActor diff --git a/lib/mv/membership/member/validations/email_change_permission.ex b/lib/mv/membership/member/validations/email_change_permission.ex deleted file mode 100644 index 2b1c041..0000000 --- a/lib/mv/membership/member/validations/email_change_permission.ex +++ /dev/null @@ -1,75 +0,0 @@ -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. - - 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 - - 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 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 has .actor; some callsites use context.actor - defp resolve_actor(changeset, context) do - ctx = changeset.context || %{} - - get_in(ctx, [:private, :actor]) || - Map.get(ctx, :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/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 1ee8ab0..f9fba1b 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,8 +8,6 @@ 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 @@ -34,8 +32,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do def validate(changeset, _opts, _context) do email_changing? = Ash.Changeset.changing_attribute?(changeset, :email) - linked_user = Loader.get_linked_user(changeset.data) - linked_user_id = if linked_user, do: linked_user.id, else: nil + linked_user_id = get_linked_user_id(changeset.data) is_linked? = not is_nil(linked_user_id) # Only validate if member is already linked AND email is changing @@ -79,4 +76,16 @@ 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 diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex index d821416..d20be7d 100644 --- a/lib/mv_web/authorization.ex +++ b/lib/mv_web/authorization.ex @@ -97,18 +97,12 @@ defmodule MvWeb.Authorization do @doc """ Checks if user can access a specific page. - Nil-safe: returns false when user is nil (e.g. unauthenticated or layout - assigns regression), so callers do not need to guard. - ## Examples iex> admin = %{role: %{permission_set_name: "admin"}} iex> can_access_page?(admin, "/admin/roles") true - iex> can_access_page?(nil, "/members") - false - iex> mitglied = %{role: %{permission_set_name: "own_data"}} iex> can_access_page?(mitglied, "/members") false diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 59e300e..45bcae0 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -97,13 +97,12 @@ defmodule MvWeb.CoreComponents do <.button navigate={~p"/"}>Home <.button disabled={true}>Disabled """ - attr :rest, :global, include: ~w(href navigate patch method data-testid) + attr :rest, :global, include: ~w(href navigate patch method) attr :variant, :string, values: ~w(primary) attr :disabled, :boolean, default: false, doc: "Whether the button is disabled" slot :inner_block, required: true - def button(assigns) do - rest = assigns.rest + def button(%{rest: rest} = assigns) do variants = %{"primary" => "btn-primary", nil => "btn-primary btn-soft"} assigns = assign(assigns, :class, Map.fetch!(variants, assigns[:variant])) diff --git a/lib/mv_web/components/layouts/sidebar.ex b/lib/mv_web/components/layouts/sidebar.ex index 26c0d7a..1d564c1 100644 --- a/lib/mv_web/components/layouts/sidebar.ex +++ b/lib/mv_web/components/layouts/sidebar.ex @@ -4,8 +4,6 @@ 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" @@ -72,56 +70,33 @@ defmodule MvWeb.Layouts.Sidebar do defp sidebar_menu(assigns) do ~H""" """ end - defp admin_menu_visible?(user) do - Enum.any?(PagePaths.admin_menu_paths(), &can_access_page?(user, &1)) - 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" @@ -144,13 +119,12 @@ defmodule MvWeb.Layouts.Sidebar do attr :icon, :string, required: true, doc: "Heroicon name for the menu group" attr :label, :string, required: true, doc: "Menu group label" - attr :testid, :string, default: nil, doc: "data-testid for stable test selectors" slot :inner_block, required: true, doc: "Submenu items" defp menu_group(assigns) do ~H""" -
  • +
    • -
    +
    - <%= if can?(@current_user, :update, member) do %> - <.link navigate={~p"/members/#{member}/edit"} data-testid="member-edit"> - {gettext("Edit")} - - <% end %> + <.link navigate={~p"/members/#{member}/edit"}>{gettext("Edit")} <:action :let={member}> - <%= 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?")} - data-testid="member-delete" - > - {gettext("Delete")} - - <% end %> + <.link + phx-click={JS.push("delete", value: %{id: member.id}) |> hide("#row-#{member.id}")} + data-confirm={gettext("Are you sure?")} + > + {gettext("Delete")} + diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index 604f166..d484672 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -39,15 +39,9 @@ defmodule MvWeb.MemberLive.Show do {MvWeb.Helpers.MemberHelpers.display_name(@member)} - <%= if can?(@current_user, :update, @member) do %> - <.button - variant="primary" - navigate={~p"/members/#{@member}/edit?return_to=show"} - data-testid="member-edit" - > - {gettext("Edit Member")} - - <% end %> + <.button variant="primary" navigate={~p"/members/#{@member}/edit?return_to=show"}> + {gettext("Edit Member")} +
    <%!-- Tab Navigation --%> diff --git a/lib/mv_web/live/user_live/index.html.heex b/lib/mv_web/live/user_live/index.html.heex index cb945e2..9314f1e 100644 --- a/lib/mv_web/live/user_live/index.html.heex +++ b/lib/mv_web/live/user_live/index.html.heex @@ -2,20 +2,13 @@ <.header> {gettext("Listing Users")} <:actions> - <%= if can?(@current_user, :create, Mv.Accounts.User) do %> - <.button variant="primary" navigate={~p"/users/new"} data-testid="user-new"> - <.icon name="hero-plus" /> {gettext("New User")} - - <% end %> + <.button variant="primary" navigate={~p"/users/new"}> + <.icon name="hero-plus" /> {gettext("New User")} + - <.table - id="users" - rows={@users} - row_id={fn user -> "row-#{user.id}" end} - row_click={fn user -> JS.navigate(~p"/users/#{user}") end} - > + <.table id="users" rows={@users} row_click={fn user -> JS.navigate(~p"/users/#{user}") end}> <:col :let={user} label={ @@ -69,23 +62,16 @@ <.link navigate={~p"/users/#{user}"}>{gettext("Show")} - <%= if can?(@current_user, :update, user) do %> - <.link navigate={~p"/users/#{user}/edit"} data-testid="user-edit"> - {gettext("Edit")} - - <% end %> + <.link navigate={~p"/users/#{user}/edit"}>{gettext("Edit")} <:action :let={user}> - <%= 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?")} - data-testid="user-delete" - > - {gettext("Delete")} - - <% end %> + <.link + phx-click={JS.push("delete", value: %{id: user.id}) |> hide("#row-#{user.id}")} + data-confirm={gettext("Are you sure?")} + > + {gettext("Delete")} + diff --git a/lib/mv_web/live/user_live/show.ex b/lib/mv_web/live/user_live/show.ex index 5114b74..e961d84 100644 --- a/lib/mv_web/live/user_live/show.ex +++ b/lib/mv_web/live/user_live/show.ex @@ -41,15 +41,9 @@ defmodule MvWeb.UserLive.Show do <.icon name="hero-arrow-left" /> {gettext("Back to users list")} - <%= if can?(@current_user, :update, @user) do %> - <.button - variant="primary" - navigate={~p"/users/#{@user}/edit?return_to=show"} - data-testid="user-edit" - > - <.icon name="hero-pencil-square" /> {gettext("Edit User")} - - <% end %> + <.button variant="primary" navigate={~p"/users/#{@user}/edit?return_to=show"}> + <.icon name="hero-pencil-square" /> {gettext("Edit User")} + diff --git a/lib/mv_web/page_paths.ex b/lib/mv_web/page_paths.ex deleted file mode 100644 index 5606c76..0000000 --- a/lib/mv_web/page_paths.ex +++ /dev/null @@ -1,42 +0,0 @@ -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 diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index c4fd57d..041507b 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -2298,7 +2298,17 @@ 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/membership/member/validations/email_change_permission.ex -#, 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." +#~ #: 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" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 0908fd8..2861f2d 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -2298,8 +2298,3 @@ 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 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 6faa102..3fe9ce3 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -2299,7 +2299,17 @@ 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/membership/member/validations/email_change_permission.ex -#, 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" +#~ #: 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 "" diff --git a/test/mv/membership/member_email_validation_test.exs b/test/mv/membership/member_email_validation_test.exs deleted file mode 100644 index d1b5a10..0000000 --- a/test/mv/membership/member_email_validation_test.exs +++ /dev/null @@ -1,236 +0,0 @@ -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) - - 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 - 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 diff --git a/test/mv_web/components/layouts/sidebar_test.exs b/test/mv_web/components/layouts/sidebar_test.exs index ff81f24..75727e3 100644 --- a/test/mv_web/components/layouts/sidebar_test.exs +++ b/test/mv_web/components/layouts/sidebar_test.exs @@ -22,14 +22,9 @@ 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", - role: %{permission_set_name: "admin"} - }, + current_user: %{id: "user-123", email: "test@example.com"}, club_name: "Test Club", mobile: mobile } @@ -149,9 +144,7 @@ defmodule MvWeb.Layouts.SidebarTest do assert menu_item_count > 0, "Should have at least one top-level menu item" # Check that nested menu groups exist - assert html =~ - ~s(
  • ) - + assert html =~ ~s(
  • ) assert html =~ ~s(role="group") assert has_class?(html, "expanded-menu-group") @@ -200,9 +193,7 @@ defmodule MvWeb.Layouts.SidebarTest do html = render_sidebar(authenticated_assigns()) # Check for nested menu structure - assert html =~ - ~s(
  • ) - + assert html =~ ~s(
  • ) assert html =~ ~s(role="group") assert html =~ ~s(aria-label="Administration") assert has_class?(html, "expanded-menu-group") @@ -530,9 +521,7 @@ defmodule MvWeb.Layouts.SidebarTest do assert html =~ ~s(role="menuitem") # Check that nested menus exist - assert html =~ - ~s(
  • ) - + assert html =~ ~s(
  • ) assert html =~ ~s(role="group") # Footer section @@ -640,9 +629,7 @@ defmodule MvWeb.Layouts.SidebarTest do html = render_sidebar(authenticated_assigns()) # expanded-menu-group structure present - assert html =~ - ~s(
  • ) - + assert html =~ ~s(
  • ) assert html =~ ~s(role="group") assert html =~ ~s(aria-label="Administration") assert has_class?(html, "expanded-menu-group") @@ -856,9 +843,7 @@ defmodule MvWeb.Layouts.SidebarTest do # Expanded menu group should have correct structure # (CSS handles hover effects, but we verify structure) - assert html =~ - ~s(
  • ) - + assert html =~ ~s(
  • ) assert html =~ ~s(role="group") end diff --git a/test/mv_web/components/sidebar_authorization_test.exs b/test/mv_web/components/sidebar_authorization_test.exs deleted file mode 100644 index 079572f..0000000 --- a/test/mv_web/components/sidebar_authorization_test.exs +++ /dev/null @@ -1,120 +0,0 @@ -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(data-testid="sidebar-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(data-testid="sidebar-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 diff --git a/test/mv_web/live/member_live_authorization_test.exs b/test/mv_web/live/member_live_authorization_test.exs deleted file mode 100644 index 9a23019..0000000 --- a/test/mv_web/live/member_live_authorization_test.exs +++ /dev/null @@ -1,102 +0,0 @@ -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 - - 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 has_element?(view, "[data-testid=member-new]") - 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, "#row-#{member.id} [data-testid=member-edit]") - refute has_element?(view, "#row-#{member.id} [data-testid=member-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 has_element?(view, "[data-testid=member-new]") - assert has_element?(view, "#row-#{member.id} [data-testid=member-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, "#row-#{member.id} [data-testid=member-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 has_element?(view, "[data-testid=member-new]") - assert has_element?(view, "#row-#{member.id} [data-testid=member-edit]") - assert has_element?(view, "#row-#{member.id} [data-testid=member-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 has_element?(view, "[data-testid=member-edit]") - 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 has_element?(view, "[data-testid=member-edit]") - 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 has_element?(view, "[data-testid=member-edit]") - end - end -end diff --git a/test/mv_web/live/user_live_authorization_test.exs b/test/mv_web/live/user_live_authorization_test.exs deleted file mode 100644 index f4b4746..0000000 --- a/test/mv_web/live/user_live_authorization_test.exs +++ /dev/null @@ -1,81 +0,0 @@ -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 - - 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 has_element?(view, "[data-testid=user-new]") - assert has_element?(view, "#row-#{user.id} [data-testid=user-edit]") - assert has_element?(view, "#row-#{user.id} [data-testid=user-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 has_element?(view, "[data-testid=user-edit]") - 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 has_element?(view, "[data-testid=user-edit]") - 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 has_element?(view, "[data-testid=user-edit]") - 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