Authentication throws are treated as valid reply()

[shaunlimjin]

I upgraded to hapi v4.0 from v1.2 and had to make some changes to my authentication strategy due to breaking changes introduced in 2.x.

I believe that because my authenticate() function contains a promise, it finishes executing before the promise is resolved and results in the following error:

Debug: hapi, internal, implementation, error 
    Error: Authentication response missing both error and credentials

This is my authenticate() function:

Within a Authentication object:

/**
 * Authenticate user via token
 * @param request
 * @param reply
 */
exports.authenticate = function (request, reply) {
    var token = getToken(request);
    if (token == null) {
        return reply(Hapi.boom.unauthorized('Header has to include Authorization key with value Bearer token.'));
    } else {
        //attempt to get user json object from session store
        this.Session.get(token)
            .then(function (user) {
                //successful authentication, renew key, return user json
                return reply(null, {credentials: user});
            }, function (err) {
                return reply(Hapi.boom.unauthorized('Session does not exist for token: ' + token));
            });
    }
};

And wiring it up:

server.auth.scheme('tokenScheme', function (server, options) {
     return Authentication;
});

server.auth.strategy('token', 'tokenScheme');

Everything was working fine in v1.2. Can someone please tell me what I'm doing wrong?

Thanks,
Shaun

[bendrucker]

Everything looks okay, although it's worth noting that passing two callbacks to then is an anti-pattern. Always use .then(success).catch(failure). Can you confirm that this.Session.get(token) resolves with the expected value? You can just drop a console.log before your reply.

[hueniverse]

Can you tell which reply() is being called?

[shaunlimjin]

Thanks for the replies! As far as I can tell, none of the reply() statements are being called.

It does not even look like the method that's supposed to resolve a promise is being called.

I modified my code for simplicity:

var Authentication = {
authenticate: function (request, reply) {
            this.testPromise().then(function () {
                console.log('this does not print either.');
                return reply(null, {credentials: {
                    email: 'test@test.com',
                    name: 'test'
                }});
            }).catch(function (err) {
                console.log(err);
            });
    },
testPromise: function () {
        console.log('this does not print.');
        var deferred = Q.defer();
        setTimeout(function () {
            deferred.resolve();
        }, 2000);
        return deferred.promise;
    }
};

module.exports = Authentication;

and

//Register our custom authentication strategy

server.auth.scheme('tokenScheme', function (server, options) {
     return Authentication;
});

server.auth.strategy('token', 'tokenScheme');

[hueniverse]

Did you put a console.log before the testPromise code? Also, the error in your initial issue means that some reply is being called, or the route would just hang. You need to figure out which one is returning. Also, can you post the route config for one route you are trying to authenticate with this?

[shaunlimjin]

A console.log before the testPromise code prints.

authenticate: function (request, reply) {
            console.log('before calling testPromise(). this prints.');
            this.testPromise().then(function () {
                console.log('this does not print either.');
                return reply(null, {credentials: {
                    email: 'test@test.com',
                    name: 'test'
                }});
            }).catch(function (err) {
                console.log(err);
            });
    }

This is an example of a route config:

    {
        path: '/clients',
        method: 'GET',
        config: {
            auth: 'token',
            handler: Admin.getClients
        }
    }

and the handler code:

/**
 * Get list of clients.
 * @param request
 * @param reply
 */
exports.getClients = function (request, reply) {
    console.log('getClients() requested by: ' + request.auth.credentials.email);
    ZServices.getClients(function (error, responseCode, body) {
        if (!error) {
            reply(body).code(responseCode);
        } else {
            reply(error).code(responseCode);
        }
    });
};

You're right, the route just hangs if I don't make any function calls in the authenticate function:

 authenticate: function (request, reply) {
        console.log('this just hangs, which is expected');
    }

If we call just any random function, we immediately get the error:

 authenticate: function (request, reply) {
        just_a_function();
    }

[bendrucker]

So your first example says reply was called with bad params? Don't see how that's possible if that message is never logged.

[shaunlimjin]

@bendrucker Exactly, I must be doing something really wrong.

For example, reply() is never called in this snippet, but the same error is thrown:

 authenticate: function (request, reply) {
        just_a_function();
    }

Full stack trace:

Debug: hapi, internal, implementation, error 
    Error: Authentication response missing both error and credentials
    at Object.exports.create (/Users/shaun/workspace/orchestration/node_modules/hapi/node_modules/boom/lib/index.js:21:17)
    at Object.exports.internal (/Users/shaun/workspace/orchestration/node_modules/hapi/node_modules/boom/lib/index.js:235:92)
    at Object.exports.badImplementation (/Users/shaun/workspace/orchestration/node_modules/hapi/node_modules/boom/lib/index.js:271:23)
    at internals.Auth._authenticate.validate (/Users/shaun/workspace/orchestration/node_modules/hapi/lib/auth.js:206:30)
    at internals.Protect.run.finish (/Users/shaun/workspace/orchestration/node_modules/hapi/lib/protect.js:48:21)
    at exports.once.wrapped (/Users/shaun/workspace/orchestration/node_modules/hapi/node_modules/hoek/lib/index.js:484:20)
    at internals.Protect.run._error (/Users/shaun/workspace/orchestration/node_modules/hapi/lib/protect.js:55:16)
    at Domain.module.exports.internals.Protect (/Users/shaun/workspace/orchestration/node_modules/hapi/lib/protect.js:29:20)
    at Domain.EventEmitter.emit (events.js:95:17)
    at process._fatalException (node.js:249:27)

[bendrucker]

Are you sure you have your bind option set properly? Seems like that's the problem and that this.Session is undefined.

[shaunlimjin]

@bendrucker Can you please elaborate on what you mean?

Anyway, if you look at my later code snippets, I stopped using this.Session and started using dummy functions.

[bendrucker]

Actually there's no bind property globally available for the server. So never mind that.

I'm assuming your dummy code is as-written and you're calling non-existent functions and triggering ReferenceErrors. Yes?

[shaunlimjin]

Nope, I fleshed out dummy functions as well.

{
authenticate: function (request, reply) {
            console.log('before calling testPromise(). this prints.');
            this.testPromise().then(function () {
                console.log('this does not print either.');
                return reply(null, {credentials: {
                    email: 'test@test.com',
                    name: 'test'
                }});
            }).catch(function (err) {
                console.log(err);
            });
    },
 testPromise: function () {
        console.log('this does not print.');
        var deferred = Q.defer();
        setTimeout(function () {
            deferred.resolve();
        }, 2000);
        return deferred.promise;
    },
    justafunction: function () {
        console.log('just a function called');
    }
}
}

[bendrucker]

Your testPromise method is clearly not being called and it's because this.testPromise is undefined. You shouldn't rely on the this value when using a framework because you don't know how your method will be called. For example, I can break your code with:

var authenticate = Authentication.authenticate;
authenticate(); // => TypeError undefined has no method 'testPromise'

This may seem contrived but with a framework it's hard to know how your methods will be called. You should assume that the context will be lost unless the docs specify otherwise.

Take a look at where your strategy will actually be called. authenticate is called on null.

[bendrucker]

Although this does unearth a separate issue, which is that authentication functions swallow exceptions

[shaunlimjin]

@bendrucker Thanks for your help!

I fixed my code by using closures to retain the reference to 'this':

//Register our custom authentication strategy
server.auth.scheme('token-auth-scheme', function(server, options) {
   return Authentication.tokenAuthentication();
});

server.auth.strategy('token', 'token-auth-scheme');

    tokenAuthentication: function () {
        var self = this;
        return {
            authenticate: function (request, reply) {
                var token = self.getToken(request);
                if (token == null) {
                    callback(Hapi.boom.unauthorized('Header has to include Authorization key with value Bearer token.'));
                } else {
                    //attempt to get user json object from redis
                    self.Session.get(token)
                        .then(function (user) {
                            //successful authentication, renew key, return user json
                            return reply(null, { credentials: user });
                        }).catch(function (err) {
                            return reply(Hapi.boom.unauthorized('Session does not exist for token: ' + token));
                        });
                }
            }
        }
    }

[hueniverse]

@bendrucker nice catch. The issue is that when the function throws, it is treated as a valid reply call.