diff --git a/lib/mv/authorization/actor.ex b/lib/mv/authorization/actor.ex index 5d337e8..ae5c019 100644 --- a/lib/mv/authorization/actor.ex +++ b/lib/mv/authorization/actor.ex @@ -26,9 +26,17 @@ defmodule Mv.Authorization.Actor do ## Security Note - `ensure_loaded/1` loads the role with `authorize?: true` (default). - The Role resource must have policies that allow an actor to read their own role. - See `Mv.Authorization.Checks.HasPermission` for the fallback implementation. + `ensure_loaded/1` loads the role with `authorize?: false` to avoid circular + 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) + - 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) + + Alternative would be to denormalize permission_set_name on User, but that + adds complexity and potential for inconsistency. """ require Logger @@ -71,8 +79,13 @@ defmodule Mv.Authorization.Actor do end defp load_role(actor) do - # Need to specify domain for Ash.load to work - case Ash.load(actor, :role, domain: Mv.Accounts) do + # SECURITY: We skip authorization here because this is a bootstrap scenario: + # - The actor is loading their OWN role (actor.role relationship) + # - This load is needed FOR authorization checks to work (circular dependency) + # - The role itself contains no sensitive data (just permission_set reference) + # - The actor is already authenticated (passed auth boundary) + # Alternative would be to denormalize permission_set_name on User. + case Ash.load(actor, :role, domain: Mv.Accounts, authorize?: false) do {:ok, loaded_actor} -> loaded_actor