Restrict member user link to admins (forbid policy)

Add ForbidMemberUserLinkUnlessAdmin check; forbid_if on Member create/update.
Fix member user-link tests: pass :user in params, assert via reload.

lib/membership/member.ex

@@ -312,14 +312,12 @@ defmodule Mv.Membership.Member do
       authorize_if expr(id == ^actor(:member_id))
     end
 
-    # GENERAL: Check permissions from user's role
+    # GENERAL: Check permissions from user's role; forbid member–user link unless admin
-    # HasPermission handles update permissions correctly:
+    # ForbidMemberUserLinkUnlessAdmin: only admins may pass :user on create/update (no-op for read/destroy).
-    # - :own_data → can update linked member (scope :linked)
+    # HasPermission: :own_data → update linked; :read_only → no update; :normal_user/admin → update all.
-    # - :read_only → cannot update any member (no update permission)
-    # - :normal_user → can update all members (scope :all)
-    # - :admin → can update all members (scope :all)
     policy action_type([:read, :create, :update, :destroy]) do
-      description "Check permissions from user's role and permission set"
+      description "Check permissions and forbid user link unless admin"
+      forbid_if Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin
       authorize_if Mv.Authorization.Checks.HasPermission
     end
 

lib/mv/authorization/checks/forbid_member_user_link_unless_admin.ex

@@ -0,0 +1,65 @@
+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 (linking a member to a user account), only admins may perform the action.
+  Non-admin users (e.g. normal_user / Kassenwart) can still create and update members
+  as long as they do not pass the `:user` argument.
+
+  ## Usage
+
+  In Member resource policies, add **before** the general HasPermission policy:
+
+      policy action_type([:create, :update]) do
+        forbid_if Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin
+        authorize_if Mv.Authorization.Checks.HasPermission
+      end
+
+  ## Behaviour
+
+  - If the action has no `:user` argument or it is nil/empty → does not forbid.
+  - If `:user` is set (e.g. `%{id: user_id}`) and actor is not admin → forbids (returns true).
+  - 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
+    actor = Actor.ensure_loaded(actor)
+
+    if user_argument_set?(authorizer) and not Actor.admin?(actor) do
+      {:ok, true}
+    else
+      {:ok, false}
+    end
+  end
+
+  defp user_argument_set?(authorizer) do
+    user_arg = get_user_argument(authorizer)
+    not is_nil(user_arg) and not empty_user_arg?(user_arg)
+  end
+
+  defp get_user_argument(authorizer) do
+    changeset = authorizer.changeset || authorizer.subject
+
+    cond do
+      is_struct(changeset, Ash.Changeset) ->
+        Ash.Changeset.get_argument(changeset, :user)
+
+      is_struct(changeset, Ash.ActionInput) ->
+        Map.get(changeset.arguments || %{}, :user)
+
+      true ->
+        nil
+    end
+  end
+
+  defp empty_user_arg?(%{} = m), do: map_size(m) == 0
+  defp empty_user_arg?(_), do: false
+end

test/mv/membership/member_policies_test.exs

@@ -403,4 +403,120 @@ defmodule Mv.Membership.MemberPoliciesTest do
       assert updated_member.first_name == "Updated"
     end
   end
+
+  describe "member user link - only admin may set or change user link" do
+    test "normal_user can create member without :user argument", %{actor: _actor} do
+      normal_user = Mv.Fixtures.user_with_role_fixture("normal_user")
+      normal_user = Mv.Authorization.Actor.ensure_loaded(normal_user)
+
+      {:ok, member} =
+        Membership.create_member(
+          %{
+            first_name: "NoLink",
+            last_name: "Member",
+            email: "nolink#{System.unique_integer([:positive])}@example.com"
+          },
+          actor: normal_user
+        )
+
+      assert member.first_name == "NoLink"
+      # Member has_one :user (FK on User side); ensure no user is linked
+      {:ok, member} = Ash.load(member, :user, domain: Mv.Membership)
+      assert is_nil(member.user)
+    end
+
+    test "normal_user cannot create member with :user argument (forbidden)", %{actor: _actor} do
+      normal_user = Mv.Fixtures.user_with_role_fixture("normal_user")
+      normal_user = Mv.Authorization.Actor.ensure_loaded(normal_user)
+      # Another user to try to link to
+      other_user = Mv.Fixtures.user_with_role_fixture("read_only")
+      other_user = Mv.Authorization.Actor.ensure_loaded(other_user)
+
+      attrs = %{
+        first_name: "Linked",
+        last_name: "Member",
+        email: "linked#{System.unique_integer([:positive])}@example.com",
+        user: %{id: other_user.id}
+      }
+
+      assert {:error, %Ash.Error.Forbidden{}} =
+               Membership.create_member(attrs, actor: normal_user)
+    end
+
+    test "normal_user can update member without :user argument", %{actor: actor} do
+      normal_user = Mv.Fixtures.user_with_role_fixture("normal_user")
+      normal_user = Mv.Authorization.Actor.ensure_loaded(normal_user)
+      unlinked_member = create_unlinked_member(actor)
+
+      {:ok, updated} =
+        Membership.update_member(unlinked_member, %{first_name: "UpdatedByNormal"},
+          actor: normal_user
+        )
+
+      assert updated.first_name == "UpdatedByNormal"
+    end
+
+    test "normal_user cannot update member with :user argument (forbidden)", %{actor: actor} do
+      normal_user = Mv.Fixtures.user_with_role_fixture("normal_user")
+      normal_user = Mv.Authorization.Actor.ensure_loaded(normal_user)
+      other_user = Mv.Fixtures.user_with_role_fixture("own_data")
+      other_user = Mv.Authorization.Actor.ensure_loaded(other_user)
+      unlinked_member = create_unlinked_member(actor)
+
+      # Passing :user in params tries to link member to other_user - only admin may do that
+      params = %{first_name: unlinked_member.first_name, user: %{id: other_user.id}}
+
+      assert {:error, %Ash.Error.Forbidden{}} =
+               Membership.update_member(unlinked_member, params, actor: normal_user)
+    end
+
+    test "admin can create member with :user argument", %{actor: _actor} do
+      admin = Mv.Fixtures.user_with_role_fixture("admin")
+      admin = Mv.Authorization.Actor.ensure_loaded(admin)
+      link_target = Mv.Fixtures.user_with_role_fixture("own_data")
+      link_target = Mv.Authorization.Actor.ensure_loaded(link_target)
+
+      attrs = %{
+        first_name: "AdminLinked",
+        last_name: "Member",
+        email: "adminlinked#{System.unique_integer([:positive])}@example.com",
+        user: %{id: link_target.id}
+      }
+
+      {:ok, member} = Membership.create_member(attrs, actor: admin)
+
+      assert member.first_name == "AdminLinked"
+      # Reload link_target to see the new member_id set by manage_relationship
+      {:ok, link_target} =
+        Ash.get(Mv.Accounts.User, link_target.id, domain: Mv.Accounts, actor: admin)
+
+      assert link_target.member_id == member.id
+    end
+
+    test "admin can update member with :user argument (link)", %{actor: actor} do
+      admin = Mv.Fixtures.user_with_role_fixture("admin")
+      admin = Mv.Authorization.Actor.ensure_loaded(admin)
+      unlinked_member = create_unlinked_member(actor)
+      link_target = Mv.Fixtures.user_with_role_fixture("read_only")
+      link_target = Mv.Authorization.Actor.ensure_loaded(link_target)
+
+      {:ok, updated} =
+        Membership.update_member(
+          unlinked_member,
+          %{user: %{id: link_target.id}},
+          actor: admin
+        )
+
+      assert updated.id == unlinked_member.id
+      # Member should now be linked to link_target (user.member_id points to this member)
+      {:ok, reloaded_user} =
+        Ash.get(Mv.Accounts.User, link_target.id,
+          domain: Mv.Accounts,
+          load: [:member],
+          actor: admin
+        )
+
+      assert reloaded_user.member_id == updated.id
+    end
+  end
 end