Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to add custom headers to the request #28

Open
sebastianmacias opened this issue Aug 23, 2018 · 19 comments · May be fixed by #47
Open

Ability to add custom headers to the request #28

sebastianmacias opened this issue Aug 23, 2018 · 19 comments · May be fixed by #47
Labels

Comments

@sebastianmacias
Copy link

Currently the only header being set is the content type:

/shurcooL/go/ctxhttp/ctxhttp.go:83

req.Header.Set("Content-Type", bodyType)

It would be helpful to allow setting custom headers in addition to the content-type since some graphql servers use headers for auth tokens and setting other variables.

Thanks

@dmitshur
Copy link
Member

dmitshur commented Sep 8, 2018

It is possible to do this by using a custom HTTP client (that sets custom request headers before delegating to the underlying HTTP client).

See https://github.com/shurcooL/githubv4#authentication for more information. Please let me know if you're satisfied with that solution or if you have any additional questions.

@tgwizard
Copy link

@dmitshur it's possible to do by creating your own HTTP Client, but it's a lot of work (just look at all the code in golang.org/x/oauth2 to set the Authorization header). This is what I do:

import "net/http"

type headerTransport struct {
	base    http.RoundTripper
	headers map[string]string
}

func NewHTTPClientWithHeaders(baseRoundTripper http.RoundTripper, headers map[string]string) *http.Client {
	if baseRoundTripper == nil {
		baseRoundTripper = http.DefaultTransport
	}

	return &http.Client{
		Transport: &headerTransport{
			base:    baseRoundTripper,
			headers: headers,
		},
	}
}

func (h *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	req2 := CloneRequest(req)
	for key, val := range h.headers {
		req2.Header.Set(key, val)
	}
	return h.base.RoundTrip(req2)
}

// CloneRequest and CloneHeader copied from https://github.com/kubernetes/apimachinery/blob/master/pkg/util/net/http.go#L424

// CloneRequest creates a shallow copy of the request along with a deep copy of the Headers.
func CloneRequest(req *http.Request) *http.Request {
	r := new(http.Request)

	// shallow clone
	*r = *req

	// deep copy headers
	r.Header = CloneHeader(req.Header)

	return r
}

// CloneHeader creates a deep copy of an http.Header.
func CloneHeader(in http.Header) http.Header {
	out := make(http.Header, len(in))
	for key, values := range in {
		newValues := make([]string, len(values))
		copy(newValues, values)
		out[key] = newValues
	}
	return out
}

Perhaps Query and Mutation can be updated to accept either a header map, or some (variadic list of) interface that allows you to mutate the request etc?

@dmitshur
Copy link
Member

The code in golang.org/x/oauth2 is more complex because it seeks to provide a general solution (rather than a very specific one) with a high degree of flexibility. E.g., it supports various token sources, including ones with a refresh token, etc.

The code you posted is a much more appropriately sized for a customized solution for your app's needs, and I recommend you continue to use that. It works with graphql's existing API and enables you to customize behavior further as needed.

Perhaps Query and Mutation can be updated to accept either a header map,

This would be quite inflexible: not everyone needs this, and it'd stop working for people as soon as they need slightly custom behavior.

or some (variadic list of) interface that allows you to mutate the request etc?

This is more viable, but it needs to be thought out well before committing to it. If you'd like to think more about this, I suggest coming up with some drafts with mock APIs and user code that uses that API. I don't expect I'll have time to work on this soon.

@tgwizard
Copy link

It seems that since #5 is desired, adding more custom HTTP handling will be tricky without implementing the transport-agnostic interface layer. I'd be very happy to pick up work on #5 if others have no desire any more.

The code you posted is a much more appropriately sized for a customized solution for your app's needs, and I recommend you continue to use that. It works with graphql's existing API and enables you to customize behavior further as needed.

It's perhaps appropriately sized, but the complexity that it entails is a bit much. You need to care and know about roundtrippers, and that they aren't supposed to mutate the request, but that a shallow-copy of the request and a deep copy of the headers is fine.

@sateeshpnv
Copy link

How about the below signature?

func (c *Client) Query(ctx context.Context, q interface{}, variables map[string]interface{}, setHeaders func(*http.Request)) error

It is backwards incompatible. However, clients can pass nil for setHeaders argument to not add any custom headers.

Full diff is given below.

diff --git a/graphql.go b/graphql.go
index 8520956..9800623 100644
--- a/graphql.go
+++ b/graphql.go
@@ -33,19 +33,19 @@ func NewClient(url string, httpClient *http.Client) *Client {
 // Query executes a single GraphQL query request,
 // with a query derived from q, populating the response into it.
 // q should be a pointer to struct that corresponds to the GraphQL schema.
-func (c *Client) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error {
-       return c.do(ctx, queryOperation, q, variables)
+func (c *Client) Query(ctx context.Context, q interface{}, variables map[string]interface{}, setHeaders func(*http.Request)) error {
+       return c.do(ctx, queryOperation, q, variables, setHeaders)
 }

 // Mutate executes a single GraphQL mutation request,
 // with a mutation derived from m, populating the response into it.
 // m should be a pointer to struct that corresponds to the GraphQL schema.
-func (c *Client) Mutate(ctx context.Context, m interface{}, variables map[string]interface{}) error {
-       return c.do(ctx, mutationOperation, m, variables)
+func (c *Client) Mutate(ctx context.Context, m interface{}, variables map[string]interface{}, setHeaders func(*http.Request)) error {
+       return c.do(ctx, mutationOperation, m, variables, setHeaders)
 }

 // do executes a single GraphQL operation.
-func (c *Client) do(ctx context.Context, op operationType, v interface{}, variables map[string]interface{}) error {
+func (c *Client) do(ctx context.Context, op operationType, v interface{}, variables map[string]interface{}, setHeaders func(*http.Request)) error {
        var query string
        switch op {
        case queryOperation:
@@ -65,7 +65,16 @@ func (c *Client) do(ctx context.Context, op operationType, v interface{}, variab
        if err != nil {
                return err
        }
-       resp, err := ctxhttp.Post(ctx, c.httpClient, c.url, "application/json", &buf)
+       req, err := http.NewRequest(http.MethodPost, c.url, &buf)
+       if err != nil {
+               return err
+       }
+       req.Header.Set("Content-Type", "application/json")
+       if setHeaders != nil {
+               setHeaders(req)
+       }
+
+       resp, err := ctxhttp.Do(ctx, c.httpClient, req)
        if err != nil {
                return err
        }

@sateeshpnv
Copy link

@dmitshur ,

If the setHeaders() approach is acceptable, I can submit a patch.

I think transport-agnostic stuff mentioned in one of the comments above can be addressed when #5 is addressed.

@riley-martine
Copy link

This would be very useful for GitHub's Schema Previews, which need a custom media type in the Accept header.

@iheanyi iheanyi linked a pull request Oct 9, 2019 that will close this issue
@iheanyi
Copy link

iheanyi commented Oct 9, 2019

@dmitshur How do you feel about the approach taken in #47? This is backwards compatible and extensible via functional options. Thoughts?

@Laisky
Copy link

Laisky commented Dec 24, 2019

you can try my fork https://github.com/Laisky/graphql , support custom headers & cookies, and fully compatible with shurcool/graphql.

@gurpreetz
Copy link

With github deprecating the current authentication method [1] , its likely we would need to at least support the addition of Authorization: token my_access_token to the library.

[1] https://developer.github.com/changes/2020-02-10-deprecating-auth-through-query-param/

@dmitshur
Copy link
Member

dmitshur commented Apr 24, 2020

@gurpreetz Thanks for pointing out that GitHub is deprecating authentication via query parameters, and recommending authentication via request headers.

However, I don't think that changes best practices for using this library. The golang.org/x/oauth2 package already provides authentication via headers, and can continue to be used for authentication. See https://github.com/shurcooL/githubv4#authentication and https://github.com/google/go-github#authentication.

@gurpreetz
Copy link

@dmitshur somehow didnt see your response.
I was already using those methods to authenticate when Github flagged it as soon to be deprecated - hence the request.
It looks like #47 might actually help move things forward.

@dmitshur
Copy link
Member

I was already using those methods to authenticate when Github flagged it as soon to be deprecated - hence the request.

Can you please elaborate? Can I reproduce this somehow?

I've looked at https://docs.github.com/en/graphql/guides/forming-calls-with-graphql#authenticating-with-graphql and I'm not seeing any changes to the authentication on GitHub's side beyond what I've already mentioned in my last comment.

If you think there is an issue, please file a new issue in https://github.com/shurcooL/githubv4/issues with more information.

@gurpreetz
Copy link

gurpreetz commented Sep 15, 2020

This is basically what I have...

	src := oauth2.StaticTokenSource(
		&oauth2.Token{
			AccessToken: githubToken,
		},
	)

	httpClient := oauth2.NewClient(context.Background(), src)
	client := githubv4.NewClient(httpClient)
..
..
        err := client.Query(.....)

I then got an email from github stating that :

your personal access token (dizzy2) using Go-http-client/1.1 was used as part of a query parameter 
to access an endpoint through the GitHub API. 
Please use the Authorization HTTP header instead, as using the `access_token` query parameter is 
deprecated. If this token is being used by an app you don't have control over, be aware that it may 
stop working as a result of this deprecation.

@dmitshur
Copy link
Member

@gurpreetz Thanks. That should not be happening, I'll need to investigate. Can you please file a new issue at https://github.com/shurcooL/githubv4/issues and specify what version of golang.org/x/oauth2 you're using?

@VanCoppenolleWout
Copy link

Hi, I needed to set the authorization in the header for my graphql request and I found a solution that works for me.

I need to provide a jwt token with my request for my backend:

This is what I added:
client = client.WithRequestModifier(setAuthHeader(token))

Full code:
`

client := graphql.NewClient("http://localhost:8080/query", nil)
client = client.WithRequestModifier(setAuthHeader(token))

variables := map[string]interface{}{
	"review": graphql.String("review string"),
	"date": graphql.String("2022-02-01 13:30:00"),
	"likes": graphql.Int(0),
	"comments": graphql.Int(0),
}

err := client.Mutate(context.Background(), &q_createreview, variables)
if err != nil {
	fmt.Println(err)
}
fmt.Println(q_createreview.CreateReview)`

Source: #77

@TizioFittizio
Copy link

@VanCoppenolleWout what is setAuthHeader there?

@ferleal
Copy link

ferleal commented May 19, 2022

It is something like this

func setAuthHeader(token string) graphql.RequestModifier {
	return func(h *http.Request) {
		h.Header.Add("Authorization", token)
	}

}

@bengiddins
Copy link

bengiddins commented May 19, 2022

Ha, found this after crafting a similar approach. Used for connecting to AWS AppSync. I already have a package named

func setHeader(header string, token string) graphql.RequestModifier {
	return func(h *http.Request) {
		h.Header.Add(header, token)
	}
}

var gqlClient *graphql.Client
gqlClient = graphql.NewClient(endpointURL, &http.Client{}).WithRequestModifier(setHeader("x-api-key", apiKey))

grihabor pushed a commit to grihabor/graphql that referenced this issue Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.