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

"Error: failed to deserialize user out of session" unwanted on production #6

Closed
hunterloftis opened this issue Jan 3, 2012 · 14 comments
Assignees

Comments

@hunterloftis
Copy link

This is subjective but I believe a deserialization error from a bad session cookie/reset redis database/other regular production hiccup should not totally stonewall the unfortunate user with the problem. As things stand now, as soon as you get a deserialization error you're essentially blacklisted, and the error will be useless information to a typical user.

More desirable production behavior includes any of:

  1. Remove the session information and treat the user as a 'fresh' user who has not logged in
  2. Allow for a configuration option that can override this behavior (eg, gracefulFailure: true)
  3. Provide an override hook for handling failed deserializations, so the developer can at least override
@jaredhanson
Copy link
Owner

Agreed. I'm leaning towards option 1 as the proper fix.

@ghost ghost assigned jaredhanson Jan 3, 2012
@mwawrusch
Copy link

strong +1 for one.

@ktmud
Copy link

ktmud commented Feb 15, 2012

+10086 for this issue...

@jwyuan
Copy link

jwyuan commented Feb 25, 2012

At least allowing the developer to handle the error would be key. +1 for at least exposing that.

@EvHaus
Copy link

EvHaus commented Mar 14, 2012

+1 on this. Is there a workaround for the issue right now?

@thiagomagro
Copy link

Hello guys, any idea when this will be fixed?

@jaredhanson
Copy link
Owner

Fixed!

You can now invalidate an existing login session by setting user to false or null when calling done inside deserializeUser.

passport.deserializeUser(function(obj, done) {
  done(null, false);  // invalidates the existing login session.
});

Most ORMs set the result to null if the record wasn't found, as is the case with Mongoose. So, this will work as expected if you are deleting user records while they have an active session. In that case, findById sets user to null, which is passed through to Passport and the session will be invalidated.

passport.deserializeUser(function(id, done) {
  User.findById(id, function (err, user) {
    done(err, user);
  });
});

This is fixed in passport@0.1.8, which has just been published to the npm registry.

@orangenotorange
Copy link

I'm having this exact same issue on version 0.1.12 with passport-facebook.

@chmanie
Copy link

chmanie commented Feb 16, 2014

Thank you! This should definitely find it's way into the documentation.

@cmwelsh
Copy link

cmwelsh commented Jun 1, 2014

I would note in the documentation that you have to return null or false. I tried returning undefined instead of null and came here through Google.

@tjwebb
Copy link

tjwebb commented Oct 25, 2014

A world where null and false are the same but are not the same as undefined makes no sense. The following are true facts of Javascript:

null == false == undefined
null !== false !== undefined

Inventing your own truth table where null == false != undefined makes things unnecessarily confusing.

mvilrokx added a commit to mvilrokx/sweepstakes that referenced this issue May 31, 2016
… that should prevent the user ending up with a stale session using passport, see jaredhanson/passport#6 for more information, haven;t tested this though
TYoung86 pushed a commit to TYoung86/sb42 that referenced this issue Sep 5, 2016
@Luchooo
Copy link

Luchooo commented Jan 15, 2019

This working for me, put the next following commands in your terminal:
redis-cli
select 1
FLUSHALL

@bengotow
Copy link

Just a heads up for folks stumbling across this—it turns out TypeORM returns undefined and not null. You should do something like (await User.findOne({where: {id: obj.id}})) || false for Passport to work properly.

airhorns added a commit to fastify/fastify-passport that referenced this issue Dec 31, 2020
I missed this nuance during the original porting -- returning false or null from a user deserializer means something special. It means the user not being found is not an error condition but just means the session has out-of-date data, and should be logged out. See jaredhanson/passport#6 for the original context on how this is a thing in passport. This recreates that functionality and tests it!
mifi added a commit to mifi/passport that referenced this issue Mar 23, 2023
@mifi
Copy link

mifi commented Mar 23, 2023

created PR to fix undefined problem #975

mifi added a commit to mifi/www.passportjs.org that referenced this issue Mar 23, 2023
It can cause subtle errors, because many query builders and ORMs return `undefined` if the entity (user) doesn't exist. See jaredhanson/passport#6 (comment)

This PR is an alternative to jaredhanson/passport#6 (comment)
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

No branches or pull requests