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

do not initialize session if uneeded. helps express-session not create ... #320

Merged
merged 1 commit into from
Aug 20, 2015

Conversation

ashelley
Copy link
Contributor

...sessions when using saveUninitialized: false option

passport creates an empty passport object {} on req.session every request. this causes express-session saveUninitialized: false not to work properly. this patch may have implications for downstream strategies but i've fixed up the tests on passport core to make sure the session is not defined instead of {}.

Please review this for inclusion as it solves:

#259

…e sessions when using saveUninitialized: false option
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.31%) to 99.69% when pulling 09ccc55 on ashelley:dont-initialize-sessions into 4dce9d9 on jaredhanson:master.

@ghost
Copy link

ghost commented Jan 23, 2015

Sure hope this makes it into master.

@doublerebel
Copy link

👍

@bryangarza
Copy link

Looks good.

@rafaelcardoso
Copy link

Thank you @ashelley , this will solve the problem that I have.
I'm waiting the merge.

@dan-dr
Copy link

dan-dr commented Feb 26, 2015

+bazillion
my poor redis

@mfmendiola
Copy link

Our mongostore is highly interested in whether this PR has landed. Any updates?

@mountHouli
Copy link

Good friends with write access, please merge this in when you can.

@rafaelcardoso
Copy link

There is anybody here? please, someone merge this ):

@wesleytodd
Copy link

I packaged up my solution to this problem, it is a hack, but it does the trick: https://github.com/wesleytodd/express-session-passport-cleanup

I will maintain this at least until this gets merged if anyone wants to use it.

@nicolashenry
Copy link

Googlebot create thousands of sessions because of the related issue. I had to filter crawlers by user agent. Please, merge this pull request.

@hnrch02
Copy link

hnrch02 commented Jul 28, 2015

@jaredhanson Any update on this?

@jaredhanson jaredhanson merged commit 09ccc55 into jaredhanson:master Aug 20, 2015
@jaredhanson
Copy link
Owner

Merged. Published to npm as passport 0.3.0. Thanks!

@ashelley
Copy link
Contributor Author

Thanks @jaredhanson

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 this pull request may close these issues.