From 8e56ef17529f3867e0d907a81f95e40276814037 Mon Sep 17 00:00:00 2001 From: Ben West Date: Sun, 27 Jul 2014 15:12:40 -0700 Subject: [PATCH 1/2] migrate to API-SECRET header @jasoncalabrese pointed out that some agents do odd things with underscores and that using a hyphen is more common. This also adds tests to ensure that the API_SECRET handling works as expected. --- lib/middleware/verify-token.js | 4 +-- tests/security.test.js | 50 ++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/lib/middleware/verify-token.js b/lib/middleware/verify-token.js index 0a390a0b5ea..a0f0b0ad0de 100644 --- a/lib/middleware/verify-token.js +++ b/lib/middleware/verify-token.js @@ -5,12 +5,12 @@ function configure (env) { function verifyAuthorization(req, res, next) { // Retrieve the secret values to be compared. var api_secret = env.api_secret; - var secret = req.params.secret ? req.params.secret : req.header('API_SECRET'); + var secret = req.params.secret ? req.params.secret : req.header('API-SECRET'); // Return an error message if the authorization fails. var unauthorized = (typeof api_secret === 'undefined' || secret != api_secret); if (unauthorized) { - res.sendJSONStatus(res, consts.HTTP_UNAUTHORIZED, 'Unauthorized', 'API_SECRET Request Header is incorect or missing.'); + res.sendJSONStatus(res, consts.HTTP_UNAUTHORIZED, 'Unauthorized', 'API-SECRET Request Header is incorect or missing.'); } else { next(); } diff --git a/tests/security.test.js b/tests/security.test.js index 573a087f41f..53e381fcd20 100644 --- a/tests/security.test.js +++ b/tests/security.test.js @@ -40,11 +40,35 @@ describe('API_SECRET', function ( ) { should.not.exist(env.api_secret); var ctx = setup_app(env, function ( ) { ctx.app.enabled('api').should.be.false; - ping_status(ctx.app, done); + ping_status(ctx.app, again); + function again ( ) { + ping_authorized_endpoint(ctx.app, 404, done); + } }); }); + it('should work fail set unauthorized', function (done) { + var known = 'b723e97aa97846eb92d5264f084b2823f57c4aa1'; + delete process.env.API_SECRET; + process.env.API_SECRET = 'this is my long pass phrase'; + var env = require('../env')( ); + env.api_secret.should.equal(known); + var ctx = setup_app(env, function ( ) { + // console.log(this.app.enabled('api')); + ctx.app.enabled('api').should.be.true; + // ping_status(ctx.app, done); + // ping_authorized_endpoint(ctx.app, 200, done); + ping_status(ctx.app, again); + function again ( ) { + ctx.app.api_secret = ''; + ping_authorized_endpoint(ctx.app, 401, done); + } + }); + + }); + + it('should work fine set', function (done) { var known = 'b723e97aa97846eb92d5264f084b2823f57c4aa1'; delete process.env.API_SECRET; @@ -54,7 +78,13 @@ describe('API_SECRET', function ( ) { var ctx = setup_app(env, function ( ) { // console.log(this.app.enabled('api')); ctx.app.enabled('api').should.be.true; - ping_status(ctx.app, done); + // ping_status(ctx.app, done); + // ping_authorized_endpoint(ctx.app, 200, done); + ping_status(ctx.app, again); + function again ( ) { + ctx.app.api_secret = env.api_secret; + ping_authorized_endpoint(ctx.app, 200, done); + } }); }); @@ -72,7 +102,7 @@ describe('API_SECRET', function ( ) { function ping_status (app, fn) { request(app) - .get('/status/.json') + .get('/status.json') .expect(200) .end(function (err, res) { // console.log(res.body); @@ -82,5 +112,19 @@ describe('API_SECRET', function ( ) { }) } + function ping_authorized_endpoint (app, fails, fn) { + request(app) + .get('/experiments/test') + .set('API-SECRET', app.api_secret || '') + .expect(fails) + .end(function (err, res) { + if (fails < 400) { + res.body.status.should.equal('ok'); + } + fn( ); + // console.log('err', err, 'res', res); + }) + } + }); From 4872169aae3692f3f4db08d728d0d377332fa94b Mon Sep 17 00:00:00 2001 From: Jason Calabrese Date: Mon, 28 Jul 2014 23:08:14 -0700 Subject: [PATCH 2/2] Revert "migrate to API-SECRET header" Cherry-picked in the wrong order This reverts commit 8e56ef17529f3867e0d907a81f95e40276814037. --- lib/middleware/verify-token.js | 4 +-- tests/security.test.js | 50 ++-------------------------------- 2 files changed, 5 insertions(+), 49 deletions(-) diff --git a/lib/middleware/verify-token.js b/lib/middleware/verify-token.js index a0f0b0ad0de..0a390a0b5ea 100644 --- a/lib/middleware/verify-token.js +++ b/lib/middleware/verify-token.js @@ -5,12 +5,12 @@ function configure (env) { function verifyAuthorization(req, res, next) { // Retrieve the secret values to be compared. var api_secret = env.api_secret; - var secret = req.params.secret ? req.params.secret : req.header('API-SECRET'); + var secret = req.params.secret ? req.params.secret : req.header('API_SECRET'); // Return an error message if the authorization fails. var unauthorized = (typeof api_secret === 'undefined' || secret != api_secret); if (unauthorized) { - res.sendJSONStatus(res, consts.HTTP_UNAUTHORIZED, 'Unauthorized', 'API-SECRET Request Header is incorect or missing.'); + res.sendJSONStatus(res, consts.HTTP_UNAUTHORIZED, 'Unauthorized', 'API_SECRET Request Header is incorect or missing.'); } else { next(); } diff --git a/tests/security.test.js b/tests/security.test.js index 53e381fcd20..573a087f41f 100644 --- a/tests/security.test.js +++ b/tests/security.test.js @@ -40,35 +40,11 @@ describe('API_SECRET', function ( ) { should.not.exist(env.api_secret); var ctx = setup_app(env, function ( ) { ctx.app.enabled('api').should.be.false; - ping_status(ctx.app, again); - function again ( ) { - ping_authorized_endpoint(ctx.app, 404, done); - } + ping_status(ctx.app, done); }); }); - it('should work fail set unauthorized', function (done) { - var known = 'b723e97aa97846eb92d5264f084b2823f57c4aa1'; - delete process.env.API_SECRET; - process.env.API_SECRET = 'this is my long pass phrase'; - var env = require('../env')( ); - env.api_secret.should.equal(known); - var ctx = setup_app(env, function ( ) { - // console.log(this.app.enabled('api')); - ctx.app.enabled('api').should.be.true; - // ping_status(ctx.app, done); - // ping_authorized_endpoint(ctx.app, 200, done); - ping_status(ctx.app, again); - function again ( ) { - ctx.app.api_secret = ''; - ping_authorized_endpoint(ctx.app, 401, done); - } - }); - - }); - - it('should work fine set', function (done) { var known = 'b723e97aa97846eb92d5264f084b2823f57c4aa1'; delete process.env.API_SECRET; @@ -78,13 +54,7 @@ describe('API_SECRET', function ( ) { var ctx = setup_app(env, function ( ) { // console.log(this.app.enabled('api')); ctx.app.enabled('api').should.be.true; - // ping_status(ctx.app, done); - // ping_authorized_endpoint(ctx.app, 200, done); - ping_status(ctx.app, again); - function again ( ) { - ctx.app.api_secret = env.api_secret; - ping_authorized_endpoint(ctx.app, 200, done); - } + ping_status(ctx.app, done); }); }); @@ -102,7 +72,7 @@ describe('API_SECRET', function ( ) { function ping_status (app, fn) { request(app) - .get('/status.json') + .get('/status/.json') .expect(200) .end(function (err, res) { // console.log(res.body); @@ -112,19 +82,5 @@ describe('API_SECRET', function ( ) { }) } - function ping_authorized_endpoint (app, fails, fn) { - request(app) - .get('/experiments/test') - .set('API-SECRET', app.api_secret || '') - .expect(fails) - .end(function (err, res) { - if (fails < 400) { - res.body.status.should.equal('ok'); - } - fn( ); - // console.log('err', err, 'res', res); - }) - } - });