In passport.authenticate, next() must be passed to a callback

[kadishmal]

passport.authenticate allows to pass a callback function as a third argument.

module.exports = function authenticate(name, options, callback) {...}

When request is made, its middleware function is called:

return function authenticate(req, res, next) {...}

which attempts to authenticate a user/client using some strategy:

(function attempt(i) {...}

Upon success, it invokes the following code:

delegate.success = function(user, info) {
    if (callback) {
      return callback(null, user, info);
    }

which, as can be seen above, calls callback if defined, then returns.

Because callback is called, the res is not ended, and next() is not called, thus the request hangs. To resolve this issue, next must be passed to callback as the last argument to allow the callback to pass on the request and continue the middleware flow.

If PR is necessary, I can provide it.

[jaredhanson]

No. If you use the custom callback, you have to send the response and call end

Sent from my iPhone

On May 1, 2014, at 7:30 PM, Esen Sagynov notifications@github.com wrote:

passport.authenticate allows to pass a callback function as a third argument.

module.exports = function authenticate(name, options, callback) {...} When request is made, its middleware function is called:

return function authenticate(req, res, next) {...} which attempts to authenticate a user/client using some strategy:

(function attempt(i) {...} Upon success, it invokes the following code:

delegate.success = function(user, info) { if (callback) { return callback(null, user, info); } which, as can be seen above, calls callback if defined, then returns. Because callback is called, the res is not ended, and next() is not called, thus the request hangs.

To resolve this issue, next must be passed to callback as the last argument. If PR is necessary, I can provide it.

— Reply to this email directly or view it on GitHub.

[kadishmal]

Imagine I have the following logic:

app.post('/auth/oauth2/token', [passport.authenticate('oauth2-client-password', { session: false }, callback), oauth2.token];

Despite the login logic I choose (my own callback or passport.authenticate), it must continue to the oauth2.token handler(s) on successful login. So, passing the response is not proper, but next is.

[jaredhanson]

Remove the callback. Use it as normal middleware like intended

Sent from my iPhone

On May 1, 2014, at 9:49 PM, Esen Sagynov notifications@github.com wrote:

Imagine I have the following logic:

app.post('/auth/oauth2/token', [passport.authenticate('oauth2-client-password', { session: false }, callback), oauth2.token]; Despite the login logic I choose (my own callback, or passport.authenticate), it must continue to the oauth2.token handler(s). So, passing response is not proper, but next() is.

— Reply to this email directly or view it on GitHub.

[kadishmal]

I did so several months already. Recently I have found that the response type of oauth2-client-password and bearer are different. The first one assigns a client object to req.user while the second one assigns a user object. Not consistent. The only option to make them consistent is to provide a callback, thus not allow passport.authenticate to assign a client object to req.user.

In the above example:

app.post('/auth/oauth2/token', [passport.authenticate('oauth2-client-password', { session: false }, callback), oauth2.token];

The task is to obtain an access token for a client which provided an authentication code. Since the client is acting on behalf of a user, req.user should not be a reference to a client, but it -either should be undefined (yet, until some logic does retrieve a user object), or- should be set to an actual user.

For this reason, the callback is a more logical solution to handle this problem.

[jaredhanson]

You are making the problem too hard. The custom callback is not the problem here. Use of client-password and bearer should occur in distinct routes, where there is no ambiguity as to what the authenticating entity is

Sent from my iPhone

On May 1, 2014, at 10:05 PM, Esen Sagynov notifications@github.com wrote:

I did so several month already. Recently I have found that the response type of oauth2-client-password and bearer are different. The first one assigns a client object to req.user while the second one assigns a user object. Not consistent. The only option to make them consistent is to provide a callback, thus not allow passport.authenticate to assign a client object to req.user.

In the above example:

app.post('/auth/oauth2/token', [passport.authenticate('oauth2-client-password', { session: false }, callback), oauth2.token]; The task is to obtain an access token for a client which provided an authentication code. Since the client is acting on behalf of a user, req.user should not be a reference to a client, but it either should be undefined (yet, until some logic does retrieve a user object), or be set to an actual user.

For this reason, the callback is a more logical solution to handle this problem.

— Reply to this email directly or view it on GitHub.

[jaredhanson]

To clarify:

The first one assigns a client object to req.user while the second one assigns a user object. Not consistent.

Why is that a problem? A route that is authenticating a client should never attempt to authenticate a client. For clarity, you can simply:

var client = req.user;

Alternatively, theres an option:

passport.authenticate('oauth2-client-passport', { assignProperty: 'client', ... }

// var client = req.client

[kadishmal]

Eventually this may be right. I suppose we may close this issue.

On Sat, May 3, 2014 at 12:20 AM, Jared Hanson notifications@github.com wrote:

To clarify:

The first one assigns a client object to req.user while the second one assigns a user object. Not consistent. Why is that a problem? A route that is authenticating a client should never attempt to authenticate a client. For clarity, you can simply: var client = req.user; Alternatively, theres an option: passport.authenticate('oauth2-client-passport', { assignProperty: 'client', ... }

// var client = req.client

Reply to this email directly or view it on GitHub: https://github.com/jaredhanson/passport/issues/237#issuecomment-42043091

[kapouer]

@jaredhanson you have to admit it is weird to have a code path that either calls next or that calls a function that cannot call next. This makes the callback option useless.

EDIT: awww unless it is meant to be used like this:

app.use(function(req, res, next) {
  passport.authenticate('bearer', {session: false}, function(err, user, info) {
    if (err) return next(err);
    next('route');
  })(req, res, next);
});

[mooocer]

I am using socket.io and want to use io.use(passport.authenticate), but it is throwing

TypeError: next is not a function

I realize that I am not passing next as an argument like in the documentation, but I couldn't figure out the solution. Stackoverflow question

Is this issue related?

[ramartinez7]

This man Jared is rude,

[AlexStormwood]

This severely-old issue still comes up very high in Google when searching for how to use custom callbacks on passport.authenticate as tidy middleware. Found a solution in another StackOverflow issue: https://stackoverflow.com/a/35983944/9319097

Basically, you gotta wrap passport.authenticate in separate middleware functions. Like so:

const routeRequiresValidJwt = function (request, response, next) {
    passport.authenticate('jwt', {session: false}, (error, user, info, status) => {
        if (error) {
            let someAuthError = new Error("Something went wrong confirming your session, please try again later.");
            someAuthError.status = 403;
            throw someAuthError;
        }
        if (!user) {
            let invalidUserError = new Error("Invalid session token, please try again later.");
            invalidUserError.status = 403;
            throw invalidUserError;
        }

        // Makes a new JWT so users stay logged in for longer if they used the app recently
        const newJwt = refreshJwt(user);
        request.freshJwt = newJwt;
        next();
    })(request, response, next);
}

It can then be imported and used in super tidy middleware calls:

router.get(
    '/private',
    routeRequiresValidJwt,
    (request, response, next) => {
        console.log("Responding on private route now...")
        response.json({
            message:"You made it!",
            newToken: request.freshJwt
        });
    }
);

I hope this helps others conform to this passport.authenticate structure while keeping their code tidy!

It would be awesome to see this documented somewhere as writing clean, minimal middleware is a pretty common need.