diff --git a/lib/strategies/basic.js b/lib/strategies/basic.js index 017455e..df051a3 100644 --- a/lib/strategies/basic.js +++ b/lib/strategies/basic.js @@ -3,6 +3,7 @@ */ const passport = require('passport-strategy'); const util = require('util'); +const { splitFirst } = require('../string.util'); /** * `BasicStrategy` constructor. @@ -67,19 +68,14 @@ BasicStrategy.prototype.authenticate = function (req) { const scheme = parts[0]; const userPass = Buffer.from(parts[1], 'base64').toString(); - const credentials = userPass.split(':'); + const [userid, password] = splitFirst(userPass, ':'); if (!/Basic/i.test(scheme)) { return this.fail(this._challenge()); } - if (credentials.length < 2) { - return this.fail(400); - } - const userid = credentials[0]; - const password = credentials[1]; - if (!userid || !password) { - return this.fail(this._challenge()); + if (userid === undefined || password === undefined) { + return this.fail(400); } const self = this; diff --git a/lib/string.util.js b/lib/string.util.js new file mode 100644 index 0000000..62ade36 --- /dev/null +++ b/lib/string.util.js @@ -0,0 +1,21 @@ +module.exports = { + + /** + * Splits a string on the first occurrence of the provided separator. + * Returns an array with one or two elements. + * @param {String} string + * @param {String} separator + */ + splitFirst: (string, separator) => { + const separatorIndex = string.indexOf(separator); + + if (separatorIndex < 0) { + return [string]; + } + + return [ + string.substring(0, separatorIndex), + string.substring(separatorIndex + 1), + ]; + }, +}; diff --git a/test/strategies/basic-test.js b/test/strategies/basic-test.js index 234346e..fe547e2 100644 --- a/test/strategies/basic-test.js +++ b/test/strategies/basic-test.js @@ -179,7 +179,7 @@ vows.describe('BasicStrategy').addBatch({ }, }, - 'strategy handling a request with credentials lacking a password': { + 'strategy handling a request with credentials lacking the " " separator': { topic: function () { return new BasicStrategy(((userid, password, done) => { done(null, {username: userid, password: password}); @@ -197,7 +197,7 @@ vows.describe('BasicStrategy').addBatch({ }; req.headers = {}; - req.headers.authorization = 'Basic Ym9iOg=='; + req.headers.authorization = 'Basic'; process.nextTick(() => { strategy.authenticate(req); }); @@ -206,12 +206,12 @@ vows.describe('BasicStrategy').addBatch({ 'should fail authentication with challenge': function (err, challenge) { // fail action was called, resulting in test callback assert.isNull(err); - assert.strictEqual(challenge, 'Basic realm="Users"'); + assert.strictEqual(challenge, 400); }, }, }, - 'strategy handling a request with credentials lacking a username': { + 'strategy handling a request with credentials containing an empty user-pass': { topic: function () { return new BasicStrategy(((userid, password, done) => { done(null, {username: userid, password: password}); @@ -229,7 +229,7 @@ vows.describe('BasicStrategy').addBatch({ }; req.headers = {}; - req.headers.authorization = 'Basic OnNlY3JldA=='; + req.headers.authorization = 'Basic '; process.nextTick(() => { strategy.authenticate(req); }); @@ -238,7 +238,139 @@ vows.describe('BasicStrategy').addBatch({ 'should fail authentication with challenge': function (err, challenge) { // fail action was called, resulting in test callback assert.isNull(err); - assert.strictEqual(challenge, 'Basic realm="Users"'); + assert.strictEqual(challenge, 400); + }, + }, + }, + + 'strategy handling a request with credentials lacking the ":" separator': { + topic: function () { + return new BasicStrategy(((userid, password, done) => { + done(null, {username: userid, password: password}); + })); + }, + + 'after augmenting with actions': { + topic: function (strategy) { + const req = {}; + strategy.success = (user) => { + this.callback(new Error('should not be called')); + }; + strategy.fail = (challenge) => { + this.callback(null, challenge); + }; + + req.headers = {}; + req.headers.authorization = 'Basic Ym9i'; // bob + process.nextTick(() => { + strategy.authenticate(req); + }); + }, + + 'should fail authentication with challenge': function (err, challenge) { + // fail action was called, resulting in test callback + assert.isNull(err); + assert.strictEqual(challenge, 400); + }, + }, + }, + + 'strategy handling a request with credentials containing an empty username': { + topic: function () { + return new BasicStrategy(((userid, password, done) => { + done(null, {username: userid, password: password}); + })); + }, + + 'after augmenting with actions': { + topic: function (strategy) { + const req = {}; + strategy.success = (user) => { + this.callback(null, user); + }; + strategy.fail = (challenge) => { + this.callback(new Error('should not be called')); + }; + + req.headers = {}; + req.headers.authorization = 'Basic OnBhc3N3b3Jk'; // :password + process.nextTick(() => { + strategy.authenticate(req); + }); + }, + + 'should not generate an error': (err, user) => { + assert.isNull(err); + }, + 'should authenticate': (err, user) => { + assert.strictEqual(user.username, ''); + assert.strictEqual(user.password, 'password'); + }, + }, + }, + + 'strategy handling a request with credentials containing an empty password': { + topic: function () { + return new BasicStrategy(((userid, password, done) => { + done(null, {username: userid, password: password}); + })); + }, + + 'after augmenting with actions': { + topic: function (strategy) { + const req = {}; + strategy.success = (user) => { + this.callback(null, user); + }; + strategy.fail = (challenge) => { + this.callback(new Error('should not be called')); + }; + + req.headers = {}; + req.headers.authorization = 'Basic Ym9iOg=='; // bob: + process.nextTick(() => { + strategy.authenticate(req); + }); + }, + + 'should not generate an error': (err, user) => { + assert.isNull(err); + }, + 'should authenticate': (err, user) => { + assert.strictEqual(user.username, 'bob'); + assert.strictEqual(user.password, ''); + }, + }, + }, + + 'strategy handling a request containing a colon in the password': { + topic: function () { + return new BasicStrategy((userid, password, done) => { + done(null, {username: userid, password: password}); + }); + }, + 'after augmenting with actions': { + topic: function (strategy) { + const req = {}; + strategy.success = user => { + this.callback(null, user); + }; + strategy.fail = () => { + this.callback(new Error('should not be called')); + }; + + req.headers = {}; + req.headers.authorization = 'Basic Ym9iOnNlY3JldDpwdw=='; // bob:secret:pw + process.nextTick(() => { + strategy.authenticate(req); + }); + }, + 'should not generate an error': (err, user) => { + assert.isNull(err); + }, + 'should authenticate': (err, user) => { + assert.strictEqual(user.username, 'bob'); + assert.strictEqual(user.password, 'secret:pw'); }, }, }, diff --git a/test/string.util-test.js b/test/string.util-test.js new file mode 100644 index 0000000..96c052b --- /dev/null +++ b/test/string.util-test.js @@ -0,0 +1,24 @@ +const vows = require('vows'); +const assert = require('assert'); +const stringUtils = require('../lib/string.util'); +const {splitFirst} = stringUtils; + +vows.describe('string utils').addBatch({ + + 'module': { + 'should export splitFirst': () => { + assert.isFunction(stringUtils.splitFirst); + }, + }, + + 'splitFirst': { + 'should split only first': () => { + assert.deepStrictEqual(splitFirst('bob:secret:pw', ':'), ['bob', 'secret:pw']); + assert.deepStrictEqual(splitFirst('a:bb:a:d', ':'), ['a', 'bb:a:d']); + }, + 'should handle non-existing seperator': () => { + assert.deepStrictEqual(splitFirst('abc', ':'), ['abc']); + }, + }, + +}).export(module);