From d7eb36cbe17d097e354824b8999a70909e2cb26b Mon Sep 17 00:00:00 2001 From: Dar Malovani Date: Tue, 23 May 2017 11:31:42 +0300 Subject: [PATCH 1/3] fix: prefer supplied url creds over auth header In a case when a request url contains HTTP credentials on the url, we want to prefer them over the auth header. The resolution is to strip the Authorization header. From: `Authorization: auth https://user:name@someapi.com/v1/repos` To: `https://user:name@someapi.com/v1/repos` --- lib/relay.js | 23 ++++++++++++++++------- test/functional/client-server.test.js | 24 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lib/relay.js b/lib/relay.js index 785f21c77..0dab653ea 100644 --- a/lib/relay.js +++ b/lib/relay.js @@ -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 diff --git a/test/functional/client-server.test.js b/test/functional/client-server.test.js index 53e7e70b7..16d24c269 100644 --- a/test/functional/client-server.test.js +++ b/test/functional/client-server.test.js @@ -128,6 +128,30 @@ test('proxy requests originating from behind the broker client', t => { }); }); + t.test('auth header is replaced when url contains token', t => { + const url = `http://githubToken@localhost:${clientPort}/echo-headers`; + 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 deleted when url contains basic auth', t => { + const url = `http://username@pass@localhost:${clientPort}/echo-headers`; + 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.notOk(responseBody['authorization'], 'token githubToken', + 'auth header was deleted, to prefer creds on the url'); + t.end(); + }); + }); + t.test('clean up', t => { client.close(); setTimeout(() => { From d6643c82c5bd7c5a3b94d1c0b8c4bc791566a796 Mon Sep 17 00:00:00 2001 From: Dar Malovani Date: Tue, 23 May 2017 16:46:36 +0300 Subject: [PATCH 2/3] test: testing correctly auth replacement --- package.json | 2 +- test/fixtures/client/filters-https.json | 12 ++++++++++ test/fixtures/client/filters.json | 12 ++++++++++ test/fixtures/server/filters.json | 2 +- test/functional/client-server.test.js | 24 ------------------- test/functional/server-client.test.js | 32 ++++++++++++++++++++++--- test/utils.js | 2 +- 7 files changed, 56 insertions(+), 30 deletions(-) diff --git a/package.json b/package.json index c2ef40008..6b36d5322 100644 --- a/package.json +++ b/package.json @@ -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 2>&1", "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", diff --git a/test/fixtures/client/filters-https.json b/test/fixtures/client/filters-https.json index f8e79e254..ed5982890 100644 --- a/test/fixtures/client/filters-https.json +++ b/test/fixtures/client/filters-https.json @@ -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", diff --git a/test/fixtures/client/filters.json b/test/fixtures/client/filters.json index ce063297a..b503e8971 100644 --- a/test/fixtures/client/filters.json +++ b/test/fixtures/client/filters.json @@ -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", diff --git a/test/fixtures/server/filters.json b/test/fixtures/server/filters.json index bbb064067..90eeddf78 100644 --- a/test/fixtures/server/filters.json +++ b/test/fixtures/server/filters.json @@ -13,7 +13,7 @@ }, { - "path": "/echo-headers", + "path": "/echo-headers/:param?", "method": "POST", "origin": "http://localhost:${originPort}" }, diff --git a/test/functional/client-server.test.js b/test/functional/client-server.test.js index 16d24c269..53e7e70b7 100644 --- a/test/functional/client-server.test.js +++ b/test/functional/client-server.test.js @@ -128,30 +128,6 @@ test('proxy requests originating from behind the broker client', t => { }); }); - t.test('auth header is replaced when url contains token', t => { - const url = `http://githubToken@localhost:${clientPort}/echo-headers`; - 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 deleted when url contains basic auth', t => { - const url = `http://username@pass@localhost:${clientPort}/echo-headers`; - 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.notOk(responseBody['authorization'], 'token githubToken', - 'auth header was deleted, to prefer creds on the url'); - t.end(); - }); - }); - t.test('clean up', t => { client.close(); setTimeout(() => { diff --git a/test/functional/server-client.test.js b/test/functional/server-client.test.js index 9bdff9fc0..a963b26c3 100644 --- a/test/functional/server-client.test.js +++ b/test/functional/server-client.test.js @@ -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`; @@ -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(); }); }); diff --git a/test/utils.js b/test/utils.js index 7dab78ce0..f1c2db927 100644 --- a/test/utils.js +++ b/test/utils.js @@ -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); }); From 1ea8450430533645862b07016f0780f04d4130ce Mon Sep 17 00:00:00 2001 From: Dar Malovani Date: Tue, 23 May 2017 18:46:19 +0300 Subject: [PATCH 3/3] fix: support node4 on init --- cli/init.js | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/init.js b/cli/init.js index ac9399106..02487309a 100644 --- a/cli/init.js +++ b/cli/init.js @@ -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`); } diff --git a/package.json b/package.json index 6b36d5322..4ee2cfe03 100644 --- a/package.json +++ b/package.json @@ -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", + "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",