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).
This commit is contained in:
Moritz 2026-01-22 23:17:55 +01:00 committed by Simon
parent 726f164b28
commit ef4df57a6f
Signed by: simon
GPG key ID: 40E7A58C4AA1EDB2
2 changed files with 20 additions and 23 deletions

View file

@ -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)

View file

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