Skip to content
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

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

Open
khajatakreemulla opened this issue Sep 24, 2022 · 16 comments · May be fixed by #986
Open

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

khajatakreemulla opened this issue Sep 24, 2022 · 16 comments · May be fixed by #986

Comments

@khajatakreemulla
Copy link

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)

  • Operating System: ubuntu
  • Node version: v14.20.0
  • passport version: v0.6.0
@jaredhanson
Copy link
Owner

Are you using session middleware?

@Riley-
Copy link

Riley- commented Sep 25, 2022

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
Copy link
Author

@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
Copy link

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
Copy link
Author

@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
Copy link

Same issue here passport@0.6.0 breaks with Sails.

@kconut
Copy link

kconut commented Nov 23, 2022

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
Copy link

Facj commented Dec 16, 2022

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
Copy link

robw00t commented Jan 17, 2023

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
Copy link

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

@kr105
Copy link

kr105 commented Mar 5, 2023

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
Copy link

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

@ghost
Copy link

ghost commented Mar 28, 2023

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
Copy link

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
Copy link

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:

v0.5.3...v0.6.0#diff-ba75157e9d5273ecf42d4a7d4538e5dad57b5aa0774cbd86ebaf109bb144c86cR21

In particularly, you can find the code that throws the error was introduced here:
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.

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
Copy link

kivra-gusahl commented Sep 4, 2023

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,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.