Merge branch 'main' into feature/export_csv

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,45 @@ 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

lib/mv/authorization/checks/actor_is_admin.ex

@@ -1,22 +1,18 @@
 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`, 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?(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

lib/mv/authorization/checks/actor_permission_set_is.ex

@@ -0,0 +1,44 @@
+defmodule Mv.Authorization.Checks.ActorPermissionSetIs do
+  @moduledoc """
+  Policy check: true when the actor's role has the given permission_set_name.
+
+  Used to restrict bypass policies (e.g. MemberGroup read by member_id) to actors
+  with a specific permission set (e.g. "own_data") so that admin with member_id
+  still gets :all scope from HasPermission, not the bypass filter.
+
+  ## Usage
+
+      # In a resource policy (both conditions must hold for the bypass)
+      bypass action_type(:read) do
+        authorize_if expr(member_id == ^actor(:member_id))
+        authorize_if {Mv.Authorization.Checks.ActorPermissionSetIs, permission_set_name: "own_data"}
+      end
+
+  ## Options
+
+  - `:permission_set_name` (required) - String or atom, e.g. `"own_data"` or `:own_data`
+  """
+  use Ash.Policy.SimpleCheck
+
+  alias Mv.Authorization.Actor
+
+  @impl true
+  def describe(opts) do
+    name = opts[:permission_set_name] || "?"
+    "actor has permission set #{name}"
+  end
+
+  @impl true
+  def match?(actor, _context, opts) do
+    case opts[:permission_set_name] do
+      nil ->
+        false
+
+      expected ->
+        case Actor.permission_set_name(actor) do
+          nil -> false
+          actual -> to_string(expected) == to_string(actual)
+        end
+    end
+  end
+end

lib/mv/authorization/checks/forbid_member_user_link_unless_admin.ex

@@ -0,0 +1,71 @@
+defmodule Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin do
+  @moduledoc """
+  Policy check: forbids setting or changing the member–user link unless the actor is admin.
+
+  Used on Member create_member and update_member actions. When the `:user` argument
+  **is present** (key in arguments, regardless of value), only admins may perform the action.
+  This covers:
+  - **Linking:** `user: %{id: user_id}` → only admin
+  - **Unlinking:** explicit `user: nil` or `user: %{}` on update_member → only admin
+  Non-admin users can create and update members only when they do **not** pass the
+  `:user` argument; omitting `:user` leaves the relationship unchanged.
+
+  ## Unlink semantics (update_member)
+
+  The Member resource uses `on_missing: :ignore` for the `:user` relationship on update.
+  So **omitting** `:user` from params does **not** change the link (no "unlink by omission").
+  Unlink is only possible by **explicitly** passing `:user` (e.g. `user: nil`), which this
+  check forbids for non-admins. Admins may link or unlink via the `:user` argument.
+
+  ## Usage
+
+  In Member resource policies, restrict to create/update only:
+
+      policy action_type([:create, :update]) do
+        forbid_if Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin
+        authorize_if Mv.Authorization.Checks.HasPermission
+      end
+
+  ## Behaviour
+
+  - If the `:user` argument **key is not present** → does not forbid.
+  - If `:user` is present (any value, including nil or %{}) and actor is not admin → forbids.
+  - If actor is nil → treated as non-admin (forbid when :user present). `Actor.admin?(nil)` is defined and returns false.
+  - If actor is admin (or system actor) → does not forbid.
+  """
+  use Ash.Policy.Check
+
+  alias Mv.Authorization.Actor
+
+  @impl true
+  def describe(_opts), do: "forbid setting member–user link unless actor is admin"
+
+  @impl true
+  def strict_check(actor, authorizer, _opts) do
+    # Nil actor: treat as non-admin (Actor.admin?(nil) returns false; no crash)
+    actor = if is_nil(actor), do: nil, else: Actor.ensure_loaded(actor)
+
+    if user_argument_present?(authorizer) and not Actor.admin?(actor) do
+      {:ok, true}
+    else
+      {:ok, false}
+    end
+  end
+
+  # Forbid when :user was passed at all (link, unlink via nil/empty, or invalid value).
+  # Check argument key presence (atom or string) for defense-in-depth.
+  defp user_argument_present?(authorizer) do
+    args = get_arguments(authorizer) || %{}
+    Map.has_key?(args, :user) or Map.has_key?(args, "user")
+  end
+
+  defp get_arguments(authorizer) do
+    subject = authorizer.changeset || authorizer.subject
+
+    cond do
+      is_struct(subject, Ash.Changeset) -> subject.arguments
+      is_struct(subject, Ash.ActionInput) -> subject.arguments
+      true -> %{}
+    end
+  end
+end

lib/mv/authorization/checks/has_permission.ex

@@ -50,6 +50,7 @@ defmodule Mv.Authorization.Checks.HasPermission do
   - **:linked** - Filters based on resource type:
     - Member: `id == actor.member_id` (User.member_id → Member.id, inverse relationship)
     - CustomFieldValue: `member_id == actor.member_id` (CustomFieldValue.member_id → Member.id → User.member_id)
+    - MemberGroup: `member_id == actor.member_id` (MemberGroup.member_id → Member.id → User.member_id)
 
   ## Error Handling
 
@@ -131,26 +132,10 @@ defmodule Mv.Authorization.Checks.HasPermission do
              resource_name
            ) do
         :authorized ->
-          # For :all scope, authorize directly
           {:ok, true}
 
         {:filter, filter_expr} ->
-          # For :own/:linked scope:
-          # - With a record, evaluate filter against record for strict authorization
-          # - Without a record (queries/lists), return false
-          #
-          # NOTE: Returning false here forces the use of expr-based bypass policies.
-          # This is necessary because Ash's policy evaluation doesn't reliably call auto_filter
-          # when strict_check returns :unknown. Instead, resources should use bypass policies
-          # with expr() directly for filter-based authorization (see User resource).
-          if record do
-            evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name)
-          else
-            # No record yet (e.g., read/list queries) - deny at strict_check level
-            # Resources must use expr-based bypass policies for list filtering
-            # Create: use a dedicated check that does not return a filter (e.g. CustomFieldValueCreateScope)
-            {:ok, false}
-          end
+          strict_check_filter_scope(record, filter_expr, actor, resource_name)
 
         false ->
           {:ok, false}
@@ -174,6 +159,15 @@ defmodule Mv.Authorization.Checks.HasPermission do
     end
   end
 
+  # For :own/:linked scope: with record evaluate filter; without record deny (resources use bypass + expr).
+  defp strict_check_filter_scope(record, filter_expr, actor, resource_name) do
+    if record do
+      evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name)
+    else
+      {:ok, false}
+    end
+  end
+
   @impl true
   def auto_filter(actor, authorizer, _opts) do
     resource = authorizer.resource
@@ -278,36 +272,28 @@ defmodule Mv.Authorization.Checks.HasPermission do
   # For :own scope with User resource: id == actor.id
   # For :linked scope with Member resource: id == actor.member_id
   defp evaluate_filter_for_strict_check(_filter_expr, actor, record, resource_name) do
-    case {resource_name, record} do
-      {"User", %{id: user_id}} when not is_nil(user_id) ->
-        # Check if this user's ID matches the actor's ID (scope :own)
-        if user_id == actor.id do
-          {:ok, true}
-        else
-          {:ok, false}
-        end
+    result =
+      case {resource_name, record} do
+        # Scope :own
+        {"User", %{id: user_id}} when not is_nil(user_id) ->
+          user_id == actor.id
 
-      {"Member", %{id: member_id}} when not is_nil(member_id) ->
-        # Check if this member's ID matches the actor's member_id
-        if member_id == actor.member_id do
-          {:ok, true}
-        else
-          {:ok, false}
-        end
+        # Scope :linked
+        {"Member", %{id: member_id}} when not is_nil(member_id) ->
+          member_id == actor.member_id
 
-      {"CustomFieldValue", %{member_id: cfv_member_id}} when not is_nil(cfv_member_id) ->
-        # Check if this CFV's member_id matches the actor's member_id
-        if cfv_member_id == actor.member_id do
-          {:ok, true}
-        else
-          {:ok, false}
-        end
+        {"CustomFieldValue", %{member_id: cfv_member_id}} when not is_nil(cfv_member_id) ->
+          cfv_member_id == actor.member_id
 
-      _ ->
-        # For other cases or when record is not available, return :unknown
-        # This will cause Ash to use auto_filter instead
-        {:ok, :unknown}
-    end
+        {"MemberGroup", %{member_id: mg_member_id}} when not is_nil(mg_member_id) ->
+          mg_member_id == actor.member_id
+
+        _ ->
+          :unknown
+      end
+
+    out = if result == :unknown, do: {:ok, :unknown}, else: {:ok, result}
+    out
   end
 
   # Extract resource name from module (e.g., Mv.Membership.Member -> "Member")
@@ -347,24 +333,20 @@ defmodule Mv.Authorization.Checks.HasPermission do
   defp apply_scope(:linked, actor, resource_name) do
     case resource_name do
       "Member" ->
-        # User.member_id → Member.id (inverse relationship)
-        # Filter: member.id == actor.member_id
-        # If actor has no member_id, return no results (use false or impossible condition)
-        if is_nil(actor.member_id) do
-          {:filter, expr(false)}
-        else
-          {:filter, expr(id == ^actor.member_id)}
-        end
+        # User.member_id → Member.id (inverse relationship). Filter: member.id == actor.member_id
+        linked_filter_by_member_id(actor, :id)
 
       "CustomFieldValue" ->
         # CustomFieldValue.member_id → Member.id → User.member_id
-        # Filter: custom_field_value.member_id == actor.member_id
-        # If actor has no member_id, return no results
-        if is_nil(actor.member_id) do
-          {:filter, expr(false)}
-        else
-          {:filter, expr(member_id == ^actor.member_id)}
-        end
+        linked_filter_by_member_id(actor, :member_id)
+
+      "MemberGroup" ->
+        # MemberGroup.member_id → Member.id → User.member_id (own linked member's group associations)
+        linked_filter_by_member_id(actor, :member_id)
+
+      "MembershipFeeCycle" ->
+        # MembershipFeeCycle.member_id → Member.id → User.member_id (own linked member's cycles)
+        linked_filter_by_member_id(actor, :member_id)
 
       _ ->
         # Fallback for other resources
@@ -372,6 +354,17 @@ defmodule Mv.Authorization.Checks.HasPermission do
     end
   end
 
+  # Returns {:filter, expr(false)} if actor has no member_id; otherwise {:filter, expr(field == ^actor.member_id)}.
+  # Used for :linked scope on Member (field :id), CustomFieldValue and MemberGroup (field :member_id).
+  defp linked_filter_by_member_id(actor, _field) when is_nil(actor.member_id) do
+    {:filter, expr(false)}
+  end
+
+  defp linked_filter_by_member_id(actor, :id), do: {:filter, expr(id == ^actor.member_id)}
+
+  defp linked_filter_by_member_id(actor, :member_id),
+    do: {:filter, expr(member_id == ^actor.member_id)}
+
   # Log authorization failures for debugging (lazy evaluation)
   defp log_auth_failure(actor, resource, action, reason) do
     Logger.debug(fn ->

lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex

@@ -0,0 +1,63 @@
+defmodule Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData do
+  @moduledoc """
+  Policy check for MemberGroup read: true only when actor has permission set "own_data"
+  AND record.member_id == actor.member_id.
+
+  Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries),
+  while admin with member_id does not match and gets :all from HasPermission.
+
+  - With a record (e.g. get by id): returns true only when own_data and member_id match.
+  - Without a record (list query): strict_check returns false; auto_filter adds filter when own_data.
+  """
+  use Ash.Policy.Check
+
+  alias Mv.Authorization.Checks.ActorPermissionSetIs
+
+  @impl true
+  def type, do: :filter
+
+  @impl true
+  def describe(_opts),
+    do: "own_data can read only member_groups where member_id == actor.member_id"
+
+  @impl true
+  def strict_check(actor, authorizer, _opts) do
+    record = get_record_from_authorizer(authorizer)
+    is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data")
+
+    cond do
+      # List query + own_data: return :unknown so authorizer applies auto_filter (keyword list)
+      is_nil(record) and is_own_data ->
+        {:ok, :unknown}
+
+      is_nil(record) ->
+        {:ok, false}
+
+      not is_own_data ->
+        {:ok, false}
+
+      record.member_id == actor.member_id ->
+        {:ok, true}
+
+      true ->
+        {:ok, false}
+    end
+  end
+
+  @impl true
+  def auto_filter(actor, _authorizer, _opts) do
+    if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") &&
+         Map.get(actor, :member_id) do
+      [member_id: actor.member_id]
+    else
+      []
+    end
+  end
+
+  defp get_record_from_authorizer(authorizer) do
+    case authorizer.subject do
+      %{data: data} when not is_nil(data) -> data
+      _ -> nil
+    end
+  end
+end

lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex

@@ -0,0 +1,62 @@
+defmodule Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData do
+  @moduledoc """
+  Policy check for MembershipFeeCycle read: true only when actor has permission set "own_data"
+  AND record.member_id == actor.member_id.
+
+  Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries),
+  while admin with member_id does not match and gets :all from HasPermission.
+
+  - With a record (e.g. get by id): returns true only when own_data and member_id match.
+  - Without a record (list query): return :unknown so authorizer applies auto_filter.
+  """
+  use Ash.Policy.Check
+
+  alias Mv.Authorization.Checks.ActorPermissionSetIs
+
+  @impl true
+  def type, do: :filter
+
+  @impl true
+  def describe(_opts),
+    do: "own_data can read only membership_fee_cycles where member_id == actor.member_id"
+
+  @impl true
+  def strict_check(actor, authorizer, _opts) do
+    record = get_record_from_authorizer(authorizer)
+    is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data")
+
+    cond do
+      is_nil(record) and is_own_data ->
+        {:ok, :unknown}
+
+      is_nil(record) ->
+        {:ok, false}
+
+      not is_own_data ->
+        {:ok, false}
+
+      record.member_id == actor.member_id ->
+        {:ok, true}
+
+      true ->
+        {:ok, false}
+    end
+  end
+
+  @impl true
+  def auto_filter(actor, _authorizer, _opts) do
+    if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") &&
+         Map.get(actor, :member_id) do
+      [member_id: actor.member_id]
+    else
+      []
+    end
+  end
+
+  defp get_record_from_authorizer(authorizer) do
+    case authorizer.subject do
+      %{data: data} when not is_nil(data) -> data
+      _ -> nil
+    end
+  end
+end

lib/mv/authorization/checks/oidc_role_sync_context.ex

@@ -0,0 +1,18 @@
+defmodule Mv.Authorization.Checks.OidcRoleSyncContext do
+  @moduledoc """
+  Policy check: true when the action is run from OIDC role sync (context.private.oidc_role_sync).
+
+  Used to allow the internal set_role_from_oidc_sync action only when called by Mv.OidcRoleSync,
+  which sets context.private.oidc_role_sync when performing the update.
+  """
+  use Ash.Policy.SimpleCheck
+
+  @impl true
+  def describe(_opts), do: "called from OIDC role sync (context.private.oidc_role_sync)"
+
+  @impl true
+  def match?(_actor, authorizer, _opts) do
+    context = Map.get(authorizer, :context) || %{}
+    get_in(context, [:private, :oidc_role_sync]) == true
+  end
+end

lib/mv/authorization/permission_sets.ex

@@ -58,6 +58,28 @@ defmodule Mv.Authorization.PermissionSets do
           pages: [String.t()]
         }
 
+  # DRY helpers for shared resource permission lists (used in own_data, read_only, normal_user, admin)
+  defp perm(resource, action, scope),
+    do: %{resource: resource, action: action, scope: scope, granted: true}
+
+  # All four CRUD actions for a resource with scope :all (used for admin)
+  defp perm_all(resource),
+    do: [
+      perm(resource, :read, :all),
+      perm(resource, :create, :all),
+      perm(resource, :update, :all),
+      perm(resource, :destroy, :all)
+    ]
+
+  # User: read/update own credentials only (all non-admin sets allow password changes)
+  defp user_own_credentials, do: [perm("User", :read, :own), perm("User", :update, :own)]
+
+  defp group_read_all, do: [perm("Group", :read, :all)]
+  defp custom_field_read_all, do: [perm("CustomField", :read, :all)]
+  defp membership_fee_type_read_all, do: [perm("MembershipFeeType", :read, :all)]
+  defp membership_fee_cycle_read_all, do: [perm("MembershipFeeCycle", :read, :all)]
+  defp role_read_all, do: [perm("Role", :read, :all)]
+
   @doc """
   Returns the list of all valid permission set names.
 
@@ -94,29 +116,22 @@ defmodule Mv.Authorization.PermissionSets do
 
   def get_permissions(:own_data) do
     %{
-      resources: [
-        # User: Can read/update own credentials only
-        # IMPORTANT: "read_only" refers to member data, NOT user credentials.
-        # All permission sets grant User.update :own to allow password changes.
-        %{resource: "User", action: :read, scope: :own, granted: true},
-        %{resource: "User", action: :update, scope: :own, granted: true},
-
-        # Member: Can read/update linked member
-        %{resource: "Member", action: :read, scope: :linked, granted: true},
-        %{resource: "Member", action: :update, scope: :linked, granted: true},
-
-        # CustomFieldValue: Can read/update/create/destroy custom field values of linked member
-        %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true},
-        %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true},
-        %{resource: "CustomFieldValue", action: :create, scope: :linked, granted: true},
-        %{resource: "CustomFieldValue", action: :destroy, scope: :linked, granted: true},
-
-        # CustomField: Can read all (needed for forms)
-        %{resource: "CustomField", action: :read, scope: :all, granted: true},
-
-        # Group: Can read all (needed for viewing groups)
-        %{resource: "Group", action: :read, scope: :all, granted: true}
-      ],
+      resources:
+        user_own_credentials() ++
+          [
+            perm("Member", :read, :linked),
+            perm("Member", :update, :linked),
+            perm("CustomFieldValue", :read, :linked),
+            perm("CustomFieldValue", :update, :linked),
+            perm("CustomFieldValue", :create, :linked),
+            perm("CustomFieldValue", :destroy, :linked)
+          ] ++
+          custom_field_read_all() ++
+          group_read_all() ++
+          [perm("MemberGroup", :read, :linked)] ++
+          membership_fee_type_read_all() ++
+          [perm("MembershipFeeCycle", :read, :linked)] ++
+          role_read_all(),
       pages: [
         # No "/" - Mitglied must not see member index at root (same content as /members).
         # Own profile (sidebar links to /users/:id) and own user edit
@@ -133,25 +148,18 @@ defmodule Mv.Authorization.PermissionSets do
 
   def get_permissions(:read_only) do
     %{
-      resources: [
-        # User: Can read/update own credentials only
-        # IMPORTANT: "read_only" refers to member data, NOT user credentials.
-        # All permission sets grant User.update :own to allow password changes.
-        %{resource: "User", action: :read, scope: :own, granted: true},
-        %{resource: "User", action: :update, scope: :own, granted: true},
-
-        # Member: Can read all members, no modifications
-        %{resource: "Member", action: :read, scope: :all, granted: true},
-
-        # CustomFieldValue: Can read all custom field values
-        %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true},
-
-        # CustomField: Can read all
-        %{resource: "CustomField", action: :read, scope: :all, granted: true},
-
-        # Group: Can read all
-        %{resource: "Group", action: :read, scope: :all, granted: true}
-      ],
+      resources:
+        user_own_credentials() ++
+          [
+            perm("Member", :read, :all),
+            perm("CustomFieldValue", :read, :all)
+          ] ++
+          custom_field_read_all() ++
+          group_read_all() ++
+          [perm("MemberGroup", :read, :all)] ++
+          membership_fee_type_read_all() ++
+          membership_fee_cycle_read_all() ++
+          role_read_all(),
       pages: [
         "/",
         # Own profile (sidebar links to /users/:id; redirect target must be allowed)
@@ -177,31 +185,38 @@ defmodule Mv.Authorization.PermissionSets do
 
   def get_permissions(:normal_user) do
     %{
-      resources: [
-        # User: Can read/update own credentials only
-        # IMPORTANT: "read_only" refers to member data, NOT user credentials.
-        # All permission sets grant User.update :own to allow password changes.
-        %{resource: "User", action: :read, scope: :own, granted: true},
-        %{resource: "User", action: :update, scope: :own, granted: true},
-
-        # Member: Full CRUD except destroy (safety)
-        %{resource: "Member", action: :read, scope: :all, granted: true},
-        %{resource: "Member", action: :create, scope: :all, granted: true},
-        %{resource: "Member", action: :update, scope: :all, granted: true},
-        # Note: destroy intentionally omitted for safety
-
-        # CustomFieldValue: Full CRUD
-        %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true},
-
-        # CustomField: Read only (admin manages definitions)
-        %{resource: "CustomField", action: :read, scope: :all, granted: true},
-
-        # Group: Can read all
-        %{resource: "Group", action: :read, scope: :all, granted: true}
-      ],
+      resources:
+        user_own_credentials() ++
+          [
+            perm("Member", :read, :all),
+            perm("Member", :create, :all),
+            perm("Member", :update, :all),
+            # destroy intentionally omitted for safety
+            perm("CustomFieldValue", :read, :all),
+            perm("CustomFieldValue", :create, :all),
+            perm("CustomFieldValue", :update, :all),
+            perm("CustomFieldValue", :destroy, :all)
+          ] ++
+          custom_field_read_all() ++
+          [
+            perm("Group", :read, :all),
+            perm("Group", :create, :all),
+            perm("Group", :update, :all),
+            perm("Group", :destroy, :all)
+          ] ++
+          [
+            perm("MemberGroup", :read, :all),
+            perm("MemberGroup", :create, :all),
+            perm("MemberGroup", :destroy, :all)
+          ] ++
+          membership_fee_type_read_all() ++
+          [
+            perm("MembershipFeeCycle", :read, :all),
+            perm("MembershipFeeCycle", :create, :all),
+            perm("MembershipFeeCycle", :update, :all),
+            perm("MembershipFeeCycle", :destroy, :all)
+          ] ++
+          role_read_all(),
       pages: [
         "/",
         # Own profile (sidebar links to /users/:id; redirect target must be allowed)
@@ -223,52 +238,39 @@ defmodule Mv.Authorization.PermissionSets do
         "/custom_field_values/:id/edit",
         # Groups overview
         "/groups",
+        # Create group
+        "/groups/new",
         # Group detail
-        "/groups/:slug"
+        "/groups/:slug",
+        # Edit group
+        "/groups/:slug/edit"
       ]
     }
   end
 
   def get_permissions(:admin) do
+    # MemberGroup has no :update action in the domain; use read/create/destroy only
+    member_group_perms = [
+      perm("MemberGroup", :read, :all),
+      perm("MemberGroup", :create, :all),
+      perm("MemberGroup", :destroy, :all)
+    ]
+
     %{
-      resources: [
-        # User: Full management including other users
-        %{resource: "User", action: :read, scope: :all, granted: true},
-        %{resource: "User", action: :create, scope: :all, granted: true},
-        %{resource: "User", action: :update, scope: :all, granted: true},
-        %{resource: "User", action: :destroy, scope: :all, granted: true},
-
-        # Member: Full CRUD
-        %{resource: "Member", action: :read, scope: :all, granted: true},
-        %{resource: "Member", action: :create, scope: :all, granted: true},
-        %{resource: "Member", action: :update, scope: :all, granted: true},
-        %{resource: "Member", action: :destroy, scope: :all, granted: true},
-
-        # CustomFieldValue: Full CRUD
-        %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true},
-
-        # CustomField: Full CRUD (admin manages custom field definitions)
-        %{resource: "CustomField", action: :read, scope: :all, granted: true},
-        %{resource: "CustomField", action: :create, scope: :all, granted: true},
-        %{resource: "CustomField", action: :update, scope: :all, granted: true},
-        %{resource: "CustomField", action: :destroy, scope: :all, granted: true},
-
-        # Role: Full CRUD (admin manages roles)
-        %{resource: "Role", action: :read, scope: :all, granted: true},
-        %{resource: "Role", action: :create, scope: :all, granted: true},
-        %{resource: "Role", action: :update, scope: :all, granted: true},
-        %{resource: "Role", action: :destroy, scope: :all, granted: true},
-
-        # Group: Full CRUD (admin manages groups)
-        %{resource: "Group", action: :read, scope: :all, granted: true},
-        %{resource: "Group", action: :create, scope: :all, granted: true},
-        %{resource: "Group", action: :update, scope: :all, granted: true},
-        %{resource: "Group", action: :destroy, scope: :all, granted: true}
-      ],
+      resources:
+        perm_all("User") ++
+          perm_all("Member") ++
+          perm_all("CustomFieldValue") ++
+          perm_all("CustomField") ++
+          perm_all("Role") ++
+          perm_all("Group") ++
+          member_group_perms ++
+          perm_all("MembershipFeeType") ++
+          perm_all("MembershipFeeCycle"),
       pages: [
+        # Explicit admin-only pages (for clarity and future restrictions)
+        "/settings",
+        "/membership_fee_settings",
         # Wildcard: Admin can access all pages
         "*"
       ]

lib/mv/authorization/role.ex

@@ -37,7 +37,8 @@ defmodule Mv.Authorization.Role do
   """
   use Ash.Resource,
     domain: Mv.Authorization,
-    data_layer: AshPostgres.DataLayer
+    data_layer: AshPostgres.DataLayer,
+    authorizers: [Ash.Policy.Authorizer]
 
   postgres do
     table "roles"
@@ -86,6 +87,13 @@ defmodule Mv.Authorization.Role do
     end
   end
 
+  policies do
+    policy action_type([:read, :create, :update, :destroy]) do
+      description "Role access: read for all permission sets, create/update/destroy for admin only (PermissionSets)"
+      authorize_if Mv.Authorization.Checks.HasPermission
+    end
+  end
+
   validations do
     validate one_of(
                :permission_set_name,
@@ -173,4 +181,18 @@ defmodule Mv.Authorization.Role do
     |> Ash.Query.filter(name == "Mitglied")
     |> Ash.read_one(authorize?: false, domain: Mv.Authorization)
   end
+
+  @doc """
+  Returns the Admin role if it exists.
+
+  Used by release tasks (e.g. seed_admin) and OIDC role sync to assign the admin role.
+  """
+  @spec get_admin_role() :: {:ok, t() | nil} | {:error, term()}
+  def get_admin_role do
+    require Ash.Query
+
+    __MODULE__
+    |> Ash.Query.filter(name == "Admin")
+    |> Ash.read_one(authorize?: false, domain: Mv.Authorization)
+  end
 end