Add option to rewrite domain in set-cookie headers

[Volune]

Sometime the set-cookie header specify a domain for the cookie. When proxying the a target using changeOrigin option, the response may contain a cookie not matching the domain of the client.

Example of such a cookie: foo=bar; domain=my.domain; expires=Tue, 12-Feb-2019 10:50:41 GMT; path=/

This pull request adds an option to rewrite the set-cookie header of the response.

Possible option values:

• disabled (default): cookieDomainRewrite: false

• rewrite all domains: cookieDomainRewrite: "my.client.domain"

• remove all domains: cookieDomainRewrite: ""

• more advanced configuration (this example removes all except one):

  cookieDomainRewrite: {
    "*": "",
    "some.domain": "some.domain"
  }
  

Let me know if I can improve this option or for any question.

[drouillard]

Looks promising

[parse]

Would love to see this merged.

[jcrugzz]

single quotes, space after if.

[Volune]

PR should be clean now, I also rebased on top of current master, and improved function comments. Let me know if I missed anything.

[jcrugzz]

Lets cache this regex so it isnt created every time the function is called. Put var domainRegex = ... up top here and use it in this function.

[jcrugzz]

@Volune Overall this looks good. I would like to see some more documentation for this option in the README, similar to what you have in this PR in your initial proposal. The only thing that kind of bothers me is having '' remove all domains. Would it be more intuitive to use an explicit null as removal? I'm really not sure what the right answer is but is there something that semantically makes more sense for that implication?

[Volune]

@jcrugzz The first reason of using '' is that it would transform Domain=some.domain to Domain=, which has the meaning of no domain, but (as far as I understand RFC 6265) is not a valid value. So I used it to remove the domain.

Also, it adds some meaning to the type of the option:

• boolean false: disable the option
• object: advanced configuration
• string: shorthand for { '*': value } configuration

I'm not against using explicit null and/or explicit undefined to remove the domain, and document it.
Let me know what's your opinion, decision.

[jcrugzz]

@Volune Thats reasonable, i think its ok. Lets just document all the options for this option in the readme, take care of that one nit i have so that we are a bit more performant so we arent creating the regex everytime and we cache it in a variable at the top of common.js (with the other one thats there. Then we will get this merged :). Thanks for bearing with me

[jcrugzz]

Also we just need a minor rebase. since i merged some other PRs

[Volune]

Should be all good.

[isaachinman]

Hey everyone, thanks for this - a very helpful feature.

Wondering if there is any built-in way to also rewrite secure/httpOnly option? In local development, I am using localhost:3000, which obviously isn't secure, so even with the domain rewrite, cookies are failing to be set.

[joeskeen]

@isaachinman I am faced with this same issue as well. Perhaps it would be good to open up a separate issue for this feature.

[pahlers]

@isaachinman Just information, node-proxy-middleware has this implemented. It can't be hard to implement this in node-http-proxy

https://github.com/gonzalocasas/node-proxy-middleware/blob/master/index.js#L113

[edit] I added this onProxyRes-function to solve the problem temporary.

onProxyRes: function (proxyRes, req, res) {
   let existingCookies = proxyRes.headers['set-cookie'],
   rewrittenCookies = [];

    if (existingCookies !== undefined) {
        if (!Array.isArray(existingCookies)) {
            existingCookies = [existingCookies];
        }

        for (let i = 0; i < existingCookies.length; i++) {
             rewrittenCookies.push(existingCookies[i].replace(/;\s*?(Secure)/i, ''));
        }

        proxyRes.headers['set-cookie'] = rewrittenCookies;
    }
}