From 3d753c5460726047fa9781ac8d7a71b2ca0f54c1 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 22 Jan 2026 23:04:56 +0100 Subject: [PATCH] Add authorize?: false to Actor.ensure_loaded SECURITY: Skip authorization for role loading to avoid circular dependency. Actor loads their OWN role, needed for authorization itself. Documented why this is safe. --- lib/mv/authorization/actor.ex | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) 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