feat: implement authorization policies for Member resource

lib/membership/member.ex

@@ -34,7 +34,8 @@ defmodule Mv.Membership.Member do
   """
   use Ash.Resource,
     domain: Mv.Membership,
-    data_layer: AshPostgres.DataLayer
+    data_layer: AshPostgres.DataLayer,
+    authorizers: [Ash.Policy.Authorizer]
 
   require Ash.Query
   import Ash.Expr
@@ -296,6 +297,40 @@ defmodule Mv.Membership.Member do
     end
   end
 
+  # Authorization Policies
+  # Order matters: Most specific policies first, then general permission check
+  policies do
+    # SYSTEM OPERATIONS: Allow operations without actor (seeds, tests, system jobs)
+    # This must come first to allow database seeding and test fixtures
+    # IMPORTANT: Use bypass so this short-circuits and doesn't require other policies
+    bypass action_type([:create, :read, :update, :destroy]) do
+      description "Allow system operations without actor (seeds, tests)"
+      authorize_if Mv.Authorization.Checks.NoActor
+    end
+
+    # SPECIAL CASE: Users can always READ their linked member
+    # This allows users with ANY permission set to read their own linked member
+    # Check using the inverse relationship: User.member_id → Member.id
+    bypass action_type(:read) do
+      description "Users can always read member linked to their account"
+      authorize_if expr(id == ^actor(:member_id))
+    end
+
+    # GENERAL: Check permissions from user's role
+    # HasPermission handles update permissions correctly:
+    # - :own_data → can update linked member (scope :linked)
+    # - :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"
+      authorize_if Mv.Authorization.Checks.HasPermission
+    end
+
+    # DEFAULT: Forbid if no policy matched
+    # Ash implicitly forbids if no policy authorized
+  end
+
   @doc """
   Filters members list based on email match priority.
 

lib/mv/authorization/checks/has_permission.ex

@@ -59,6 +59,7 @@ defmodule Mv.Authorization.Checks.HasPermission do
   def strict_check(actor, authorizer, _opts) do
     resource = authorizer.resource
     action = get_action_from_authorizer(authorizer)
+    record = get_record_from_authorizer(authorizer)
 
     cond do
       is_nil(actor) ->
@@ -76,12 +77,12 @@ defmodule Mv.Authorization.Checks.HasPermission do
         {:ok, false}
 
       true ->
-        strict_check_with_permissions(actor, resource, action)
+        strict_check_with_permissions(actor, resource, action, record)
     end
   end
 
   # Helper function to reduce nesting depth
-  defp strict_check_with_permissions(actor, resource, action) do
+  defp strict_check_with_permissions(actor, resource, action, record) do
     with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- actor,
          {:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name),
          permissions <- PermissionSets.get_permissions(ps_atom),
@@ -93,9 +94,15 @@ defmodule Mv.Authorization.Checks.HasPermission do
              actor,
              resource_name
            ) do
-        :authorized -> {:ok, true}
-        {:filter, _} -> {:ok, :unknown}
-        false -> {:ok, false}
+        :authorized ->
+          {:ok, true}
+
+        {:filter, filter_expr} ->
+          # For strict_check on single records, evaluate the filter against the record
+          evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name)
+
+        false ->
+          {:ok, false}
       end
     else
       %{role: nil} ->
@@ -150,15 +157,60 @@ defmodule Mv.Authorization.Checks.HasPermission do
     end
   end
 
-  # Helper to extract action from authorizer
+  # Helper to extract action type from authorizer
+  # CRITICAL: Must use action_type, not action.name!
+  # Action types: :create, :read, :update, :destroy
+  # Action names: :create_member, :update_member, etc.
+  # PermissionSets uses action types, not action names
   defp get_action_from_authorizer(authorizer) do
     case authorizer.subject do
-      %{action: %{name: action}} -> action
-      %{action: action} when is_atom(action) -> action
+      %{action_type: action_type} when action_type in [:create, :read, :update, :destroy] ->
+        action_type
+
+      # Fallback for older Ash versions or different subject shapes
+      %{action: %{type: action_type}} when action_type in [:create, :read, :update, :destroy] ->
+        action_type
+
+      _ ->
+        nil
+    end
+  end
+
+  # Helper to extract record from authorizer for strict_check
+  defp get_record_from_authorizer(authorizer) do
+    case authorizer.subject do
+      %{data: data} when not is_nil(data) -> data
       _ -> nil
     end
   end
 
+  # Evaluate filter expression for strict_check on single records
+  # 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
+      {"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
+
+      {"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
+
+      _ ->
+        # For other cases or when record is not available, return :unknown
+        # This will cause Ash to use auto_filter instead
+        {:ok, :unknown}
+    end
+  end
+
   # Extract resource name from module (e.g., Mv.Membership.Member -> "Member")
   defp get_resource_name(resource) when is_atom(resource) do
     resource |> Module.split() |> List.last()
@@ -190,21 +242,24 @@ defmodule Mv.Authorization.Checks.HasPermission do
   end
 
   # Scope: linked - Filter based on user relationship (resource-specific!)
-  # Uses Ash relationships: Member has_one :user, CustomFieldValue belongs_to :member
+  # IMPORTANT: Understand the relationship direction!
+  # - User belongs_to :member (User.member_id → Member.id)
+  # - Member has_one :user (inverse, no FK on Member)
   defp apply_scope(:linked, actor, resource_name) do
     case resource_name do
       "Member" ->
-        # Member has_one :user → filter by user.id == actor.id
-        {:filter, expr(user.id == ^actor.id)}
+        # User.member_id → Member.id (inverse relationship)
+        # Filter: member.id == actor.member_id
+        {:filter, expr(id == ^actor.member_id)}
 
       "CustomFieldValue" ->
-        # CustomFieldValue belongs_to :member → member has_one :user
-        # Traverse: custom_field_value.member.user.id == actor.id
-        {:filter, expr(member.user.id == ^actor.id)}
+        # CustomFieldValue.member_id → Member.id → User.member_id
+        # Filter: custom_field_value.member_id == actor.member_id
+        {:filter, expr(member_id == ^actor.member_id)}
 
       _ ->
-        # Fallback for other resources: try user relationship first, then user_id
-        {:filter, expr(user.id == ^actor.id or user_id == ^actor.id)}
+        # Fallback for other resources
+        {:filter, expr(user_id == ^actor.id)}
     end
   end
 

lib/mv/authorization/checks/no_actor.ex

@@ -0,0 +1,60 @@
+defmodule Mv.Authorization.Checks.NoActor do
+  @moduledoc """
+  Custom Ash Policy Check that allows actions when no actor is present.
+
+  This is primarily used for:
+  - Database seeding (priv/repo/seeds.exs)
+  - Test fixtures that create data without authentication
+  - Background jobs that operate on behalf of the system
+
+  ## Security Note
+
+  This check should only be used for specific actions where system-level
+  access is appropriate. It should always be combined with other policy
+  checks that validate actor-based permissions when an actor IS present.
+
+  ## Usage in Policies
+
+      policies do
+        # Allow seeding and system operations
+        policy action_type(:create) do
+          authorize_if NoActor
+        end
+        
+        # Check permissions when actor is present
+        policy action_type([:read, :create, :update, :destroy]) do
+          authorize_if HasPermission
+        end
+      end
+
+  ## Behavior
+
+  - Returns `{:ok, true}` when actor is nil (allows action)
+  - Returns `{:ok, :unknown}` when actor is present (delegates to other policies)
+  - `auto_filter` returns nil (no filtering needed)
+  """
+
+  use Ash.Policy.Check
+
+  @impl true
+  def describe(_opts) do
+    "allows actions when no actor is present (for seeds and system operations)"
+  end
+
+  @impl true
+  def strict_check(actor, _authorizer, _opts) do
+    if is_nil(actor) do
+      # No actor present - allow (for seeds, tests, system operations)
+      {:ok, true}
+    else
+      # Actor present - let other policies decide
+      {:ok, :unknown}
+    end
+  end
+
+  @impl true
+  def auto_filter(_actor, _authorizer, _opts) do
+    # No filtering needed - this check only validates presence/absence of actor
+    nil
+  end
+end

mix.exs

@@ -76,6 +76,7 @@ defmodule Mv.MixProject do
       {:mix_audit, "~> 2.1", only: [:dev, :test], runtime: false},
       {:sobelow, "~> 0.14", only: [:dev, :test], runtime: false},
       {:credo, "~> 1.7", only: [:dev, :test], runtime: false},
+      {:picosat_elixir, "~> 0.1", only: [:dev, :test]},
       {:ecto_commons, "~> 0.3"},
       {:slugify, "~> 1.3"}
     ]

mix.lock

@@ -60,6 +60,7 @@
   "phoenix_pubsub": {:hex, :phoenix_pubsub, "2.2.0", "ff3a5616e1bed6804de7773b92cbccfc0b0f473faf1f63d7daf1206c7aeaaa6f", [:mix], [], "hexpm", "adc313a5bf7136039f63cfd9668fde73bba0765e0614cba80c06ac9460ff3e96"},
   "phoenix_template": {:hex, :phoenix_template, "1.0.4", "e2092c132f3b5e5b2d49c96695342eb36d0ed514c5b252a77048d5969330d639", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}], "hexpm", "2c0c81f0e5c6753faf5cca2f229c9709919aba34fab866d3bc05060c9c444206"},
   "phoenix_view": {:hex, :phoenix_view, "2.0.4", "b45c9d9cf15b3a1af5fb555c674b525391b6a1fe975f040fb4d913397b31abf4", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}], "hexpm", "4e992022ce14f31fe57335db27a28154afcc94e9983266835bb3040243eb620b"},
+  "picosat_elixir": {:hex, :picosat_elixir, "0.2.3", "bf326d0f179fbb3b706bb2c15fbc367dacfa2517157d090fdfc32edae004c597", [:make, :mix], [{:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "f76c9db2dec9d2561ffaa9be35f65403d53e984e8cd99c832383b7ab78c16c66"},
   "plug": {:hex, :plug, "1.19.1", "09bac17ae7a001a68ae393658aa23c7e38782be5c5c00c80be82901262c394c0", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "560a0017a8f6d5d30146916862aaf9300b7280063651dd7e532b8be168511e62"},
   "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"},
   "postgrex": {:hex, :postgrex, "0.21.1", "2c5cc830ec11e7a0067dd4d623c049b3ef807e9507a424985b8dcf921224cd88", [:mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "27d8d21c103c3cc68851b533ff99eef353e6a0ff98dc444ea751de43eb48bdac"},