Skip to content

Commit

Permalink
Merge pull request #42 from snyk/fix/use_supplied_http_creds
Browse files Browse the repository at this point in the history
fix: prefer supplied url creds over auth header
  • Loading branch information
Dar Malovani authored May 23, 2017
2 parents 4d08d7f + 1ea8450 commit 74c0020
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 14 deletions.
2 changes: 1 addition & 1 deletion cli/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = (args) => {
const dir = path.resolve(root, project);

return fs.readdir(root).then(files => {
if (!files.includes(project)) {
if (files.indexOf(project) === -1) {
throw new Error(`${project} is an invalid template name`);
}

Expand Down
23 changes: 16 additions & 7 deletions lib/relay.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,22 @@ function responseHandler(filterRules, config) {
const parsed = parse(result);
const auth = parsed.auth;

// put the token in the authorization header instead of on the URL
// if it's there.
if (auth && !auth.includes(':')) {
headers.authorization = `token ${auth}`;
// then strip from the url
delete parsed.auth;
result = format(parsed);
if (auth) {
// if URL contains basic auth,
// remove authorization header to prefer auth on the URL.
if (auth.includes(':')) {
delete headers.authorization;
}

// if URL contains token auth,
// put the token in the authorization header
// instead of on the URL.
else {
headers.authorization = `token ${auth}`;
// then strip from the url
delete parsed.auth;
result = format(parsed);
}
}

// if the request is all good - and at this point it is, we'll check
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"start": "node lib",
"dev": "node lib | ./node_modules/.bin/bunyan",
"es5ify": "babel --ignore=node_modules . -d .",
"test": "CI=1 tap -R spec test/**/*.test.js --timeout 60 2>&1 | ./node_modules/.bin/bunyan",
"test": "CI=1 tap -R spec test/**/*.test.js --timeout 60",
"lint": "eslint cli/*.js lib/**/*.js",
"check-tests": "! grep 'test.only' test/**/*.test.js -n",
"cover": "tap test/**/*.test.js --timeout 60 --cov --coverage-report=lcov",
Expand Down
12 changes: 12 additions & 0 deletions test/fixtures/client/filters-https.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@
]
},

{
"path": "/echo-headers/github",
"method": "POST",
"origin": "http://githubToken@localhost:${originPort}"
},

{
"path": "/echo-headers/bitbucket",
"method": "POST",
"origin": "http://bitbucketUser:bitbucketPassword@localhost:${originPort}"
},

{
"path": "/echo-headers",
"method": "POST",
Expand Down
12 changes: 12 additions & 0 deletions test/fixtures/client/filters.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@
]
},

{
"path": "/echo-headers/github",
"method": "POST",
"origin": "http://githubToken@localhost:${originPort}"
},

{
"path": "/echo-headers/bitbucket",
"method": "POST",
"origin": "http://bitbucketUser:bitbucketPassword@localhost:${originPort}"
},

{
"path": "/echo-headers",
"method": "POST",
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/server/filters.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
},

{
"path": "/echo-headers",
"path": "/echo-headers/:param?",
"method": "POST",
"origin": "http://localhost:${originPort}"
},
Expand Down
32 changes: 29 additions & 3 deletions test/functional/server-client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ test('proxy requests originating from behind the broker server', t => {
// wait for the client to successfully connect to the server and identify itself
server.io.on('connection', socket => {
socket.on('identify', token => {
t.plan(12);
t.plan(16);

t.test('successfully broker POST', t => {
const url = `http://localhost:${serverPort}/broker/${token}/echo-body`;
Expand Down Expand Up @@ -176,8 +176,34 @@ test('proxy requests originating from behind the broker server', t => {

t.test('content-length is set without chunked http', t => {
const url = `http://localhost:${serverPort}/broker/${token}/echo-headers`;
request({ url, method: 'get' }, (err, res) => {
t.ok(res.headers['Content-Length'], 'found content-length header');
request({ url, method: 'post'}, (err, res) => {
t.ok(res.headers['content-length'], 'found content-length header');
t.end();
});
});

t.test('auth header is replaced when url contains token', t => {
const url = `http://localhost:${serverPort}/broker/${token}/echo-headers/github`;
const headers = {Authorization: 'broker auth'};
request({ url, method: 'post', headers }, (err, res) => {
const responseBody = JSON.parse(res.body);
t.equal(res.statusCode, 200, '200 statusCode');
t.equal(responseBody.authorization, 'token githubToken',
'auth header was replaced by github token');
t.end();
});
});

t.test('auth header is is replaced when url contains basic auth', t => {
const url = `http://localhost:${serverPort}/broker/${token}/echo-headers/bitbucket`;
const headers = {};
request({ url, method: 'post', headers }, (err, res) => {
const responseBody = JSON.parse(res.body);
t.equal(res.statusCode, 200, '200 statusCode');
const auth = responseBody.authorization.replace('Basic ', '');
const encodedAuth = Buffer.from(auth, 'base64').toString('utf-8');
t.equal(encodedAuth, 'bitbucketUser:bitbucketPassword',
'auth header is set correctly');
t.end();
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ echoServer.post('/echo-body/:param?', (req, res) => {
res.send(req.body);
});

echoServer.post('/echo-headers', (req, res) => {
echoServer.post('/echo-headers/:param?', (req, res) => {
res.json(req.headers);
});

Expand Down

0 comments on commit 74c0020

Please sign in to comment.