From 5ee91ca02dd33768c7cf411108b13159d4bb8c8b Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Thu, 21 Mar 2013 22:43:21 -0700 Subject: [PATCH 1/9] Initial session to auth rewrite --- lib/auth/basic.js | 32 +++++++---------- lib/auth/bewit.js | 6 +--- lib/auth/cookie.js | 9 +++-- lib/auth/hawk.js | 15 +++----- lib/auth/index.js | 75 ++++++++++++++++++++++------------------ lib/auth/oz.js | 6 +--- lib/request.js | 11 +++--- lib/server.js | 7 ++-- test/integration/auth.js | 6 ++-- test/unit/auth/index.js | 67 +++++++++++++++++++++-------------- test/unit/validation.js | 2 +- 11 files changed, 122 insertions(+), 114 deletions(-) diff --git a/lib/auth/basic.js b/lib/auth/basic.js index 9681d8568..f2fd91b1b 100755 --- a/lib/auth/basic.js +++ b/lib/auth/basic.js @@ -53,45 +53,39 @@ internals.Scheme.prototype.authenticate = function (request, callback) { return callback(Boom.badRequest('Bad header internal syntax', 'Basic')); } - var credentials = { - username: credentialsParts[0], - password: credentialsParts[1] - }; + var username = credentialsParts[0]; + var password = credentialsParts[1]; - this.settings.loadUserFunc(credentials.username, function (err, user) { + this.settings.loadUserFunc(username, function (err, user) { if (err) { - request.log(['auth', 'user'], err); - return callback(err, null, true); + return callback(err, null, { log: { tags: ['auth', 'user'], data: err } }); } if (!user) { - request.log(['auth', 'error', 'user', 'unknown']); - return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, true); + return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, { log: { tags: ['auth', 'error', 'user', 'unknown'] } }); } if (!user.hasOwnProperty('password') || !user.id || - user.id !== credentials.username) { + user.id !== username) { - request.log(['auth', 'error', 'user', 'invalid']); - return callback(Boom.internal('Bad user object received for Basic auth validation'), null, true); + return callback(Boom.internal('Bad user object received for Basic auth validation'), null, { log: { tags: ['auth', 'error', 'user', 'invalid'] } }); } if (typeof self.settings.hashPasswordFunc === 'function') { - credentials.password = self.settings.hashPasswordFunc(credentials.password); + password = self.settings.hashPasswordFunc(password); } // Check password - if (!Cryptiles.fixedTimeComparison(user.password, credentials.password)) { - request.log(['auth', 'error', 'user', 'password']); - return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, true); + if (!Cryptiles.fixedTimeComparison(user.password, password)) { + return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, { log: { tags: ['auth', 'error', 'user', 'password'] } }); } // Authenticated - var session = { + var credentials = { id: user.id, app: '', scope: user.scope, @@ -99,9 +93,9 @@ internals.Scheme.prototype.authenticate = function (request, callback) { ext: Utils.clone(user) // ext.tos }; - delete session.ext.password; + delete credentials.ext.password; - return callback(null, session); + return callback(null, credentials); }); }; diff --git a/lib/auth/bewit.js b/lib/auth/bewit.js index 69b62f4f6..5974d852d 100755 --- a/lib/auth/bewit.js +++ b/lib/auth/bewit.js @@ -31,10 +31,6 @@ internals.Scheme.prototype.authenticate = function (request, callback) { Hawk.uri.authenticate(request.raw.req, this.settings.getCredentialsFunc, { hostHeaderName: this.settings.hostHeaderName }, function (err, credentials, bewit) { - if (credentials) { - credentials.authExt = bewit.ext; - } - - return callback(err, credentials); + return callback(err, credentials, { artifacts: bewit }); }); }; \ No newline at end of file diff --git a/lib/auth/cookie.js b/lib/auth/cookie.js index d5397189a..0581671b7 100755 --- a/lib/auth/cookie.js +++ b/lib/auth/cookie.js @@ -66,8 +66,7 @@ internals.Scheme.prototype.authenticate = function (request, callback) { request.clearState(self.settings.cookie); } - request.log(['auth', 'validate'], err); - return unauthenticated(Boom.unauthorized('Invalid cookie'), session, true); + return unauthenticated(Boom.unauthorized('Invalid cookie'), session, { log: { tags: ['auth', 'validate'], data: err}}); } if (override) { @@ -78,10 +77,10 @@ internals.Scheme.prototype.authenticate = function (request, callback) { }); }; - var unauthenticated = function (err, session, wasLogged) { + var unauthenticated = function (err, session, options) { if (!self.settings.redirectTo) { - return callback(err, session, wasLogged); + return callback(err, session, options); } var uri = self.settings.redirectTo; @@ -96,7 +95,7 @@ internals.Scheme.prototype.authenticate = function (request, callback) { uri += self.settings.appendNext + '=' + encodeURIComponent(request.url.path); } - return callback(new Redirection(uri), session, wasLogged); + return callback(new Redirection(uri), session, options); }; validate(); diff --git a/lib/auth/hawk.js b/lib/auth/hawk.js index 57bc12db1..1387168af 100755 --- a/lib/auth/hawk.js +++ b/lib/auth/hawk.js @@ -31,19 +31,14 @@ internals.Scheme.prototype.authenticate = function (request, callback) { Hawk.server.authenticate(request.raw.req, this.settings.getCredentialsFunc, { hostHeaderName: this.settings.hostHeaderName }, function (err, credentials, artifacts) { - if (credentials) { - credentials.artifacts = artifacts; - delete credentials.artifacts.credentials; // Needed to prevent utils.clone from creating a maximum call stack error - } - - return callback(err, credentials); + return callback(err, credentials, { artifacts: artifacts }); }); }; -internals.Scheme.prototype.authenticatePayload = function (payload, credentials, contentType, callback) { +internals.Scheme.prototype.authenticatePayload = function (request, callback) { - var isValid = Hawk.server.authenticatePayload(payload, credentials, credentials.artifacts.hash, contentType); + var isValid = Hawk.server.authenticatePayload(request.rawBody, request.auth.credentials, request.auth.artifacts.hash, request.raw.req.headers['content-type']); return callback(isValid ? null : Boom.unauthorized('Payload is invalid')); }; @@ -51,8 +46,8 @@ internals.Scheme.prototype.authenticatePayload = function (payload, credentials, internals.Scheme.prototype.responseHeader = function (request, callback) { - var artifacts = Utils.clone(request.session.artifacts); - artifacts.credentials = request.session; + var artifacts = Utils.clone(request.auth.artifacts); + artifacts.credentials = request.auth.credentials; var options = { contentType: request.response._headers['Content-Type'] diff --git a/lib/auth/index.js b/lib/auth/index.js index 4d7e2f206..afd545337 100755 --- a/lib/auth/index.js +++ b/lib/auth/index.js @@ -168,14 +168,12 @@ internals.Auth.prototype.authenticate = function (request, next) { var authErrors = []; var strategyPos = 0; - request.isAuthenticated = false; - var authenticate = function () { // Injection - if (request.session) { - return validate(null, request.session); + if (request.auth.credentials) { + return validate(null, request.auth.credentials); } // Authenticate @@ -185,7 +183,8 @@ internals.Auth.prototype.authenticate = function (request, next) { if (config.mode === 'optional' || config.mode === 'try') { - request.session = null; + request.auth.isAuthenticated = false; + request.auth.credentials = null; request.log(['auth', 'unauthenticated']); return next(); } @@ -197,16 +196,21 @@ internals.Auth.prototype.authenticate = function (request, next) { return strategy.authenticate(request, validate); }; - var validate = function (err, session, wasLogged) { + var validate = function (err, credentials, options) { + + options = options || {}; // Unauthenticated - if (!err && !session) { - return next(Boom.internal('Authentication response missing both error and session')); + if (!err && !credentials) { + return next(Boom.internal('Authentication response missing both error and credentials')); } if (err) { - if (!wasLogged) { + if (options.log) { + request.log(options.log.tags, options.log.data); + } + else { request.log(['auth', 'unauthenticated'], err); } @@ -215,7 +219,9 @@ internals.Auth.prototype.authenticate = function (request, next) { err.response.code !== 401) { // An actual error (not just missing authentication) if (config.mode === 'try') { - request.session = session; + request.auth.isAuthenticated = false; + request.auth.credentials = credentials; + request.auth.artifacts = options.artifacts; request.log(['auth', 'unauthenticated', 'try'], err); return next(); } @@ -234,15 +240,16 @@ internals.Auth.prototype.authenticate = function (request, next) { // Authenticated - request.session = session; - request.session._strategy = self._strategies[config.strategies[strategyPos - 1]]; + request.auth.credentials = credentials; + request.auth.artifacts = options.artifacts; + request.auth._strategy = self._strategies[config.strategies[strategyPos - 1]]; // Check scope if (config.scope && - (!session.scope || session.scope.indexOf(config.scope) === -1)) { + (!credentials.scope || credentials.scope.indexOf(config.scope) === -1)) { - request.log(['auth', 'error', 'scope'], { got: session.scope, need: config.scope }); + request.log(['auth', 'error', 'scope'], { got: credentials.scope, need: config.scope }); return next(Boom.forbidden('Insufficient scope (\'' + config.scope + '\' expected)')); } @@ -250,9 +257,9 @@ internals.Auth.prototype.authenticate = function (request, next) { var tos = (config.hasOwnProperty('tos') ? config.tos : null); if (tos && - (!session.ext || !session.ext.tos || session.ext.tos < tos)) { + (!credentials.ext || !credentials.ext.tos || credentials.ext.tos < tos)) { - request.log(['auth', 'error', 'tos'], { min: tos, received: session.ext && session.ext.tos }); + request.log(['auth', 'error', 'tos'], { min: tos, received: credentials.ext && credentials.ext.tos }); return next(Boom.forbidden('Insufficient TOS accepted')); } @@ -264,32 +271,32 @@ internals.Auth.prototype.authenticate = function (request, next) { if (entity === 'any') { request.log(['auth']); - request.isAuthenticated = true; + request.auth.isAuthenticated = true; return next(); } // Entity: 'user' if (entity === 'user') { - if (!session.user) { - request.log(['auth', 'error'], 'User session required'); - return next(Boom.forbidden('Application session cannot be used on a user endpoint')); + if (!credentials.user) { + request.log(['auth', 'error'], 'User credentials required'); + return next(Boom.forbidden('Application credentials cannot be used on a user endpoint')); } request.log(['auth']); - request.isAuthenticated = true; + request.auth.isAuthenticated = true; return next(); } // Entity: 'app' - if (session.user) { - request.log(['auth', 'error'], 'App session required'); - return next(Boom.forbidden('User session cannot be used on an application endpoint')); + if (credentials.user) { + request.log(['auth', 'error'], 'App credentials required'); + return next(Boom.forbidden('User credentials cannot be used on an application endpoint')); } request.log(['auth']); - request.isAuthenticated = true; + request.auth.isAuthenticated = true; return next(); }; @@ -304,19 +311,19 @@ internals.Auth.authenticatePayload = function (request, next) { if (!config || !config.payload || - !request.isAuthenticated) { + !request.auth.isAuthenticated) { return next(); } if (config.payload === 'optional' && - (!request.session.artifacts.hash || - typeof request.session._strategy.authenticatePayload !== 'function')) { + (!request.auth.artifacts.hash || + typeof request.auth._strategy.authenticatePayload !== 'function')) { return next(); } - request.session._strategy.authenticatePayload(request.rawBody, request.session, request.raw.req.headers['content-type'], function (err) { + request.auth._strategy.authenticatePayload(request, function (err) { return next(err); }); @@ -329,14 +336,14 @@ internals.Auth.responseHeader = function (request, next) { var config = auth.routeConfig(request); if (!config || - !request.isAuthenticated) { + !request.auth.isAuthenticated) { return next(); } - if (!request.session || - !request.session._strategy || - typeof request.session._strategy.responseHeader !== 'function') { + if (!request.auth.credentials || + !request.auth._strategy || + typeof request.auth._strategy.responseHeader !== 'function') { return next(); } @@ -348,7 +355,7 @@ internals.Auth.responseHeader = function (request, next) { return next(); } - request.session._strategy.responseHeader(request, function (err) { + request.auth._strategy.responseHeader(request, function (err) { return next(err); }); diff --git a/lib/auth/oz.js b/lib/auth/oz.js index 1a488594d..40d04ab5f 100755 --- a/lib/auth/oz.js +++ b/lib/auth/oz.js @@ -71,11 +71,7 @@ internals.Scheme.prototype.authenticate = function (request, callback) { Oz.request.authenticate(request.raw.req, this.settings.encryptionPassword, { isHttps: this.settings.isHttps }, function (err, ticket, ext) { - if (ticket) { - ticket.authExt = ext; - } - - return callback(err, ticket); + return callback(err, ticket, { artifacts: ext }); }); }; diff --git a/lib/request.js b/lib/request.js index 6f79cbd75..6b1e28b6f 100755 --- a/lib/request.js +++ b/lib/request.js @@ -48,8 +48,11 @@ exports = module.exports = internals.Request = function (server, req, res, optio this.response = null; this.isReplied = false; - this.session = null; // Does not mean authenticated { id, [app], [scope], [user], [ext.tos] } - this.isAuthenticated = false; + this.auth = { + isAuthenticated: false, + credentials: null, // Special keys: 'app', 'user', 'scope', 'tos' + artifacts: null // Scheme-specific artifacts + }; this.pre = {}; this.analytics = { @@ -58,8 +61,8 @@ exports = module.exports = internals.Request = function (server, req, res, optio // Apply options - if (options.session) { - this.session = options.session; + if (options.credentials) { + this.auth.credentials = options.credentials; } // Defined elsewhere: diff --git a/lib/server.js b/lib/server.js index bc3bc8ee0..8deb49dc4 100755 --- a/lib/server.js +++ b/lib/server.js @@ -302,8 +302,9 @@ internals.Server.prototype.auth = function (name, options) { internals.Server.prototype.inject = function (options, callback) { - var requestOptions = (options.session ? { session: options.session } : null); - delete options.session; + var settings = Utils.clone(options); + var requestOptions = (settings.credentials ? { credentials: settings.credentials } : null); + delete settings.credentials; var onEnd = function (res) { @@ -319,7 +320,7 @@ internals.Server.prototype.inject = function (options, callback) { }; var needle = this._dispatch(requestOptions); - Shot.inject(needle, options, onEnd); + Shot.inject(needle, settings, onEnd); }; diff --git a/test/integration/auth.js b/test/integration/auth.js index 4a3b69487..68035e407 100755 --- a/test/integration/auth.js +++ b/test/integration/auth.js @@ -81,7 +81,7 @@ describe('Auth', function () { var doubleHandler = function (request) { - var options = { method: 'POST', url: '/basic', headers: { authorization: basicHeader('john', '12345') }, session: request.session }; + var options = { method: 'POST', url: '/basic', headers: { authorization: basicHeader('john', '12345') }, credentials: request.auth.credentials }; server.inject(options, function (res) { @@ -611,7 +611,7 @@ describe('Auth', function () { var hawkChangeHandler = function (request) { - request.session.algorithm = 'ha'; + request.auth.credentials.algorithm = 'ha'; request.reply.payload('Success').send(); }; @@ -1464,7 +1464,7 @@ describe('Auth', function () { server.route({ method: 'GET', path: '/resource', handler: function () { - expect(this.session.something).to.equal('new'); + expect(this.auth.credentials.something).to.equal('new'); return this.reply('resource'); }, config: { auth: 'default' } diff --git a/test/unit/auth/index.js b/test/unit/auth/index.js index 82e7255ab..483cbff2a 100755 --- a/test/unit/auth/index.js +++ b/test/unit/auth/index.js @@ -127,6 +127,7 @@ describe('Auth', function () { strategies: ['test'] } }, + auth: {}, log: function () { }, raw: { res: { @@ -210,7 +211,7 @@ describe('Auth', function () { var test = function (scheme) { - it('doesn\'t throw an error when a session exists and entity is any (' + scheme.scheme + ')', function (done) { + it('doesn\'t throw an error when credentials exist and entity is any (' + scheme.scheme + ')', function (done) { var options = { auth: scheme @@ -219,12 +220,14 @@ describe('Auth', function () { var server = new Hapi.Server('localhost', 0, options); var request = { - session: {}, + auth: { + credentials: {} + }, _timestamp: Date.now(), route: { auth: { entity: 'any', - strategies: [scheme.scheme] + strategies: ['default'] } }, log: function () { }, @@ -238,7 +241,7 @@ describe('Auth', function () { }); }); - it('doesn\'t throw an error when a session exists and entity defaults to any (' + scheme.scheme + ')', function (done) { + it('doesn\'t throw an error when credentials exist and entity defaults to any (' + scheme.scheme + ')', function (done) { var options = { auth: scheme @@ -247,12 +250,13 @@ describe('Auth', function () { var server = new Hapi.Server('localhost', 0, options); var request = { - session: { + auth: { + credentials: {} }, _timestamp: Date.now(), route: { auth: { - strategies: [scheme.scheme] + strategies: ['default'] } }, log: function () { }, @@ -266,7 +270,7 @@ describe('Auth', function () { }); }); - it('doesn\'t throw an error when a session exists with a user and user entity specified (' + scheme.scheme + ')', function (done) { + it('doesn\'t throw an error when credentials exist with a user and user entity specified (' + scheme.scheme + ')', function (done) { var options = { auth: scheme @@ -275,14 +279,16 @@ describe('Auth', function () { var server = new Hapi.Server('localhost', 0, options); var request = { - session: { - user: 'test' + auth: { + credentials: { + user: 'test' + } }, _timestamp: Date.now(), route: { auth: { entity: 'user', - strategies: [scheme.scheme] + strategies: ['default'] } }, log: function () { }, @@ -296,7 +302,7 @@ describe('Auth', function () { }); }); - it('throws an error when a session exists without a user and user entity is specified (' + scheme.scheme + ')', function (done) { + it('throws an error when credentials exist without a user and user entity is specified (' + scheme.scheme + ')', function (done) { var options = { auth: scheme @@ -305,12 +311,14 @@ describe('Auth', function () { var server = new Hapi.Server('localhost', 0, options); var request = { - session: {}, + auth: { + credentials: {} + }, _timestamp: Date.now(), route: { auth: { entity: 'user', - strategies: [scheme.scheme] + strategies: ['default'] } }, log: function () { }, @@ -325,7 +333,7 @@ describe('Auth', function () { }); }); - it('throws an error when a session exists without a app and app entity is specified (' + scheme.scheme + ')', function (done) { + it('throws an error when credentials exist without a app and app entity is specified (' + scheme.scheme + ')', function (done) { var options = { auth: scheme @@ -334,14 +342,16 @@ describe('Auth', function () { var server = new Hapi.Server('localhost', 0, options); var request = { - session: { - user: 'test' + auth: { + credentials: { + user: 'test' + } }, _timestamp: Date.now(), route: { auth: { entity: 'app', - strategies: [scheme.scheme] + strategies: ['default'] } }, log: function () { }, @@ -356,7 +366,7 @@ describe('Auth', function () { }); }); - it('doesn\'t throw an error when a session exists with a app and app entity is specified (' + scheme.scheme + ')', function (done) { + it('doesn\'t throw an error when credentials exist with a app and app entity is specified (' + scheme.scheme + ')', function (done) { var options = { auth: scheme @@ -365,14 +375,16 @@ describe('Auth', function () { var server = new Hapi.Server('localhost', 0, options); var request = { - session: { - app: 'test' + auth: { + credentials: { + app: 'test' + } }, _timestamp: Date.now(), route: { auth: { entity: 'app', - strategies: [scheme.scheme] + strategies: ['default'] } }, log: function () { }, @@ -395,14 +407,16 @@ describe('Auth', function () { var server = new Hapi.Server('localhost', 0, options); var request = { - session: { - user: 'test' + auth: { + credentials: { + user: 'test' + } }, _timestamp: Date.now(), route: { auth: { entity: 'wrongEntity', - strategies: [scheme.scheme] + strategies: ['default'] } }, log: function () { }, @@ -418,7 +432,7 @@ describe('Auth', function () { }); }); - it('throws an error when missing session and bad request (' + scheme.scheme + ')', function (done) { + it('throws an error when missing credentials and bad request (' + scheme.scheme + ')', function (done) { var server = { settings: {}, @@ -434,6 +448,9 @@ describe('Auth', function () { strategies: ['default'] } }, + auth: { + credentials: {} + }, log: function () { }, raw: { res: { diff --git a/test/unit/validation.js b/test/unit/validation.js index 09867742e..a86ea491d 100755 --- a/test/unit/validation.js +++ b/test/unit/validation.js @@ -183,7 +183,7 @@ describe('Validation', function () { }); expect(rates[0]).to.be.greaterThan(8); // accept a 15 point margin - expect(rates[49]).to.be.lessThan(43); + expect(rates[49]).to.be.lessThan(44); done(); }); From fa0c4928fceaa17079fef8aec87a98aec0bebc92 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Thu, 21 Mar 2013 22:55:21 -0700 Subject: [PATCH 2/9] misc --- lib/payload.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/payload.js b/lib/payload.js index 5abcdda8c..6ccc2be34 100755 --- a/lib/payload.js +++ b/lib/payload.js @@ -101,7 +101,7 @@ exports.read = function (request, next) { var payloadBuf = new Buffer(payload, encoding); if (!internals.isDeflate(payloadBuf) && !internals.isGzip(payloadBuf)) { - return finish(Boom.badRequest('Invalid gzip')); + return finish(Boom.badRequest('Invalid compressed payload')); } Zlib.unzip(payloadBuf, function (err, buffer) { // Err shouldn't exist since the buffer is validated above From c98738379e52e68a974bc19450bb5bff41b069cb Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Thu, 21 Mar 2013 23:00:35 -0700 Subject: [PATCH 3/9] Fix inject clone --- lib/server.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/server.js b/lib/server.js index 8deb49dc4..e434cdbd6 100755 --- a/lib/server.js +++ b/lib/server.js @@ -302,9 +302,8 @@ internals.Server.prototype.auth = function (name, options) { internals.Server.prototype.inject = function (options, callback) { - var settings = Utils.clone(options); - var requestOptions = (settings.credentials ? { credentials: settings.credentials } : null); - delete settings.credentials; + var requestOptions = (options.credentials ? { credentials: options.credentials } : null); + delete options.credentials; var onEnd = function (res) { @@ -320,7 +319,7 @@ internals.Server.prototype.inject = function (options, callback) { }; var needle = this._dispatch(requestOptions); - Shot.inject(needle, settings, onEnd); + Shot.inject(needle, options, onEnd); }; From ecb3d14c900ba5139839841e3c543a0a96b32389 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Fri, 22 Mar 2013 00:08:25 -0700 Subject: [PATCH 4/9] Refactor Basic --- examples/auth.js | 10 +++++--- lib/auth/basic.js | 34 ++++++++++++--------------- lib/auth/index.js | 4 ++-- test/integration/auth.js | 18 +++++--------- test/integration/pack/--auth/index.js | 2 +- test/unit/auth/index.js | 3 +-- 6 files changed, 32 insertions(+), 39 deletions(-) diff --git a/examples/auth.js b/examples/auth.js index 59415d67e..496d4f254 100755 --- a/examples/auth.js +++ b/examples/auth.js @@ -24,12 +24,16 @@ internals.hashPassword = function (password) { internals.users = { john: { - id: 'john', - password: internals.hashPassword('john') + user: 'john' } }; +internals.passwords = { + john: 'john' +}; + + internals.credentials = { 'john': { id: 'john', @@ -41,7 +45,7 @@ internals.credentials = { internals.loadUser = function (username, callback) { - callback(null, internals.users[username]); + callback(null, internals.users[username], internals.hashPassword(internals.passwords[username])); }; diff --git a/lib/auth/basic.js b/lib/auth/basic.js index f2fd91b1b..990c08f5c 100755 --- a/lib/auth/basic.js +++ b/lib/auth/basic.js @@ -56,21 +56,27 @@ internals.Scheme.prototype.authenticate = function (request, callback) { var username = credentialsParts[0]; var password = credentialsParts[1]; - this.settings.loadUserFunc(username, function (err, user) { + this.settings.loadUserFunc(username, function (err, credentials, hashedPassword) { if (err) { return callback(err, null, { log: { tags: ['auth', 'user'], data: err } }); } - if (!user) { - return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, { log: { tags: ['auth', 'error', 'user', 'unknown'] } }); + if (!credentials) { + return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, { log: { tags: ['auth', 'error', 'basic', 'user', 'unknown'] } }); } - if (!user.hasOwnProperty('password') || - !user.id || - user.id !== username) { + if (!credentials.user || + credentials.user !== username) { - return callback(Boom.internal('Bad user object received for Basic auth validation'), null, { log: { tags: ['auth', 'error', 'user', 'invalid'] } }); + return callback(Boom.internal('Bad user object received for Basic auth validation'), null, { log: { tags: ['auth', 'error', 'basic', 'user', 'invalid'] } }); + } + + if (hashedPassword === null || + hashedPassword === undefined || + typeof hashedPassword !== 'string') { + + return callback(Boom.internal('Bad password received for Basic auth validation'), null, { log: { tags: ['auth', 'error', 'basic', 'password', 'invalid'] } }); } if (typeof self.settings.hashPasswordFunc === 'function') { @@ -79,22 +85,12 @@ internals.Scheme.prototype.authenticate = function (request, callback) { // Check password - if (!Cryptiles.fixedTimeComparison(user.password, password)) { - return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, { log: { tags: ['auth', 'error', 'user', 'password'] } }); + if (!Cryptiles.fixedTimeComparison(hashedPassword, password)) { + return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, { log: { tags: ['auth', 'error', 'basic', 'password'] } }); } // Authenticated - var credentials = { - id: user.id, - app: '', - scope: user.scope, - user: user.id, - ext: Utils.clone(user) // ext.tos - }; - - delete credentials.ext.password; - return callback(null, credentials); }); }; diff --git a/lib/auth/index.js b/lib/auth/index.js index afd545337..0ad7f1b74 100755 --- a/lib/auth/index.js +++ b/lib/auth/index.js @@ -257,9 +257,9 @@ internals.Auth.prototype.authenticate = function (request, next) { var tos = (config.hasOwnProperty('tos') ? config.tos : null); if (tos && - (!credentials.ext || !credentials.ext.tos || credentials.ext.tos < tos)) { + (!credentials.tos || credentials.tos < tos)) { - request.log(['auth', 'error', 'tos'], { min: tos, received: credentials.ext && credentials.ext.tos }); + request.log(['auth', 'error', 'tos'], { min: tos, received: credentials.tos }); return next(Boom.forbidden('Insufficient TOS accepted')); } diff --git a/test/integration/auth.js b/test/integration/auth.js index 68035e407..d6dec78a8 100755 --- a/test/integration/auth.js +++ b/test/integration/auth.js @@ -44,13 +44,10 @@ describe('Auth', function () { if (id === 'john') { return callback(null, { - id: 'john', - password: hashPassword('12345'), + user: 'john', scope: [], - ext: { - tos: 100 - } - }); + tos: 100 + }, hashPassword('12345')); } else if (id === 'jane') { return callback(Hapi.error.internal('boom')); @@ -1238,13 +1235,10 @@ describe('Auth', function () { if (id === 'john') { return callback(null, { - id: 'john', - password: '12345', + user: 'john', scope: [], - ext: { - tos: 100 - } - }); + tos: 100 + }, '12345'); } else if (id === 'jane') { return callback(Hapi.error.internal('boom')); diff --git a/test/integration/pack/--auth/index.js b/test/integration/pack/--auth/index.js index 9d7ef1c50..7b0c3ff9d 100755 --- a/test/integration/pack/--auth/index.js +++ b/test/integration/pack/--auth/index.js @@ -10,7 +10,7 @@ exports.register = function (pack, options, next) { var loadUser = function (id, callback) { if (id === 'john') { - return callback(null, { id: 'john', password: '12345' }); + return callback(null, { user: 'john' }, '12345'); } return callback(null, null); diff --git a/test/unit/auth/index.js b/test/unit/auth/index.js index 483cbff2a..e8b00d5d5 100755 --- a/test/unit/auth/index.js +++ b/test/unit/auth/index.js @@ -143,7 +143,6 @@ describe('Auth', function () { } }; - var server = { settings: {}, route: function () { } @@ -154,7 +153,7 @@ describe('Auth', function () { scheme: 'basic', loadUserFunc: function (username, callback) { - return callback(null, { id: 'steve', password: 'password' }); + return callback(null, { user: 'steve' }, 'password'); } } }; From 886a1740df49aec9d55e9d2e40fe3ea80e139484 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Fri, 22 Mar 2013 09:50:31 -0700 Subject: [PATCH 5/9] Remove basic auth requirement for credentials content --- lib/auth/basic.js | 8 +++----- test/integration/auth.js | 23 +++++++++++++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/auth/basic.js b/lib/auth/basic.js index 990c08f5c..bf4fd5167 100755 --- a/lib/auth/basic.js +++ b/lib/auth/basic.js @@ -59,17 +59,15 @@ internals.Scheme.prototype.authenticate = function (request, callback) { this.settings.loadUserFunc(username, function (err, credentials, hashedPassword) { if (err) { - return callback(err, null, { log: { tags: ['auth', 'user'], data: err } }); + return callback(err, null, { log: { tags: ['auth', 'basic'], data: err } }); } if (!credentials) { return callback(Boom.unauthorized('Bad username or password', 'Basic'), null, { log: { tags: ['auth', 'error', 'basic', 'user', 'unknown'] } }); } - if (!credentials.user || - credentials.user !== username) { - - return callback(Boom.internal('Bad user object received for Basic auth validation'), null, { log: { tags: ['auth', 'error', 'basic', 'user', 'invalid'] } }); + if (typeof credentials !== 'object') { + return callback(Boom.internal('Bad credentials object received for Basic auth validation'), null, { log: { tags: ['auth', 'error', 'basic', 'credentials', 'invalid'] } }); } if (hashedPassword === null || diff --git a/test/integration/auth.js b/test/integration/auth.js index d6dec78a8..38053de33 100755 --- a/test/integration/auth.js +++ b/test/integration/auth.js @@ -52,8 +52,11 @@ describe('Auth', function () { else if (id === 'jane') { return callback(Hapi.error.internal('boom')); } - else if (id === 'invalid') { - return callback(null, {}); + else if (id === 'invalid1') { + return callback(null, 'bad'); + } + else if (id === 'invalid2') { + return callback(null, {}, null); } else { return callback(null, null); @@ -202,9 +205,21 @@ describe('Auth', function () { }); }); - it('returns an error on invalid user lookup error', function (done) { + it('returns an error on non-object credentials error', function (done) { + + var request = { method: 'POST', url: '/basic', headers: { authorization: basicHeader('invalid1', '12345') } }; + + server.inject(request, function (res) { + + expect(res.result).to.exist; + expect(res.result.code).to.equal(500); + done(); + }); + }); + + it('returns an error on missing password error', function (done) { - var request = { method: 'POST', url: '/basic', headers: { authorization: basicHeader('invalid', '12345') } }; + var request = { method: 'POST', url: '/basic', headers: { authorization: basicHeader('invalid2', '12345') } }; server.inject(request, function (res) { From 7856b998c8e2f3c49289ea55086008ca61f897f3 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Fri, 22 Mar 2013 10:12:29 -0700 Subject: [PATCH 6/9] Cookie auth api --- lib/auth/cookie.js | 15 +++++++++------ lib/request.js | 2 ++ test/integration/auth.js | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/auth/cookie.js b/lib/auth/cookie.js index 0581671b7..21285d7ed 100755 --- a/lib/auth/cookie.js +++ b/lib/auth/cookie.js @@ -108,13 +108,16 @@ internals.Scheme.prototype.extend = function (request) { // Decorate request - request.setSession = function (session) { + Utils.assert(!request.session, 'Conflicting use of request.session'); - request.setState(self.settings.cookie, session); - }; + request.session = { + set: function (session) { - request.clearSession = function () { + request.setState(self.settings.cookie, session); + }, + clear: function () { - request.clearState(self.settings.cookie); - }; + request.clearState(self.settings.cookie); + } + } }; diff --git a/lib/request.js b/lib/request.js index 6b1e28b6f..7587f10d9 100755 --- a/lib/request.js +++ b/lib/request.js @@ -54,6 +54,8 @@ exports = module.exports = internals.Request = function (server, req, res, optio artifacts: null // Scheme-specific artifacts }; + this.session = null; // Special key used by cookie auth as well as other extension session-based schemes (only one allowed) + this.pre = {}; this.analytics = { startTime: now diff --git a/test/integration/auth.js b/test/integration/auth.js index 38053de33..b9205981f 100755 --- a/test/integration/auth.js +++ b/test/integration/auth.js @@ -1464,7 +1464,7 @@ describe('Auth', function () { auth: { mode: 'try' }, handler: function () { - this.setSession({ user: this.params.user }); + this.session.set({ user: this.params.user }); return this.reply(this.params.user); } } @@ -1482,7 +1482,7 @@ describe('Auth', function () { server.route({ method: 'GET', path: '/logout', handler: function () { - this.clearSession(); + this.session.clear(); return this.reply('logged-out'); }, config: { auth: 'default' } }); From e14e0bb6873474c4375f61b6fd88e81a9688194b Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Fri, 22 Mar 2013 21:58:37 -0700 Subject: [PATCH 7/9] Expose state.prepareValue --- lib/index.js | 4 + lib/pack.js | 1 + lib/state.js | 212 ++++++++++++++++++++++++++++----------------------- 3 files changed, 120 insertions(+), 97 deletions(-) diff --git a/lib/index.js b/lib/index.js index 66c179f37..0a79a0c20 100755 --- a/lib/index.js +++ b/lib/index.js @@ -28,6 +28,10 @@ internals.export = function () { return new module.exports.Server(arguments[0], arguments[1], arguments[2]); }; + + module.exports.state = { + prepareValue: require('./state').prepareValue + }; }; internals.export(); diff --git a/lib/pack.js b/lib/pack.js index 06c3cbe09..e61b2b950 100755 --- a/lib/pack.js +++ b/lib/pack.js @@ -220,6 +220,7 @@ internals.Pack.prototype._register = function (plugin, permissions, options, cal var root = step(); root.version = Utils.version; + root.hapi = require('./'); if (permissions.views) { root.views = function (options) { diff --git a/lib/state.js b/lib/state.js index 8dd767997..95963efd1 100755 --- a/lib/state.js +++ b/lib/state.js @@ -312,73 +312,56 @@ exports.generateSetCookieHeader = function (cookies, definitions, callback) { return callback(Boom.internal('Invalid cookie name: ' + cookie.name)); } - // Encode value - - encode(cookie.value, options, function (err, value) { + // Prepare value (encode, sign) + exports.prepareValue(cookie.name, cookie.value, options, function (err, value) { + if (err) { - return callback(Boom.internal('Failed to encode cookie (' + cookie.name + ') value: ' + err.message )); - } - - // Validate value - - if (value && - (typeof value !== 'string' || !value.match(internals.valueRegx))) { - - return callback(Boom.internal('Invalid cookie value: ' + cookie.value)); + return callback(err); } - // Sign cookie + // Construct cookie - sign(cookie.name, value, options.sign, function (err, signed) { + var segment = cookie.name + '=' + (value || ''); - if (err) { - return callback(Boom.internal('Failed to sign cookie (' + cookie.name + ') value: ' + err.message)); - } + if (options.ttl !== null && + options.ttl !== undefined) { // Can be zero - // Construct cookie - - var segment = cookie.name + '=' + (signed || ''); + var expires = new Date(options.ttl ? Date.now() + options.ttl : 0); + segment += '; Max-Age=' + Math.floor(options.ttl / 1000) + '; Expires=' + expires.toUTCString(); + } - if (options.ttl !== null && - options.ttl !== undefined) { // Can be zero + if (options.isSecure) { + segment += '; Secure'; + } - var expires = new Date(options.ttl ? Date.now() + options.ttl : 0); - segment += '; Max-Age=' + Math.floor(options.ttl / 1000) + '; Expires=' + expires.toUTCString(); - } + if (options.isHttpOnly) { + segment += '; HttpOnly'; + } - if (options.isSecure) { - segment += '; Secure'; + if (options.domain) { + var domain = options.domain.toLowerCase(); + if (!domain.match(internals.domainLabelLenRegx)) { + return callback(Boom.internal('Cookie domain too long: ' + options.domain)); } - if (options.isHttpOnly) { - segment += '; HttpOnly'; + if (!domain.match(internals.domainRegx)) { + return callback(Boom.internal('Invalid cookie domain: ' + options.domain)); } - if (options.domain) { - var domain = options.domain.toLowerCase(); - if (!domain.match(internals.domainLabelLenRegx)) { - return callback(Boom.internal('Cookie domain too long: ' + options.domain)); - } - - if (!domain.match(internals.domainRegx)) { - return callback(Boom.internal('Invalid cookie domain: ' + options.domain)); - } + segment += '; Domain=' + domain; + } - segment += '; Domain=' + domain; + if (options.path) { + if (!options.path.match(internals.pathRegx)) { + return callback(Boom.internal('Invalid cookie path: ' + options.path)); } - if (options.path) { - if (!options.path.match(internals.pathRegx)) { - return callback(Boom.internal('Invalid cookie path: ' + options.path)); - } - - segment += '; Path=' + options.path; - } + segment += '; Path=' + options.path; + } - header.push(segment); - return next(); - }); + header.push(segment); + return next(); }); }, function (err) { @@ -387,76 +370,111 @@ exports.generateSetCookieHeader = function (cookies, definitions, callback) { }); }; - var encode = function (value, definition, encodeCallback) { + format(); +}; - // Encodings: 'base64json', 'base64', 'form', 'iron', 'none' - if (value === undefined) { - return encodeCallback(null, value); - } +exports.prepareValue = function (name, value, options, callback) { - if (!definition || - !definition.encoding || - definition.encoding === 'none') { + // Encode value - return encodeCallback(null, value); - } + internals.encode(value, options, function (err, encoded) { - if (definition.encoding === 'iron') { - Iron.seal(value, definition.password, definition.iron || Iron.defaults, function (err, sealed) { + if (err) { + return callback(Boom.internal('Failed to encode cookie (' + name + ') value: ' + err.message )); + } - if (err) { - return encodeCallback(err); - } + // Validate value - return encodeCallback(null, sealed); - }); + if (encoded && + (typeof encoded !== 'string' || !encoded.match(internals.valueRegx))) { - return; + return callback(Boom.internal('Invalid cookie value: ' + value)); } - var result = value; + // Sign cookie - try { - switch (definition.encoding) { - case 'base64': - result = (new Buffer(value, 'binary')).toString('base64'); - break; - case 'base64json': - var stringified = JSON.stringify(value); - result = (new Buffer(stringified, 'binary')).toString('base64'); - break; - case 'form': - result = Querystring.stringify(value); - break; + internals.sign(name, encoded, options.sign, function (err, signed) { + + if (err) { + return callback(Boom.internal('Failed to sign cookie (' + name + ') value: ' + err.message)); } - } - catch (err) { - return encodeCallback(err); - } + + return callback(null, signed); + }); + }); +}; - return encodeCallback(null, result); - }; - var sign = function (name, value, options, signCallback) { +internals.encode = function (value, options, callback) { - if (value === undefined || - !options) { + // Encodings: 'base64json', 'base64', 'form', 'iron', 'none' - return signCallback(null, value); - } + if (value === undefined) { + return callback(null, value); + } - Iron.hmacWithPassword(options.password, options.integrity || Iron.defaults.integrity, [internals.macPrefix, name, value].join('\n'), function (err, mac) { + if (!options || + !options.encoding || + options.encoding === 'none') { + + return callback(null, value); + } + + if (options.encoding === 'iron') { + Iron.seal(value, options.password, options.iron || Iron.defaults, function (err, sealed) { if (err) { - return signCallback(err); + return callback(err); } - var signed = value + '.' + mac.salt + '*' + mac.digest; - return signCallback(null, signed); + return callback(null, sealed); }); - }; - format(); + return; + } + + var result = value; + + try { + switch (options.encoding) { + case 'base64': + result = (new Buffer(value, 'binary')).toString('base64'); + break; + case 'base64json': + var stringified = JSON.stringify(value); + result = (new Buffer(stringified, 'binary')).toString('base64'); + break; + case 'form': + result = Querystring.stringify(value); + break; + } + } + catch (err) { + return callback(err); + } + + return callback(null, result); }; + +internals.sign = function (name, value, options, callback) { + + if (value === undefined || + !options) { + + return callback(null, value); + } + + Iron.hmacWithPassword(options.password, options.integrity || Iron.defaults.integrity, [internals.macPrefix, name, value].join('\n'), function (err, mac) { + + if (err) { + return callback(err); + } + + var signed = value + '.' + mac.salt + '*' + mac.digest; + return callback(null, signed); + }); +}; + + \ No newline at end of file From ecbabcaf3c949bf77ca0c980755c24f04d34328c Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Mon, 25 Mar 2013 22:54:05 -0700 Subject: [PATCH 8/9] Flip pack.api() args order --- lib/pack.js | 9 ++++++--- test/integration/pack/--test1/lib/index.js | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/pack.js b/lib/pack.js index e61b2b950..2371b384d 100755 --- a/lib/pack.js +++ b/lib/pack.js @@ -150,16 +150,19 @@ internals.Pack.prototype._register = function (plugin, permissions, options, cal var methods = { length: selection.servers.length, - api: function (set, key) { + api: function (/* key, value */) { + var key = (arguments.length === 2 ? arguments[0] : null); + var value = (arguments.length === 2 ? arguments[1] : arguments[0]); + selection.servers.forEach(function (server) { server.plugins[plugin.name] = server.plugins[plugin.name] || {}; if (key) { - server.plugins[plugin.name][key] = set; + server.plugins[plugin.name][key] = value; } else { - Utils.merge(server.plugins[plugin.name], set); + Utils.merge(server.plugins[plugin.name], value); } }); }, diff --git a/test/integration/pack/--test1/lib/index.js b/test/integration/pack/--test1/lib/index.js index 355955136..c110a4ad5 100755 --- a/test/integration/pack/--test1/lib/index.js +++ b/test/integration/pack/--test1/lib/index.js @@ -9,7 +9,7 @@ exports.register = function (pack, options, next) { pack.select('test').route({ path: '/test1', method: 'GET', handler: function () { this.reply('testing123'); } }); pack.api(internals.math); - pack.api(internals.text.glue, 'glue'); + pack.api('glue', internals.text.glue); return next(); }; From 3a4bf13bf76a92faa1cfa8ad30f44429652fa79a Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Tue, 26 Mar 2013 03:19:41 -0700 Subject: [PATCH 9/9] Add preAuth ext --- lib/ext.js | 3 ++- lib/request.js | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ext.js b/lib/ext.js index dd7b66208..3d0bba7e4 100755 --- a/lib/ext.js +++ b/lib/ext.js @@ -18,6 +18,7 @@ module.exports = internals.Ext = function () { this._events = { onRequest: null, // New request, before handing over to the router (allows changes to the request method, url, etc.) + onPreAuth: null, // After cookie parse and before authentication onPreHandler: null, // After validation and body parsing, before route handler onPostHandler: null // After route handler returns, before sending response }; @@ -36,7 +37,7 @@ internals.Ext.prototype._add = function (event, func, options, plugin) { options = options || {}; - Utils.assert(['onRequest', 'onPreHandler', 'onPostHandler'].indexOf(event) !== -1, 'Unknown event type: ' + event); + Utils.assert(['onRequest', 'onPreAuth', 'onPreHandler', 'onPostHandler'].indexOf(event) !== -1, 'Unknown event type: ' + event); // Validate rules diff --git a/lib/request.js b/lib/request.js index 7587f10d9..b30838dec 100755 --- a/lib/request.js +++ b/lib/request.js @@ -271,6 +271,7 @@ internals.Request.prototype._execute = function (route) { var funcs = [ // 'onRequest' in Server State.parseCookies, + ext('onPreAuth'), Auth.authenticate, // Authenticates the raw.req object internals.queryExtensions, Validation.query,