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

Cookies and sessions get overwritten when session not detected (after redirect, or ad hoc) #223

Open
LordPraslea opened this issue Nov 5, 2024 · 2 comments

Comments

@LordPraslea
Copy link

I'm currently facing a very disturbing error which I was able to replicate on multiple instances and which all lead to a bug in the session manager.

Whenever the cookie is set to SameSiteStrictMode then a new session gets created instead of reusing the old one.

Example: A user is doing an online purchase and from the checkout, gets redireced to stripe,paypal, whatever, then upon payment or cancellation (s)he gets redirected back to a success or cancellation page. But instead of showing that page, the user's greeted with the login page (due to the token not working)

This also occurs out of the blue on many occasions when the user simply refreshes the page without a logical explanation. Debugging the LoadAndSave function did indeed show that a new cookie/token was regenerated.

	sessionManager := scs.New()
	sessionManager.Cookie.SameSite = http.SameSiteStrictMode
	sessionManager.Cookie.Name = "my_session"
	sessionManager.Cookie.Secure = true

The bug is replicated with both badgerstore and sqlitestore

I've experimented for hours on different days to figure out the best settings.

Setting SameSite to LAX seems to temporarily mend this issue.

The problem would be forgiven since samesite strict disallows sending cookies, then a simple refresh would normally work and the user would still be logged in. However, if the token/session is rewritten then it's a new session so it creates friction and purely refreshing does not work anymore.

And yes, I'm redirecting to the same domain.

@jum
Copy link
Contributor

jum commented Nov 8, 2024

This is basically part of same site strict mode means, your cookies will not cross site boundaries when using third parties like payment services or oauth flows. The easy fix is using lax mode. I fixed the issue by remembering my cookie state behind the code token that is send back in oauth flows and merging the session values into the new session. May be the payment flows have a similar feature?

@LordPraslea
Copy link
Author

This is basically part of same site strict mode means, your cookies will not cross site boundaries when using third parties like payment services or oauth flows. The easy fix is using lax mode. I fixed the issue by remembering my cookie state behind the code token that is send back in oauth flows and merging the session values into the new session. May be the payment flows have a similar feature?

  1. That the cookie is not sent in Strict Mode is true, when I'd refresh, then I'd expect the existing cookie/session to be passed AND to be logged in again, currently, a new session is issued which breaks the existing application. I'd imagine many scenarios where this breaks, including when someone clicks on a link they've received via email or simply sharing an link to view a specific page.
  2. Such a token/session could be useful, however they kind of introduce the extra security hassle and bugs associated with such implementations, yet they don't not fix point 1.

All I can think about are hacky solutions (such as an extra lax cookie as to NOT reissue a session). Or a way to set the cookie to lax before sending the user to a payment processor and then back to strict after the payment is finalized/cancelled.

Indeed, lax mode is what I've used and seems to be the true solution for the problems outlined in point 1. and above with new sessions being issued.

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

No branches or pull requests

2 participants