diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex index d20be7d..d821416 100644 --- a/lib/mv_web/authorization.ex +++ b/lib/mv_web/authorization.ex @@ -97,12 +97,18 @@ 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 45bcae0..59e300e 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -97,12 +97,13 @@ defmodule MvWeb.CoreComponents do <.button navigate={~p"/"}>Home <.button disabled={true}>Disabled """ - attr :rest, :global, include: ~w(href navigate patch method) + attr :rest, :global, include: ~w(href navigate patch method data-testid) attr :variant, :string, values: ~w(primary) attr :disabled, :boolean, default: false, doc: "Whether the button is disabled" slot :inner_block, required: true - def button(%{rest: rest} = assigns) do + def button(assigns) do + rest = assigns.rest 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 1d564c1..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,33 +72,56 @@ 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" @@ -119,12 +144,13 @@ 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""" -
  • +
  • - <%!-- 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 9314f1e..cb945e2 100644 --- a/lib/mv_web/live/user_live/index.html.heex +++ b/lib/mv_web/live/user_live/index.html.heex @@ -2,13 +2,20 @@ <.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"} data-testid="user-new"> + <.icon name="hero-plus" /> {gettext("New User")} + + <% end %> - <.table id="users" rows={@users} row_click={fn user -> JS.navigate(~p"/users/#{user}") end}> + <.table + id="users" + rows={@users} + row_id={fn user -> "row-#{user.id}" end} + row_click={fn user -> JS.navigate(~p"/users/#{user}") end} + > <:col :let={user} label={ @@ -62,16 +69,23 @@ <.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"} data-testid="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?")} + data-testid="user-delete" + > + {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..5114b74 100644 --- a/lib/mv_web/live/user_live/show.ex +++ b/lib/mv_web/live/user_live/show.ex @@ -41,9 +41,15 @@ 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"} + data-testid="user-edit" + > + <.icon name="hero-pencil-square" /> {gettext("Edit User")} + + <% end %> 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 diff --git a/test/mv_web/components/layouts/sidebar_test.exs b/test/mv_web/components/layouts/sidebar_test.exs index 75727e3..ff81f24 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 } @@ -144,7 +149,9 @@ 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") @@ -193,7 +200,9 @@ 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") @@ -521,7 +530,9 @@ 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 @@ -629,7 +640,9 @@ 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") @@ -843,7 +856,9 @@ 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 new file mode 100644 index 0000000..079572f --- /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(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 new file mode 100644 index 0000000..9a23019 --- /dev/null +++ b/test/mv_web/live/member_live_authorization_test.exs @@ -0,0 +1,102 @@ +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 new file mode 100644 index 0000000..f4b4746 --- /dev/null +++ b/test/mv_web/live/user_live_authorization_test.exs @@ -0,0 +1,81 @@ +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