Skip to content

Conversation

@zackattackz
Copy link
Contributor

Prior to this commit, replace_refs in the policy authorizer did not handle a case for unrelated exists expressions (no :path or :at_path).

Notes

Path handling in replace_refs

When recursively calling replace_refs on an unrelated exists, I ignored the path for the new stack entry, passing an empty list:

replace_refs(expr, %{
  acc
  | stack: [{resource, [], action, domain} | acc.stack]
})

The reasoning is that since this is a "new" level of the expression, starting at resource, we do not care about the current path in the stack.

The second test "filter_input with nested unrelated exists respects field level authorization" attempts to cover this case, though messing with different values for path at this point all seemed to work anyways?

All to say I'm not too sure about this, and would suggest closer review.

Default read action

We use the default read action here, since there is no existing way to pass in a custom action for an unrelated exists. This is in line with existing behavior at some other places like here and here

Stack Trace

An example of the stack trace produced by attempting to filter in this way prior to this commit:

** (Ash.Error.Unknown) 
     Bread Crumbs:
       > Exception raised in: Ash.Test.Resource.UnrelatedExistsTest.User.read

     Unknown Error

     * ** (FunctionClauseError) no function clause matching in :lists.droplast/1
       (stdlib 7.1) lists.erl:441: :lists.droplast([])
       (ash 3.15.0) lib/ash/can.ex:555: Ash.Can.alter_query/5
       (ash 3.15.0) lib/ash.ex:1824: Ash.can/3
       (ash 3.15.0) lib/ash/actions/read/read.ex:88: Ash.Actions.Read.run/3
       (ash 3.15.0) lib/ash.ex:2790: Ash.read/2
       (ash 3.15.0) lib/ash.ex:2725: Ash.read!/2
       test/resource/unrelated_exists_test.exs:466: Ash.Test.Resource.UnrelatedExistsTest."test unrelated exists with filter_input filter_input with nested unrelated exists respects field level authorization"/1
       (ex_unit 1.19.0) lib/ex_unit/runner.ex:528: ExUnit.Runner.exec_test/2
       (stdlib 7.1) timer.erl:599: :timer.tc/2
       (ex_unit 1.19.0) lib/ex_unit/runner.ex:450: anonymous fn/6 in ExUnit.Runner.spawn_test_monitor/4
     code: |> Ash.read!(actor: user1, authorize?: true)
     stacktrace:
       (stdlib 7.1) lists.erl:441: :lists.droplast([])
       (ash 3.15.0) lib/ash/policy/authorizer/authorizer.ex:1078: Ash.Policy.Authorizer.related_with_action/2
       (ash 3.15.0) lib/ash/policy/authorizer/authorizer.ex:1009: Ash.Policy.Authorizer.replace_refs/2
       (ash 3.15.0) lib/ash/policy/authorizer/authorizer.ex:975: Ash.Policy.Authorizer.replace_refs/2
       (ash 3.15.0) lib/ash/policy/authorizer/authorizer.ex:1012: Ash.Policy.Authorizer.replace_refs/2
       (ash 3.15.0) lib/ash/policy/authorizer/authorizer.ex:730: Ash.Policy.Authorizer.alter_filter/3
       (ash 3.15.0) lib/ash/can.ex:555: Ash.Can.alter_query/5
       (elixir 1.19.0) lib/enum.ex:2520: Enum."-reduce/3-lists^foldl/2-0-"/3
       (ash 3.15.0) lib/ash.ex:1824: Ash.can/3
       (ash 3.15.0) lib/ash/actions/read/read.ex:2124: Ash.Actions.Read.authorize_query/2
       (ash 3.15.0) lib/ash/actions/read/read.ex:586: Ash.Actions.Read.do_read/5
       (ash 3.15.0) lib/ash/actions/read/read.ex:435: Ash.Actions.Read.do_run/3
       (ash 3.15.0) lib/ash/actions/read/read.ex:89: anonymous fn/3 in Ash.Actions.Read.run/3
       (ash 3.15.0) lib/ash/actions/read/read.ex:88: Ash.Actions.Read.run/3
       (ash 3.15.0) lib/ash.ex:2790: Ash.read/2
       (ash 3.15.0) lib/ash.ex:2725: Ash.read!/2
       test/resource/unrelated_exists_test.exs:466: (test)

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@zackattackz
Copy link
Contributor Author

Actually instead of using domain, would it make more sense to use Ash.Resource.Info.domain(resource) ?


%Ash.Query.Exists{expr: expr, resource: resource, related?: false} = exists ->
[{_, _, _, domain} | _] = acc.stack
action = Ash.Resource.Info.primary_action!(resource, :read)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add Ash.Resource.Info.domain(resource) || domain.

Otherwise looks good 🤔

| stack: [{resource, [], action, domain} | acc.stack]
})

{%{exists | expr: expr}, %{acc | stack: tl(acc.stack)}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you can just do original_stack, no need to use the new stack and do tl(acc.stack)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Force pushed an update to to this + above + rebased over main.

Also I applied the original_stack suggestion to the other Exists case, to remove the tl(acc.stack) from there as well just to be consistent. If you prefer to leave that I can change it back though.

Prior to this commit, replace_refs in the policy authorizer did not handle a case for unrelated exists expressions (no :path or :at_path).
@zackattackz zackattackz force-pushed the fix/unrel-exists-auth-case branch from c89815c to 9bf7fda Compare February 8, 2026 21:38
@zachdaniel zachdaniel merged commit cb0dbc5 into ash-project:main Feb 8, 2026
45 checks passed
@zachdaniel
Copy link
Contributor

🚀 Thank you for your contribution! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants