rkusa / koa-passport

Passport middleware for Koa
MIT License
774 stars 55 forks source link

req.session.regenerate is not a function #181

Open haydarai opened 2 years ago

haydarai commented 2 years ago

It seems to be because of this https://github.com/jaredhanson/passport/issues/907, probably a good idea to downgrade the passport version for now.

younes-io commented 2 years ago

@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 2 years ago

@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.

nemphys commented 2 years ago

@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 2 years ago

@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.

nemphys commented 2 years ago

@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 :-) )

CherryDT commented 1 year ago

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 1 year ago

@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 1 year ago

I've added a fix to koa-session with which I am able to use koa-passport correctly, see: https://github.com/koajs/session/pull/221

rkusa commented 1 year ago

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 https://github.com/rkusa/koa-passport/pull/187#issuecomment-1326235941). Any opinions?

nemphys commented 1 year ago

@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 1 year ago

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

nemphys commented 1 year ago

Nice! What about koa-generic-session?

lehni commented 1 year ago

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.

ilonaand commented 1 year ago

"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 1 year ago

@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?

ilonaand commented 1 year ago

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 1 year ago

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.