From 2ec9215829f4fe85d4b7ce5ba45d2dbfb8e4fe09 Mon Sep 17 00:00:00 2001 From: Tim Macfarlane Date: Tue, 27 Mar 2018 14:06:20 +0200 Subject: [PATCH 1/3] better authorizer error handling --- index.js | 4 +++- test.js | 34 +++++++++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index ee08756..b163088 100644 --- a/index.js +++ b/index.js @@ -69,7 +69,9 @@ function buildMiddleware(options) { } function authorizerCallback(err, approved) { - assert.ifError(err) + if (err) { + return next(err) + } if(approved) return next() diff --git a/test.js b/test.js index 9bf122a..1556ea1 100644 --- a/test.js +++ b/test.js @@ -4,6 +4,7 @@ const express = require('express') const supertest = require('supertest'); var app = express() +app.set('env', 'test') //Requires basic auth with username 'Admin' and password 'secret1234' var staticUserAuth = basicAuth({ @@ -98,15 +99,24 @@ app.get('/realmfunction', realmFunctionAuth, function(req, res) { //Custom authorizer checking if the username starts with 'A' and the password with 'secret' function myAuthorizer(username, password) { - return username.startsWith('A') && password.startsWith('secret') + if(username.startsWith('A') && password.startsWith('secret')) + return true + else if (username.startsWith('error')) + throw new Error('authorizer error') + else + return false } //Same but asynchronous function myAsyncAuthorizer(username, password, cb) { - if(username.startsWith('A') && password.startsWith('secret')) - return cb(null, true) - else - return cb(null, false) + setTimeout(function () { + if(username.startsWith('A') && password.startsWith('secret')) + return cb(null, true) + else if (username.startsWith('error')) + return cb(new Error('authorizer error')) + else + return cb(null, false) + }, 1) } function getUnauthorizedResponse(req) { @@ -165,6 +175,13 @@ describe('express-basic-auth', function() { .expect(401, done) }) + it('should return 500 if authoriser rejects', function(done) { + supertest(app) + .get(endpoint) + .auth('error', 'stuff') + .expect(500, done) + }) + it('should accept fitting credentials', function(done) { supertest(app) .get(endpoint) @@ -189,6 +206,13 @@ describe('express-basic-auth', function() { .expect(401, done) }) + it('should return 500 if authoriser rejects', function(done) { + supertest(app) + .get(endpoint) + .auth('error', 'stuff') + .expect(500, done) + }) + it('should accept fitting credentials', function(done) { supertest(app) .get(endpoint) From 0a7cbf4dcd7337d5a57927cd56c7bae0aaa0c79d Mon Sep 17 00:00:00 2001 From: Tim Macfarlane Date: Tue, 27 Mar 2018 14:10:23 +0200 Subject: [PATCH 2/3] allow authorizers to return promises --- index.js | 16 ++++++++++++++-- test.js | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index b163088..c24d0ae 100644 --- a/index.js +++ b/index.js @@ -43,8 +43,20 @@ function buildMiddleware(options) { if(isAsync) return authorizer(authentication.name, authentication.pass, authorizerCallback) - else if(!authorizer(authentication.name, authentication.pass)) - return unauthorized() + else { + var result = authorizer(authentication.name, authentication.pass) + if (result && typeof result.then === 'function') { + return result.then(function (isAuthorized) { + authorizerCallback(undefined, isAuthorized) + }, function (error) { + authorizerCallback(error) + }).catch(function (e) { + console.log('e', e) + }) + } else if (!result) { + return unauthorized() + } + } return next() diff --git a/test.js b/test.js index 1556ea1..f84a1a7 100644 --- a/test.js +++ b/test.js @@ -31,6 +31,11 @@ var asyncAuth = basicAuth({ authorizeAsync: true }) +//Uses a custom promise-based authorizer function +var promiseAuth = basicAuth({ + authorizer: myPromiseAuthorizer +}) + //Uses a custom response body function var customBodyAuth = basicAuth({ users: { 'Foo': 'bar' }, @@ -77,6 +82,10 @@ app.get('/async', asyncAuth, function(req, res) { res.status(200).send('You passed') }) +app.get('/promise', promiseAuth, function(req, res) { + res.status(200).send('You passed') +}) + app.get('/custombody', customBodyAuth, function(req, res) { res.status(200).send('You passed') }) @@ -119,6 +128,16 @@ function myAsyncAuthorizer(username, password, cb) { }, 1) } +//Same but returns promise +function myPromiseAuthorizer(username, password) { + if(username.startsWith('A') && password.startsWith('secret')) + return Promise.resolve(true) + else if (username.startsWith('error')) + return Promise.reject(new Error('authorizer error')) + else + return Promise.resolve(false) +} + function getUnauthorizedResponse(req) { return req.auth ? ('Credentials ' + req.auth.user + ':' + req.auth.password + ' rejected') : 'No credentials provided' } @@ -221,6 +240,37 @@ describe('express-basic-auth', function() { }) }) + describe('promise authorizer', function() { + const endpoint = '/promise' + + it('should reject on missing header', function(done) { + supertest(app) + .get(endpoint) + .expect(401, done) + }) + + it('should reject on wrong credentials', function(done) { + supertest(app) + .get(endpoint) + .auth('dude', 'stuff') + .expect(401, done) + }) + + it('should return 500 if authoriser rejects', function(done) { + supertest(app) + .get(endpoint) + .auth('error', 'stuff') + .expect(500, done) + }) + + it('should accept fitting credentials', function(done) { + supertest(app) + .get(endpoint) + .auth('Aererer', 'secretiveStuff') + .expect(200, 'You passed', done) + }) + }) + describe('custom response body', function() { it('should reject on missing header and generate resposne message', function(done) { supertest(app) From 98e6158461f4ac7f4e7b9c89273bcf0cdfce7487 Mon Sep 17 00:00:00 2001 From: Tim Macfarlane Date: Tue, 27 Mar 2018 14:18:48 +0200 Subject: [PATCH 3/3] some documentation --- README.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 580eac0..87ef07e 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,8 @@ one of the three passed credentials. Alternatively, you can pass your own `authorizer` function, to check the credentials however you want. It will be called with a username and password and is expected to -return `true` or `false` to indicate that the credentials were approved or not: +return `true` or `false`, or a `Promise` of `true` or `false` to indicate that the +credentials were approved or not: ```js app.use(basicAuth( { authorizer: myAuthorizer } )) @@ -75,8 +76,18 @@ function myAuthorizer(username, password) { ``` This will authorize all requests with credentials where the username begins with -`'A'` and the password begins with `'secret'`. In an actual application you would -likely look up some data instead ;-) +`'A'` and the password begins with `'secret'`. + +Alternatively, you can use an `async` function or return a promise: + +```js +app.use(basicAuth( { authorizer: myAuthorizer } )) + +async function myAuthorizer(username, password) { + const user = await database.findUser(username) + return user.comparePassword(password) +} +``` ### Custom Async Authorization