Strange behavior for passport.authenticate() function

[Brozish]

Hi there!

I'm trying koa-passport and discovered a strange behavior on passport.authenticate() function call.
This function does return next() and this breaks my actions sequence (see details below).
If I change return next() in koa-passport lib to return, everything works just fine (at least as I expect it to be).

Can anyone advice on this and help me to clarify if there is a bug or I'm just doing something wrong?

My example application code that makes output located at:
https://gist.github.com/Brozish/69e8147f323bbb044f3efd21e5d0df2f

create file version.js and copy example application code

yarn init
yarn add koa koa-router koa-bodyparser koa-passport passport-jwt mongoose winston
node version.js

Query application with curl or similar tool to see output:

curl -X GET \
  http://localhost:3000/users \
  -H 'Authorization: Bearer eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjViZmQwNTYwYWZmZjcxMjBmOTBkZTY4NSIsImVtYWlsIjoiaXNzdWVAaXNzdWUuaXNzdWUiLCJpYXQiOjE1NDMzMDg2NTcsImV4cCI6MTU3NDg0NDY1N30.RuyTMiiZRS6j8XhWPzPSKL4jacn2aRnaWLQQ6K5z9F7aWQgBoehl-1eXddRy_QfSxRF4cILPUhSWa1PnK78ocg' \
  -H 'cache-control: no-cache'

My application console output if koa-passport does return next() (wrong actions sequence):

• startAuth
• beforeKoaPassport
• jwtStrategy
• startGetUsers
• endGetUsers
• afterKoaPassport
• endAuth

My application console output if koa-passport does return (everything is OK):

• startAuth
• beforeKoaPassport
• jwtStrategy
• afterKoaPassport
• endAuth
• startGetUsers
• endGetUsers

Koa-passport lib code that breaks my app can be found at:
116fa48

git blame ./lib/framework/koa.js --date=short -L 14
116fa48 (rkusa 2015-11-20 149) return next()

My full application code:
https://bitbucket.org/Brozish/node.js/src/master/

Any help on this subject would be appreciated. If this is a bug, I would be happy to make a pull request or being mentioned in commit with a fix.

[devt3000]

Weird, my JWT strategy isn't being executed. Maybe this is a fix?

[rkusa]

Hey, just to let you know, it is on my todo to look into it, I just wasn't able to find the time, yet.

[rkusa]

First, thanks for the good issue. A good description, example, and even a blame to track down the origin of the change that introduced the behaviour 💙

I think we cannot change return next() to return because this would break using the passport.authenticate as a middleware. Instead, I'd suggest the following change f3e1708, which would allow the following usage:

const [user] = await passport.authenticate('jwt', { session: false })(ctx)

The change basically allows calling passport.authenticate without having to add a callback. In this case it now

• forwards errors to the returned promise,
• returnes the callback's result to the returned promise
  (not the (ctx) instead of `(ctx, next)

Please let me know if this change would work for you. (though I have to fix the tests for this change, before being able to publish them)

[rkusa]

I re-evaluated the change I proposed above and not think that it is not a good idea, since it basically tries to make passport.authenticate both callable as a middleware and as an inline function. The failing tests are a prove, that doing so is error-prone.

I am wondering whether your example should just be?

const auth = async (ctx, next) => {
  logger.info('startAuth')

  if (!ctx.state.user) {
    ctx.throw(401, 'Unauthorized')
  }

  logger.info('endAuth')

  await next()
}

const getUsers = async (ctx) => {
  logger.info('startGetUsers')

  const users = await User.find({})

  ctx.body = {
    payload: users
  }

  logger.info('endGetUsers')
}

router.get(
  '/users',
  passport.authenticate('jwt', { session: false }),
  auth,
  getUsers
)

[Brozish]

Hey Markus!

Thank you very much for your response and the example given. The middleware approach worked for me!
You may close this issue.

[Deleted user]

For those who don't like this piece of code:

router.get(
  '/users',
  passport.authenticate('jwt', { session: false }),
  auth,
  getUsers
);

You can try something like this:

const passport = require('koa-passport');

const privateRoute = (ctx, next) => {
  return passport.authenticate('jwt', { session: false }, async (err, user) => {
    if (err || !user) {
      ctx.throw(401, 'Unauthorized');
    } else {
      await ctx.login(user);
      await next();
    }
  })(ctx);
};

And then use it like this:

router.get(
  '/users',
  privateRoute,
  getUsers
);

I was trying to find such solution for 5 hours. I guess i need to leave it here, maybe it will help somebody in the future.

[katlimruiz]

For those who don't like this piece of code:

router.get(
  '/users',
  passport.authenticate('jwt', { session: false }),
  auth,
  getUsers
);

You can try something like this:

const passport = require('koa-passport');

const privateRoute = (ctx, next) => {
  return passport.authenticate('jwt', { session: false }, async (err, user) => {
    if (err || !user) {
      ctx.throw(401, 'Unauthorized');
    } else {
      await ctx.login(user);
      await next();
    }
  })(ctx);
};

And then use it like this:

router.get(
  '/users',
  privateRoute,
  getUsers
);

I was trying to find such solution for 5 hours. I guess i need to leave it here, maybe it will help somebody in the future.

this is exactly what I've been looking for, though I dont quite get exactly what is doing (to me, this should return a middleware object, not the execution of it) ... but it works :)

[ezze]

@ghost, thanks a lot for sharing this solution! I was also searching for hours how to combine authenticate middleware from koa-passport and custom callbacks to return my API responses are JSON instead of plain text. Using something like

const jwtAuth = passport.authenticate('jwt', { session: false }, (error, user) => {...});
router.get('/path', jwtAuth, nextMiddleware);

prevents nextMiddleware from being called.

The following allows me both to provide a custom response and to proceed to nextMiddleware:

const jwtAuth = async(ctx, next) => {
  await passport.authenticate('jwt', { session: false }, async(error, user) => {
    if (error || !user) {
      throw Boom.unauthorized();
    }
    await next();
  })(ctx, next);
};