graphql-kit / graphql-faker

🎲 Mock or extend your GraphQL API with faked data. No coding required.
MIT License
2.69k stars 225 forks source link

Add a --return-headers option #124

Closed recurser closed 1 year ago

recurser commented 4 years ago

This allows --return-headers to be specified for proxying response headers from the GraphQL server (ie. the opposite of the --forward-headers option). In my case I needed to be able to set cookies in the GraphQL response, but it will work for any headers.

IvanGoncharov commented 4 years ago

@recurser Thanks for PR 👍 I think it totally reasonable to proxy Set-Cookies and similar headers by default so people spend less time figuring what's wrong and what option will fix it. Do you see any problem with proxying such headers by default?

IvanGoncharov commented 4 years ago

Actually thinking about that we can also change code to proxy Authorization header by default.

recurser commented 4 years ago

@IvanGoncharov agreed, I think it is probably reasonable to proxy all response headers by default, since it is what the proxied API maintainers intend - what do you think?

How about if I proxy all headers by default if the option isn't specified? Then if people want to restrict headers for some reason they can specify individual headers, but cookies etc will work out of the box.

IvanGoncharov commented 4 years ago

@recurser Not sure about all headers since some of them request specific and shouldn't even be proxied for simple proxies. Can you build a default whitelist for both request/respond headers based on this list: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers

recurser commented 4 years ago

@IvanGoncharov sounds good 👍 Updated - how does it look now?

IvanGoncharov commented 4 years ago

@recurser Thanks a lot for compiling a list of headers 👍 I need to review them and see if allowing these headers will not break something, will try to do that over the weekend.

If you have time, can you please remove headers that affect the format of the context of request or response? E.g. Content-Length since our response also contains faked data and data can be encoded differently.

recurser commented 4 years ago

@IvanGoncharov sure thing - I've removed everything Content-related to be on the safe side. Feel free to tweak the list or squash the commits if you'd like, or let me know if you'd like any others removed.