Correct logOut using Passport. ClearCookie doesn't delete cookies.

[frederikhors]

I'm using PassportJS and this code for logout:

  .get("/logout", async (req, res) => {
    await req.logout();
    req.session = null;
    await res.clearCookie(process.env.PROJECT_TITLE.toLowerCase());
    await res.clearCookie(`${process.env.PROJECT_TITLE.toLowerCase()}.sig`);
    return res.redirect("/");
  });

It just changes the cookies but don't delete them. Why?

It does delete them if I use just this code:

  .get("/logout", async (req, res) => {
    await res.clearCookie(process.env.PROJECT_TITLE.toLowerCase());
    await res.clearCookie(`${process.env.PROJECT_TITLE.toLowerCase()}.sig`);
    return res.redirect("/");
  });

Where am I wrong?

[dougwilson]

The only difference is that req.logout is no longer there. I have no idea what that code does, so not sure why removing it would affect the cookie.

[frederikhors]

I think req.logout() is from Passport to remove it's info from session (cookie).

But also if I just use req.session = null; it doesn't delete cookies.

Why?

[dougwilson]

I'm not sure. I can investigate the issue, though. Please provide all the following so I can reproduce the issue:

1. Version of Node.js, this module, and all other involved modules
2. Complete source code I can copy and paste and run without modification.
3. Complete instructions on how to reproduce the issue after running the above code.

Thanks!

[mariusa]

Same here. The steps here didn't help:
jaredhanson/passport#246

   req.logout()
    req.session = null //doesn't remove the session, on next page load it's present again with the old data.
    res.json({})

Using

        "passport": "0.4.x",
        "passport-local": "1.x.x",

[dougwilson]

I can investigate but need to be able to reproduce the issue. Please provide the three pieces from #104 (comment) so I can take a look :)

[aegyed91]

this is the workaround i found as of now

do not call req.logout, use req.clearCookie

// config.js
module.exports = {
  baseCookieOptions: {
    name: 'app.session',
    httpOnly: true,
    signed: true,
    secret: 'secret'
  }
}

// init-cookie.js
module.exports = (app) => {
  app.use(cookieParser())
  app.use(
    cookieSession({
      ...baseCookieOptions,
      secure: isProd,
      maxAge: 30 * 24 * 60 * 60 * 1000 // 30 days
    })
  )
}

// auth-route.js
router.get('logout', (req, res, next) => {
  // manually set cookie headers, cause `req.logout` does not work
  // req.logout()
  req.secret = baseCookieOptions.secret
  res.clearCookie('app.session', baseCookieOptions)
  res.clearCookie('app.session.sig', baseCookieOptions)

  res.json({
    status: 'ok'
  })
})

Version of Node.js, this module, and all other involved modules:

"cookie-parser": "^1.4.3",
"cookie-session": "^2.0.0-beta.3",
"express": "~4.16.3",
"passport": "^0.4.0",
"passport-local": "^1.0.0"

$ node -v
v10.8.0
$ npm -v
6.2.0

[dougwilson]

I'm just going to close this for now since it's been three posts yet it doesn't seem like anyone is interested in actually providing the info I need to investigate. You're welcome to make a pull request with a fix though, if you cannot provide code I can investigate 👍

[frederikhors]

@dougwilson sorry for the delay.

What you asked:

1. NodeJS 10 (but same problem with 8, I don't think it matters now); other versions in package.json;
2. You can find here what you need: https://acoustic-red.glitch.me. And also you can use Glitch's button to "Remix" or view source code:

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

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

test and test.sig:

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

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('/')
})

[dougwilson]

You cannot use both req.session = null to clear the session and res.clearCookie, as they end up conflicting (this module will override your clear commands because it think you want the req.session = null behavior. If you want the cookies completely gone on log out, rather than just being set to an empty session, your the following:

app.get('/logout', async (req, res) => {
  await req.logout();
  res.clearCookie("test", {path:"/",httpOnly:true})
  res.clearCookie("test.sig", {path:"/",httpOnly:true})
  return res.redirect('/')
})

[frederikhors]

I updated the code on https://acoustic-red.glitch.me.

As you can see it doesn't work.

Actual code:

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

[dougwilson]

Use the code I posted above.

[frederikhors]

@dougwilson thanks.

The difference is just this: {path:"/",httpOnly:true}?

Why If I use this instead it works deleting all cookies?

app.get('/logout', async (req, res) => {
  res.clearCookie("test")
  res.clearCookie("test.sig")
  return res.redirect('/')
})

[frederikhors]

@dougwilson I tried, as you can see.

It doesn't work also with your code:

app.get('/logout', async (req, res) => {
  await req.logout();
  res.clearCookie("test", {path:"/",httpOnly:true})
  res.clearCookie("test.sig", {path:"/",httpOnly:true})
  return res.redirect('/')
})

Cookies after "Logout" are still there.

https://acoustic-red.glitch.me

[dougwilson]

I just tried https://acoustic-red.glitch.me/ in Google Chrome and it worked just fine. Both test and tes.sig were there after login. The going through Logout they were gone. I'm looking in Inspector > Application > Cookies > https://acoustic-red.glitch.me

[dougwilson]

Screen shot after login:

Screen shot after logout:

[frederikhors]

@dougwilson I'm sorry because it seems I'm crazy. My Chrome still doesn't work. And also another PC.

After "Login" (redirect on "private" page):

After "Logout" (redirect on Home Page):

So the cookies are still there but are different!

Now if you click again on "Logout" the cookies are gone!

[dougwilson]

I still cannot replicate on the site you provided. I tried a different machine with the same result I posted above.

[frederikhors]

@tsm91, can you try what I say here: #104 (comment)

@dougwilson I tried another PC (NOT A MY PC!) right now and is the same (Chrome and Firefox).

How is it possible?!

[dougwilson]

Perhaps we are not doing exactly the same clicks / interactions on the site and that is causing the difference. Can you write down, click by click, exactly what you do on the site? Also, let's start a new Incognito session as well to make sure we have the same state too.

[frederikhors]

@dougwilson OK. Incognito mode always.

1. click "Login" link
2. login with form, it redirects on "/private" and create cookies
3. click "Logout" link, it redirects on Home Page ("/") and cookies are still there
4. the cookies are still there but are different than before!
5. now if you click again on "Logout" the cookies are gone!

[aegyed91]

@federomero i checked the demo page and followed the steps.
I experience the same behaviour.
The first time i logout it will change the cookie contents (maybe passport implicitly creates an anonymous session?)
The second time i click logout, the cookies are gone.

Note: after i logout the first time and the cookie contents are changed, i am no longer able to visit the private page. Which is a good.

[frederikhors]

@tsm91 yes, it's good but the cookies are still there.

[frederikhors]

@dougwilson, any hint on this? I'm not the only one.

[dougwilson]

I'm not sure. I haven't retried yet. And the module source is open and free for modification. Don't let me hold you up, you're always welcome to fix the issue in the source and can make a pull request to contribute it back.

[frederikhors]

In this (#100) thread @aliencorp suggests:

Add res.end(); after req.session = null;

Does this make sense?

[frederikhors]

I'm not sure. I haven't retried yet. And the module source is open and free for modification. Don't let me hold you up, you're always welcome to fix the issue in the source and can make a pull request to contribute it back.

I would really like to be able to create PR that solve the problem. If I had known how to do it I would have already done it.

[dougwilson]

Then just wait until I have some time to take another look.

[frederikhors]

@dougwilson perfect! (Perhaps reopening the issue could help you remember the todo.)

[dougwilson]

I haven't forgotten. I don't look through evey repo every day to determine what I need to do. I have a todo list. I can reopen this if it will make you happy but won't make any difference for when I can get to it.

[dougwilson]

So I'm taking a look and it seems to be some kind of conflict between passport and this module. I'm not very familiar with passport, but I'm trying to dig in. Is there a way to attach a live debugger to get a break point in the server on that glitch.com website?

[dougwilson]

Basically during the logout route, something is doing req.session.passport = { user: 1 } and that is getting set in the cookie. This module sets the cookie at the last minute if you don't explicitly do req.session.save(), which is why it still sets after you attempt to clear the cookie (your clear commands are not provided by this module and this module has no idea you're trying to clear the cookies).

The req.session = null does "clear" them, in the sense that the value of the session is cleared, even if there is still a cookie set.

[dougwilson]

The "remix" I made here: https://pale-bathtub.glitch.me/ seems to behave as you're expecting.

[frederikhors]

@dougwilson your remix is just:

await req.logout(); >> to >> //await req.logout();, right?

I think await req.logout(); should work and after that AWAIT it should does NOTHING!

Isn't it?

Is secure to just use res.clearCookie() and not also a req.logout? What do you think about?

Thanks for your effort on these problems.

[dougwilson]

Hi @frederikhors

your remix is just: await req.logout(); >> to >> //await req.logout();, right?

correct, that was the only change in my remix.

I think await req.logout(); should work and after that AWAIT it should does NOTHING!

The issue I'm seeing is that req.logout is altering the session, which is why the session is getting updated in your logout request.

I think await req.logout(); should work and after that AWAIT it should does NOTHING!

I'm not very familiar with passport. Maybe can you explain exact what req.logout is doing apart from altering req.session? We may be able to determine this by understanding the specifics of what req.logout does.

But what I found is that the cookie is getting set on your logout because of the following:

(1) req.logout alters the req.session object, so a need to set the cookie is noted by this module
(2) the code calls clearcookie, which has nothing to do with this module and this module has no idea your code did that. clearing a cookie is just setting a cookie with an expiration date in the past
(3) the response ends and this module sees that (a) the req.session object was changed, thus it knows it needs to set the new value and (b) req.session.save() hasn't been called, so it will automatically save the changes for you

So it seems like you have one of two options:

(a) don't touch the req.session if you don't want a new value to be saved in the cookie (this is why I commented out req.logout()

OR

(b) call req.session.save() to explicitly save the changes to the session that req.logout() made and then do the clear cookie calls.

I hope that helps 👍

[dougwilson]

This is an example remix that will do the req.logout() and clear the cookie in your logout route: https://glitch.com/edit/#!/tiny-chinchilla

[frederikhors]

@dougwilson I love you. Thanks a lot for your commitment!

I found this about req.logout(): http://www.passportjs.org/docs/logout

Specifically:

Invoking logout() will remove the req.user property and clear the login session (if any).

So with just res.clearCookie() I don't delete req.user so now I'm using what you suggested:

await req.logout()
req.session = null
req.sessionOptions.maxAge = 0
return res.redirect('/')

It works very well.

Am I wrong in something?

[dougwilson]

So if all req.logout does is clear the res.session.user (which is why the module is trying to set the cookie as this is a modification to req.session), then that is pointless as that object is stored only in the cookie. This means that just clearing the cookie already does that and the req.logout is redundant.

[frederikhors]

It refers to req.user not res.session.user. I think this is not the same, right?

[dougwilson]

Ah. I have no idea. Where does req.user come from?

[frederikhors]

I think from Passport. But I think it's okay to have both await req.logout() and req.session = null. Because also in the future it can does more than today. And I still null the session and delete cookie with maxAge. I'm all right. :)