MemberGroup: restrict bypass to own_data via MemberGroupReadLinkedForOwnData

- ActorPermissionSetIs check; bypass policy filters by member_id for own_data only.
- Admin with member_id still gets :all via HasPermission. Tests added.

lib/membership/member_group.ex

@@ -42,7 +42,6 @@ defmodule Mv.Membership.MemberGroup do
     data_layer: AshPostgres.DataLayer,
     authorizers: [Ash.Policy.Authorizer]
 
-  import Ash.Expr
   require Ash.Query
 
   postgres do
@@ -58,13 +57,13 @@ defmodule Mv.Membership.MemberGroup do
     end
   end
 
-  # Authorization: read uses bypass for :linked (own_data list) then HasPermission for :all;
+  # Authorization: read uses bypass for :linked (own_data only) then HasPermission for :all;
   # create/destroy use HasPermission (normal_user + admin only).
-  # Order: bypass first so own_data gets expr filter; HasPermission then authorizes :all for others.
+  # Single check: own_data gets filter via auto_filter; admin does not match, gets :all from HasPermission.
   policies do
     bypass action_type(:read) do
       description "own_data: read only member_groups where member_id == actor.member_id"
-      authorize_if expr(member_id == ^actor(:member_id))
+      authorize_if Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData
     end
 
     policy action_type(:read) do

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/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

test/mv/membership/member_group_policies_test.exs

@@ -184,6 +184,39 @@ defmodule Mv.Membership.MemberGroupPoliciesTest do
       assert mg.id in ids
     end
 
+    test "admin with member_id set (linked to member) still reads all member_groups", %{
+      actor: actor
+    } do
+      # Admin linked to a member (e.g. viewing as member context) must still get :all scope,
+      # not restricted to linked member's groups (bypass is only for own_data).
+      admin = Mv.Fixtures.user_with_role_fixture("admin")
+      linked_member = create_member_fixture()
+      other_member = create_member_fixture()
+      group_a = create_group_fixture()
+      group_b = create_group_fixture()
+
+      admin =
+        admin
+        |> Ash.Changeset.for_update(:update, %{})
+        |> Ash.Changeset.force_change_attribute(:member_id, linked_member.id)
+        |> Ash.update(actor: actor)
+
+      {:ok, admin} = Ash.load(admin, :role, domain: Mv.Accounts, actor: actor)
+
+      mg_linked = create_member_group_fixture(linked_member.id, group_a.id)
+      mg_other = create_member_group_fixture(other_member.id, group_b.id)
+
+      {:ok, list} =
+        Mv.Membership.MemberGroup
+        |> Ash.read(actor: admin, domain: Mv.Membership)
+
+      ids = Enum.map(list, & &1.id)
+      assert mg_linked.id in ids, "Admin with member_id must see linked member's MemberGroups"
+
+      assert mg_other.id in ids,
+             "Admin with member_id must see all MemberGroups (:all), not only linked"
+    end
+
     test "can create member_group", %{user: user, actor: _actor} do
       member = create_member_fixture()
       group = create_group_fixture()