-
Notifications
You must be signed in to change notification settings - Fork 107
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
Inconsistent behavior when destroying a session #143
Comments
I suspect the problem is actually in your code (the logout route): router.get('/logout', (req, res) => {
// Had to call both of these for some reason to get the session to actually be logged out correctly
req.logout((err) => {
if (err) log.error(HELPERS.auth, 'Unable to logout', err)
})
req.session.destroy() // removing this with 3.0.0 makes the error go away
res.redirect('/')
}) The Changing the above code block to: router.get('/logout', (req, res) => {
req.logout((err) => {
if (err) {
log.error(HELPERS.auth, 'Unable to logout', err)
}
// Logout failed or successful. In either case, redirect the user.
res.redirect('/')
})
}) Should resolve the passport related error and also the inconsistent logout behavior that you described. |
This might not be an actual bug, or could be an issue with Passport, but I'm experiencing this inconsistent behavior after updating this lib, so I'm starting here first.
Problem:
When a user calls the
/auth/logout
route, the server errors. It was working previously on2.1.6
, but after recently updating and changing the implementation, the same code now crashes.Version:
I recently upgraded from
^2.1.6
to^3.0.0
.Related Libs:
Error
Stack Trace
The property that is undefined is the request.
Codeblock from Passport.js
Code
Logout Route
Old Session Store
New Session Store
Solutions
So far these have appeared to "solve" the problem, but given the inconsistency, maybe something in the lib needs to be adjusted?
request.destroy()
callThank you! I've been using your library in my OSS projects for several years and it's been great :)
The text was updated successfully, but these errors were encountered: