Policies for updates can generate invalid ecto code when loading other resources
Code of Conduct
- [x] I agree to follow this project's Code of Conduct
AI Policy
- [x] I agree to follow this project's AI Policy, or I agree that AI was not used while creating this issue.
Versions
When upgrading from 3.5.23 to 3.5.34
Operating system
ubuntu 24, macos
Current Behavior
I have a policy that requires loading a related resource to check some conditions. This generates an invalid ecto query that tries to insert a subquery into the from of an update_all query.
In our case:
We have some resource A, eg Organisations, that has a has_many relationship with some other resource B, eg UserOrganisations,
Previously we had a policy that uses an expr( ) filter to check if an actor is an admin;
policy action(:update) do
authorize_if expr(exists(user_organisations, user_id == ^actor(:id) and role == "admin"))
end
When bumping ash, this seems to generate invalid ecto-- it tries to insert a subquery in the from of an update_all
* ** (Ecto.QueryError) `update_all` does not allow subqueries in `from` in query:
from o0 in subquery(from o0 in Accounts.Organisation,
as: 0,
left_join: u1 in subquery(from u0 in Accounts.UserOrganisation,
as: 500,
where: is_nil(
type(
as(500).archived_at,
{:parameterized,
{Ash.Type.UtcDatetimeUsec.EctoType,
precision: :microsecond, cast_dates_as: :start_of_day, timezone: :utc}}
)
),
For us this also occurs in a similar way with a forbid_unless statement
forbid_unless relates_to_actor_via([:user_organisation, :user])
where the user_organisation again is generating a subquery.
This is the only similar kinda situation with ecto:
https://groups.google.com/g/elixir-ecto/c/4rjWQoyYhmI/m/9Vz14hUfAgAJ
Reproduction
Create a policy on a resource that requires loading another resource. So a resource A with has_many B. Have a policy that tries to do something on A depending on some property in B, eg if B is an admin.
Expected Behavior
Ash should not generate invalid ecto code or at least fail to compile? If it tries to insert a subquery in from, should that not become a seperate query before somehow?
Would it be possible @StrongFennecs to make a reproduction repository for this?
Yeah I think a reproduction will be necessary. I will revisit once a reproduction exists (if @ken-kost doesn't get to it first 😉 )
@ken-kost your repro was against ash core but this would only happen on ash_postgres tests.
@ken-kost your repro was against ash core but this would only happen on ash_postgres tests.
Okay, so I made a test on ash_postgres.
Only on post with author named John can this action be called.
in Post:
policy action(:update_for_john) do
authorize_if(expr(exists(author, first_name == "John" and id == ^actor(:id))))
end
made a :update_for_john action and test is:
author =
AshPostgres.Test.Author
|> Ash.Changeset.for_create(:create, %{first_name: "is", last_name: "match"})
|> Ash.create!()
post =
AshPostgres.Test.Post
|> Ash.Changeset.for_create(:create, %{title: "match"})
|> Ash.Changeset.manage_relationship(:author, author, type: :append_and_remove)
|> Ash.create!()
assert {:error, %Ash.Error.Forbidden{}} =
post
|> Ash.Changeset.for_update(:update_for_john, %{}, authorize?: true, actor: author)
|> Ash.update()
john =
AshPostgres.Test.Author
|> Ash.Changeset.for_create(:create, %{first_name: "John", last_name: "match"})
|> Ash.create!()
johns_post =
AshPostgres.Test.Post
|> Ash.Changeset.for_create(:create, %{title: "match"})
|> Ash.Changeset.manage_relationship(:author, john, type: :append_and_remove)
|> Ash.create!()
|> Ash.load!(:author)
{:ok, _johns_post} =
johns_post
|> Ash.Changeset.for_update(:update_for_john, %{title: "John's match"},
authorize?: true,
actor: john
)
|> Ash.update()
and it passes. I also tried without exists and that also works.
Awesome thanks for that. In that case we'll need a reproduction from the original poster :)
Sure, yes looks like the issue is actually bumping ash postgres from 2.6.8 to 2.6.15 or 2.6.16 actually and not ash - ash can be bumped fine.
I think theres probably a few more authorisations involved with our example, Ill have to go try come up with a example at some point later this week.
Seems like adding require_atomic? false in corresponding locations where update is called that triggers the authorisation fixed it in our tests -- So possible more like a bad error message?
Unlikely, I think it just sends you down a different code path. We should fix the original underlying bug.