hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.1k stars 2.77k forks source link

Allow * for column permission #2104

Open leoalves opened 5 years ago

leoalves commented 5 years ago

Checking the documentation for the Metadata Permission API I can see there is an option for * to select all fields from a table.

The console does not allow * when setting permissions. You can toogle all columns on or off, but all it does is select all available columns.

Not having * in the console can cause 2 issues.

This feature will be a time saver when setting permissions into multiple tables. And since the backend already supports it. It shouldn't be that hard.

leoalves commented 5 years ago

I did this change in the console. https://github.com/leoalves/graphql-engine/commit/dbe99cd17368efdf0fa872580b855100c163bea6

But the server does not recognize as all fields. It recognizes as all fields at the time the permission was created.

Is this a bug or that is it the expected behavior? I think it would be a good feature to have.

Not sure if it's possible, since all the generated queries have the field name and not *.

Thanks

leoalves commented 5 years ago

Hi @rikinsk,

I think this should be tagged as server.

I did the changes in the console to allow * to be selected.
https://github.com/leoalves/graphql-engine/commit/dbe99cd17368efdf0fa872580b855100c163bea6

But the server gives permissions to all columns at the moment the permission is saved. When adding additional columns to the table, the permission does not get updated, even tough the permission columns is saved as *. There should be some kind of re executing the permissions logic when the table is changed.

Thanks

rikinsk-zz commented 5 years ago

This needs server to change behavior for columns value. should mean all columns at any point in time and not just columns at the time of definition

tirumaraiselvan commented 5 years ago

@rikinsk @leoalves Although this is a good dev-mode experience, this might be a dangerous from a production/reproducibility point-of-view.

@0x777 has strong opinions on this.

Let's see if we can come up with a decent solution.

tirumaraiselvan commented 5 years ago

I have had this discussion in the context of Event Trigger payloads where "*" means all columns (anytime) as described in this issue. See: #1651

0x777 commented 5 years ago

I thought that the server already has support for * for columns in select permissions, however it doesn't work as expected. It is kind of non-trivial (though very possible) to keep our internal structures in sync when a column is added/deleted through SQL for the kind of behaviour that is expected with *.

0x777 commented 5 years ago

To clarify the metadata API's * behaves like how Postgres treats * in views. For example, if you have

create view v as select * from t

and then add a column to table t it does not reflect in the view v. * takes the columns that are present at the time of the view's definition. This is the same with permissions in graphql-engine.

Since that's not the behaviour that people expect when the specify * I say we don't expose this in the console. I'm gonna change the priority to 'longterm' given the complexity of the change that is required.

ElixirMike commented 4 years ago

I want to add my opinion that permissions here should work similar to how Postgres RLS works. In RLS, you assign a policy to the table, which then applies to ALL fields in the table...so that when you add new fields, the permissions continue to work.

Here the current flow is very undesirable. Anytime a new field is added, the only solution to add permissions is a complete deletion and then complete re-apply of all permissions. This is a heavy transaction for Hasura and slow (especially when the schema gets bigger).

I really hope there is a fix for this. Another work-around would be to suppo;rt Postgres RLS by passing the sessions variables to postgres on a given call. I've used a tool called PostGrest which does this...where claims in the JWT token can be accessed dircetly in Postgres. This then allows me to use RLS to setup my permissions which would just work going forward. I'd be fine with either solution, but the speed issue of having to constantly delete/re-apply permissions on adding fields is too slow.

tirumaraiselvan commented 4 years ago

Currently, reload_metadata after adding a new column works. Dropping a column and then reload_metadata also works if the drop is done via pure sql (with metadata check, it throws a dependency error). We can perhaps do the "reload" on column add or drop internally so that this feature becomes possible. Related: https://github.com/hasura/graphql-engine/issues/4712

ewashAgora commented 3 years ago

Can someone please provide an update on this issue? Will this issue be resolved soon? In the meantime, is there a suggested workaround other than having to go into the console and manually update the permissions to include newly added columns? Is there something in the API or CLI we can use to update permissions on modified tables? Thank you so much for your help.

pnoyens commented 2 years ago

@ewashAgora What I'm doing now to make it a bit more efficient is to keep the complete metadata dir inside my backend project. Whenever I add new fields to the schema, or alter names of existing fields, I just use my IDE to replace all column permissions in the metadata folder. While having a lot of tables already in my project, it saves me tons of time compared to altering permissions in the UI console. Basically, everything you can alter in the UI, you can also set in these metadata yaml files. Especially if you have complex permission logic for some tables, this could be a big time saver!

Camsteack commented 2 years ago

Hi ! Any update on this feature ? It would be really nice, especially to simplify the metadata files that get really verbose because of all the fields being re listed for each permission type. It also add quite a lot of over head when you are working with a multi tenant schema based application.

Thanks a lot !

aminebeh commented 1 year ago

Up