Auth errors are not consistent

Issues GraphQL
, , ,
1

If I set auth for all modification endpoints: add, delete and update; then add and delete throw errors when I don’t have permissions.

On addType:

{
  "errors": [
    {
      "message": "mutation failed because authorization failed"
    }
  ],

On deleteType:

{
  "errors": [
    {
      "message": "mutation failed because Dgraph execution failed because line 4 column 2: Only aggregation/math functions allowed inside empty blocks. Got: ...",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ]
    }
  ],

On updateType, I get no errors whatsoever, but just my return list is empty.

Ideally, it should all be consistent. Either throw clear errors, or always return an empty list.

3 Likes
2

Hi Manish,
yeah errors should be consistent.We will look into it and discuss it internally.

3

@mrjn I tried add and delete with auth on a schema given in docs. https://graphql.dgraph.io/doc/authorization/mutations/

For add type , It is giving error as you mentioned and for delete it is working fine. Means if i don’t have permissions then it doesn’t delete anything.

The error you are getting looks something different. Can you please give the schema and the steps to reproduce this, So that we can see exactly what’s happening.

4

You can try here: https://beneficial-baseball-9463.us-west-2.aws.cloud.dgraph.io/graphql

That’s the issue right. For add, you get an auth error. For delete, you don’t get the error, but it doesn’t do anything. It should either be silent in both, or be sending errors in both (also, in delete operations).

5

The error on deleteType is a bug. No error and an empty result should have been returned there if you don’t have permissions. We have created a JIRA ticket for it and @arijit is looking into fixing this.

For addType mutation, we should change the behaviour to not return an authorization error. This would make the behaviour consistent with that of update and delete. Adding this to our backlog.

The other approach would have been to return auth errors from update/delete mutations. That would have leaked information about the data that is there even though the user doesn’t have permission to it. Since this is not considered as a good security practice, we have decided against it.

1 Like
6

These two PRs fixes the issue.

https://github.com/dgraph-io/dgraph/pull/6350

https://github.com/dgraph-io/dgraph/pull/6329

2 Likes