When redirecting to a different host, strip Authorization header

[bitinn]

This is a tricky one, on the surface, security first, why would you want to do that?

But:

• There isn't a Spec / RFC enforcing it.
• Some single sign-on solutions even rely on it.

Request that do implement such a thing:

• https://github.com/request/request/blob/b12a624/lib/redirect.js#L134

So does curl (but only when using a http proxy?)

• https://github.com/curl/curl/blob/6beb0eee/lib/http.c#L710
• http://stackoverflow.com/questions/37865875/stopping-curl-from-sending-authorization-header-on-302-redirect

Either way, I am not rushing to fix this, but a heads-up if anyone is using node-fetch for authorisation.

[TimothyGu]

The reason why the Fetch Standard doesn't enforce it is because it doesn't allow fetching a URL with credentials in the first place. See Request() step 6.3:

6. If input is a string, then run these substeps:
   1. Let parsedURL be the result of parsing input with baseURL.
   2. If parsedURL is failure, then throw a TypeError.
   3. If parsedURL includes credentials, then throw a TypeError.

I think we should make this change.

[Richienb]

@bitinn @TimothyGu For this issue, should we stick as close as possible to the spec and deny credentials or just strip the Authorisation header?

[bitinn]

@Richienb I am slightly confused reading Timothy's comment, because mine was about stripping the Authorization header during redirect, he's saying credentials in the URL are denied (which is correct, MDN says Chrome denies it for all cases).

I am pretty sure in the Auth header scenario, Fetch is controlled by credentials mode, which we don't support yet.

So either we now support it, or we just dropped the credentials on redirect using either omit or same-origin policy.

(supporting credentials is probably a better choice, our current behavior is basically include, which would work in most cases, but not secure by default; but it's a bit of work to properly drop cookies and auth headers and TLS client-cert...)

[Richienb]

@bitinn I'll add this to the roadmap.

[bitinn]

close via #1449