feat(GraphQL): Auth on interfaces should work correctly while mutating them.

[minhaj-shakeel]

Fixes GRAPHQL-741.

---

This change is 

Docs Preview:

[pawanrawal]

The feature looks good functionally and is a blocker for 20.11 hence approving it. @minhaj-shakeel would need to address some of the comments when he is back.

Reviewed 11 of 12 files at r3.
Reviewable status: 10 of 11 files reviewed, 8 unresolved discussions (waiting on @MichaelJCompton, @minhaj-shakeel, and @pawanrawal)

---

graphql/e2e/auth/update_mutation_test.go, line 170 at r3 (raw file):

func getAllPosts(t *testing.T, users []string, roles []string, answers []bool) ([]*Question, []*Answer, []*FbPost, []string) {
	Questions, getAllQuestionIds := getAllQuestions(t, users, answers)

looks a bit strange, why don't you just query the interface posts directly instead of querying all implementing types?

---

graphql/e2e/auth/update_mutation_test.go, line 373 at r3 (raw file):

	testCases := []TestCase{{
		name:   "Only 1 Question Node, whose text is `A Question` satisfies the below `user` and `ans`",

Why was this removed?

---

graphql/e2e/auth/update_mutation_test.go, line 473 at r3 (raw file):

			gqlResponse := params.ExecuteAsPost(t, graphqlURL)
			fmt.Println(string(gqlResponse.Data))

needs to be removed

---

graphql/resolve/auth_delete_test.yaml, line 737 at r3 (raw file):

      PostRoot as var(func: uid(Post1)) @filter(((uid(QuestionAuth2) AND uid(QuestionAuth3)) OR uid(AnswerAuth4)))
      Post1 as var(func: uid(0x1, 0x2)) @filter(type(Post))
      Question1 as var(func: type(Question))

Same comment as for update, this dataset can be reduced to a smaller size here.

---

graphql/resolve/auth_update_test.yaml, line 856 at r3 (raw file):

      PostRoot as var(func: uid(Post1)) @filter(((uid(QuestionAuth2) AND uid(QuestionAuth3)) OR uid(FbPostAuth4) OR uid(AnswerAuth5)))
      Post1 as var(func: uid(0x123, 0x456)) @filter(type(Post))
      Question1 as var(func: type(Question))

This can definitely be optimized, since we have the filter on the interface, we could apply the same filter on all the implementing types to restrict the initial data set. So uid(0x123, 0x456) filter should be applied to all of Question, FbPost and Answer.

---

graphql/resolve/auth_update_test.yaml, line 964 at r3 (raw file):

    }

- name: "Updating interface having no own auth rules but some implementing types have auth rules and they are not satisfied."

I don't see B or C having any auth rules in graphql/e2e/auth/schema.graphql so this comment seems incorrect.

---

graphql/resolve/mutation_rewriter.go, line 676 at r3 (raw file):

	}

	// For interface, empty delete mutation should be returned if Auth rules are

Why should it be an empty delete mutation, is this only called for delete or also for updates?

---

graphql/resolve/schema.graphql, line 304 at r3 (raw file):

}

type B implements A {

doesn't look like this is used anywhere, the file that's used is in e2e/auth/schema.graphql

[pawanrawal]

Just saw that CI is failing, please fix that. Looks like auth on types implementing interfaces is broken with this change.

[abhimanyusinghgaur]

Reviewable status: 10 of 11 files reviewed, 8 unresolved discussions (waiting on @MichaelJCompton, @minhaj-shakeel, and @pawanrawal)

---

graphql/e2e/auth/update_mutation_test.go, line 170 at r3 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

looks a bit strange, why don't you just query the interface posts directly instead of querying all implementing types?

seems he is also using the implementing types separately, maybe that is the reason.

---

graphql/e2e/auth/update_mutation_test.go, line 373 at r3 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Why was this removed?

Done

---

graphql/resolve/auth_update_test.yaml, line 964 at r3 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

I don't see B or C having any auth rules in graphql/e2e/auth/schema.graphql so this comment seems incorrect.

C seems to have auth rules.

---

graphql/resolve/schema.graphql, line 304 at r3 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

doesn't look like this is used anywhere, the file that's used is in e2e/auth/schema.graphql

It seems an interface now requires an implementing type for the tests to work. This particular one fails otherwise:

TestMutationRewriting/Delete_Mutation_Rewriting/Deleting_an_interface_with_just_a_field_with_@id_directive