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

Authorization header shouldn't be forwarded across redirects #450

Closed
jbt opened this issue Feb 28, 2013 · 8 comments
Closed

Authorization header shouldn't be forwarded across redirects #450

jbt opened this issue Feb 28, 2013 · 8 comments

Comments

@jbt
Copy link

jbt commented Feb 28, 2013

Authorization headers are getting preserved even after redirects to a different server, which breaks if the target of the redirect isn't expecting Authorization (e.g. Amazon S3 throws a 400 error).

Ideally the behaviour would be the same as in curl or requesting the URL in a browser

For example:

var app = require('express')();

app.get('*', function(req, res){
  res.redirect('http://aws-lightbox.s3.amazonaws.com/source/jquery.fancybox.css');
});

app.listen(5678)

require('request')('http://foo:bar@localhost:5678/', function(err,resp){
  console.log('Request', resp.statusCode);
});

require('child_process').exec('curl -L -w "%{http_code}" -o /dev/null http://foo:bar@localhost:5678/', function(err, resp){
  console.log('Curl', resp)
});

Outputs:

Request 400
Curl 200

This is currently breaking NPM with links to GitHub private repo tarballs using basic auth, as they redirect to S3

@mikeal
Copy link
Member

mikeal commented Feb 28, 2013

i could have sworn this was fixed in a pull request, are you sure this is an issue on master?

@jbt
Copy link
Author

jbt commented Feb 28, 2013

Yep, pretty sure, happens with both the version on NPM and direct from github.

@ArVan
Copy link

ArVan commented Sep 8, 2014

Hi all, I'm also experiencing this issue. Any news here? I'm, using the latest npm version (2.42.0)

@mikeal
Copy link
Member

mikeal commented Sep 8, 2014

this was just fixed, and should be fixed in the latest npm version.

@nylen
Copy link
Member

nylen commented Sep 8, 2014

You're thinking of @isaacs change to remove headers when proxying, right? That's a different issue - we still need to remove authorization headers on redirect to a different host. #1058 is a good start on this.

@mikeal
Copy link
Member

mikeal commented Sep 9, 2014

ahh, yea, good catch :)

@simov
Copy link
Member

simov commented Sep 10, 2015

@mikofski a better approach would be to create a new issue, and reference all other issues there instead of spamming the same content in all threads. GitHub puts back reference links to them.

I removed your posts in the other 3 issues.

@simov
Copy link
Member

simov commented Sep 10, 2015

Yep, open another one, and link to the other issue from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants