User already exists error for a request without JWT on a schema with Auth defined

Issues GraphQL
, ,
1

Steps to reproduce

  1. Create schema with Auth and Add Authorization with claims: {“email”: "harshad@harshad.com, “isAuthenticated”: “True”, “role”: “USER”}
interface Node {
  id: ID!
}
	
type User implements Node 
@auth(
	add: { and: [
		{ rule: "{ $isAuthenticated: { eq: \"True\" } }" },
		{ rule: "{ $role: { eq: \"USER\" } }"},
		{ rule: """query($email: String!) {
								queryUser(filter: { username: { eq: $email}}) {
											id
									}
								}
				"""
		},
	]},
	query: { and: [
		{ rule: "{ $isAuthenticated: { eq: \"True\" } }" },
		{ rule: "{ $role: { eq: \"USER\" } }"},
		{ rule: """
                    query($email: String!) {
                        queryUser(filter: { username: { eq: $email}}) {
                                username
                            }
                        }
                """},
	]},
)
{
		username: String! @id @search(by: [hash])
		displayName: String @search(by: [exact])
		phoneNumber: String @search(by: [exact])
		photoUrl: String @search(by: [exact])
}
  1. Now create a user with queryVariables without JWT header.
mutation AU($users: AddUserInput!) {
  addUser(input: [$users]) {
    numUids
    user {
      id
      displayName
      username
    }
  }
}

Expected: "message": "mutation failed because authorization failed"
Got: "message": "mutation failed because authorization failed"

THIS IS GOOD!

  1. Now using a valid JWT create a user.
mutation AU($users: AddUserInput!) {
  addUser(input: [$users]) {
    numUids
    user {
      id
      displayName
      username
    }
  }
}

queryVariables

{
	"users": {
		"displayName": "harshad",
		"username": "harshad@harshad.com",
		"photoUrl": "https://someurl23.com"
	}
}

  1. You should see successful return of the user. Another Good News !

  2. Try and run the same mutation this time Without any JWT header value.

Expected: “mutation failed because authorization failed”
Got:

  "errors": [
    {
      "message": "couldn't rewrite query for mutation addUser because id harshad@harshad.com already exists for type User"
    }
  ],
  "data": {
    "addUser": {
      "numUids": 0,
      "user": []
    }
  },
  "extensions": {
    "touched_uids": 4,
    "tracing": {
      "version": 1,
      "startTime": "2020-08-15T12:24:53.4089533Z",
      "endTime": "2020-08-15T12:24:53.4196013Z",
      "duration": 10647600,
      "execution": {
        "resolvers": [
          {
            "path": [
              "addUser"
            ],
            "parentType": "Mutation",
            "fieldName": "addUser",
            "returnType": "AddUserPayload",
            "startOffset": 141200,
            "duration": 10453100,
            "dgraph": [
              {
                "label": "mutation",
                "startOffset": 257600,
                "duration": 5287200
              },
              {
                "label": "query",
                "startOffset": 8644300,
                "duration": 1887600
              }
            ]
          }
        ]
      }
    }
  }
}

You see an issue here?!

Notes:

  1. If the mutation is executed for the first time then we do not get any errors given valid JWT.
  2. Run the same mutation without JWT token. When a record already exist the error actually reveals there is an existing record. Now if you look closely at the query I am passing username field as part of Query Variables . It appears that validation somehow is done using query variable as opposed to the JWT variable which should be injected via dgraph.

The way I see this is potential security risk as any personal accessing the endpoint can constantly try and retrieve information from endpoint pretending to be other user without being the real user. This same could be applied to other model objects as well.

@mrjn, @arijit , @slotlocker2

Please let me know if I am doing something wrong here. Just incase !

Regards
Harshad

1 Like
2

I think the only thing here is that Dgraph is verifying the input as valid before trying any rules. This makes sense as some rules do queries that could cost tue db transactions that would not be needed if the input was not valid input in the first place. The input is invalid because the user uses the @id directive which prevents duplicates.

So what you are expecting is that the auth rules be applied before validating input. This may not be the desired effect across the board, but I do see the problem with exposing data that is otherwise not readable and not writable to a user that can now make guesses until he finds valid data. What could he do with that then though? Well a pen tester might then use that valid email address and start a fishing scam against someone he knows is a user.

3 Likes
3

That is precisely what i am referring to as well.

5

@harshadbhatia this is a good observation, we should not be revealing that the user already exists if the mutation doesn’t pass the auth rules. We’ll work on fixing this.

1 Like
8

I have raised a PR to fix the following issue.

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

1 Like
10

Closing this issue as the fix is already in master and would also be part of the 20.11 release.