Bug: Permission filters should silently exclude nodes, not throw errors

[amcgee]

Current behavior

Performing a read query with a permissions filter throws InsufficientPermission errors if any resulting object would fail the permission check.

This is a security vulnerability as it exposes the existence of database objects which the authenticated user (or public) does not have permission to view.

Ignoring security for a minute (but don't ignore security) from a purely logistical application development perspective this behavior requires that permission filters are either: 1) replicated client-side in order to prevent InsufficientPermission errors, which could be another potential attack vector in some applications; OR 2) replicated in a custom resolver, which somewhat defeats the purpose of graphcool-generated CRUD operations.

Both of these options are also logistical development hassles which could easily cause problems when the two instances of the permission filter get out of sync.

Reproduction

For instance, with the following simple permission filter (adapted from the docs auth tutorial) -

types.graphql

type Post @model {
  id: ID! @isUnique
  published: Boolean!
}

graphcool.yml

permissions:
  - operation: Post.read
    query: viewPost.graphql

viewPost.graphql

query viewPostQuery($node_id: ID!) {
  SomePostExists(filter: {
    id: $node_id,
    published: true
  })
}

executing the following query after populating the database with an unpublished post (createPost(published:false)) yields an insufficientPermissions error (or several, if multiple fields are queried) :

query {
  allPosts {
    id
    published
  }
}

Note that the following query succeeds and represents the expected behavior outlined below:

query {
  allPosts(filter:{published:true}) {
    id
    published
  }
}

Expected behavior

The above query should yield an empty list if no nodes pass the permission filter, or silently exclude those nodes which do not pass from the resulting list.

The graphcool API should not expose the existence of objects in the database which the authenticated user (or the public) does not have permissions to view. A disallowed read operation should be indistinguishable from a read operation referencing a non-existent object

I think the correct behavior should be to return an empty list whenever a query yields a list (i.e. allPosts, including relation list fields) and to behave as if the node does not exist when a query response is singular but excluded by a permission filter. For example, Post(id:"<existing_id_of_unpublished_post>") should be the same as Post(id:"<nonexistent_id>") and should yield:

{
  data: {
    Post: null
  }
}

The logical extension of this issue is to also mask the existence of permission-excluded fields from the results of a read query, but that has its own set of tradeoffs regarding how graphql client implementations (like Apollo) could deal with missing keys in a response, so that's a topic for another day.

---

I would be happy to dig into fixing this if someone can point me to the right part of the codebase (and repository, since there are many and it's not obvious what lives where). Let me know.

[amcgee]

Related to #58