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.92k stars 1.24k forks source link

req.logout not working with "local strategy" #246

Open pensierinmusica opened 10 years ago

pensierinmusica commented 10 years ago

The req.logout method doesn't seem to delete the session values when invoked.

I'm using passport-local and cookie-session, with Express.

It looks like similar issues are discussed on StackOverflow.

Any ideas what might be causing the problem?

tybenz commented 10 years ago

Same thing here. Using passport-http, basic session, with restify.

I've tried both:

req.logout();
res.send( { message: 'Successfully logged out' } );

and:

req.session.destroy( function ( err ) {
    res.send( { message: 'Successfully logged out' } );
});

Neither of them are working. Any suggestions?

mathiasm74 commented 10 years ago

Got the same problem. Seems to be due to changes in Express 4.0.

phof commented 10 years ago

Same issue here, also using Express 4 + cookie-session.

iamsenseiken commented 10 years ago

This is still a problem it would seem... anyone have a good solution yet?

johnmastri commented 10 years ago

Same issue here

chriskrycho commented 9 years ago

I'm in the same boat. Any update?

itsmichaeldiego commented 9 years ago

Same issue here. Express 4 + Passport

It seems only happening in Chrome (Version 38.0.2125.122.) Is that possible ?

chriskrycho commented 9 years ago

Follow-up: for me, it appears to have been a dependency issue (possibly deriving from an automated refactoring that didn't correctly exclude node_modules). I nuked my local node_modules directory, ran npm install, and got a working session logout.

Note that for my tests with jasmine-node and request to work correctly, I had to make sure to supply a request.jar() instance in the jar field of the request object.

Here's my logout:

// Express middleware function for logging out a user. The action is successful
// if the user is no longer authenticated.
var logout = function (req, res, next) {
  // Get rid of the session token. Then call `logout`; it does no harm.
  req.logout();
  req.session.destroy(function (err) {
    if (err) { return next(err); }
    // The response should indicate that the user is no longer authenticated.
    return res.send({ authenticated: req.isAuthenticated() });
  });
};

This works fine with express@4.10.1, express-session@1.9.1, passport@0.2.1, passport-local@1.0.0. I'm using the connect-pg-simple session store middleware with it, with no issues.

akashdeepshah commented 8 years ago

same here, I'm using "express": "^4.13.3","express-session": "^1.12.1", "passport": "^0.3.2","passport-local": "^1.0.0", "sequelize": "^3.14.1".

when logouts it shows req.user = null, but after page refresh it comes again, I think problem is in

passport.deserializeUser(function (id, done) { User.findById(id).then(function (user) { var usr = user.get({ plain: true }); done(null, usr); }) });

any help??

webus commented 8 years ago

Same issue here. Any update ? @akashdeepshah you have found the solution with function passport.deserializeUser ?

akashdeepshah commented 8 years ago

@webus , Previously I was using angular service to logout and it failed, and then I tried directly and it worked !!! hope in your case it works too..

omidgolparvar commented 8 years ago

Same issue here. Express 4 + Passport

How can I fix this? Should I downgrade Express from 4 to 3 ?!

omidgolparvar commented 8 years ago

Dears @akashdeepshah @webus @pensierinmusica @tybenz @mathiasm74 @phof @samuraiken @johnmastri @chriskrycho @MichhDiego It think I found a solution. Please check it out and share your results. I use Express v4.13.4 and Passport v0.3.2 in my project. When I want to checkout logged in user, I use req.isAuthenticated() and store it's returned value in a variable. for example:

var _LoggedIn = (req.isAuthenticated() ? true : false);

so, I don't use req.user anymore, and it works for me, and I hope it works for all of us. finally, if I did anything wrong, tell me please. thanks. :)

itsmichaeldiego commented 8 years ago

@omidgolparvar I am really thankful for your answer, but that project is dead! Haha, thanks again anyways.

assadtony commented 8 years ago

Any update on this? Still having this issue. I am using req.logOut() and req.session.destroy(). When I do, the session seems to be destroyed, but when I refresh it still has the old request Authentication and user.

I am using req.isAuthenticated() and it is still getting back a "true" after refresh.

Any recommendations?

itsmichaeldiego commented 8 years ago

@assadtony Did you check https://github.com/jaredhanson/passport/issues/246#issuecomment-207683415 ?

assadtony commented 8 years ago

Yeah, just figured it out, some other post helped me out with this solution, and it worked for me in conjunction with the comment mentioned. Thanks @MichhDiego ;)

app.use(session({ ... resave: false, .... }));

peacemakr commented 7 years ago

@omidgolparvar can you explain how that helps you logout from the session for user?

Ryca77 commented 7 years ago

@assadtony You referred to some other post helping you find the solution to this. Can you remember what that was? I'm having the same issue, it looks like the session is being destroyed but when I return to the login page the user is still authenticated.

sattha commented 7 years ago

this may help, http://stackoverflow.com/a/43429283/2077479

Harrisonkamau commented 7 years ago

I had the same problem too, but the following workaround solved it: // Express Session app.use(session({ secret: 'secret', saveUninitialized: false, resave: false, cookie: { maxAge: 1000 } })); Then on the logout route router.get('/logout', function (req, res) { req.logOut(); // remove all session data req.session = null; res.redirect('/login'); });

Cheers.

mschipperheyn commented 7 years ago

I have the same problem. session null or destroy solutions seems a bit ham fisted. If you just want to log out and keep other session variables, this doesn't work. Why doesn't req.logout() simply work? Seems like a bug to me. If there is a reason why logout couldn't work, it at least should throw a warning. The underlying problems seems to be that the relevant session variables aren't actually cleared.

kathleentully commented 7 years ago

So I am having these issues using Express 4.15.4, passport 0.4.0, passport-oauth2 1.4.0, cookie-session 1.3.1. Symptoms are the same as described above. I have:

app.get('/logout', (req, res) => {
  req.logout();
  res.redirect('/');
});

My / path is protected - if the user is not logged in, it will redirect to login. I added this line to my middleware router: console.log(req.path, ':', req.isAuthenticated()); In Chrome, I navigated to /, confirmed I was still logged in, then navigated to /logout. I ended up back at / logged in. But interestingly, this is how I got there:

/ : true
/logout : true
/ : false
/login : false
/login/callback : false
/ : true

So it looks like (at least in my case), it is logging me out, but when forwarded to passport.authenticate('oauth2'), I'm automatically logged back in without prompt. Any thoughts? Is this symptom the same for everyone else here?

mschipperheyn commented 7 years ago

Just logged a similar issue on express

ghost commented 7 years ago

@kathleentully Try creating an express session and use it to destroy the current_user: const expressSession = require('cookie-session');

// you can set the expiry date let expiryDate = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); // 7 days

const session = expressSession( { secret: 'very_secret', saveUninitialized: true, resave: false, cookie: { secureProxy: true, httpOnly: true, expires: expiryDate } });

app.use(session);

// Log out the user app.get('/logout', (req, res) => { req.logout(); req.session.destroy(); res.redirect('/'); })

Hope this works :)

aayush1408 commented 6 years ago

I am having trouble getting my system to log out with PassportJS. It seems the logout route is being called, but its not removing the session. I want it to return 401

ghost commented 6 years ago

This is still a problem it would seem... anyone have a solution yet?

kievo23 commented 6 years ago

Having the same problem here

ghost commented 6 years ago

@kievo23 Would you mind sharing a gist of the failing code?

boomkim commented 6 years ago

I had same issue so i just looked over the source code of the passport library. In passport/lib/sessionmanager.js , I found this code

SessionManager.prototype.logOut = function(req, cb) {
  if (req._passport && req._passport.session) {
    delete req._passport.session.user;
  } 
  cb && cb();
}

so i changed this code like this

SessionManager.prototype.logOut = function(req, cb) {
  if(req._passport){
    console.log(req._passport);
  }
   if (req._passport && req._passport.session) {
    delete req._passport.session.user;
  } else {
    delete req.session.passport;
  }
  cb && cb();
}

and below is what my console shows

{ instance: 
   Authenticator {
     _key: 'passport',
     _strategies: { session: [Object], local: [Object], facebook: [Object] },
     _serializers: [ [Function] ],
     _deserializers: [ [Function] ],
     _infoTransformers: [],
     _framework: 
      { initialize: [Function: initialize],
        authenticate: [Function: authenticate] },
     _userProperty: 'user',
     _sm: SessionManager { _key: 'passport', _serializeUser: [Function: bound ] },
     Authenticator: [Function: Authenticator],
     Passport: [Function: Authenticator],
     Strategy: { [Function: Strategy] Strategy: [Circular] },
     strategies: { SessionStrategy: [Object] } } }

I think it is because we have req._passport but req._passport.session. Although My version of logOut is working, library seems to be updated.

ghost commented 6 years ago

Thanks boomkim https://github.com/boomkim

On Mon, Jan 8, 2018 at 12:56 PM, boomkim notifications@github.com wrote:

I had same issue so i just looked over the source code of the passport library. In passport/lib/sessionmanager.js , I found this code

SessionManager.prototype.logOut = function(req, cb) { if (req._passport && req._passport.session) { delete req._passport.session.user; } cb && cb(); }

so i changed this code like this

SessionManager.prototype.logOut = function(req, cb) { if(req._passport){ console.log(req._passport); } if (req._passport && req._passport.session) { delete req._passport.session.user; } else { delete req.session.passport; } cb && cb(); }

and below is what my console shows { instance: Authenticator { _key: 'passport', _strategies: { session: [Object], local: [Object], facebook: [Object] }, _serializers: [ [Function] ], _deserializers: [ [Function] ], _infoTransformers: [], _framework: { initialize: [Function: initialize], authenticate: [Function: authenticate] }, _userProperty: 'user', _sm: SessionManager { _key: 'passport', _serializeUser: [Function: bound ] }, Authenticator: [Function: Authenticator], Passport: [Function: Authenticator], Strategy: { [Function: Strategy] Strategy: [Circular] }, strategies: { SessionStrategy: [Object] } } }

I think it is because we have req._passport but req._passport.session. Although My version of logOut is working, library seems to be updated.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jaredhanson/passport/issues/246#issuecomment-355911277, or mute the thread https://github.com/notifications/unsubscribe-auth/AelRYnGuTu1O0-00wvOpuBkLi_cWl4Ebks5tIdhMgaJpZM4B7_eT .

elipeters commented 6 years ago

@boomkim can you please explain what you have done?

As far as I can tell from your code you are saying that if req._passport && req._passport.session doesn't exist then delete the session and yet I'm gettting...

SessionManager.prototype.logOut = function(req, cb) {
  if (req._passport && req._passport.session) {
    console.log(req.session.passport.user); //prints User ID
    delete req._passport.session.user;
    console.log(req.session.passport.user); //prints "undefined"
  } 
  cb && cb();
}
boomkim commented 6 years ago

@elipeters As far as I know, for this version, since we don't have req._passport.session object, we cannot get into if clause to run 'delete req._passport.session.user;'

what I'm saying is this logout module is not perfect now so if you want to execute logout, you need to delete session by yourself. you can execute session delete by changing code like this if(req.session.passport){ delete req.session.passport; }

kievo23 commented 6 years ago

@kamauharrison it was actually a mistake in my route. its working well

kievo23 commented 6 years ago

req.logout(); req.session = null; Works for me

kievo23 commented 6 years ago

it actually worked on mine, the problem with mine was the order of my routes

On Wed, Jun 20, 2018 at 7:29 PM, Joshua McKenzie notifications@github.com wrote:

No fix for this yet?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jaredhanson/passport/issues/246#issuecomment-398813144, or mute the thread https://github.com/notifications/unsubscribe-auth/AM8Cq_sLcsvHMPOP4YTDHM0dA1iw_8_Aks5t-nhbgaJpZM4B7_eT .

--

Kelvin Chege The Web Developer

frederikhors commented 6 years ago

Dear people, I think I have the same doubts here: https://github.com/expressjs/cookie-session/issues/104#issuecomment-416234872

Reported from there:

Reproduction: https://acoustic-red.glitch.me. And also you can use Glitch's button to "Remix" or view source code.

You can see the navbar menu in the top of the page:

image

You can click on "Login" > Login button (form already compiled) > et voilà, you can see the cookies:

test and test.sig:

image

After you can use "Logout" menu link to logout and you can see the cookies already there, but different:

image

What I'm asked in this issue is if it is correct this behaviour: am I wrong?

This is the code for logout:

app.get('/logout', async (req, res) => {
  await req.logout();
  req.session = null;
  res.clearCookie("test")
  res.clearCookie("test.sig")
  return res.redirect('/')
})
shumiyao commented 4 years ago

Anyone here using "remember-me" strategy by any chance?

Just like most cases discussed above, the session data of my web app kept on coming back alive like zombie no matter how many times I log out, despite I observed in the console that the session data was certainly being deleted with req.logout() or setting null to the req.session.

In my case remember-me was causing the issue when a user is logged in with the remember me option enabled. Solution in my case was to delete remember me token saved in database when the user logs out.

dimahinev commented 3 years ago

Hi everyone Try to set cookie in your session with httpOnly: false, this way you can change or remove your connect.sid

app.use(
  session({
    ..., cookie: {
      path: '/',
      httpOnly: false,
      secure: false,
      maxAge: null,
    },
  })
)

app.get('/logout', function (req, res) {
  req.logout()
  res.send('Logout Success!!!')
})

It worked for me on localhost !! (on heroku not working)

tomasregina commented 3 years ago

I came across similiar issue. I forgot to put { withCredentials: true } on client when axios request is send, which caused unexpected sessions behavior. After that, logout is working right, but don't expect, that cookie will be deleted. Logout action just 'invalidate' the session in DB (user info will be empty)

sandesnp commented 3 years ago

The issue persists. Logout does seem to work but when I send a request back in it seems to deserialize the previous saved id in cookies.

satyamsovan123 commented 3 years ago

7 years, and here I'm crying for the issue to be resolved! Please, help.

ashwin2001-cloud commented 3 years ago

This works:

app.get('/logout', (req, res)=>{ res.clearCookie('Session_name'); res.redirect('/'); });

But, want to know why req.logout() is not deleteing session.

ddaniel27 commented 2 years ago

Seems like req.logout() doesn't remove the cookie in the frontend, this also happens with req.session.destroy() or res.clearCookie(), so what I came out is use a little JS snippet like this document.cookie = cookieName + '=; expires=Thu, 01 Jan 1970 00:00:01 GMT;' this works for me, I hope this can hope anyone else :)

kodie commented 2 years ago

I tried all of the solutions here and just couldn't get anything to work. No matter what I did, I could not get the user logged out unless I manually deleted the connect.gid cookie.

UNTILL

I changed my /logout route on the server-side from a POST to a DELETE and then it started working. 🤷

ricomasgu commented 2 years ago

For me was changing the HTTP method. I was experiencing errors on: router.post('/logout', (req, res) => { // logout the user using passport req.logout(); res.json({ message: 'Successful logout' }); })

I solved with: router.delete('/logout', (req, res) => { // logout the user using passport req.logout(); res.json({ message: 'Successful logout' }); })

GureSinghh commented 2 years ago

app.get('/logout',(req,res)=>{ req.logOut(()=>{ res.redirect('/login') });

})

tiromodibedi commented 2 years ago

On you main app.js file you should configure passport before the auth routes (login, logout). Like this:

app.use(passport.initialize()); // initialize passport
passport.use(new LocalStrategy(User.authenticate()));
passport.serializeUser(User.serializeUser()); // method supported by passport-local mongoose
passport.deserializeUser(User.deserializeUser()); // method supported by passport-local mongoose

app.use('/', indexRouter);
app.use('/access-control', accessControlRouter); // my auth routes

Before I had it the other way round and I kept getting the same error

tradengr commented 1 year ago

I tried all of the solutions here and just couldn't get anything to work. No matter what I did, I could not get the user logged out unless I manually deleted the connect.gid cookie.

UNTILL

I changed my /logout route on the server-side from a POST to a DELETE and then it started working. shrug

Worked for me! Thanks @kodie!

reigns29 commented 1 year ago

Yes , using delete instead of post request works here !! Thanks @kodie !