rkusa/koa-passport

req.session.regenerate is not a function

haydarai opened this issue ยท 17 comments

It seems to be because of this jaredhanson/passport#907, probably a good idea to downgrade the passport version for now.

@rkusa : can you please downgrade ? This is really blocking me and I'm rethinking to switch to express if this is going to take a long time :/

rkusa commented

@younes-io @haydarai Sole purpose of koa-passport 5.x was to upgrade passport to v6. So if you have to use an older version of passport, you can use koa-passport 4.x for now.

@rkusa koa-passport 5 uses passport v6 whereas koa-passport 4 uses passport v4. Since passport v5 seems to be the way to go for now, how does one handle that?

rkusa commented

@nemphys Fair point. Passport v5 looks to me like it just stops extending http.IncomingMessage.prototype, which koa-passport already did from the begining (and all other minor releases are about fixes/compatibility for that change). However, I might be wrong. In this case, you could depend on koa-passport@4 and pin passport to v5 - the combination should work fine I think.

@rkusa right, this is doable, but I just realised that it beats my purpose to get rid of the passport < 0.6.0 vulnerability (which is the reason I wan to update koa-passport :-) )

Is there any ETA for this? I'm considering my options for what the best way would be to solve this problem within the next few weeks (waiting for a fix or refactoring)

lehni commented

@rkusa passport is defined as a dependency of koa-passport, not a peer dependency. So when using koa-passport@4, passport@0.4 will be used, not v0.5. Also, please note that all passport versions starts with 0., this is currently wrong in the README which speaks of v5 and v6, etc. See https://github.com/jaredhanson/passport/tags

lehni commented

I've added a fix to koa-session with which I am able to use koa-passport correctly, see: koajs/session#221

rkusa commented

I am contemplating whether I should merge #187 (add a workaround to koa-passport) or point everyone to a custom middleware as a workaround (see #187 (comment)). Any opinions?

@rkusa I suppose the 2nd option is better; merging something like that in koa-passport would just be an ugly fix, so anyone interested (including myself) should take the responsibility of adding it to the codebase.

lehni commented

koa-session v6.4.0 was just released, with my PR linked above merged (koajs/session#221). This solves the issue without any workarounds needed.

Nice! What about koa-generic-session?

lehni commented

Perhaps the MR can serve as a scaffolding for the same change there? I don't use koa-generic-session, so won't have the time to look into it.

"koa-passport": "^6.0.0",
"koa-router": "^12.0.0",
"koa-session": "^6.4.0",

We get error: 2023-02-07T12:19:47.935Z error: uncaughtException: Cannot read properties of null (reading 'regenerate')
TypeError: Cannot read properties of null (reading 'regenerate')
at C:\d\node_modules\passport\lib\sessionmanager.js:83:17
at C:\d\node_modules\koa-session\lib\session.js:156:26
at processTicksAndRejections (node:internal/process/task_queues:95:5)

Please help, how to fix that?

lehni commented

@ilonaand the failing line of code is here:

req.session.regenerate(...)

https://github.com/jaredhanson/passport/blob/72119401792ddda24e7c2b652d8d3e2decdbee5d/lib/sessionmanager.js#L83

So it looks like you don't have a session. Are you sure you're actually using the koa-session plugin?

Thanks, but we using koa-session

import session from 'koa-session';
import passport from 'koa-passport';

passport.serializeUser((user: unknown, done) => {
log.info('serializeUser', user);
done(null, (user as IUser).id);
});

passport.deserializeUser((id: string, done) => {
try {
log.info('deserializeUser', id);
const user = userService.findOne(id);
done(null, user);
} catch (err) {
done(err);
}
});

.use(session(Config, app))
.use(passport.initialize())
.use(passport.session())

The user logs in without errors, the session is created, and the error occurs immediately after logging out

lehni commented

Very strange. We have a setup very similar to do this, and it all works for us. The error message does hint at req.session missing, so the problem lies somewhere there.