feat(GraphQL): Add introspection headers to custom logic (#5775)

* Add introspection headers in custom logic * Fix introspection test * Addressed comments * Added tests * Addressed comments * Fix tests * Fix schema test * Update output for subscription schema test * Address comment
dgraph-io · Jul 3, 2020 · cf5df22 · cf5df22
1 parent c6a0dfa
commit cf5df22
Show file tree

Hide file tree

Showing 39 changed files with 199 additions and 45 deletions.
diff --git a/graphql/e2e/custom_logic/custom_logic_test.go b/graphql/e2e/custom_logic/custom_logic_test.go
@@ -165,7 +165,7 @@ func TestCustomQueryShouldForwardHeaders(t *testing.T) {
 				 url: "http://mock:8888/verifyHeaders",
 				 method: "GET",
 				 forwardHeaders: ["X-App-Token", "X-User-Id"],
-				 secretHeaders: ["Github-Api-Token", "X-App-Token"]
+				 secretHeaders: ["Github-Api-Token"]
 		 })
 	 }
 	 
@@ -204,12 +204,12 @@ func TestCustomNameForwardHeaders(t *testing.T) {
 				 url: "http://mock:8888/verifyCustomNameHeaders",
 				 method: "GET",
 				 forwardHeaders: ["X-App-Token:App", "X-User-Id"],
-				 secretHeaders: ["Authorization:Github-Api-Token", "X-App-Token"]
+				 secretHeaders: ["Authorization:Github-Api-Token"]
+				 introspectionHeaders: ["API:Github-Api-Token"]
 		 })
 	 }
 
 		 # Dgraph.Secret Github-Api-Token "random-fake-token"
-		 # Dgraph.Secret X-App-Token "should-be-overriden"
 	 `
 	updateSchemaRequireNoGQLErrors(t, schema)
 	time.Sleep(2 * time.Second)
@@ -254,8 +254,9 @@ func TestSchemaIntrospectionForCustomQueryShouldForwardHeaders(t *testing.T) {
 				http: {
 				  url: "http://mock:8888/validatesecrettoken"
 				  method: "POST"
-				  secretHeaders: ["GITHUB-API-TOKEN"]
-				  graphql: "query($yo: CountryInput!) {countries(filter: $yo)}"
+				  forwardHeaders: ["Content-Type"]
+				  introspectionHeaders: ["GITHUB-API-TOKEN"]
+          graphql: "query($yo: CountryInput!) {countries(filter: $yo)}"
 				}
 			  )
 		  }

diff --git a/graphql/schema/gqlschema.go b/graphql/schema/gqlschema.go
@@ -106,6 +106,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go
@@ -1599,40 +1599,82 @@ func customDirectiveValidation(sch *ast.Schema,
 
 	// 12. Finally validate the given graphql operation on remote server, when all locally doable
 	// validations have finished
-	si := httpArg.Value.Children.ForName("skipIntrospection")
 	var skip bool
-	if si != nil {
-		skip, err = strconv.ParseBool(si.Raw)
-		if err != nil {
-			errs = append(errs, gqlerror.ErrorPosf(graphql.Position,
-				"Type %s; Field %s; skipIntrospection in @custom directive can only be "+
-					"true/false, found: `%s`.",
-				typ.Name, field.Name, si.Raw))
+	iHeaders := make(map[string]string)
+	if body != nil || graphql != nil {
+		var errPos *ast.Position
+		switch {
+		case body != nil:
+			errPos = body.Position
+		case graphql != nil:
+			errPos = graphql.Position
+		default:
+			// this case is not possible, as requiredFields will have non-0 length only if there was
+			// some body or graphql. Written only to satisfy logic flow, so that errPos is always
+			// non-nil.
+			errPos = dir.Position
+		}
+		si := httpArg.Value.Children.ForName("skipIntrospection")
+		if si != nil {
+			skip, err = strconv.ParseBool(si.Raw)
+			if err != nil {
+				errs = append(errs, gqlerror.ErrorPosf(errPos,
+					"Type %s; Field %s; skipIntrospection in @custom directive can only be "+
+						"true/false, found: `%s`.",
+					typ.Name, field.Name, si.Raw))
+			}
 		}
-	}
 
-	forwardHeaders := httpArg.Value.Children.ForName("forwardHeaders")
-	if forwardHeaders != nil {
-		for _, h := range forwardHeaders.Children {
-			key := strings.Split(h.Value.Raw, ":")
-			if len(key) > 2 {
-				return append(errs, gqlerror.ErrorPosf(graphql.Position,
-					"Type %s; Field %s; forwardHeaders in @custom directive should be of the form 'remote_headername:local_headername' or just 'headername'"+
-						", found: `%s`.",
-					typ.Name, field.Name, h.Value.Raw))
+		forwardHeaders := httpArg.Value.Children.ForName("forwardHeaders")
+		fHeaders := make(map[string]bool)
+		if forwardHeaders != nil {
+			for _, h := range forwardHeaders.Children {
+				key := strings.Split(h.Value.Raw, ":")
+				if len(key) > 2 {
+					return append(errs, gqlerror.ErrorPosf(errPos,
+						"Type %s; Field %s; forwardHeaders in @custom directive should be of the form 'remote_headername:local_headername' or just 'headername'"+
+							", found: `%s`.",
+						typ.Name, field.Name, h.Value.Raw))
+				}
+				fHeaders[key[0]] = true
 			}
 		}
-	}
 
-	secretHeaders := httpArg.Value.Children.ForName("secretHeaders")
-	if secretHeaders != nil {
-		for _, h := range secretHeaders.Children {
-			key := strings.Split(h.Value.Raw, ":")
-			if len(key) > 2 {
-				return append(errs, gqlerror.ErrorPosf(graphql.Position,
-					"Type %s; Field %s; secretHeaders in @custom directive should be of the form 'remote_headername:local_headername' or just 'headername'"+
-						", found: `%s`.",
-					typ.Name, field.Name, h.Value.Raw))
+		secretHeaders := httpArg.Value.Children.ForName("secretHeaders")
+		if secretHeaders != nil {
+			for _, h := range secretHeaders.Children {
+				secretKey := strings.Split(h.Value.Raw, ":")
+				if len(secretKey) > 2 {
+					return append(errs, gqlerror.ErrorPosf(errPos,
+						"Type %s; Field %s; secretHeaders in @custom directive should be of the form 'remote_headername:local_headername' or just 'headername'"+
+							", found: `%s`.",
+						typ.Name, field.Name, h.Value.Raw))
+				}
+				if fHeaders != nil {
+					if fHeaders[secretKey[0]] {
+						return append(errs, gqlerror.ErrorPosf(errPos,
+							"Type %s; Field %s; secretHeaders and forwardHeaders in @custom directive cannot have overlapping headers"+
+								", found: `%s`.",
+							typ.Name, field.Name, h.Value.Raw))
+					}
+				}
+			}
+		}
+
+		introspectionHeaders := httpArg.Value.Children.ForName("introspectionHeaders")
+		if introspectionHeaders != nil {
+			for _, h := range introspectionHeaders.Children {
+				key := strings.Split(h.Value.Raw, ":")
+				if len(key) == 1 {
+					key = []string{h.Value.Raw, h.Value.Raw}
+				}
+				if len(key) > 2 {
+					return append(errs, gqlerror.ErrorPosf(errPos,
+						"Type %s; Field %s; introspectionHeaders in @custom directive should be of the form 'remote_headername:local_headername' or just 'headername'"+
+							", found: `%s`.",
+						typ.Name, field.Name, h.Value.Raw))
+				}
+				iHeaders[key[0]] = key[1]
 			}
 		}
 	}
@@ -1642,18 +1684,16 @@ func customDirectiveValidation(sch *ast.Schema,
 	}
 
 	if graphql != nil && !skip && graphqlOpDef != nil {
-		secretHeaders := httpArg.Value.Children.ForName("secretHeaders")
 		headers := http.Header{}
-		if secretHeaders != nil {
-			for _, h := range secretHeaders.Children {
-				key := strings.Split(h.Value.Raw, ":")
-				if len(key) == 1 {
-					key = []string{h.Value.Raw, h.Value.Raw}
-				}
-				// We try and fetch the value from the stored secrets.
-				val := secrets[key[1]]
-				headers.Add(key[0], string(val))
+		for key, val := range iHeaders {
+			// We try and fetch the value from the stored secrets.
+			value, ok := secrets[val]
+			if !ok {
+				return append(errs, gqlerror.ErrorPosf(graphql.Position,
+					"Type %s; Field %s; introspectionHeaders in @custom directive should use secrets to store the header value. To do that specify `%s` in this format '#Dgraph.Secret name value' at the bottom of your schema file.",
+					typ.Name, field.Name, val,))
 			}
+			headers.Add(key, string(value))
 		}
 		if err := validateRemoteGraphql(&remoteGraphqlMetadata{
 			parentType:   typ,

diff --git a/graphql/schema/testdata/schemagen/output/authorization.graphql b/graphql/schema/testdata/schemagen/output/authorization.graphql
@@ -68,6 +68,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/comments-and-descriptions.graphql b/graphql/schema/testdata/schemagen/output/comments-and-descriptions.graphql
@@ -73,6 +73,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/custom-mutation.graphql b/graphql/schema/testdata/schemagen/output/custom-mutation.graphql
@@ -61,6 +61,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/custom-nested-types.graphql b/graphql/schema/testdata/schemagen/output/custom-nested-types.graphql
@@ -78,6 +78,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/custom-query-mixed-types.graphql b/graphql/schema/testdata/schemagen/output/custom-query-mixed-types.graphql
@@ -62,6 +62,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/custom-query-not-dgraph-type.graphql b/graphql/schema/testdata/schemagen/output/custom-query-not-dgraph-type.graphql
@@ -61,6 +61,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/custom-query-with-dgraph-type.graphql b/graphql/schema/testdata/schemagen/output/custom-query-with-dgraph-type.graphql
@@ -57,6 +57,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/deprecated.graphql b/graphql/schema/testdata/schemagen/output/deprecated.graphql
@@ -57,6 +57,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/...stdata/schemagen/output/dgraph-reverse-directive-on-concrete-type-with-interfaces.graphql b/...stdata/schemagen/output/dgraph-reverse-directive-on-concrete-type-with-interfaces.graphql
@@ -71,6 +71,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/dgraph-reverse-directive-with-interfaces.graphql b/graphql/schema/testdata/schemagen/output/dgraph-reverse-directive-with-interfaces.graphql
@@ -71,6 +71,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/field-with-id-directive.graphql b/graphql/schema/testdata/schemagen/output/field-with-id-directive.graphql
@@ -70,6 +70,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/...schema/testdata/schemagen/output/field-with-reverse-predicate-in-dgraph-directive.graphql b/...schema/testdata/schemagen/output/field-with-reverse-predicate-in-dgraph-directive.graphql
@@ -64,6 +64,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/hasInverse-with-interface-having-directive.graphql b/graphql/schema/testdata/schemagen/output/hasInverse-with-interface-having-directive.graphql
@@ -81,6 +81,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/hasInverse-with-interface.graphql b/graphql/schema/testdata/schemagen/output/hasInverse-with-interface.graphql
@@ -82,6 +82,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/hasInverse-with-type-having-directive.graphql b/graphql/schema/testdata/schemagen/output/hasInverse-with-type-having-directive.graphql
@@ -81,6 +81,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/hasInverse.graphql b/graphql/schema/testdata/schemagen/output/hasInverse.graphql
@@ -62,6 +62,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/hasInverse_withSubscription.graphql b/graphql/schema/testdata/schemagen/output/hasInverse_withSubscription.graphql
@@ -62,6 +62,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/ignore-unsupported-directive.graphql b/graphql/schema/testdata/schemagen/output/ignore-unsupported-directive.graphql
@@ -64,6 +64,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/interface-with-id-directive.graphql b/graphql/schema/testdata/schemagen/output/interface-with-id-directive.graphql
@@ -66,6 +66,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/interface-with-no-ids.graphql b/graphql/schema/testdata/schemagen/output/interface-with-no-ids.graphql
@@ -66,6 +66,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/interfaces-with-types-and-password.graphql b/graphql/schema/testdata/schemagen/output/interfaces-with-types-and-password.graphql
@@ -88,6 +88,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/interfaces-with-types.graphql b/graphql/schema/testdata/schemagen/output/interfaces-with-types.graphql
@@ -88,6 +88,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/no-id-field-with-searchables.graphql b/graphql/schema/testdata/schemagen/output/no-id-field-with-searchables.graphql
@@ -56,6 +56,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/no-id-field.graphql b/graphql/schema/testdata/schemagen/output/no-id-field.graphql
@@ -68,6 +68,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/password-type.graphql b/graphql/schema/testdata/schemagen/output/password-type.graphql
@@ -57,6 +57,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/searchables-references.graphql b/graphql/schema/testdata/schemagen/output/searchables-references.graphql
@@ -66,6 +66,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }
 

diff --git a/graphql/schema/testdata/schemagen/output/searchables.graphql b/graphql/schema/testdata/schemagen/output/searchables.graphql
@@ -83,6 +83,7 @@ input CustomHTTP {
 	mode: Mode
 	forwardHeaders: [String!]
 	secretHeaders: [String!]
+	introspectionHeaders: [String!]
 	skipIntrospection: Boolean
 }