Skip to content

Commit

Permalink
Don't forward authorization header across requests to different hosts
Browse files Browse the repository at this point in the history
Fixes #450 and fixes #860.
  • Loading branch information
nylen committed Oct 17, 2014
1 parent 986b941 commit 210b326
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions request.js
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,9 @@ Request.prototype.onRequestResponse = function (response) {
return
}

// Save the original host before any redirect (if it changes, we need to
// remove any authorization headers)
self.originalHost = self.headers.host
if (self.setHost) {
self.removeHeader('host')
}
Expand Down Expand Up @@ -1252,6 +1255,12 @@ Request.prototype.onRequestResponse = function (response) {
self.removeHeader('host')
self.removeHeader('content-type')
self.removeHeader('content-length')
if (self.uri.hostname !== self.originalHost.split(':')[0]) {
// Remove authorization if changing hostnames (but not if just
// changing ports or protocols). This matches the behavior of curl:
// https://github.com/bagder/curl/blob/6beb0eee/lib/http.c#L710
self.removeHeader('authorization')
}
}
}

Expand Down

2 comments on commit 210b326

@sbrinkmann
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By turning off the authentication while redirecting, you break some SSO mechanisms such as SAML. There should be a flag which keeps the authentication header, even when there is redirect to another host.

@nylen
Copy link
Member Author

@nylen nylen commented on 210b326 Jan 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make an issue for this? Discussion on commits is harder to follow.

Please sign in to comment.