Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query caching and whitelisting #22

Closed
vektah opened this issue Feb 23, 2018 · 12 comments
Closed

Query caching and whitelisting #22

vektah opened this issue Feb 23, 2018 · 12 comments

Comments

@vektah
Copy link
Collaborator

vektah commented Feb 23, 2018

hashing incoming queries and caching the document would improve parse times for large queries.

It would also serve as a great point to whitelist allowed queries.

Open question: how should the whitelist be specified?

Perhaps Persisted Queries?

@0xSalman
Copy link

I would be interested to see how hashing/caching the queries would work.

However, whitelisting queries (persisted queries) defeats the purpose of using GraphQL. On paper it seems like a great idea but it takes away the flexibility of GraphQL.

@vektah
Copy link
Collaborator Author

vektah commented Feb 28, 2018

Hashing / caching would be pretty straight forward to implement, but would need to be measured before it lands. I wonder if there are any good graphql benchmark suites out there?

I think persisted queries offer a few big benefits:

  • locking down a graph to have a similar exposure footprint as REST.
  • protecting against malicious users: maybe something like https://github.com/ivome/graphql-query-complexity is probably better suited for public apis.
  • deleting deprecated fields: in a microservice environment knowing who is using what parts of the graph lets you fail in CI if you remove something thats still in use.

@andrewmunro
Copy link

Adding complexity somehow to the context object in the resolver would be very nice! We were thinking of implementing some kind of ACL layer that rate limits complexity per user...

I imagine this would tie in to #5 which gives us access to the query in the resolver?

@0xSalman
Copy link

0xSalman commented Mar 3, 2018

@vektah

locking down a graph to have a similar exposure footprint as REST.

If it was me, I would use REST instead. Why use GraphQL if I want to make it behave like REST? Not to mention, you can only add queries (cannot change or delete). So any change to an existing query requires updating the clients. This adds lots of maintenance in a microservices environment and if you have multiple user facing clients then REST style API versioning will come into play.

protecting against malicious users: maybe something like https://github.com/ivome/graphql-query-complexity is probably better suited for public apis.

In my humble opinion, this is definitely a much better option. You can truly secure your API but also keep benefiting from GraphQL flexibility.

deleting deprecated fields: in a microservice environment knowing who is using what parts of the graph lets you fail in CI if you remove something thats still in use.

I think this is a point against persisted GraphQL queries (PGQ)

Lastly, I am not totally against PGQ and I know it has some benefits but I think other options (i.e., depth limit, complexity/cost analysis) are better practices to secure GraphQL API.

Happy coding! 😄

@ghost
Copy link

ghost commented Mar 3, 2018 via email

@0xSalman
Copy link

0xSalman commented Mar 4, 2018

@gedw99

That's authentication/authorization and it does not prevent an authorized user to make N+1 & infinite circular relationship type of query request

@ghost
Copy link

ghost commented Mar 4, 2018

@salmana1 But for security dont yo want 2 things:

  1. Before a query is made to know what the user can and cant do. Hence you can restrict the Nav and also restrict the queries they could type in a query IDE.
  2. After they make a query, and it hits the server, check each table they are hitting

I guess you knwo this but lots of projects use casbin to do policy based restrictions. Its very popular, and one of the useful aspects is that its runtime based, which is what you want because you want to apply the security in terms of what a roles can access at runtime.
https://github.com/casbin/casbin

If there is something i am missing in the big picture let me know...

@imiskolee
Copy link

first, i think cache is a field level or object level feature. its looks like

type Object @cache(10s) {
   id : Int @cache(10s)
   name: String @cache(20s)
   price: @cache(Never)
}

in the blow case, we can define a obejct level cache time and field level cache time.

@vektah
Copy link
Collaborator Author

vektah commented May 8, 2018

Thats a different kind of caching. This issue is for the incoming query objects, not the resolver results

@vektah
Copy link
Collaborator Author

vektah commented Jul 19, 2018

I've broken this out on the project board.

@vektah vektah closed this as completed Jul 19, 2018
@xzyfer
Copy link

xzyfer commented Oct 17, 2018

Were persisted queries implemented?
If so it'd be great to have the featured listed in the readme :)

@vektah
Copy link
Collaborator Author

vektah commented Oct 17, 2018

Nope, just caching. Would be pretty easy to add the hooks required to lookup a query by ID and let the user implement the store

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

No branches or pull requests

5 participants