From ef4df57a6faecfbda47a5c896a8f048424d9a37b Mon Sep 17 00:00:00 2001
From: Moritz <moritz.m@local-it.org>
Date: Thu, 22 Jan 2026 23:17:55 +0100
Subject: [PATCH] Restrict Actor.ensure_loaded to Mv.Accounts.User only

Pattern match on %Mv.Accounts.User{} instead of generic actor.
Clearer intention, prevents accidental authorization bypasses.
Non-User actors are returned as-is (no-op).
---
 lib/mv/authorization/actor.ex        | 31 +++++++++++++---------------
 test/mv/authorization/actor_test.exs | 12 +++++------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/lib/mv/authorization/actor.ex b/lib/mv/authorization/actor.ex
index ae5c019..3482043 100644
--- a/lib/mv/authorization/actor.ex
+++ b/lib/mv/authorization/actor.ex
@@ -1,10 +1,10 @@
 defmodule Mv.Authorization.Actor do
   @moduledoc """
-  Helper functions for ensuring actors have required data loaded.
+  Helper functions for ensuring User actors have required data loaded.
 
   ## Actor Invariant
 
-  Authorization policies (especially HasPermission) require that the actor
+  Authorization policies (especially HasPermission) require that the User actor
   has their `:role` relationship loaded. This module provides helpers to
   ensure this invariant is maintained across all entry points:
 
@@ -13,6 +13,12 @@ defmodule Mv.Authorization.Actor do
   - Background jobs
   - Tests
 
+  ## Scope
+
+  This module ONLY handles `Mv.Accounts.User` resources. Other resources with
+  a `:role` field are ignored (returned as-is). This prevents accidental
+  authorization bypasses and keeps the logic focused.
+
   ## Usage
 
       # In LiveView on_mount
@@ -30,7 +36,7 @@ defmodule Mv.Authorization.Actor do
   dependency (actor needs role loaded to be authorized, but loading role requires
   authorization). This is safe because:
 
-  - The actor is loading their OWN role (actor.role relationship)
+  - The actor (User) is loading their OWN role (user.role relationship)
   - This load is needed FOR authorization checks to work
   - The role itself contains no sensitive data (just permission_set reference)
   - The actor is already authenticated (passed auth boundary)
@@ -42,11 +48,12 @@ defmodule Mv.Authorization.Actor do
   require Logger
 
   @doc """
-  Ensures the actor has their `:role` relationship loaded.
+  Ensures the actor (User) has their `:role` relationship loaded.
 
   - If actor is nil, returns nil
   - If role is already loaded, returns actor as-is
   - If role is %Ash.NotLoaded{}, loads it and returns updated actor
+  - If actor is not a User, returns as-is (no-op)
 
   ## Examples
 
@@ -61,23 +68,13 @@ defmodule Mv.Authorization.Actor do
   """
   def ensure_loaded(nil), do: nil
 
-  def ensure_loaded(%{role: %Ash.NotLoaded{}} = actor) do
-    # Only attempt to load if actor is a valid Ash resource
-    if ash_resource?(actor) do
-      load_role(actor)
-    else
-      # Not an Ash resource (e.g., plain map in tests) - return as-is
-      actor
-    end
+  # Only handle Mv.Accounts.User - clear intention, no accidental other resources
+  def ensure_loaded(%Mv.Accounts.User{role: %Ash.NotLoaded{}} = user) do
+    load_role(user)
   end
 
   def ensure_loaded(actor), do: actor
 
-  # Check if actor is a valid Ash resource
-  defp ash_resource?(actor) do
-    is_struct(actor) and Ash.Resource.Info.resource?(actor.__struct__)
-  end
-
   defp load_role(actor) do
     # SECURITY: We skip authorization here because this is a bootstrap scenario:
     # - The actor is loading their OWN role (actor.role relationship)
diff --git a/test/mv/authorization/actor_test.exs b/test/mv/authorization/actor_test.exs
index 9c83bd3..e542301 100644
--- a/test/mv/authorization/actor_test.exs
+++ b/test/mv/authorization/actor_test.exs
@@ -72,13 +72,13 @@ defmodule Mv.Authorization.ActorTest do
       assert result.role.id == role.id
     end
 
-    test "handles load errors gracefully (returns original actor)" do
-      # Create a plain map (not a real Ash resource)
-      fake_actor = %{id: "fake", role: %Ash.NotLoaded{field: :role}}
+    test "returns non-User actors as-is (no-op)" do
+      # Create a plain map (not Mv.Accounts.User)
+      other_actor = %{id: "fake", role: %Ash.NotLoaded{field: :role}}
 
-      # Should not crash, returns original
-      result = Actor.ensure_loaded(fake_actor)
-      assert result == fake_actor
+      # Should return as-is (pattern match doesn't apply to non-User)
+      result = Actor.ensure_loaded(other_actor)
+      assert result == other_actor
     end
   end
 end