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.
This commit is contained in:
Moritz 2026-01-22 23:04:56 +01:00 committed by Simon
parent 69836978be
commit 3d753c5460
Signed by: simon
GPG key ID: 40E7A58C4AA1EDB2

View file

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