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:
parent
e60bb6926f
commit
f3abade7ad
1 changed files with 18 additions and 5 deletions
|
|
@ -26,9 +26,17 @@ defmodule Mv.Authorization.Actor do
|
||||||
|
|
||||||
## Security Note
|
## Security Note
|
||||||
|
|
||||||
`ensure_loaded/1` loads the role with `authorize?: true` (default).
|
`ensure_loaded/1` loads the role with `authorize?: false` to avoid circular
|
||||||
The Role resource must have policies that allow an actor to read their own role.
|
dependency (actor needs role loaded to be authorized, but loading role requires
|
||||||
See `Mv.Authorization.Checks.HasPermission` for the fallback implementation.
|
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
|
require Logger
|
||||||
|
|
@ -71,8 +79,13 @@ defmodule Mv.Authorization.Actor do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp load_role(actor) do
|
defp load_role(actor) do
|
||||||
# Need to specify domain for Ash.load to work
|
# SECURITY: We skip authorization here because this is a bootstrap scenario:
|
||||||
case Ash.load(actor, :role, domain: Mv.Accounts) do
|
# - 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} ->
|
{:ok, loaded_actor} ->
|
||||||
loaded_actor
|
loaded_actor
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue