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

Adding redirectLocationTrusted option to retain authorization header on redirect domain change #2876

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,7 @@ The first argument can be either a `url` or an `options` object. The only requir
- `followAllRedirects` - follow non-GET HTTP 3xx responses as redirects (default: `false`)
- `followOriginalHttpMethod` - by default we redirect to HTTP method GET. you can enable this property to redirect to the original HTTP method (default: `false`)
- `maxRedirects` - the maximum number of redirects to follow (default: `10`)
- `redirectLocationTrusted` - if a redirect causes a domain change then the authorization header is, by default, prevented from being sent to the new location. If this option is set to true then the authorization header will be retained.
- `removeRefererHeader` - removes the referer header when a redirect happens (default: `false`). **Note:** if true, referer header set in the initial request is preserved during redirect chain.

---
Expand Down
12 changes: 9 additions & 3 deletions lib/redirect.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ function Redirect (request) {
this.redirects = []
this.redirectsFollowed = 0
this.removeRefererHeader = false
this.redirectLocationTrusted = false
}

Redirect.prototype.onRequest = function (options) {
Expand All @@ -40,6 +41,9 @@ Redirect.prototype.onRequest = function (options) {
if (options.followOriginalHttpMethod !== undefined) {
self.followOriginalHttpMethod = options.followOriginalHttpMethod
}
if (options.redirectLocationTrusted !== undefined) {
self.redirectLocationTrusted = options.redirectLocationTrusted
}
}

Redirect.prototype.redirectTo = function (response) {
Expand Down Expand Up @@ -131,9 +135,11 @@ Redirect.prototype.onResponse = function (response) {
request.removeHeader('host')
request.removeHeader('content-type')
request.removeHeader('content-length')
if (request.uri.hostname !== request.originalHost.split(':')[0]) {
// Remove authorization if changing hostnames (but not if just
// changing ports or protocols). This matches the behavior of curl:
if (request.uri.hostname !== request.originalHost.split(':')[0] &&
!self.redirectLocationTrusted) {
// Unless specifically set via 'redirectLocationTrusted' remove any 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
request.removeHeader('authorization')
}
Expand Down
20 changes: 18 additions & 2 deletions tests/test-redirect-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var request = require('../index')
var util = require('util')
var tape = require('tape')
var destroyable = require('server-destroy')
var extend = require('extend')

var s = server.createServer()
var ss = server.createSSLServer()
Expand Down Expand Up @@ -66,9 +67,10 @@ function handleRequests (srv) {
handleRequests(s)
handleRequests(ss)

function runTest (name, redir, expectAuth) {
function runTest (name, redir, additionalOpts, expectAuth) {
tape('redirect to ' + name, function (t) {
request(redir.src, function (err, res, body) {
var opts = extend({url: redir.src}, additionalOpts)
request(opts, function (err, res, body) {
t.equal(err, null)
t.equal(res.request.uri.href, redir.dst)
t.equal(res.statusCode, 200)
Expand All @@ -83,19 +85,33 @@ function runTest (name, redir, expectAuth) {
function addTests () {
runTest('same host and protocol',
redirect.from('http', 'localhost').to('http', 'localhost'),
{},
true)

runTest('same host different protocol',
redirect.from('http', 'localhost').to('https', 'localhost'),
{},
true)

runTest('different host same protocol',
redirect.from('https', '127.0.0.1').to('https', 'localhost'),
{},
false)

runTest('different host and protocol',
redirect.from('http', 'localhost').to('https', '127.0.0.1'),
{},
false)

runTest('different host same protocol, redirectLocationTrusted enabled',
redirect.from('https', '127.0.0.1').to('https', 'localhost'),
{redirectLocationTrusted: true},
true)

runTest('different host and protocol, redirectLocationTrusted enabled',
redirect.from('http', 'localhost').to('https', '127.0.0.1'),
{redirectLocationTrusted: true},
true)
}

tape('setup', function (t) {
Expand Down