craftql icon indicating copy to clipboard operation
craftql copied to clipboard

Prevent querying of everything in the same entry type

Open exophunk opened this issue 7 years ago • 4 comments

I am struggling to find a way to add basic security to my app. I understand that I can scope access to certain entry types with tokens.

But «inside» this entry type, you are allowed to query anything. In my example, I have a entry type Coupons, that have a field code which is a random String like AK298K6844X.

In the app, you can open a coupon page like this: https://myapp.com/coupons/AK298K6844X. Then, on this page, I run a CraftQL query to load the specific coupon:

query coupons($code: String) {
    entries: entries(section: [coupons], code: $code) {
        ...on Coupons {
            name,
            code,
        }
    }
}

Result:

{
  "data": {
    "entries": [
      {
        "name": "Some Coupon",
        "code": "e9P06SWjSm"
      }
    ]
  }
}

This works fine and it returns the given coupon. Also, the randomness of the coupon code is secure enough to not randomly guess another coupon. With traditional REST, this is quite easy and safe enough, as you can simply provide an endpoint that only returns single coupon items.

With GraphQL though, you can simply omit the parameter $code and the query will return every coupon available.

Result:

{
  "data": {
    "entries": [
      {
        "name": "Some Coupon",
        "code": "e9P06SWjSm"
      },
      ... every other coupon!
    ]
  }
}

Most approaches to secure this doesn't seem to help: – Token Scopes can't prevent this – Authentication can't prevent this, an authenticated user can still query all coupons

It feels like Query whitelisting is the only thing that would prevent clients from fetching whatever they want, right? Or can anyone think of another approach to secure access?

A similar problem is with CraftQL queries that also query related entry types. The token scope need to allow access to every entry type that is part of the query, but this way you open access to anything on the related entry type.

For example:

coupon -> product -> owner I maybe only want to allo querying the related owner "name", but allowing entry type "owner" in the scope, the client can now fetch all owners...

exophunk avatar May 30 '18 11:05 exophunk

If query whitelisting is the only option, is there a way to add a middleware/filter or something before any CraftQL query that filters allowed queries? For example with this: persistgraphql

If it's possible to add a filter from another (custom) plugin, I would try that.

exophunk avatar May 30 '18 11:05 exophunk

Thanks for raising this @exophunk. I'm torn because adding a middleware filter would make sense but then you need to parse the AST to make sure that the query is the one you expect before you apply any sort of validation to it. E.g., if you wanted to allow someone to select all blog posts but not to select all coupon codes you'd have to do that check in your filter.

Maybe an easier way would be to allow custom fields to be added to the schema. Then you could add a coupon(code: String!) query field. That would require the code and then you could exclude coupons from the generic entries field…

markhuot avatar May 30 '18 14:05 markhuot

Okay, I played around with this a bit and found a reasonable solution to do query whitelisting. With the build tool persistgraphql, you can quite easily scan your codebase for graphql queries and export them to a .json file. You can export queries from gql files or from inline code.

Then, I've extended your Controller and added a Filter, which checks the queries against a whitelist. If the query is not in there, the query is not allowed.

I've quickly put the code into a craft-plugin, so you can check it out: https://github.com/exophunk/craft-craftql-whitelist

The plugin ist not quite ready for public use, because:

– It's not on composer and meant to be placed in the /plugins folder. – It has some hardcoded strings, for example I hardcode overwrite the /api route: – It has some example variables validation code from my current project, also hardcoded – I wonder what the performance implications are of reading a json-file on every request, maybe there is a better way. – I wonder if there ia a better way to apply a filter to your controller, without extending it and overwriting the route. – It has no support or thoughts on mutations yet

So I hope this helps you if you have any plans to integrate whitelisting directly into CraftQL. I think this makes most sense. If you would like to keep this as an external plugin, we could think of making your CraftQL plugin more easily extendable and I would further develop and release the plugin.

Cheers, Robert

exophunk avatar Jun 04 '18 15:06 exophunk

I'm having the same issues - I can't make a whole channel public, as only some entries and/or fields are meant for a certain user to see. So I also need some whitelisting feature.

I can't imagine nobody else needs this and is requesting this ... Or do I miss something obvious to work around this? Thanks, Gert

gertst avatar Oct 24 '19 19:10 gertst