fix(GraphQL): Fix auth rewriting for nested queries when RBAC rule is true.

[arijitAD]

Fixes #GRAPHQL-583
GraphQL query:

    query {
      queryContact {
        id
        nickName
        tasks {
          id
          name
          occurrances {
            due
            comp
          }
        }
      }
    }

If tasks and occurrence have RBAC rules evaluated to true then we would return early and not rewrite the query properly.
Previously: Here Task4 and TaskOccurance3 will be reported as undefined variables.

    query {
      queryContact(func: uid(ContactRoot)) {
        id : uid
        nickName : Contact.nickName
        tasks : Contact.tasks @filter(uid(Task4)) {
          id : uid
          name : Task.name
          occurrances : Task.occurrances @filter(uid(TaskOccurance3)) {
            due : TaskOccurance.due
            comp : TaskOccurance.comp
            dgraph.uid : uid
          }
        }
      }
      ContactRoot as var(func: uid(Contact5))
      Contact5 as var(func: type(Contact))
    }

Current:

    query {
      queryContact(func: uid(ContactRoot)) {
        id : uid
        nickName : Contact.nickName
        tasks : Contact.tasks @filter(uid(Task4)) {
          id : uid
          name : Task.name
          occurrances : Task.occurrances @filter(uid(TaskOccurance3)) {
            due : TaskOccurance.due
            comp : TaskOccurance.comp
            dgraph.uid : uid
          }
        }
      }
      ContactRoot as var(func: uid(Contact5))
      Contact5 as var(func: type(Contact))
      var(func: uid(ContactRoot)) {
        Task1 as Contact.tasks
      }
      Task4 as var(func: uid(Task1))
      var(func: uid(Task1)) {
        TaskOccurance2 as Task.occurrances
      }
      TaskOccurance3 as var(func: uid(TaskOccurance2))
    }

---

This change is 

Docs Preview:

[pawanrawal]

Mostly looking good, got some questions here.

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @arijitAD and @MichaelJCompton)

---

graphql/resolve/auth_query_test.yaml, line 141 at r1 (raw file):

    }

- name: "Deep RBAC rule - Level 1 type without auth."

How is this case without auth? Isn't this the case where all levels are true?

---

graphql/resolve/auth_test.go, line 338 at r1 (raw file):

			// Check for unused variables.
			_, err = gql.Parse(gql.Request{Str: dgraph.AsString(dgQuery)})

Can you verify before merging that this was indeed failing before your change?

---

graphql/resolve/query_rewriter.go, line 745 at r1 (raw file):

		var fieldAuth []*gql.GraphQuery
		var authFilter *gql.FilterTree

Hoping its safe for fieldAuth to be nil here for the code that follows when rbac != schema.Uncertain

---

graphql/resolve/query_rewriter.go, line 747 at r1 (raw file):

		var authFilter *gql.FilterTree
		// If RBAC rules are evaluated to `Uncertain` then we add the Auth rules.
		if rbac == schema.Uncertain {

What are the values possible here apart from schema.Uncertain?

[arijitAD]

Reviewable status: 6 of 8 files reviewed, 4 unresolved discussions (waiting on @MichaelJCompton and @pawanrawal)

---

graphql/resolve/auth_query_test.yaml, line 141 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

How is this case without auth? Isn't this the case where all levels are true?

tasks field which is at level 1 doesn't have any auth rules.

---

graphql/resolve/auth_test.go, line 338 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Can you verify before merging that this was indeed failing before your change?

I have verified it.

---

graphql/resolve/query_rewriter.go, line 745 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Hoping its safe for fieldAuth to be nil here for the code that follows when rbac != schema.Uncertain

Yes. fieldAuth is only appended to authQueries. Appening nil slice to an existing slice doesn't do anything.

---

graphql/resolve/query_rewriter.go, line 747 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

What are the values possible here apart from schema.Uncertain?

schema.Postive. In that case we don't need to add auth rules. But we need the below logic for nested queries.

[pawanrawal]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arijitAD and @MichaelJCompton)

---

graphql/resolve/auth_query_test.yaml, line 141 at r1 (raw file):

Previously, arijitAD (Arijit Das) wrote…

tasks field which is at level 1 doesn't have any auth rules.

Ah I see, got confused betweens tasks and adminTasks