-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Strange behavior for passport.authenticate() function #125
Comments
Weird, my JWT strategy isn't being executed. Maybe this is a fix? |
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. |
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
The change basically allows calling
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) |
I re-evaluated the change I proposed above and not think that it is not a good idea, since it basically tries to make 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
) |
Hey Markus! Thank you very much for your response and the example given. The middleware approach worked for me! |
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 :) |
@ghost, thanks a lot for sharing this solution! I was also searching for hours how to combine const jwtAuth = passport.authenticate('jwt', { session: false }, (error, user) => {...});
router.get('/path', jwtAuth, nextMiddleware); prevents The following allows me both to provide a custom response and to proceed to const jwtAuth = async(ctx, next) => {
await passport.authenticate('jwt', { session: false }, async(error, user) => {
if (error || !user) {
throw Boom.unauthorized();
}
await next();
})(ctx, next);
}; |
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 toreturn
, 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
Query application with curl or similar tool to see output:
My application console output if koa-passport does
return next()
(wrong actions sequence):My application console output if koa-passport does
return
(everything is OK):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.
The text was updated successfully, but these errors were encountered: