No access to context in Strategy.
atassis opened this issue · 17 comments
Hello. When I try to get user data in strategy, I can't get the state data. For example, I need to authenticate user to tether his data of different socials, so in express I'ld need to set passReqToCallback: true in strategy.
For now I made a hack in my project- in file lib/framework/request.js I changed exports.create function to this:
const req = Object.create(ctx.request, properties)
// add passport http.IncomingMessage extensions
req.login = IncomingMessageExt.logIn
req.logIn = IncomingMessageExt.logIn
req.logout = IncomingMessageExt.logOut
req.logOut = IncomingMessageExt.logOut
req.isAuthenticated = IncomingMessageExt.isAuthenticated
req.isUnauthenticated = IncomingMessageExt.isUnauthenticated
req.ctx = ctx;
return req
}
Just added req.ctx=ctx; before return req so I can use ctx in my strategy.
For compatibility reasons, the user object is also accessible from req.user. So you should be able to use passReqToCallback. Or do you need something else from the ctx?
@rkusa, For most circumstances, I really do need req.user only. But in some cases, I need to get some specific data from context. Anyway, my variant is not breaking passport behaviour. I am already using it with koa-passport-ot module and getting the ctx by destructuring req.
I pass the database connection through middlewares via ctx.state so having access to that in the strategy would be nice, for persistence.
A workaround that does not break the compatibility with passport would be to add the context to the req object: req.ctx. However, I am not sure if I want to add this to koa-passport, because it is simple to implement this workaround on your own and I think most users don't need this functionality. What do you guys think?
I think passport is biased by express/connect, but having if not full ctx but at least the state would be beneficial.
It definitely is biased by express/connect, which is really unfortunate. I've checked the source of passport and its strategies again. There really is no other way than adding a state getter to the req object. I don't like this solution, but since the user getter already "pollutes" the req object, also adding adding a state getter is probably not worse. Especially since there is a real use case behind this.
Remaining question would be whether the req.state getter should be added by default or only when a specific option is activated? Opinions are welcome
@rkusa I see no reason not to add ctx to req. With ES6 properties destructuring it you can easily do ({ ctx }, ...) => {}, which gives you full control under your context in strategy. I agree with such a misfortune as lack of passport analogue for koa and thinking about writing koa-based one. For myself, I created temp package, koa-passport-ot.
Isn't something like
const passport = require('koa-passport')
app.use((ctx, next) => { ctx.req.ctx = ctx; return next() })
app.use(passport.initialize())
app.use(passport.session())a good enough solution?
If you prefer shorter code, it should also be possible to write something like:
app.use((ctx, next) => (ctx.req.ctx = ctx, next()))@rkusa first off, thank you for koa-passport it is seriously a life saver!
For what it's worth, I also needed this functionality. I might be worth adding a little disclaimer to the Readme with a link to this issue and the workaround, that way people can find it easily. I do think it feels like something that many apps will need if they're anything more than prototypes, since limiting connections to the database seems like a pretty standard use case.
Another dude here wanting access to the context in a Strategy.
@ianstormtaylor @basickarl Are you guys interested in the whole context or would the ctx.state be sufficient?
Another vote for ctx. In my case, I need access to headers to get Token Lifetime information.
Vote here for the full ctx. No need for an arbitrary limitation.
A req.ctx getter is now available in v4.0.0
@leebenson Just published (it is a major release because of the passport update to 0.4.x)