Refactor: NoActor to SimpleCheck with compile-time environment check

This prevents security issues where :create/:read without actor would
be allowed in production. Now all operations require an actor in production.

lib/membership/member.ex

@@ -298,11 +298,12 @@ defmodule Mv.Membership.Member do
   # Authorization Policies
   # Order matters: Most specific policies first, then general permission check
   policies do
-    # SYSTEM OPERATIONS: Allow operations without actor (seeds, tests, system jobs)
+    # SYSTEM OPERATIONS: Allow CRUD operations without actor
-    # This must come first to allow database seeding and test fixtures
+    # In test: All operations allowed (for test fixtures)
-    # IMPORTANT: Use bypass so this short-circuits and doesn't require other policies
+    # In production: Only :create and :read allowed (enforced by NoActor.check)
+    # :read is needed for internal Ash lookups (e.g., relationship validation during user creation).
     bypass action_type([:create, :read, :update, :destroy]) do
-      description "Allow system operations without actor (seeds, tests)"
+      description "Allow system operations without actor (seeds, tests, internal lookups)"
       authorize_if Mv.Authorization.Checks.NoActor
     end
 

lib/mv/authorization/checks/no_actor.ex

@@ -2,22 +2,27 @@ defmodule Mv.Authorization.Checks.NoActor do
   @moduledoc """
   Custom Ash Policy Check that allows actions when no actor is present.
 
-  This is primarily used for:
+  **IMPORTANT:** This check ONLY works in test environment for security reasons.
-  - Database seeding (priv/repo/seeds.exs)
+  In production/dev, ALL operations without an actor are denied.
-  - 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
+  This check uses compile-time environment detection to prevent accidental
-  access is appropriate. It should always be combined with other policy
+  security issues in production. In production, ALL operations (including :create
-  checks that validate actor-based permissions when an actor IS present.
+  and :read) will be denied if no actor is present.
+
+  For seeds and system operations in production, use an admin actor instead:
+
+      admin_user = get_admin_user()
+      Ash.create!(resource, attrs, actor: admin_user)
 
   ## Usage in Policies
 
       policies do
-        # Allow seeding and system operations
+        # Allow system operations without actor (TEST ENVIRONMENT ONLY)
-        policy action_type(:create) do
+        # In test: All operations allowed
+        # In production: ALL operations denied (fail-closed)
+        bypass action_type([:create, :read, :update, :destroy]) do
           authorize_if NoActor
         end
         
@@ -29,32 +34,41 @@ defmodule Mv.Authorization.Checks.NoActor do
 
   ## Behavior
 
-  - Returns `{:ok, true}` when actor is nil (allows action)
+  - In test environment: Returns `true` when actor is nil (allows all operations)
-  - Returns `{:ok, :unknown}` when actor is present (delegates to other policies)
+  - In production/dev: Returns `false` when actor is nil (denies all operations - fail-closed)
-  - `auto_filter` returns nil (no filtering needed)
+  - Returns `false` when actor is present (delegates to other policies)
   """
 
-  use Ash.Policy.Check
+  use Ash.Policy.SimpleCheck
+
+  # Compile-time check: Only allow no-actor bypass in test environment
+  @allow_no_actor_bypass Mix.env() == :test
+  # Alternative (if you want to control via config):
+  # @allow_no_actor_bypass Application.compile_env(:mv, :allow_no_actor_bypass, false)
 
   @impl true
   def describe(_opts) do
-    "allows actions when no actor is present (for seeds and system operations)"
+    if @allow_no_actor_bypass do
-  end
+      "allows actions when no actor is present (test environment only)"
-
-  @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
+      "denies all actions when no actor is present (production/dev - fail-closed)"
-      {:ok, :unknown}
     end
   end
 
   @impl true
-  def auto_filter(_actor, _authorizer, _opts) do
+  def match?(nil, _context, _opts) do
-    # No filtering needed - this check only validates presence/absence of actor
+    # Actor is nil
-    nil
+    if @allow_no_actor_bypass do
+      # Test environment: Allow all operations
+      true
+    else
+      # Production/dev: Deny all operations (fail-closed for security)
+      false
+    end
+  end
+
+  def match?(_actor, _context, _opts) do
+    # Actor is present - don't match (let other policies decide)
+    false
   end
 end