Skip to content

Commit

Permalink
authenticate callback signature. Closes #1281
Browse files Browse the repository at this point in the history
  • Loading branch information
Eran Hammer committed Jan 2, 2014
1 parent 0e1e68f commit f84f514
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 38 deletions.
22 changes: 10 additions & 12 deletions lib/auth/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,63 +22,61 @@ exports = module.exports = internals.Scheme = function (server, options) {

// Basic Authentication

internals.Scheme.prototype.authenticate = function (request, callback) {
internals.Scheme.prototype.authenticate = function (request, reply) {

var self = this;

callback = Utils.nextTick(callback);

var req = request.raw.req;
var authorization = req.headers.authorization;
if (!authorization) {
return callback(Boom.unauthorized(null, 'Basic'));
return reply(Boom.unauthorized(null, 'Basic'));
}

var parts = authorization.split(/\s+/);

if (parts[0] &&
parts[0].toLowerCase() !== 'basic') {

return callback(Boom.unauthorized(null, 'Basic'));
return reply(Boom.unauthorized(null, 'Basic'));
}

if (parts.length !== 2) {
return callback(Boom.badRequest('Bad HTTP authentication header format', 'Basic'));
return reply(Boom.badRequest('Bad HTTP authentication header format', 'Basic'));
}

var credentialsParts = new Buffer(parts[1], 'base64').toString().split(':');
if (credentialsParts.length !== 2) {
return callback(Boom.badRequest('Bad header internal syntax', 'Basic'));
return reply(Boom.badRequest('Bad header internal syntax', 'Basic'));
}

var username = credentialsParts[0];
var password = credentialsParts[1];

if (!username && !this.settings.allowEmptyUsername) {
return callback(Boom.unauthorized('HTTP authentication header missing username', 'Basic'));
return reply(Boom.unauthorized('HTTP authentication header missing username', 'Basic'));
}

this.settings.validateFunc(username, password, function (err, isValid, credentials) {

credentials = credentials || null;

if (err) {
return callback(err, credentials, { log: { tags: ['auth', 'basic'], data: err } });
return reply(err, { credentials: credentials, log: { tags: ['auth', 'basic'], data: err } });
}

if (!isValid) {
return callback(Boom.unauthorized('Bad username or password', 'Basic'), credentials);
return reply(Boom.unauthorized('Bad username or password', 'Basic'), { credentials: credentials });
}

if (!credentials ||
typeof credentials !== 'object') {

return callback(Boom.badImplementation('Bad credentials object received for Basic auth validation'), null, { log: { tags: 'credentials' } });
return reply(Boom.badImplementation('Bad credentials object received for Basic auth validation'), { log: { tags: 'credentials' } });
}

// Authenticated

return callback(null, credentials);
return reply(null, { credentials: credentials });
});
};

Expand Down
4 changes: 2 additions & 2 deletions lib/auth/bewit.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ exports = module.exports = internals.Scheme = function (server, options) {

// Hawk Authentication

internals.Scheme.prototype.authenticate = function (request, callback) {
internals.Scheme.prototype.authenticate = function (request, reply) {

Hawk.server.authenticateBewit(request.raw.req, this.settings.getCredentialsFunc, this.settings.hawk, function (err, credentials, bewit) {

return callback(err, credentials, { artifacts: bewit });
return reply(err, { credentials: credentials, artifacts: bewit });
});
};
16 changes: 7 additions & 9 deletions lib/auth/cookie.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,10 @@ exports = module.exports = internals.Scheme = function (server, options) {

// Cookie Authentication

internals.Scheme.prototype.authenticate = function (request, callback) {
internals.Scheme.prototype.authenticate = function (request, reply) {

var self = this;

callback = Utils.nextTick(callback);

var validate = function () {

// Check cookie
Expand All @@ -59,7 +57,7 @@ internals.Scheme.prototype.authenticate = function (request, callback) {
}

if (!self.settings.validateFunc) {
return callback(null, session);
return reply(null, { credentials: session });
}

self.settings.validateFunc(session, function (err, isValid, credentials) {
Expand All @@ -71,21 +69,21 @@ internals.Scheme.prototype.authenticate = function (request, callback) {
request.clearState(self.settings.cookie);
}

return unauthenticated(Boom.unauthorized('Invalid cookie'), session, { log: (err ? { data: err } : 'Failed validation') });
return unauthenticated(Boom.unauthorized('Invalid cookie'), { credentials: credentials, log: (err ? { data: err } : 'Failed validation') });
}

if (credentials) {
request.setState(self.settings.cookie, credentials);
}

return callback(null, credentials || session);
return reply(null, { credentials: credentials || session });
});
};

var unauthenticated = function (err, session, options) {
var unauthenticated = function (err, result) {

if (!self.settings.redirectTo) {
return callback(err, session, options);
return reply(err, result);
}

var uri = self.settings.redirectTo;
Expand All @@ -100,7 +98,7 @@ internals.Scheme.prototype.authenticate = function (request, callback) {
uri += self.settings.appendNext + '=' + encodeURIComponent(request.url.path);
}

return callback(request.generateResponse().redirect(uri), session, options);
return reply(request.generateResponse().redirect(uri), result);
};

validate();
Expand Down
4 changes: 2 additions & 2 deletions lib/auth/hawk.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ exports = module.exports = internals.Scheme = function (server, options) {

// Hawk Authentication

internals.Scheme.prototype.authenticate = function (request, callback) {
internals.Scheme.prototype.authenticate = function (request, reply) {

Hawk.server.authenticate(request.raw.req, this.settings.getCredentialsFunc, this.settings.hawk, function (err, credentials, artifacts) {

return callback(err, credentials, { artifacts: artifacts });
return reply(err, { credentials: credentials, artifacts: artifacts });
});
};

Expand Down
19 changes: 10 additions & 9 deletions lib/auth/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ internals.Auth.prototype.authenticate = function (request, next) {
// Injection

if (request.auth.credentials) {
return validate(null, request.auth.credentials);
return validate(null, { credentials: request.auth.credentials });
}

// Authenticate
Expand All @@ -204,19 +204,19 @@ internals.Auth.prototype.authenticate = function (request, next) {
return strategy.authenticate(request, validate);
};

var validate = function (err, credentials, options) {
var validate = function (err, results) {

options = options || {};
results = results || {};

// Unauthenticated

if (!err && !credentials) {
if (!err && !results.credentials) {
return next(Boom.badImplementation('Authentication response missing both error and credentials'));
}

if (err) {
if (options.log) {
request.log(['hapi', 'auth', 'error', config.strategies[strategyPos]].concat(options.log.tags), options.log.data);
if (results.log) {
request.log(['hapi', 'auth', 'error', config.strategies[strategyPos]].concat(results.log.tags), results.log.data);
}
else {
request.log(['hapi', 'auth', 'error', 'unauthenticated'], err);
Expand All @@ -228,8 +228,8 @@ internals.Auth.prototype.authenticate = function (request, next) {

if (config.mode === 'try') {
request.auth.isAuthenticated = false;
request.auth.credentials = credentials;
request.auth.artifacts = options.artifacts;
request.auth.credentials = results.credentials;
request.auth.artifacts = results.artifacts;
request.log(['hapi', 'auth', 'error', 'unauthenticated', 'try'], err);
return next();
}
Expand All @@ -248,8 +248,9 @@ internals.Auth.prototype.authenticate = function (request, next) {

// Authenticated

var credentials = results.credentials;
request.auth.credentials = credentials;
request.auth.artifacts = options.artifacts;
request.auth.artifacts = results.artifacts;
request.auth._strategy = self._strategies[config.strategies[strategyPos - 1]];

// Check scope
Expand Down
6 changes: 3 additions & 3 deletions test/integration/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ describe('Auth', function () {

server.route([
{ method: 'POST', path: '/hawk', handler: hawkHandler, config: { auth: true } },
{ method: 'POST', path: '/hawkValidate', handler: hawkHandler, config: { auth: true, validate: { query: { } } } },
{ method: 'POST', path: '/hawkValidate', handler: hawkHandler, config: { auth: true, validate: { query: {} } } },
{ method: 'POST', path: '/hawkError', handler: hawkErrorHandler, config: { auth: true } },
{ method: 'POST', path: '/hawkStream', handler: hawkStreamHandler, config: { auth: true } },
{ method: 'POST', path: '/hawkOptional', handler: hawkHandler, config: { auth: { mode: 'optional' } } },
Expand Down Expand Up @@ -1061,7 +1061,7 @@ describe('Auth', function () {

authenticate: function (request, callback) {

callback(null, {});
callback(null, { credentials: {} });
}
}
}
Expand Down Expand Up @@ -1202,7 +1202,7 @@ describe('Auth', function () {

it('returns a 401 response when missing the authorization header', function (done) {

var request = { method: 'POST', url: 'http://example.com:8080/multiple'};
var request = { method: 'POST', url: 'http://example.com:8080/multiple' };

server.inject(request, function (res) {

Expand Down
11 changes: 11 additions & 0 deletions test/integration/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,17 @@ describe('Response', function () {
done();
});
});

it('ignores result when err provided in reply(err, result)', function (done) {

var server = new Hapi.Server();
server.route({ method: 'GET', path: '/', handler: function (request, reply) { reply(Hapi.error.badRequest(), 'steve'); } });
server.inject('/', function (res) {

expect(res.statusCode).to.equal(400);
done();
});
});
});

describe('Buffer', function () {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ describe('Auth', function () {
implementation: {
authenticate: function (request, callback) {

return callback(null, null, false);
return callback(null, null);
}
}
}
Expand Down

0 comments on commit f84f514

Please sign in to comment.