jaredhanson / passport

Simple, unobtrusive authentication for Node.js.
https://www.passportjs.org?utm_source=github&utm_medium=referral&utm_campaign=passport&utm_content=about
MIT License
22.94k stars 1.24k forks source link

passport.js throws login session error after upgrading from 0.4.1 to 0.6.0 #939

Open khajatakreemulla opened 2 years ago

khajatakreemulla commented 2 years ago

error: Server Error: error: Error: Login sessions require session support. Did you forget to use express-session middleware? at SessionStrategy.authenticate (/home/khaja/Desktop/flow-web/node_modules/passport/lib/strategies/session.js:46:41) at attempt (/home/khaja/Desktop/flow-web/node_modules/passport/lib/middleware/authenticate.js:369:16) at authenticate (/home/khaja/Desktop/flow-web/node_modules/passport/lib/middleware/authenticate.js:370:7) at Layer.handle [as handle_request] (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:317:13) at /home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:335:12) at next (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:275:10) at initialize (/home/khaja/Desktop/flow-web/node_modules/passport/lib/middleware/initialize.js:98:5) at Layer.handle [as handle_request] (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:317:13) at /home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:335:12) at next (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:275:10) at /home/khaja/Desktop/flow-web/node_modules/sails/lib/hooks/http/get-configured-http-middleware-fns.js:85:20 at app._privateSessionMiddleware (/home/khaja/Desktop/flow-web/node_modules/sails/lib/hooks/session/index.js:467:20) at session (/home/khaja/Desktop/flow-web/node_modules/sails/lib/hooks/http/get-configured-http-middleware-fns.js:83:9) at Layer.handle [as handle_request] (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:317:13) at /home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:335:12) at next (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:275:10) at cookieParser (/home/khaja/Desktop/flow-web/node_modules/cookie-parser/index.js:71:5) at Layer.handle [as handle_request] (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/layer.js:95:5) at trim_prefix (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:317:13) at /home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:284:7 at Function.process_params (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:335:12) at next (/home/khaja/Desktop/flow-web/node_modules/express/lib/router/index.js:275:10)

jaredhanson commented 2 years ago

Are you using session middleware?

Riley- commented 2 years ago

I, too, have this issue and have moved back to 0.5.3 as a result (resolved the issue).

I do not use session middleware, but the error was thrown when using the passport-github2 strategy with passport 0.6.0.

khajatakreemulla commented 2 years ago

@jaredhanson we are using sails.js (an express framework). it w as working well in earlier version. we are using passport with passport-local strategy.

JaimeGSandoval commented 2 years ago

Unfortunately I'm running into the same issue as @khajatakreemulla. I'm trying to destroy a session after req.logout() and I get the same error output. I ended up reverting back to 0.5.3 and now it's working.

khajatakreemulla commented 1 year ago

@jaredhanson did you get a chance to look at this, do i need to know is there any recent changes which will resolve this issue.

boxymoron commented 1 year ago

Same issue here passport@0.6.0 breaks with Sails.

kconut commented 1 year ago

Hi, also encountered this issue when we upgraded from passport 0.4.1 to 0.6.0 with Sails 1.5.3.

We couldn't revert back to the older passport version because of a vulnerability that gets flagged by snyk. What we ended up doing is to just use express-session as http middleware rather than sails' built in session config.

Facj commented 1 year ago

Hi,

I've also encountered this error when updationf from 0.5.2 to 0.6.0. I'm using passport-jwt 4.0.0.

robw00t commented 1 year ago

Fyi I had this same problem after upgrading from 0.5.3 to 0.6.0. I found the solution as such:

this.passport.authenticate('saml', {
                failureRedirect: '/',
                failureFlash: true,
                session: false,
            }),

If you explicitly specify session:false in passport.authenticate then 0.6.0 behaves like 0.5.3 did & doesn't throw the session error.

I didn't have to do that in 0.5.3. I also noticed that I had to specify session:false everywhere I called passport.authenticate.

CharmiBhikadiya commented 1 year ago

@robw00t specifying session:false in passport.authenticate does not work for me. Did you install express-session?

kr105 commented 1 year ago

I just found the same problem and adding session: false to passport.authenticate the problem was gone using Passport 0.6.0 as pointed by @robw00t .

Here is an example.

router.post('/auth/login', passport.authenticate('local', {session: false}), (req, res) => {
    // Create a token
    var token = authenticate.getToken({_id: req.user._id});

    // Response
    res.statusCode = 200;
    res.setHeader('Content-Type', 'application/json');
    res.json({success: true, token: token, status: 'You are successfully logged in!'});
});
Kleyguerth commented 1 year ago

I'm having the same problem, specifying session: false doesn't work. The error is thrown on req.logout

ghost commented 1 year ago

for older versions of passport 0.4.0 we dont need a callback for req.logout(); function but we need for 0.6.0

flowwishthebest commented 1 year ago

This is not good, but perhaps someone will do the same way.

I have the same problem on version 0.6.0 (version 0.5.3 is fine). I don't have sessions and have to upgrade to the latest version so I mocked the session in req.

function mockSession(req) {
  if (!req.session) {
    req.session = {
      save(cb) { cb(); },
      regenerate(cb) { cb(); }
    };
  }
}

passport.use(new SamlStrategy(
  { passReqToCallback: true },
  (req, profile, done) => {
    mockSession(req);
    // ...
  }),
);

app.post(
  '/login',
  passport.authenticate('saml', {
    session: false /* not working, but maybe later */
  }),
  (req, res) => {
    // ...
  }
);
markstos commented 1 year ago

I ran into this and worked to isolate the issue. As @flowwishthebest found, the issue is introduced 0.6.0-- 0.5.3 is not affected. If you want review the "diff" between the two release, it's here:

https://github.com/jaredhanson/passport/compare/v0.5.3...v0.6.0#diff-ba75157e9d5273ecf42d4a7d4538e5dad57b5aa0774cbd86ebaf109bb144c86cR21

In particularly, you can find the code that throws the error was introduced here: https://github.com/jaredhanson/passport/compare/v0.5.3...v0.6.0#diff-ba75157e9d5273ecf42d4a7d4538e5dad57b5aa0774cbd86ebaf109bb144c86cR14

I believe to trigger it, to you need to have not set the session: key in your call to authenticate(). As with some others, setting session: false in my call to authenticate fixed it.

My code was not calling req.login() before, and we are were not using session: true anywhere and we were not loading express-session anywhere.

It appears that 0.6.0, unless session: false is set then authenticate() will call req.logIn() which in turn will expect req.session to exist.

Some people here reported that setting session: false did not fix the issue for them. That's because of this old bug / implemented feature in the login function.

In that case, if req.session did not exist-- whether that was a bug or not-- it was initialized to an empty object. The comment in 0.5.3 said "TODO: Error if session isn't available". So it looks like the intent had to been to require req.session or throw an error all along, but it was not implemented until 0.6.0.

https://github.com/jaredhanson/passport/compare/v0.5.3...v0.6.0#diff-ba75157e9d5273ecf42d4a7d4538e5dad57b5aa0774cbd86ebaf109bb144c86cL18

If that's your case, I guess you need to follow docs to load use express-session.

kivra-gusahl commented 1 year ago

I encountered this as well while running a sailsjs app. The problem is that sails is disabling session on requests for assets. So main "main" request would work but I got the error for all css and js assets. My solution was to add this to the session config so that we enable sessions for all requests:

isSessionDisabled: () => false,