Cleanup auth interface. Closes #1288

docs/Reference.md

@@ -951,9 +951,26 @@ Registers an authentication scheme where:
 
 The `scheme` method must return an object with the following keys:
 
-- `authenticate(request, reply)` - required function called on each incoming request configured with the authentication scheme.
-- `payload(request, callback)` - optional function called to authenticate the request payload.
-- `response(request, response, callback)` - optional function called to decorate the response with authentication headers.
+- `authenticate(request, reply)` - required function called on each incoming request configured with the authentication scheme where:
+    - `request` - the request object.
+    - `reply(err, result)` - the interface the authentication method must call when done where:
+        - `err` - if not `null`, indicates failed authentication.
+        - `result` - an object containing:
+            - `credentials` - the authenticated credentials. Required if `err` is `null`.
+            - `artifacts` - optional authentication artifacts.
+            - `log` - optional object used to customize the request authentication log which supports:
+                - `data` - log data.
+                - `tags` - additional tags.
+- `payload(request, next)` - optional function called to authenticate the request payload where:
+    - `request` - the request object.
+    - `next(err)` - the continuation function the method must called when done where:
+        - `err` - if `null`, payload successfully authenticated. If `false`, indicates that authentication could not be performed
+          (e.g. missing payload hash). If set to any other value, it is used as an error response.
+- `response(request, next)` - optional function called to decorate the response with authentication headers before the response
+  headers or payload is written where:
+    - `request` - the request object.
+    - `next(err)` - the continuation function the method must called when done where:
+        - `err` - if `null`, successfully applied. If set to any other value, it is used as an error response.
 
 #### `server.auth.strategy(name, scheme, [mode], [options])`
 

lib/auth.js

@@ -39,6 +39,7 @@ internals.Auth.prototype.strategy = function (name, scheme /*, mode, options */)
     var options = (arguments.length === 4 ? arguments[3] : arguments[2]);
 
     Utils.assert(name, 'Authentication strategy must have a name');
+    Utils.assert(name !== 'bypass', 'Cannot use reserved strategy name: bypass');
     Utils.assert(!this._strategies[name], 'Authentication strategy name already exists');
     Utils.assert(scheme && this._schemes[scheme], name, 'has an unknown scheme:', scheme);
     Utils.assert(!mode || !this._defaultStrategy.name, 'Cannot set default required strategy more than once:', name, '- already set to:', this._defaultStrategy);
@@ -147,10 +148,10 @@ internals.Auth.prototype._authenticate = function (request, next) {
         // Injection
 
         if (request.auth.credentials) {
-            return validate(null, { credentials: request.auth.credentials });
+            return validate('bypass', null, { credentials: request.auth.credentials });
         }
 
-        // Authenticate
+        // Find next strategy
 
         if (strategyPos >= config.strategies.length) {
 
@@ -166,27 +167,28 @@ internals.Auth.prototype._authenticate = function (request, next) {
             return next(Boom.unauthorized('Missing authentication', authErrors));
         }
 
+        var strategy = config.strategies[strategyPos];
+        ++strategyPos;
+
         // Generate reply interface
 
         var savedResults = undefined;
         var transfer = function (err) {
 
-            validate(err, savedResults);
+            validate(strategy, err, savedResults);
         };
 
         var root = function (err, result) {
 
             savedResults = result;
-            return (err ? reply._root(err) : validate(err, result));
+            return (err ? reply._root(err) : validate(strategy, err, result));
         };
 
         var reply = Handler.replyInterface(request, transfer, root);
-
-        var strategy = self._strategies[config.strategies[strategyPos++]];           // Increments counter after fetching current strategy
-        return strategy.authenticate(request, reply);
+        return self._strategies[strategy].authenticate(request, reply);
     };
 
-    var validate = function (err, result) {
+    var validate = function (strategy, err, result) {
 
         result = result || {};
 
@@ -198,7 +200,7 @@ internals.Auth.prototype._authenticate = function (request, next) {
 
         if (err) {
             if (result.log) {
-                request.log(['hapi', 'auth', 'error', config.strategies[strategyPos]].concat(result.log.tags), result.log.data);
+                request.log(['hapi', 'auth', 'error', strategy].concat(result.log.tags), result.log.data);
             }
             else {
                 request.log(['hapi', 'auth', 'error', 'unauthenticated'], err);
@@ -210,6 +212,7 @@ internals.Auth.prototype._authenticate = function (request, next) {
 
                 if (config.mode === 'try') {
                     request.auth.isAuthenticated = false;
+                    request.auth.strategy = strategy;
                     request.auth.credentials = result.credentials;
                     request.auth.artifacts = result.artifacts;
                     request.log(['hapi', 'auth', 'error', 'unauthenticated', 'try'], err);
@@ -231,17 +234,17 @@ internals.Auth.prototype._authenticate = function (request, next) {
         // Authenticated
 
         var credentials = result.credentials;
+        request.auth.strategy = strategy;
         request.auth.credentials = credentials;
         request.auth.artifacts = result.artifacts;
-        request.auth._strategy = self._strategies[config.strategies[strategyPos - 1]];
 
         // Check scope
 
         if (config.scope) {
             if (!credentials ||                                                                                         // Missing credentials
                 !credentials.scope ||                                                                                   // Credentials missing scope
                 (typeof config.scope === 'string' && credentials.scope.indexOf(config.scope) === -1) ||                 // String scope isn't in credentials
-                !Utils.intersect(config.scope, credentials.scope).length) {                                             // Array scope doesn't intersect credentials
+                (Array.isArray(config.scope) && !Utils.intersect(config.scope, credentials.scope).length)) {            // Array scope doesn't intersect credentials
 
                 request.log(['hapi', 'auth', 'scope', 'error'], { got: credentials && credentials.scope, need: config.scope });
                 return next(Boom.forbidden('Insufficient scope - ' + config.scope + ' expected'));
@@ -303,45 +306,43 @@ internals.Auth.payload = function (request, next) {
 
     var auth = request.server.auth;
     var config = auth._routeConfig(request);
-
     if (!config ||
         !config.payload ||
-        !request.auth.isAuthenticated) {
+        !request.auth.isAuthenticated ||
+        request.auth.strategy === 'bypass') {
 
         return next();
     }
 
-    if (config.payload === 'optional' &&
-        (!request.auth.artifacts.hash ||
-        typeof request.auth._strategy.payload !== 'function')) {
-
-        return next();
-    }
+    var strategy = auth._strategies[request.auth.strategy];
+    strategy.payload(request, function (err) {
 
-    request.auth._strategy.payload(request, function (err) {
+        if (err === false) {
+            return next(config.payload === 'optional' ? null : Boom.unauthorized('Missing payload authentication'));
+        }
 
         return next(err);
     });
 };
 
 
-internals.Auth.response = function (request, response, next) {
+internals.Auth.response = function (request, next) {
 
     var auth = request.server.auth;
     var config = auth._routeConfig(request);
-
     if (!config ||
-        !request.auth.isAuthenticated) {
+        !request.auth.isAuthenticated ||
+        request.auth.strategy === 'bypass') {
 
         return next();
     }
 
+    var strategy = auth._strategies[request.auth.strategy];
     if (!request.auth.credentials ||
-        !request.auth._strategy ||
-        typeof request.auth._strategy.response !== 'function') {
+        typeof strategy.response !== 'function') {
 
         return next();
     }
 
-    request.auth._strategy.response(request, response, next);
+    strategy.response(request, next);
 };

lib/handler.js

@@ -155,27 +155,32 @@ internals.wrap = function (result, request, finalize) {
 
     var response = Response.wrap(result, request);
 
-    response.hold = function () {
+    if (response.isBoom) {
+        finalize(response)
+    }
+    else {
+        response.hold = function () {
 
-        response.hold = undefined;
+            response.hold = undefined;
 
-        response.send = function () {
+            response.send = function () {
 
-            response.send = undefined;
-            finalize(response);
-        };
+                response.send = undefined;
+                finalize(response);
+            };
 
-        return response;
-    };
+            return response;
+        };
 
-    process.nextTick(function () {
+        process.nextTick(function () {
 
-        response.hold = undefined;
+            response.hold = undefined;
 
-        if (!response.send) {
-            finalize(response);
-        }
-    });
+            if (!response.send) {
+                finalize(response);
+            }
+        });
+    }
 
     return response;
 };

lib/response/headers.js

@@ -10,8 +10,9 @@ var Auth = null;                        // Delay load due to circular dependenci
 var internals = {};
 
 
-exports.apply = function (response, request, next) {
+exports.apply = function (request, next) {
 
+    var response = request.response;
     if (response._payload.size &&
         typeof response._payload.size === 'function') {
 
@@ -31,7 +32,7 @@ exports.apply = function (response, request, next) {
             return next(err);
         }
 
-        internals.auth(response, request, next);
+        internals.auth(request, next);
     });
 };
 
@@ -184,9 +185,9 @@ internals.state = function (response, request, next) {
 };
 
 
-internals.auth = function (response, request, next) {
+internals.auth = function (request, next) {
 
     Auth = Auth || require('../auth');
-    Auth.response(request, response, next);
+    Auth.response(request, next);
 };
 

lib/response/index.js

@@ -28,7 +28,9 @@ exports.wrap = function (result, request) {
 };
 
 
-internals.setup = function (response, request, next) {
+internals.setup = function (request, next) {
+
+    var response = request.response;
 
     var headers = function () {
 
@@ -43,7 +45,7 @@ internals.setup = function (response, request, next) {
             response.statusCode = response._payload.statusCode;
         }
 
-        Headers.apply(response, request, function (err) {
+        Headers.apply(request, function (err) {
 
             if (err) {
                 return next(err);
@@ -100,7 +102,7 @@ exports.send = function (request, callback) {
             return fail(response);
         }
 
-        internals.setup(response, request, function (err) {
+        internals.setup(request, function (err) {
 
             if (err) {
                 request._setResponse(err);
@@ -117,8 +119,9 @@ exports.send = function (request, callback) {
         var response = new Plain(error.payload, request);
         response.code(error.statusCode);
         Utils.merge(response.headers, error.headers);
+        request._setResponse(response);
 
-        internals.setup(response, request, function (err) {
+        internals.setup(request, function (err) {
 
             // Return the original error (which is partially prepared) instead of having to prepare the result error
             return internals.transmit(response, request, callback);