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

fix(middleware/session): CookieSameSite default "Lax" #1638

merged 4 commits into from Dec 2, 2021


Copy link

@sixcolors sixcolors commented Dec 1, 2021 states that CookieSameSite default = false.

This is incorrect; default is "Lax"

TODO in session.go to change to "Strict" should be removed. Browser defaults are Lax. Fiber moving to default "Strict" is unadvisable as it will cause unexpected behavior. Strict requires HTTP first-party context and disregards requests initiated by third parties. So after navigating to the site from a third party site the safe HTTP methods (GET, HEAD, or OPTIONS) will not set the cookie. For example, a login page would not work by default when navigating to it from google or any other non-first party link.

"With Strict, the cookie is only sent to the site where it originated. Lax is similar, except that cookies are sent when the user navigates to the cookie's origin site. For example, by following a link from an external site. None specifies that cookies are sent on both originating and cross-site requests, but only in secure contexts (i.e., if SameSite=None then the Secure attribute must also be set). If no SameSite attribute is set, the cookie is treated as Lax."

The behaviour the team likely sought to prevent is SameSite=None, which blocks cookie set on cross-origin requests.

This PR closes #1638

@ReneWerner87 ReneWerner87 merged commit 693f3c5 into gofiber:master Dec 2, 2021
14 checks passed
@sixcolors sixcolors deleted the 1637-CookieSameSite-default branch Dec 2, 2021
Copy link

ReneWerner87 commented Dec 6, 2021

doc changed gofiber/docs@b67c531

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

Successfully merging this pull request may close these issues.

2 participants