Skip to content
This repository has been archived by the owner on Sep 2, 2022. It is now read-only.

Improve reusability of permission queries #189

Closed
altschuler opened this issue Apr 27, 2017 · 6 comments
Closed

Improve reusability of permission queries #189

altschuler opened this issue Apr 27, 2017 · 6 comments

Comments

@altschuler
Copy link

altschuler commented Apr 27, 2017

I'm seeing a common pattern in my permission queries, many of them are basically identical except for the X in SomeXExists.

I'm not sure exactly how the semantics would be, but there must be some way of making a general purpose, maybe variable-based, query definition to be reused. In addition to keeping things dry, it would make permission queries much less prone to typos/errors = more secure 🛡️

@marktani
Copy link
Contributor

That's a super cool idea! This could be something like a CurrentTypeExists query, and you could attach permission queries to operations on multiple types. I like!

@altschuler
Copy link
Author

I think interfaces would solve this elegantly, so you define a permission query for an interface, and you can then add that query to any type that implements the interface.

And in terms of UI, instead of having to add the query for each type you could get a list of all the types that implement the interface and simply tick them off as required.

@HariSeldon23
Copy link

I really like the idea of the interface. However I have a much simpler use case, whereby I would like to bulk update 20 of my 30 types with the Authenticated permission. Instead of having to manually adjust each operation in the UI, it would be great if we had a multi select and then apply Authenticated to the selected records.

@kbrandwijk
Copy link
Contributor

@altschuler On one hand, I like the idea of attaching a permission query to an interface. From a user perspective, it makes sense too. But looking at how GraphQL works under the hood, I think we need to be careful not to make too many abstractions that deviate too much from the GraphQL internals.

A permission query is essentialy a resolver function. Resolver functions for interfaces do not exist in GraphQL. So under the hood, if you define a permission query for an interface, that would mean attaching that permission query to all the resolvers for all types that implement that interface. When you add a new type, with an existing interface, that would mean linking the already existing permission query to the resolver of that new type. And that would all happen 'under the hood'.

Also conceptually, there's nothing concrete about an interface. It just defines common fields, without any implementation.

The thing that might come closer to what we're actually trying to achieve here, might be type inheritance. The problem is, GraphQL does not support type inheritance (yet). So now we're back to copying again.

So technically, we will always end up with one resolver function (permission query), and the desired functionality to create once, reuse (DRY). Let's make the GUI reflect that implementation as closely as possible (the less 'magic' the better), and allow the user to define a permission query, and explicitly attach it to more than one type. Of course, in order for that to work, we need more generic queries, like CurrentTypeExist as @marktani mentioned and probably also something like ParentType, or the choice to implement permissions as functions instead of queries. That would give the most freedom, especially with access to the context like event hook functions have.

@dukuo
Copy link

dukuo commented Dec 11, 2017

I'm not sure if this would be a solution for this case, but what about multiple queries? If all return true then it allow operation. This way the permission logic is a little bit modular, since right now you can only do multiple queries within one file.

- permissions
   - operation: Model.operation
     authenticated: true
     query: 
       - ./src/common/permissions.graphql:IsSomeUserRole
       - ./src/model/permissions.graphql:SomethingSpecificToThisModel

@marktani
Copy link
Contributor

This issue has been moved to graphcool/graphcool-framework.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants