Query complexity analysis, protection against malicious queries

[ivome]

I couldn't really find a good solution to protect this library against malicious queries and to perform a query analysis prior to running the execution of the query. The best approach to the problem was from the sangria-graphql library.

So I implemented a similar solution for the graphql-js server which you can checkout here:
https://github.com/ivome/graphql-query-complexity

Since every graphql server that is exposed to the public needs something like this, I was wondering if we could make this part of the core library.

There are two major points which would have to be addressed and don't feel 100% clean yet:

• The complexity can change based on the input variables so we need it in the validation rule. I solved that by injecting the variables along with the configuration. Maybe there is a better solution because the variables are already passed to the execute function. It would be nicer to have the variables available in the validation rules. This might also be relevant for other kinds of validations. But that's probably a bigger change?
• The definition of field configuration would have to be extended by the complexity field. I think it makes most sense to define that in the field configuration directly.

Any feedback appreciated! Or maybe there is an even better approach to handle this?

[orefalo]

+1

[wincent]

Hm, I could have sworn that there was already some prior discussion on this on one of our repos, but I can't find it right now (the closest I can find is #998, but that's not the one I am thinking of).

Since every graphql server that is exposed to the public needs something like this

The way we've dealt with this at FB is to pretty much disallow execution of arbitrary queries by dealing with persisted queries alone (see: graphql/express-graphql#109). This isn't just about reducing attack surface area, although that is a nice win: it's mostly motivated by performance wins (which are huge) and a bunch of useful side-effects like making applications of logging, visualization and analytics more tractable.

It's not clear to me that every publicly-exposed server needs this at all, and your library shows that this is the kind of thing that can certainly be solved in "user-space", even if you don't want to go down the persisted query route, but please don't take this as an attempt to shut down the discussion. Carry on!

[leebyron]

Please feel free to send this as a PR to the utilities/ directory if you feel like including it in the core library.