get selected fields in create/update action
Hi, I have this policy to check if a user is only selecting fields that they are allowed to see. This worked well for reads because it was possible to get the selected/loaded fields from the query. Is it possible to do something similar for
mutations
38 Replies
🤔 currently, not really. You can get the selected fields
because there is a
select
option on changeset
but we would need to add load
to changeset as well
which would make sense to do 🙂
Let me take a lookI thought I read somewhere in the docs that it's not possible to load stuff with
mutations
. But its been a while so not sureYeah, there is a way to load them but its done with
api.load
after the initial create
I'll give you the first sneak preview of what I'm working on this week to solve for this need
It will come w/ a similar thing to
%Ash.NotLoaded{}
called %Ash.ForbiddenField{}
and it will resolve that field to a forbidden error in gql (the entire request won't be forbidden, but the fields you can't see will have a forbidden error in their resolution
The other aspect of this though is that we will actually deprecate loading?
checks in policies
You can still use selecting
, but everything related to can you load X should be done as a field policyFortunately I never used loading?, I did it all with this extension so far. But until now it was almost exlcusivly reads.
Yeah, there is an issue w/ that though
specifically we don't rerun a read action on
api.load
all of our authorization is done as "row level"
so if you can see a row, it lets you api.load
anything on it.
AshGraphql
first gets the record and uses api.load
meaning your policies might not be doing what you think they are doing 😢But the query for the first resource would still be getting the
load
list in the query right? If so I would already check for this on the first resource, if not I do have policies on all the related resources as well so I should be good for now. But definitly something to considerthe current implementation it will not get the load, no
the load happens later
this will trigger the policy on the loaded resource though right?
yeah, all resource policies are still run
but aggregates/calculations will always appear to not be being loaded
Great, in that case I will be able to sleep tonight 🤣
So like if you have a calculation you're locking down using that check you showed me to see if its being loaded
it will always appear that its not being loaded
selected attributes will always be correct though
ok, that probably means all my calculations are not secured, because there are no policies attached to calculation directly, same with aggregates. But relationships should be ok
Well, if you're locking down calcs/aggs that way, I'm sorry
Its definitely not clear that this is how it works in the docs, hopefully will have field policies done this week
Just to make sure, policies for calculation would only work correctly with the
loading?
policy right now?policies for aggregates and calculations won't work with ash_graphql essentially no matter how it works
Actually, just calculations, yeah
because it does use
Ash.Query.load
for aggregates
actually...I can make this better sooner
one sec
I added something to core recently that should let me make AshGraphql
use Ash.Query.load
for calculations
So then the only problem would be if you tried to use loading?
with a relationship
i.e in your case you have get_loads
that will always appear emptySounds like these kinds of policy expectations would a good thing to add to my app's integration tests for the GraphQL API. 😅
ah, okay but there is an implementation detail you need to know about for your
get_calculations
using this new stuff:
So there may actually be a way to solve this without the need for field_policies
(although field policies would be nice to have). The primary reason that ash_graphql
can't use Ash.Query.load
is because the caller might do this:
Right now, there is not a way to load the same relationship two different ways
so AshGraphql
uses api.load
after the fact to account for that. But if I add Ash.Query.load_relationship_as
in the same way we have Ash.Query.load_calculation_as
, and add a relationships
key to the resource similar to calculations
and aggregates
for "anonymous" calcs/aggs, then we can create a single Ash.Query.load
statement, and then your policies will work 100% as you have them
(as long as you add that get_calculations/1
change I mentioned above)
I could potentially do that to start, and then add field policies after. That would prevent any unexpected behavior for current implementations. But once field_policy
is added, I'd still want people to use that because it will play nice with api.load
if someone uses it elsewhere
Okay @barnabasj if you try main
of ash_graphql
with the latest release of ash
(2.9.19), and make that get_calculations/1
change, that check should now honor loaded calcs in ash_graphql
thanks i will have a look at it shortly
As for the original question: I'm going to be adding
Ash.Changeset.load
and your policy will then be usable on changesets as well in almost exactly the same way.
You should do it with aggregates too, actually:
@Blibs might be interesting for you
A quick test looked promising. The policy seems to work, but I'm getting an error down the line. Not 100% sure if that is just coincidental or related. Will take a closer look tomorrow
😢 Hmm...yeah something is up there
I'll have it raise a more informative (but still raise) error in that case that could potentially help track the issue down.
okay, I've added
Ash.Changeset.load
to ash core, and used it in AshGraphql
The only remaining "issue" is that AshGraphql
loads related resources after-the-fact, not in the main query
Alright, so the last thing to cover this stuff/wrap it up is the ability to load the same relationship multiple times in different ways. I'm not entirely sure how difficult this is going to end up being 🙂 But once I do that, I can add utilities like Ash.Changeset.loading(changeset) => [%Attribute{}, %Calculation{}, %Aggregate{}]
and Ash.Query.loading
that will get you the list of things being loaded, to abstract away some of that complexity.So far everything seems to work, thank you.
After some further tests, two problems emerged. I found a place where we use the aliases already. There we get this error now:
And I had a calculation that worked before now triggers this error:
I tried to debug the second error a bit, and it seems it is looking for a hotel relationship on the hotel instead of the items, but I'm not sure why
Will look into these both today.
ash: 8fdd319697ff39262fc4162f38fd6c55e2b9b189 is the last commit that I know worked for us in relation to the query/loads. Policy problems aside
This is the load statement in the calc that now fails:
Okay, so I just pushed up a fix for the first issue
the second one might be more complicated
If you're still around, the error message I was outputting there is actually wrong a bit
i can try it shortly
awesome, thanks
actually we need both
okay, just pushed a better message up to main, hopefully will help me track the error down
Getting the same error here: https://discordapp.com/channels/711271361523351632/1115269100860616735/1115929808187179033
The calc load now givs me this error:
what I do not understand about this error message is why is the Hotel the resource, because the calculation is on the collection which is not in the error message so the load would be collection -> hotel_itinerary_items -> hotel -> country
are you sure its the same error? could there be a different stacktrace?
let me check,
ok, yeah stack trace is bigger :facepalm:
okay, pushed up another fix for that
just more places we're thinking we can do
to_string(calc.name)
but we can't always
could potentially be more, its hard to find them all the time 😆 I'm surprised tests in ash_graphql
didn't find this. I guess its because it didn't test dynamic calcs with calcs with dependencies
okay, and I think I found the issue for the dependency issue also
so main
should have fixes for both hopefully
I logged the calculations in the query, I was just wondering why the calc_name was not the original calc name. Is there a way to get the original calc name from the alias?
oh
yeah that is what that is supposed to be
thanks for pointing that out
calc_name
should be nil
for anonymous calculations (which you can't do through the api) and :the_resource_calc_name
otherwise
okay, pushed a fix for that as wellgreat, thanks. now i can get the correct calc name and check if the user is allowed to get it too 🚀
🥳 🥳 🥳
Just clicked through all the pages, looks like everything is working now :party_parrot: