"Not Authorised!" on a field make the containing object null

[AlessandroFerrariFood]

First of all I think that graphql-shield add a clean, generic and maintainable security layer to graphql.
It don't replace ad-hoc fine-grained security check in some resolver for special cases but make easy to add uniform basic access control on all your schema.
So kudos to you!

I'm implementing a field level access policies with graphql-shield but I'm having a strange issue.
When a field is "Not Authorised!" the parent object is set to null.
Here is an example.

Permissions:
const permissions = shield({ Query: { node: allow, }, User: { name: allow, secret: deny } });

Query:
{ node (id: 'myId') { ... on User { name, secret } } }

Expected result:
{ "data": { "node": { name: "My name" } }, "errors": [ { "message": "Not Authorised!", "locations": [ { "line": 31, "column": 3 } ], "path": [ "node", "secret" ] } ] }

Actual result:
{ "data": { "node": null }, "errors": [ { "message": "Not Authorised!", "locations": [ { "line": 31, "column": 3 } ], "path": [ "node", "secret" ] } ] }

It's not an errorPolicy issue on the client because this is extracted from the raw http response.
Only parent object is set to null.
A connection query has all fields normally returned but edges are like this
edges { edge { node: null }, edge { node: null }, edge { node: null }, edge { node: null } }

I'd like to adopt graphql-shield so let me know if I can help to solve this issue.
Thanks.

[maticzav]

Hey @AlessandroFerrariFood 👋

Could you share the chunk of your schema that is being affected? 🙂

[AlessandroFerrariFood]

I have a big proprietary application I can't share but I assembled a strongly reduced but complete version to test the issue on.
Here is its schema
schema.txt
Permission are
const permissions = shield({ User: { firstName: allow, lastName: allow, secret: deny } });
Following query
{ currentUser { name } }
correctly give
{ "data": { "currentUser": { "name": "Paolo Rossi" } } }

Following query
{ currentUser { name secret } }
wrongly give
{ "data": { "currentUser": null }, "errors": [ { "message": "Not Authorised!", "locations": [ { "line": 5, "column": 5 } ], "path": [ "currentUser", "secret" ] } ] }
but I expected
{ "data": { "currentUser": { "name": "Paolo Rossi" } }, "errors": [ { "message": "Not Authorised!", "locations": [ { "line": 5, "column": 5 } ], "path": [ "currentUser", "secret" ] } ] }
or similar.

[maticzav]

Perfect, thank you so much for this input. 🎉

So, the reason you are experiencing this issue has to do with how GraphQL processes responses. I'll use SDL to present the idea, but I believe you should have no problem translating this into your schema as well.

GraphQL uses strongly typed language. That means that there's a particular "contract" between client and server which makes sure no result is unpredictable. My thinking is this; if I am querying type book by its id and want to access id, name and content field which are all non-nullable I rely on the fact that every single field in a book instance will have value once I get the result. This way, I don't have to write checks on the frontend to make sure every field, in fact, has a value.

Imagine a situation where one of the fields throws an error. In such a scenario, GraphQL can no longer ensure that all fields in Book will have a value. Hence, to keep the contract trustworthy, it has to return null.

GraphQL Shield works on a similar pattern; If the user has no access to a particular field, we throw an error. Makes sense, doesn't it? In your specific case, the problem is that secret is non-nullable field. Taking into consideration the above section, we can easily understand why in case the customer is not authenticated, user returns null. If it weren't so, GraphQL would break the contract.

My vague remodelling of your schema looks something like this;

type Query {
  node: User!
}

type User {
  name: String!
  ...
  secret: String!
}

Now to fix the error, you've probably guessed by now, you have to change non-null secret to nullable secret.

I hope this helps you solve the problem! 🙂

A quick advice; thinking of schema as a contract between your client and server could change the way you perceive it. For the field to be non-nullable, you have to ensure that no matter what, the client can receive it.

PS.: Check out node Query field. Can you ensure that there will always be a user no matter the id?

[AlessandroFerrariFood]

You are right!
I missed the not-null.
Changing solved it.

Thank you very much!

PS: node Query field called with wrong id returns a not found graphql error

[mkaradeniz]

Hey,

I have a related issue to this. Let's say my schema looks like this:

type Resource {
  id: ID!
  createdBy: User
}

type User {
  id: ID!
  name: String!
}

I now have a field rule on Resource to deny unauthenticated users from querying createdBy.

When I now query Resource unauthenticated like this:

{
  resources {
    id
    createdBy {
      id
      name
    }
  }
}

I would have expected to get this result:

{
  "data": {
    "resources": [
      {
        "id": 1,
        "createdBy": null
      },
      {
        "id": 2,
        "createdBy": null
      }
    ]
  }
}

But I get this:

{
  "error": {
    "errors": [
      {
        "message": "Not Authorised!",
        "locations": [
          {
            "line": 4,
            "column": 5
          }
        ],
        "path": [
          "resources",
          0,
          "createdBy"
        ]
      },
      {
        "message": "Not Authorised!",
        "locations": [
          {
            "line": 4,
            "column": 5
          }
        ],
        "path": [
          "resources",
          1,
          "createdBy"
        ]
      }
    ],
    "data": {
      "resources": [
        {
          "id": 1,
          "createdBy": null
        },
        {
          "id": 2,
          "createdBy": null
        }
      ]
    }
  }
}

Is there a way to suppress this error so it just returns null for createdBy and be done with it?

A bit more of context: In the app I'm working on I can't know which permission the user has, so I want to still be able to write the queries oblivious to this and just return null to fields the user isn't allowed to query (as long as the fields are nullable of course) so I can handle it in the frontend. Is this something I can actually achieve with graphql-shield?

[maticzav]

Hey, let's see what's going on!

tldr; I think the "problem" is on the frontend because it defaults to error and doesn't consume the data below

So, I think there are some settings regarding how client handles errors. What I wanted to ask before is how does the data in the error case relate to your schema (the rugs part).

[mkaradeniz]

Oh gosh, sorry. I wanted to edit the code I posted so it is more clear, but I forgot to edit the error part. I edited my original post.

The frontend itself isn't built yet, this is all from the playground.

I think it all boils down to this: If someone queries a nullable field without permission to access that field, I simply want to return null instead of throwing an error.

[maticzav]

I think it all boils down to this: If someone queries a nullable field without permission to access that field, I simply want to return null instead of throwing an error.

It makes sense, no worries! 🙂

I mean, the easiest way to do that is to make fields nullable, as you mentioned. It's not possible to not throw a permission error, but that shouldn't be a problem, in my opinion.

Why's ignoring the error, and only collecting the data, not an option?

[mkaradeniz]

Why's ignoring the error, and only collecting the data, not an option?

The problem I was facing is, that (when using nexus framework) errors were returning 500 status codes. With this hack graphql-nexus/nexus#761 (comment) I was able to change it. Now it does return an errors array but also the data so I should be able to ignore it later

So I guess it was more of a nexus issue than a graphql-shield one, though I would've preferred to not throw an error but I get that this is probably not intended.

Thanks so much :)

[fforres]

In case other people are looking for the same, link that @mkaradeniz is not existing any more (Maybe this one was the same?)
graphql-nexus/nexus#383 (comment)

For context, what we want to do, is format the response (in this case in apollo) so it will include the data also :)

const server = new ApolloServer({
  // more config
  formatResponse: (response, { operationName }) => {
    if (response && operationName) {
      return {
        ...response,
        data: response.data || {
          [operationName]: null,
        },
      }
    }

    return {
      data: null,
    }
  },
})