fix(GraphQL): Fix query rewriting for auth delete when deleting types with inverse field. (#6350)

graphql/e2e/auth/auth_test.go

@@ -43,6 +43,13 @@ var (
 	metaInfo *testutil.AuthMeta
 )
 
+type Tweets struct {
+	Id        string `json:"id,omitempty"`
+	Text      string `json:"text,omitempty"`
+	Timestamp string `json:"timestamp,omitempty"`
+	User      User   `json:"user,omitempty"`
+}
+
 type User struct {
 	Username string `json:"username,omitempty"`
 	Age      uint64 `json:"age,omitempty"`
@@ -127,8 +134,8 @@ type Task struct {
 }
 
 type TaskOccurrence struct {
-	Id string `json:"id,omitempty"`
-	Due string `json:"due,omitempty"`
+	Id   string `json:"id,omitempty"`
+	Due  string `json:"due,omitempty"`
 	Comp string `json:"comp,omitempty"`
 }
 
@@ -149,6 +156,7 @@ type uidResult struct {
 }
 
 type Tasks []Task
+
 func (tasks Tasks) add(t *testing.T) {
 	getParams := &common.GraphQLParams{
 		Query: `
@@ -432,7 +440,7 @@ func TestAuthRulesWithMissingJWT(t *testing.T) {
 func TestOrderAndOffset(t *testing.T) {
 	tasks := Tasks{
 		Task{
-			Name:        "First Task four occurrence",
+			Name: "First Task four occurrence",
 			Occurrences: []*TaskOccurrence{
 				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
 				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
@@ -441,7 +449,7 @@ func TestOrderAndOffset(t *testing.T) {
 			},
 		},
 		Task{
-			Name:       "Second Task single occurrence",
+			Name: "Second Task single occurrence",
 			Occurrences: []*TaskOccurrence{
 				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
 			},
@@ -451,7 +459,7 @@ func TestOrderAndOffset(t *testing.T) {
 			Occurrences: []*TaskOccurrence{},
 		},
 		Task{
-			Name:        "Fourth Task two occurrences",
+			Name: "Fourth Task two occurrences",
 			Occurrences: []*TaskOccurrence{
 				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},
 				{Due: "2020-07-19T08:00:00", Comp: "2020-07-19T08:00:00"},

graphql/e2e/auth/delete_mutation_test.go

@@ -361,3 +361,67 @@ func TestDeleteNestedFilter(t *testing.T) {
 		})
 	}
 }
+
+func TestDeleteRBACRuleInverseField(t *testing.T) {
+	mutation := `
+	mutation addTweets($tweet: AddTweetsInput!){
+      addTweets(input: [$tweet]) {
+        numUids
+      }
+    }
+	`
+
+	addTweetsParams := &common.GraphQLParams{
+		Headers: getJWT(t, "foo", ""),
+		Query:   mutation,
+		Variables: map[string]interface{}{"tweet": Tweets{
+			Id:        "tweet1",
+			Text:      "abc",
+			Timestamp: "2020-10-10",
+			User: User{
+				Username: "foo",
+			},
+		}},
+	}
+
+	gqlResponse := addTweetsParams.ExecuteAsPost(t, graphqlURL)
+	require.Nil(t, gqlResponse.Errors)
+
+	testCases := []TestCase{
+		{
+			user:   "foobar",
+			result: `{"deleteTweets":{"numUids":0,"tweets":[]}}`,
+		},
+		{
+			user:   "foo",
+			result: `{"deleteTweets":{"numUids":1,"tweets":[ {"text": "abc"}]}}`,
+		},
+	}
+
+	mutation = `
+	mutation {
+      deleteTweets(
+        filter: {
+          text: {anyoftext: "abc"}
+        }) {
+		numUids
+        tweets {
+          text
+        }
+      }
+    }
+	`
+
+	for _, tcase := range testCases {
+		t.Run(tcase.role+tcase.user, func(t *testing.T) {
+			deleteTweetsParams := &common.GraphQLParams{
+				Headers: getJWT(t, tcase.user, tcase.role),
+				Query:   mutation,
+			}
+
+			gqlResponse := deleteTweetsParams.ExecuteAsPost(t, graphqlURL)
+			require.Nil(t, gqlResponse.Errors)
+			require.JSONEq(t, string(gqlResponse.Data), tcase.result)
+		})
+	}
+}

graphql/e2e/auth/schema.graphql

@@ -23,6 +23,20 @@ type User @auth(
   tickets: [Ticket] @hasInverse(field: assignedTo)
   secrets: [UserSecret]
   issues: [Issue]
+  tweets: [Tweets] @hasInverse(field: user)
+}
+
+type Tweets @auth (
+    add: { rule: "{$USER: { eq: \"foo\" } }"},
+    delete: { rule: "{$USER: { eq: \"foo\" } }"},
+    update: { rule: "{$USER: { eq: \"foo\" } }"}
+){
+    id: String! @id
+    text: String! @search(by: [fulltext])
+    user: User
+    timestamp: DateTime! @search
+    score: Int @search
+    streams: String @search
 }
 
 type UserSecret @auth(

graphql/resolve/auth_delete_test.yaml

@@ -24,6 +24,73 @@
       UserSecretAuth2 as var(func: uid(UserSecret1)) @filter(eq(UserSecret.ownedBy, "user1")) @cascade
     }
 
+- name: "Delete with inverse field and RBAC true"
+  gqlquery: |
+    mutation {
+      deleteTweets(
+        filter: {
+          text: {anyoftext: "abc"}
+        }) {
+        tweets {
+          text
+        }
+      }
+    }
+  jwtvar:
+    USER: "foo"
+  dgmutations:
+    - deletejson: |
+        [
+          { "uid": "uid(x)" },
+          {
+            "User.tweets" : [{"uid":"uid(x)"}],
+            "uid" : "uid(User2)"
+          }
+        ]
+  dgquery: |-
+    query {
+      x as deleteTweets(func: uid(TweetsRoot)) {
+        uid
+        User2 as Tweets.user
+      }
+      TweetsRoot as var(func: uid(Tweets1))
+      Tweets1 as var(func: type(Tweets)) @filter(anyoftext(Tweets.text, "abc"))
+      tweets(func: uid(Tweets3)) {
+        text : Tweets.text
+        dgraph.uid : uid
+      }
+      Tweets3 as var(func: uid(Tweets4))
+      Tweets4 as var(func: uid(x))
+    }
+
+- name: "Delete with inverse field and RBAC false"
+  gqlquery: |
+    mutation {
+      deleteTweets(
+        filter: {
+          text: {anyoftext: "abc"}
+        }) {
+        tweets {
+          text
+        }
+      }
+    }
+  dgmutations:
+    - deletejson: |
+        [
+          { "uid": "uid(x)" }
+        ]
+  dgquery: |-
+    query {
+      x as deleteTweets()
+      tweets(func: uid(Tweets1)) {
+        text : Tweets.text
+        dgraph.uid : uid
+      }
+      Tweets1 as var(func: uid(Tweets2))
+      Tweets2 as var(func: uid(x))
+    }
+
 - name: "Delete with deep auth"
   gqlquery: |
     mutation deleteTicket($filter: TicketFilter!) {
@@ -288,13 +355,18 @@
           {
             "Ticket.assignedTo" : [ {"uid":"uid(x)"} ],
             "uid" : "uid(Ticket4)"
+          },
+          {
+            "Tweets.user" : {"uid":"uid(x)"},
+            "uid" : "uid(Tweets5)"
           }
         ]
   dgquery: |-
     query {
       x as deleteUser(func: uid(UserRoot)) {
         uid
         Ticket4 as User.tickets
+        Tweets5 as User.tweets
       }
       UserRoot as var(func: uid(User1)) @filter((uid(UserAuth2) AND uid(UserAuth3)))
       User1 as var(func: type(User)) @filter(eq(User.username, "userxyz"))

graphql/resolve/mutation_rewriter.go

@@ -670,6 +670,43 @@ func RewriteUpsertQueryFromMutation(m schema.Mutation, authRw *authRewriter) *gq
 	return dgQuery
 }
 
+// removeNodeReference removes any reference we know about (via @hasInverse) into a node.
+func removeNodeReference(m schema.Mutation, authRw *authRewriter,
+	qry *gql.GraphQuery) []interface{} {
+	var deletes []interface{}
+	for _, fld := range m.MutatedType().Fields() {
+		invField := fld.Inverse()
+		if invField == nil {
+			// This field be a reverse edge, in that case we need to delete the incoming connections
+			// to this node via its forward edges.
+			invField = fld.ForwardEdge()
+			if invField == nil {
+				continue
+			}
+		}
+		varName := authRw.varGen.Next(fld.Type(), "", "", false)
+
+		qry.Children = append(qry.Children,
+			&gql.GraphQuery{
+				Var:  varName,
+				Attr: invField.Type().DgraphPredicate(fld.Name()),
+			})
+
+		delFldName := fld.Type().DgraphPredicate(invField.Name())
+		del := map[string]interface{}{"uid": MutationQueryVarUID}
+		if invField.Type().ListType() == nil {
+			deletes = append(deletes, map[string]interface{}{
+				"uid":      fmt.Sprintf("uid(%s)", varName),
+				delFldName: del})
+		} else {
+			deletes = append(deletes, map[string]interface{}{
+				"uid":      fmt.Sprintf("uid(%s)", varName),
+				delFldName: []interface{}{del}})
+		}
+	}
+	return deletes
+}
+
 func (drw *deleteRewriter) Rewrite(
 	ctx context.Context,
 	m schema.Mutation) ([]*UpsertMutation, error) {
@@ -703,44 +740,14 @@ func (drw *deleteRewriter) Rewrite(
 	}
 
 	deletes := []interface{}{map[string]interface{}{"uid": "uid(x)"}}
-
-	// we need to delete this node with ^^ and then any reference we know about
-	// (via @hasInverse) into this node.
-	for _, fld := range m.MutatedType().Fields() {
-		invField := fld.Inverse()
-		if invField == nil {
-			// This field be a reverse edge, in that case we need to delete the incoming connections
-			// to this node via its forward edges.
-			invField = fld.ForwardEdge()
-			if invField == nil {
-				continue
-			}
-		}
-		varName := varGen.Next(fld.Type(), "", "", false)
-
-		qry.Children = append(qry.Children,
-			&gql.GraphQuery{
-				Var:  varName,
-				Attr: invField.Type().DgraphPredicate(fld.Name()),
-			})
-
-		delFldName := fld.Type().DgraphPredicate(invField.Name())
-		del := map[string]interface{}{"uid": MutationQueryVarUID}
-		if invField.Type().ListType() == nil {
-			deletes = append(deletes,
-				map[string]interface{}{
-					"uid":      fmt.Sprintf("uid(%s)", varName),
-					delFldName: del})
-		} else {
-			deletes = append(deletes,
-				map[string]interface{}{
-					"uid":      fmt.Sprintf("uid(%s)", varName),
-					delFldName: []interface{}{del}})
-		}
+	// We need to remove node reference only if auth rule succeeds.
+	if qry.Attr != m.ResponseName()+"()" {
+		// We need to delete the node and then any reference we know about (via @hasInverse)
+		// into this node.
+		deletes = append(deletes, removeNodeReference(m, authRw, qry)...)
 	}
 
 	b, err := json.Marshal(deletes)
-
 	var finalQry *gql.GraphQuery
 	// This rewrites the Upsert mutation so we can query the nodes before deletion. The query result
 	// is later added to delete mutation result.