Review cleanups, no behavior change to the authorization model.
* Normalize the trusted-actor bypass to `MapScoped.trusted()` in all 18
resources. Half previously used the bare `MapScoped.Trusted` module and half
the helper. The blocks are meant to be scanned as one repeated template, so
an inconsistent first line defeats the point.
* Drop the `WriteDirect` check. After update/destroy moved to filter checks it
had a single caller, `MapDefaultSettings.create`, where it is equivalent to
`create_map_matches_token/0`: map_id is `allow_nil? false` with no
InjectMapFromActor, so the "absent map_id" branch is unreachable and an
omitted map_id fails validation before persisting. Removes ~35 lines and one
concept from the policy module. Comments in map.ex / map_system.ex /
map_connection.ex that referenced it by name were updated.
* Document why CreateMapMatchesToken reads params before the attribute and
checks both key types. This is the check enforcing the foreign-map_id
breaking change, and the ordering is load-bearing: InjectMapFromActor
overwrites the attribute with the token's map before policies run, so
reading only the attribute would compare the token map against itself and
authorize every foreign map_id.
The map-duplication fixes that were previously bundled here have moved to
their own branch (fix/map-duplication-attrs) -- they are unrelated to /api/v1
scoping and fix a bug that reproduces on main.
Two guards closing out the authz work.
internal_authz_regression_test.exs exercises the real internal call sites
that pass an `actor:`, so tightening a policy later cannot silently break the
app's own paths. Each test names the production site it mirrors.
Two corrections to the planned tests, both from reading the callers:
* Map.duplicate is invoked with conn.assigns[:current_character], a
Character (map_api_controller.ex:1276), and the :duplicate action sets
owner_id from context.actor.id. Map.owner is a belongs_to Character, so a
User actor would write a User id into a Character FK. The test uses a
Character and asserts owner_id comes back correct.
* :available is never called with a Character. Its only callers
(Maps.get_available_maps/1, Acls.get_available_acls/1) pass a User, and
FilterMapsByRoles reads actor.characters, which a Character struct lacks.
The suite reloads the User with `load: :characters` the way
CheckJsonApiAuth does, and covers the Character bypass via the write and
duplicate tests instead.
authz_includes_leak_test.exs covers `include=`, the one path that reaches a
resource without going through its own route. Every refutation is paired
with a positive control so it cannot pass vacuously.
Note Character is NOT an AshJsonApi resource (extensions: [AshCloak] only),
so ?include=owner emits a resource identifier with "type": null and an empty
attributes object -- no Character field is serialized at all. The negative
assertion is therefore about sensitive attributes being absent rather than
about the type name, which the planned test would have asserted and which
would never have matched.
MapSystem's primary :read action has its own always-on
FilterSystemsByActorMap preparation, which independently scopes every
read to the actor's map. The prior InTokenMap filter/3 test round-tripped
through that action, so it measured the preparation rather than the
check under test — stubbing InTokenMap.build_filter/2 to expr(true)
left the suite green. Assert directly on the raw filter/3 return value
instead. Verified by actually stubbing build_filter/2 to always-match,
confirming the new assertion fails, then reverting.
InTokenMap/ParentInTokenMap called Ash.Expr.ref/2 with reversed args
and, even fixed, dynamic relationship-path refs don't resolve through
filter hydration. Build filters as nested keyword lists instead (a
form FilterCheck.filter/3 accepts directly and Ash resolves natively
through relationships). Also fixes CreateParentInTokenMap's internal
parent lookup, which silently returned no rows without an actor due
to MapSystem's actor-scoped read preparation. Adds direct filter/3
tests against real queries/DB rows for both FilterCheck modules plus
positive/negative CreateParentInTokenMap coverage.
Guarzo