Skip to content

Commit

Permalink
migrate to API-SECRET header
Browse files Browse the repository at this point in the history
@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.
  • Loading branch information
bewest committed Jul 27, 2014
1 parent c7def83 commit ff84000
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 5 deletions.
4 changes: 2 additions & 2 deletions lib/middleware/verify-token.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
50 changes: 47 additions & 3 deletions tests/security.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
});

});
Expand All @@ -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);
Expand All @@ -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);
})
}

});

1 comment on commit ff84000

@jasoncalabrese
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that case matters too, curl sends at upper case, but it seems that nginx converts to lowercase when proxying to Node. Also since we have an API_SECRET env var with different contents, I think a new hader would make sense. Maybe api_token?

Please sign in to comment.