From 7229dc5681eb1986983c701627eabaf0aaadd705 Mon Sep 17 00:00:00 2001 From: Apoorv Vardhan Date: Mon, 8 Jun 2020 19:59:05 +0530 Subject: [PATCH 1/5] Added support for custom names in headers --- graphql/schema/rules.go | 8 ++++++-- graphql/schema/schemagen.go | 6 +++++- graphql/schema/wrappers.go | 16 ++++++++++++---- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index 90f5fa6a45f..00f4d6b95a5 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -1613,9 +1613,13 @@ func customDirectiveValidation(sch *ast.Schema, headers := http.Header{} if secretHeaders != nil { for _, h := range secretHeaders.Children { + key := strings.Split(h.Value.Raw, ":") + if len(key) != 2 { + continue + } // We try and fetch the value from the stored secrets. - val := secrets[h.Value.Raw] - headers.Add(h.Value.Raw, string(val)) + val := secrets[key[1]] + headers.Add(key[0], string(val)) } } if err := validateRemoteGraphql(&remoteGraphqlMetadata{ diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go index 8ba1c261fe0..dc1a7574a7d 100644 --- a/graphql/schema/schemagen.go +++ b/graphql/schema/schemagen.go @@ -259,7 +259,11 @@ func getAllowedHeaders(sch *ast.Schema, definitions []string) string { return } for _, h := range forwardHeaders.Children { - headers[h.Value.Raw] = struct{}{} + key := strings.Split(h.Value.Raw, ":") + if len(key) != 2 { + continue + } + headers[key[1]] = struct{}{} } } diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go index 8e86543dac3..06949aa04ff 100644 --- a/graphql/schema/wrappers.go +++ b/graphql/schema/wrappers.go @@ -847,8 +847,12 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err if secretHeaders != nil { hc.RLock() for _, h := range secretHeaders.Children { - val := string(hc.secrets[h.Value.Raw]) - fconf.ForwardHeaders.Set(h.Value.Raw, val) + key := strings.Split(h.Value.Raw, ":") + if len(key) != 2 { + continue + } + val := string(hc.secrets[key[1]]) + fconf.ForwardHeaders.Set(key[0], val) } hc.RUnlock() } @@ -857,8 +861,12 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err if forwardHeaders != nil { for _, h := range forwardHeaders.Children { // We would override the header if it was also specified as part of secretHeaders. - reqHeaderVal := f.op.header.Get(h.Value.Raw) - fconf.ForwardHeaders.Set(h.Value.Raw, reqHeaderVal) + key := strings.Split(h.Value.Raw, ":") + if len(key) != 2 { + continue + } + reqHeaderVal := f.op.header.Get(key[1]) + fconf.ForwardHeaders.Set(key[0], reqHeaderVal) } } From b6abf68b24098d0eac4283abaedfbcb3cfdfa8be Mon Sep 17 00:00:00 2001 From: Apoorv Vardhan Date: Mon, 8 Jun 2020 20:34:58 +0530 Subject: [PATCH 2/5] Continue to support old header definition --- graphql/schema/rules.go | 6 +++--- graphql/schema/schemagen.go | 2 +- graphql/schema/wrappers.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index 00f4d6b95a5..140ae35abed 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -1615,11 +1615,11 @@ func customDirectiveValidation(sch *ast.Schema, for _, h := range secretHeaders.Children { key := strings.Split(h.Value.Raw, ":") if len(key) != 2 { - continue + 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)) + val := secrets[key[0]] + headers.Add(key[1], string(val)) } } if err := validateRemoteGraphql(&remoteGraphqlMetadata{ diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go index dc1a7574a7d..2b0e3bbcca5 100644 --- a/graphql/schema/schemagen.go +++ b/graphql/schema/schemagen.go @@ -261,7 +261,7 @@ func getAllowedHeaders(sch *ast.Schema, definitions []string) string { for _, h := range forwardHeaders.Children { key := strings.Split(h.Value.Raw, ":") if len(key) != 2 { - continue + key = []string{h.Value.Raw, h.Value.Raw} } headers[key[1]] = struct{}{} } diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go index 06949aa04ff..6890c5c7d4b 100644 --- a/graphql/schema/wrappers.go +++ b/graphql/schema/wrappers.go @@ -849,7 +849,7 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err for _, h := range secretHeaders.Children { key := strings.Split(h.Value.Raw, ":") if len(key) != 2 { - continue + key = []string{h.Value.Raw, h.Value.Raw} } val := string(hc.secrets[key[1]]) fconf.ForwardHeaders.Set(key[0], val) @@ -863,7 +863,7 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err // We would override the header if it was also specified as part of secretHeaders. key := strings.Split(h.Value.Raw, ":") if len(key) != 2 { - continue + key = []string{h.Value.Raw, h.Value.Raw} } reqHeaderVal := f.op.header.Get(key[1]) fconf.ForwardHeaders.Set(key[0], reqHeaderVal) From 2b4df9c3ace9c0e8528dad52ef14dcb1196c9a75 Mon Sep 17 00:00:00 2001 From: Apoorv Vardhan Date: Tue, 9 Jun 2020 16:16:05 +0530 Subject: [PATCH 3/5] Addressed comments --- graphql/schema/rules.go | 11 ++++++++--- graphql/schema/schemagen.go | 4 +++- graphql/schema/wrappers.go | 8 ++++++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index 140ae35abed..6ddb0be4752 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -1614,12 +1614,17 @@ func customDirectiveValidation(sch *ast.Schema, if secretHeaders != nil { for _, h := range secretHeaders.Children { key := strings.Split(h.Value.Raw, ":") - if len(key) != 2 { + if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} + } else if len(key) > 2 { + return 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) } // We try and fetch the value from the stored secrets. - val := secrets[key[0]] - headers.Add(key[1], string(val)) + val := secrets[key[1]] + headers.Add(key[0], string(val)) } } if err := validateRemoteGraphql(&remoteGraphqlMetadata{ diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go index 2b0e3bbcca5..d6ba96c521f 100644 --- a/graphql/schema/schemagen.go +++ b/graphql/schema/schemagen.go @@ -260,8 +260,10 @@ func getAllowedHeaders(sch *ast.Schema, definitions []string) string { } for _, h := range forwardHeaders.Children { key := strings.Split(h.Value.Raw, ":") - if len(key) != 2 { + if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} + } else if len(key) > 2 { + continue } headers[key[1]] = struct{}{} } diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go index 6890c5c7d4b..6110758abdd 100644 --- a/graphql/schema/wrappers.go +++ b/graphql/schema/wrappers.go @@ -848,8 +848,10 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err hc.RLock() for _, h := range secretHeaders.Children { key := strings.Split(h.Value.Raw, ":") - if len(key) != 2 { + if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} + } else if len(key) > 2 { + continue } val := string(hc.secrets[key[1]]) fconf.ForwardHeaders.Set(key[0], val) @@ -862,8 +864,10 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err for _, h := range forwardHeaders.Children { // We would override the header if it was also specified as part of secretHeaders. key := strings.Split(h.Value.Raw, ":") - if len(key) != 2 { + if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} + } else if len(key) > 2 { + continue } reqHeaderVal := f.op.header.Get(key[1]) fconf.ForwardHeaders.Set(key[0], reqHeaderVal) From bd3a154b97d7c5b9f01ba7b75fca4cbbc8057e6f Mon Sep 17 00:00:00 2001 From: Apoorv Vardhan Date: Fri, 12 Jun 2020 21:45:31 +0530 Subject: [PATCH 4/5] Added integration test --- graphql/e2e/custom_logic/cmd/main.go | 21 ++++++++++ graphql/e2e/custom_logic/custom_logic_test.go | 41 ++++++++++++++++++- graphql/schema/rules.go | 4 +- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/graphql/e2e/custom_logic/cmd/main.go b/graphql/e2e/custom_logic/cmd/main.go index 36ac72590c1..11bf843aa1f 100644 --- a/graphql/e2e/custom_logic/cmd/main.go +++ b/graphql/e2e/custom_logic/cmd/main.go @@ -295,6 +295,26 @@ func verifyHeadersHandler(w http.ResponseWriter, r *http.Request) { check2(w.Write([]byte(`[{"id":"0x3","name":"Star Wars"}]`))) } +func verifyCustomNameHeadersHandler(w http.ResponseWriter, r *http.Request) { + err := verifyRequest(r, expectedRequest{ + method: http.MethodGet, + urlSuffix: "/verifyCustomNameHeaders", + body: "", + headers: map[string][]string{ + "X-App-Token": {"app-token"}, + "X-User-Id": {"123"}, + "Authorization": {"random-fake-token"}, + "Accept-Encoding": nil, + "User-Agent": nil, + }, + }) + if err != nil { + check2(w.Write([]byte(err.Error()))) + return + } + check2(w.Write([]byte(`[{"id":"0x3","name":"Star Wars"}]`))) +} + func twitterFollwerHandler(w http.ResponseWriter, r *http.Request) { err := verifyRequest(r, expectedRequest{ method: http.MethodGet, @@ -1114,6 +1134,7 @@ func main() { http.HandleFunc("/favMovies/", getFavMoviesHandler) http.HandleFunc("/favMoviesPost/", postFavMoviesHandler) http.HandleFunc("/verifyHeaders", verifyHeadersHandler) + http.HandleFunc("/verifyCustomNameHeaders", verifyCustomNameHeadersHandler) http.HandleFunc("/twitterfollowers", twitterFollwerHandler) // for mutations diff --git a/graphql/e2e/custom_logic/custom_logic_test.go b/graphql/e2e/custom_logic/custom_logic_test.go index 42761efd622..6a3d506b119 100644 --- a/graphql/e2e/custom_logic/custom_logic_test.go +++ b/graphql/e2e/custom_logic/custom_logic_test.go @@ -168,6 +168,45 @@ func TestCustomQueryShouldForwardHeaders(t *testing.T) { secretHeaders: ["Github-Api-Token", "X-App-Token"] }) } + + # Dgraph.Secret Github-Api-Token "random-fake-token" + # Dgraph.Secret app "should-be-overriden" + ` + updateSchemaRequireNoGQLErrors(t, schema) + time.Sleep(2 * time.Second) + + query := ` + query { + verifyHeaders(id: "0x123") { + id + name + } + }` + params := &common.GraphQLParams{ + Query: query, + Headers: map[string][]string{ + "X-App-Token": []string{"app-token"}, + "X-User-Id": []string{"123"}, + "Random-header": []string{"random"}, + }, + } + + result := params.ExecuteAsPost(t, alphaURL) + require.Nil(t, result.Errors) + expected := `{"verifyHeaders":[{"id":"0x3","name":"Star Wars"}]}` + require.Equal(t, expected, string(result.Data)) +} + +func TestCustomNameForwardHeaders(t *testing.T) { + schema := customTypes + ` + type Query { + verifyHeaders(id: ID!): [Movie] @custom(http: { + url: "http://mock:8888/verifyCustomNameHeaders", + method: "GET", + forwardHeaders: ["X-App-Token:App", "X-User-Id"], + secretHeaders: ["Authorization:Github-Api-Token", "X-App-Token"] + }) + } # Dgraph.Secret Github-Api-Token "random-fake-token" # Dgraph.Secret X-App-Token "should-be-overriden" @@ -185,7 +224,7 @@ func TestCustomQueryShouldForwardHeaders(t *testing.T) { params := &common.GraphQLParams{ Query: query, Headers: map[string][]string{ - "X-App-Token": []string{"app-token"}, + "App": []string{"app-token"}, "X-User-Id": []string{"123"}, "Random-header": []string{"random"}, }, diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index 6ddb0be4752..9f8a1514951 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -1617,10 +1617,10 @@ func customDirectiveValidation(sch *ast.Schema, if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} } else if len(key) > 2 { - return gqlerror.ErrorPosf(graphql.Position, + 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) + typ.Name, field.Name, h.Value.Raw)) } // We try and fetch the value from the stored secrets. val := secrets[key[1]] From 5b20d74a5d6890527a45b44cb4419f66983a0bd1 Mon Sep 17 00:00:00 2001 From: Apoorv Vardhan Date: Tue, 16 Jun 2020 21:46:46 +0530 Subject: [PATCH 5/5] Moved validations to rules.go --- graphql/schema/rules.go | 31 ++++++++++++++++++++++++++----- graphql/schema/schemagen.go | 2 -- graphql/schema/wrappers.go | 4 ---- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index 9f8a1514951..a8a49af38d1 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -1604,6 +1604,32 @@ func customDirectiveValidation(sch *ast.Schema, } } + 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)) + } + } + } + + 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)) + } + } + } + if errs != nil { return errs } @@ -1616,11 +1642,6 @@ func customDirectiveValidation(sch *ast.Schema, key := strings.Split(h.Value.Raw, ":") if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} - } else 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)) } // We try and fetch the value from the stored secrets. val := secrets[key[1]] diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go index d6ba96c521f..13fbccd4545 100644 --- a/graphql/schema/schemagen.go +++ b/graphql/schema/schemagen.go @@ -262,8 +262,6 @@ func getAllowedHeaders(sch *ast.Schema, definitions []string) string { key := strings.Split(h.Value.Raw, ":") if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} - } else if len(key) > 2 { - continue } headers[key[1]] = struct{}{} } diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go index 6110758abdd..a9f5a16e6fe 100644 --- a/graphql/schema/wrappers.go +++ b/graphql/schema/wrappers.go @@ -850,8 +850,6 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err key := strings.Split(h.Value.Raw, ":") if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} - } else if len(key) > 2 { - continue } val := string(hc.secrets[key[1]]) fconf.ForwardHeaders.Set(key[0], val) @@ -866,8 +864,6 @@ func getCustomHTTPConfig(f *field, isQueryOrMutation bool) (FieldHTTPConfig, err key := strings.Split(h.Value.Raw, ":") if len(key) == 1 { key = []string{h.Value.Raw, h.Value.Raw} - } else if len(key) > 2 { - continue } reqHeaderVal := f.op.header.Get(key[1]) fconf.ForwardHeaders.Set(key[0], reqHeaderVal)