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

req.session.destroy() does not remove cookies with the latest version of NextJS when hosting on Vercel #274

Closed
dpyzo0o opened this issue Dec 9, 2020 · 17 comments
Labels

Comments

@dpyzo0o
Copy link

dpyzo0o commented Dec 9, 2020

Hi, I have come across a very annoying bug recently. If you upgrade nextjs to the latest version, which is 10.0.3, and deploy the application on Vercel, the method req.session.destroy() does not remove the cookies.

Here is a repo to reproduce this issue, it's just a redeployment of the next-icon-session's nextjs example but with nextjs upgraded to 10.0.3.

Steps to reproduce:

  1. login
  2. logout (first time it will succeed with a 200 return)
  3. login again
  4. logout (this time it will fail to remove the cookie with a 304 return)

next-iron-session's nextjs example:

1607513150016

after upgrading nextjs to 10.0.3:

1607513145304

Additional information: This issue only happens when deploying on vercel, it works correctly when I run it locally.


Edit: After downgrading nextjs to 10.0.0, it works correcly on vercel.

@vvo
Copy link
Owner

vvo commented Dec 9, 2020

Thanks for the detailed bug report and first analysis.
The failing version is 10.0.2. Now we need to:

  • understand if they fixed a bug, which in turn made our code buggy (and we should update our code)
  • or if they introduced a bug (and we need to find it, and they/we need to fix it)
  • find/identity the right change here: https://github.com/vercel/next.js/releases/tag/v10.0.2
  • or maybe the change in on the Vercel platform itself

I am doing some tests right now, let me know how it goes on your side

@vvo
Copy link
Owner

vvo commented Dec 9, 2020

More info on the issue: this is because of the caching mechanism. Now why has it changed on the latest Next.js versions: I don't know and I think you should write to the Vercel support to know more.

A possible workaround for now would be to manually set caching headers (informing not to cache) on all your API routes that are setting up cookies like login and logout.

@dpyzo0o
Copy link
Author

dpyzo0o commented Dec 9, 2020

More info on the issue: this is because of the caching mechanism. Now why has it changed on the latest Next.js versions: I don't know and I think you should write to the Vercel support to know more.

A possible workaround for now would be to manually set caching headers (informing not to cache) on all your API routes that are setting up cookies like login and logout.

I have tried to manually set the headers res.setHeader('cache-control', 'public, max-age=0, must-revalidate') and it does not seem to work. I'm not an expert on the http caching so I don't know if I'm doing the right way...

@dpyzo0o
Copy link
Author

dpyzo0o commented Dec 9, 2020

This might be the related change https://github.com/vercel/next.js/pull/18986/files

@vvo
Copy link
Owner

vvo commented Dec 9, 2020

Hey there @dpyzo0o there's definitely something strange that changed between Vercel/Next.js, in the meantime you can set res.setHeader("cache-control", "no-store, max-age=0"); on your logout route and that should do it, even on Vercel.

Let me know!

@dpyzo0o
Copy link
Author

dpyzo0o commented Dec 9, 2020

@vvo Thanks, it works.

@vvo
Copy link
Owner

vvo commented Dec 9, 2020

@dpyzo0o I just updated the next-iron-session repository and now recommend another solution: just make sure to call any route that uses destroy via a POST request. Most proxies and browsers (100%?) will never cache POST requests unless badly or weirdly configured.

The two solutions have the same effect, but using POST for logout is more common practice I think.

Thanks!

@vvo vvo closed this as completed Dec 9, 2020
vvo added a commit that referenced this issue Dec 9, 2020
Before this commit, our examples were showcasing the use of GET /logout. And
/logout would then session.destroy(). But GET requests can be cached (cdns,
browsers) which makes logout sometimes fails. There are multiple ways to solve
this but ultimately logout routes should be POST requests, this is a common way
to solve this.

fixes #274

Also upgraded most deps
@vvo
Copy link
Owner

vvo commented Feb 10, 2021

🎉 This issue has been resolved in version 4.1.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

@vvo vvo added the released label Feb 10, 2021
@Deivaras
Copy link

🎉 This issue has been resolved in version 4.1.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

Everything works on:
Google Chrome Version 88.0.4324.150 (Official Build) (64-bit) with
"next": "^10.0.6",
"next-iron-session": "^4.1.11"
on development and production on Vercel. Also sometimes it takes two clicks to login.

Does not work on Vercel with:
Firefox 85.0.2 (64-bit);
Firefox developer edition 86.0b8 (64-bit);

@vvo
Copy link
Owner

vvo commented Feb 11, 2021

hey @Deivaras I believe this notification was long due sorry about that.

Are you using POST requests for logout routes now? I do have that double clicks to logout issue yup, did not investigate (not login though, login is always OK).

@Deivaras
Copy link

Deivaras commented Feb 12, 2021

hey @Deivaras I believe this notification was long due sorry about that.

Are you using POST requests for logout routes now? I do have that double clicks to logout issue yup, did not investigate (not login though, login is always OK).

Yes, I'm doing POST request on logout fetch and also on Chrome sometimes when trying to login (after first click) I see the cookie and then you have to click login the second time, while cookie being replaced and only then you got logged in.
`

} onClick={async (e) => { e.preventDefault(); await mutateUser(fetchJson('/api/logout'), { method: 'POST' }); }} > Logout `

@vvo
Copy link
Owner

vvo commented Feb 16, 2021

Hey there, I believe this is now fixed, have a look at the updated example: 7ffc8bb

I cannot reproduce this bad behavior anymore.

@elie222
Copy link

elie222 commented Jul 27, 2022

Facing this issue now.

import { NextApiRequest, NextApiResponse } from 'next';
import { withSessionRoute } from '@utils/iron-router';

const handler = async (req: NextApiRequest, res: NextApiResponse) => {
    req.session.destroy();
    res.send({ ok: true });
};

export default withSessionRoute(handler);

We receive ok response but the session isn't destroyed when we try another api route straight after. Locally this works fine. On Vercel it doesn't for some reason.

@elie222
Copy link

elie222 commented Jul 28, 2022

Changing my endpoint to POST seems to have fixed it now

@garryshield
Copy link

work out with this

// ...
await new Promise<void>((resolve, reject) => {
      req.session.destroy((err) => {
        if (err) {
          reject(err)
        } else {
          res.clearCookie('ACCESS_TOKEN', {
            domain: '.xxx.com'
          })
          res.clearCookie('REFRESH_TOKEN', {
            domain: '.xxx.com'
          })
          res.clearCookie('connect.sid', {
            domain: '.xxx.com'
          })

          resolve()
        }
      })
    })
// ...

@behzad-ahmadi
Copy link

behzad-ahmadi commented Mar 6, 2023

The problem is not solved yet?!

UPDATE:
the request should be POST to solve the problem.

@johnsmera
Copy link

Version 13:
After 3 hours, changing the request to POST solved the problem. Shouldn't this have been more explicit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants