Conversation
The `grants do ... end` block previously only lived on resources. Now it can also be declared on an `Ash.Domain` via the `AshGrant.Domain` extension, as an alternative or *complement* to the resource-level block — teams that prefer centralizing RBAC across a bounded context no longer need to repeat grants on every resource, while resources can still declare their own grants for local policies. Resource and domain grants merge in `AshGrant.Info.grants/1` (resource wins on grant-name conflicts, same policy as `scope` inheritance). A new domain-level transformer synthesizes the `GrantsResolver` when the domain has grants, and a new domain-level verifier reuses the extracted reference-validation helper to check `on:`/action/scope at compile time. Resource verifier also now recognizes scopes inherited from the domain when verifying a resource's own grants — a closed gap that came up while adding domain tests.
…grant merge
Review feedback on the domain-level grants DSL:
- Domain-level `permission` now takes the target resource as a required
*second positional* argument instead of the `on:` keyword:
# Before
permission :manage_posts, :*, :always, on: MyApp.Blog.Post
# After
permission :manage_posts, MyApp.Blog.Post, :*, :always
Reads more directly when the target is always explicit (as it must be on
a domain) and mirrors how the resource reads when you ignore `on:`.
Implemented as a distinct domain-only entity (`@domain_permission`) with
`args: [:name, :on, :action, :scope]`. The resource-level `@permission`
is unchanged — it keeps the 3-arg form with optional `on:` keyword for
cross-resource cases. One entity per level avoids the
"optional-positional-in-the-middle" corner of Spark's macro expansion.
`instance` remains a keyword option in both (hardcoded instance IDs are
rare; dynamic ones use a resolver).
- `AshGrant.Info.merge_domain_grants/2` now uses `Enum.uniq_by(list, & &1.name)`
instead of building a name MapSet and rejecting. `MapSet.new/1` dedupes
by struct equality (wouldn't dedupe by `:name`); `MapSet.new/2` with a
transform builds a set of names but loses the original structs.
`Enum.uniq_by/2` is the right primitive for "dedupe by key, keep first" —
one call, no intermediate set.
Today every `permission` has to name a scope. Teams that want a simple
unrestricted grant (`permission :read, User, :read`) have to declare
`scope :always, true` and pass `:always` everywhere.
The permission-string core and the `Check` write path already treated
`nil` scope as "no row filter". The gap was on the read path and the DSL.
This closes both.
Runtime:
- `Evaluator.get_all_scopes/4` stops dropping `nil` scopes so consumers
can decide what to do with them.
- `FilterCheck.build_filter_with_instances/6` and
`CanPerform.build_rbac_expression/3` short-circuit to `true` when any
matching permission has `nil` scope, same as `"always"`/`"all"`/`"global"`.
DSL:
- `@permission` (resource) and `@domain_permission` (domain) now take
`{:optional, :scope, nil}` in `args` and `required: false, default: nil`
in the schema. `permission :read_user, :read` and
`permission :read_user, User, :read` are now valid.
- `ValidateGrantReferences` skips the scope lookup when `scope == nil`.
- `GrantsResolver.stringify/1` renders `nil` scope as an empty trailing
segment (`"post:*:read:"`), which `Permission.parse/1` round-trips to
`scope: nil` via the existing 4-part parser.
UX guardrail:
- With optional scope, `permission :name, :read, :always` at the domain
level silently binds `on: :read` (a plain atom). The verifier now
rejects non-module atoms with an explicit "is not a module — did you
forget the target?" message instead of falling through.
Docs:
- README quick-start now leads with the no-scope form and only declares
the scopes grants actually reference (no more ceremonial
`scope :always, true`).
- CLAUDE.md's permission-format section notes that the trailing scope
segment is optional.
Tests:
- `AshGrant.Evaluator.get_all_scopes/4` now pins the nil-preserved list.
- New `AshGrant.OptionalScopeTest` covers DSL parsing, the resolver
round-trip (emit → parse → `scope: nil`), and the end-to-end
`has_access?` path for both resource- and domain-level declarations.
- `AshGrant.DomainGrantsTest` updated to check the new non-module-atom
verifier warning instead of the old arity-error path (which optional
scope removed).
All 1126 tests pass. Credo + dialyzer unchanged (only pre-existing
warnings remain).
Previously, declaring both `grants do ... end` and an explicit `resolver` on the same resource (or domain) was a compile error. Some cases need both — static RBAC + ABAC belong in declarative grants, but dynamic per-row permissions (e.g. DB-backed sharing) need a resolver function. `AshGrant.GrantsResolver` now evaluates declared grants *and* calls any user-declared resolver, concatenating the permission lists. Deny-wins in `AshGrant.Evaluator` continues to hold: a deny from either source overrides an allow from either source. Changes: - Delete `AshGrant.Transformers.SynthesizeGrantsResolver` and `AshGrant.Domain.Transformers.SynthesizeGrantsResolver`. They used to set the `:resolver` option to `AshGrant.GrantsResolver`, overwriting any user-declared resolver. Synthesis now happens at read time in `AshGrant.Info.resolver/1` — if any grants are declared (resource or domain), it returns `GrantsResolver`; otherwise the user's explicit `:resolver` flows through unchanged. - `AshGrant.Info.raw_resolver/1` (new) returns the user's explicit resolver without the synthesis, used by `GrantsResolver.resolve/2` to avoid a loop. - `AshGrant.GrantsResolver.resolve/2` is now a two-step call: evaluate declared grants, then call the user resolver (if any) and concat. User resolver errors are logged and treated as empty so a raising callback never breaks the grants side. - Drop the "both declared → DslError" check in `AshGrant.Transformers.NormalizeGrants`. - `AshGrant.Verifiers.ValidateResolverPresent` and `AshGrant.Verifiers.ValidateScopes` now treat declared grants (on the resource or inherited from the domain) as a valid source, so a grants-only resource no longer trips the "no resolver" warning. - Closed the "resource has custom `resolver` + domain has `grants`" gap I had documented as a known quirk last PR. Domain grants now apply on top of the resource's custom resolver — the test that pinned the old shadowing behavior is flipped to pin the new merge behavior. Tests: - Resource- and domain-level "rejects declaring both" tests replaced with positive "accepts both and merges" tests covering admin / dynamic / no-match actors. - `Domain.Info.resolver/1` no longer returns `GrantsResolver` after synthesis — the assertion now uses `Info.resolver/1` on a resource in the domain instead, which reflects the user-facing contract. Full suite: 1126 tests, 0 failures. Credo + format clean.
|
Minor: |
Previously, every domain-level `permission` had to name its target
resource as a positional argument (`permission :x, MyApp.Post, :read,
:always`). That doesn't match how `Ash.Policy.Authorizer` works at the
domain level — domain policies cover every resource/action unless
narrowed with `policy resource_is(...)`. The grants DSL now follows the
same model.
Domain grants are now declared with the **resource-level shape**:
grants do
# Broadcast — applies to every resource in the domain
grant :admin, expr(^actor(:role) == :admin) do
permission :manage_all, :*, :always
end
# Scoped to one resource — Ash's `policy resource_is/1` analog
grant :auditor, expr(^actor(:role) == :auditor) do
permission :audit_posts, :read, :always, on: MyApp.Blog.Post
end
end
`AshGrant.GrantsResolver` substitutes `context.resource` at runtime
when `permission.on == nil`, so a single domain permission lights up
every resource the domain owns. Use the `on:` keyword to scope a
domain permission to a single resource.
Implementation:
- Delete the separate `@domain_permission`, `@domain_grant`,
`@domain_grants` entities and the `domain_grants_section/0` helper
in `lib/ash_grant/dsl.ex`. Domains now use the same `@permission` /
`@grant` / `@grants` entities as resources via `grants_section/0`.
- `AshGrant.Domain.Dsl` swaps `domain_grants_section()` for
`grants_section()`.
- `AshGrant.GrantsResolver.to_permission_string/2` takes the current
resource and substitutes it when `permission.on == nil`.
- `AshGrant.Verifiers.GrantReferences.validate_permission/5` skips the
resource/action/scope reference checks when `permission.on == nil`
(the target isn't known until runtime). Still validates everything
when `on:` is specified.
Tests:
- `test/support/grants_only_domain.ex` rewritten to use broadcast
permissions plus one resource-scoped permission for coverage.
- `test/ash_grant/domain_grants_test.exs` reorganized around three
buckets: broadcast applies to every resource, scoped (`on:`) applies
only to its target, and the merge / override / custom-resolver
behaviour from earlier PRs.
- New compile-time test pins that `permission :name, :action, :scope`
with no `on:` compiles cleanly at the domain level.
- `test/ash_grant/optional_scope_test.exs` updated for the new domain
syntax.
README quick-start and `AshGrant.Domain` @moduledoc rewritten to lead
with the broadcast example. Full suite: 1127 tests, 0 failures. Credo
+ format clean.
Cross-resource permissions via `on:` were the last source of mental overhead in the grants DSL. With domain grants now broadcast by default, the `on:` knob is redundant: if a permission targets resource R, you declare it inside R's grants block. End of story. Removed: - The `on:` schema option on `@permission`. Spark now rejects `permission :name, :action, :scope, on: SomeResource` at parse time. - The `looks_like_module?/1` heuristic in `AshGrant.Verifiers.GrantReferences`. It was a workaround from when `:on` was the required second positional arg — typing `permission :name, :read, :always` would silently bind `:read` to `:on`, so the verifier needed a "did you forget the target?" probe. Without a user-facing `:on`, that whole branch is dead code. - The cross-resource branches in `AshGrant.Verifiers.GrantReferences` (target != caller, ensure external resource is loaded, look up its scopes/actions). Permissions now point at exactly one of: nil (domain broadcast — skip checks) or the enclosing module (resource-level after `NormalizeGrants`). Both cases reduce to a flat `local_actions` / `local_scopes` lookup. - The `:auditor` grant from the test fixture (it used the scoped `on: GrantsDomainPost` form) and the four "warns when on: …" tests in `domain_grants_test.exs`. Reference checks for resource-level grants are still covered by `grants_dsl_test.exs`. Updated docs (README, `AshGrant.Domain` @moduledoc, `AshGrant.Domain.Dsl` @moduledoc, `@permission` and `@grants` describes) to lead with the "location of the grant is what scopes it" model. Internally `AshGrant.Dsl.Permission` keeps an `:on` field — `NormalizeGrants` sets it at the resource level, and the resolver/verifier use it — but it's no longer part of the public surface. Full suite: 1121 tests, 0 failures. Credo + format clean.
|
|
||
| defp build_rbac_expression(scopes, scope_resolver, resource) do | ||
| if "always" in scopes or "all" in scopes or "global" in scopes do | ||
| # `nil` scope = permission declared without a scope = unrestricted, same |
There was a problem hiding this comment.
We should not have special logic for always / all / global
| # Check for global access from RBAC. `nil` represents a permission | ||
| # declared without a scope — it means "no row filter" (same as the | ||
| # `Check` write path, which treats nil scope as `true`). | ||
| has_global_access = |
There was a problem hiding this comment.
Same here. no special logic for always / all / global
| @@ -0,0 +1,30 @@ | |||
| defmodule AshGrant.Domain.Verifiers.ValidateGrantReferences do | |||
There was a problem hiding this comment.
Is this now dead code after the last refactor?
|
|
||
| case permission.on do | ||
| nil -> | ||
| # Domain-level broadcast — target unknown until runtime. |
There was a problem hiding this comment.
why would that not be known until runtime, we have all the resources defined in the domain
| end) | ||
| end | ||
|
|
||
| defp resolve_user(actor, resource, context) do |
There was a problem hiding this comment.
why is this called resolve_user? I find the user confusing
Addresses PR review feedback from @barnabasJ and @C-Sinclair. **No more `:always` / `:all` / `:global` special-casing.** `FilterCheck`, `CanPerform`, and `Check` all had a hardcoded "if this scope name is one of these strings, treat as global" shortcut. Replaced with the single `nil in scopes` test — the only sentinel for "no row filter." Named scopes — including legacy `:always` — go through the regular scope-resolver path: declare `scope :always, true` (or any name) and the resolver returns `true`, which the filter combinator collapses to "no constraint." Same primitive, no magic strings. **Domain verifier earns its keep.** Comment correctly pointed out we *do* know all the domain's resources at compile time. Rewrote `AshGrant.Domain.Verifiers.ValidateGrantReferences` to validate every broadcast permission against every resource declared in the domain's `resources do` block: - `action: :foo` must be defined on every resource (or `:*`). - `scope: :foo` must resolve on every resource — declared locally or inherited from this domain (uses `AshGrant.Info.scopes/1` so the inheritance merge applies). Resources that aren't loaded yet at verifier time (the typical domain↔resource compile dance) are skipped silently; the resource's own verifier still catches its grants when it compiles. Two new compile-warning tests pin the action and scope cases. **Renamed `resolve_user` / `call_user_resolver`.** "User" was overloaded — it shows up everywhere as the actor. Renamed to `resolve_via_resolver` / `call_resolver`, which says what the function does without conflating it with the actor. **Unified `merge_domain_scopes/2`.** `merge_domain_grants/2` already used `Enum.uniq_by(list ++ domain, & &1.name)`; scopes were still on the older `MapSet.new/2 + Enum.reject` pattern. Same job, two idioms — folded both onto `Enum.uniq_by/2`. Full suite: 1123 tests, 0 failures. Credo dropped one refactoring opportunity (10 → 9) thanks to the scopes simplification. Format clean.
Summary
AshGrant.Domainextension to acceptgrants do ... endalongside the existingresolverandscopesupport. Teams that want to centralize RBAC on the domain can now do so instead of — or in addition to — declaring grants on each resource.AshGrant.Info.grants/1; on grant-name conflicts, the resource wins (same override policyscopeuses).SynthesizeGrantsResolvertransformer wires the domain's resolver toAshGrant.GrantsResolverwhen the domain has grants, and enforces mutual exclusivity with an explicit domainresolver.ValidateGrantReferencesverifier reuses the extracted reference-validation helper to check that each permission'son:/action:/scope:resolves at compile time.What changed
New API surface
AshGrant.Domain.Dslgains agrantssection (reusesAshGrant.Dsl.grants_section/0).AshGrant.Domain.Infogainsgrants/1,get_grant/2,permissions/1.Runtime merge
AshGrant.Info.grants/1reads resource entities and appends any domain grants whose.nameisn't already on the resource (mirror ofmerge_domain_scopes/2).Compile-time
GrantsResolverwhen grants are declared; errors if an explicitresolveris also set.on:is anAsh.Resource,action:exists on the target,scope:is defined on the target (or inherited from its domain).AshGrant.Verifiers.GrantReferencesso resource and domain verifiers don't duplicate.Interaction summary (from
AshGrant.Domain@moduledoc)GrantsResolver) + grantsresolverresolverTest plan
mix test test/ash_grant/domain_grants_test.exs— 27 new tests covering domain-only flow, resource+domain merge, same-name override, cross-resource grants, custom resolver shadowing, and compile-time verification (mutual exclusivity raises, reference-validation warnings captured viacapture_io).mix test— full suite, 1116 tests, 0 failures.mix credo— no new issues (10 pre-existing refactoring opportunities, all unrelated).mix dialyzer— no new warnings (pre-existingMix.*+info.ex:414warnings unchanged).Reviewer notes
scope/resolverpattern and avoids the known compile-time cycle between domains withcode_interfaceentries and their resources. See the @moduledoc onAshGrant.Domain.InfoandAshGrant.Info.grants/1for the rationale.capture_io(:stderr, ...)rather thanassert_raise, following the note inValidateResolverPresent's @moduledoc. The one mutual-exclusivity test does useassert_raisebecause that check lives in a transformer.