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

set sameSite: lax by default #277

Merged
merged 1 commit into from
Jan 26, 2024
Merged

set sameSite: lax by default #277

merged 1 commit into from
Jan 26, 2024

Conversation

gurgunday
Copy link
Member

@gurgunday gurgunday commented Jan 24, 2024

closes #268

There isn't a connection between secure and sameSite, so it doesn't make sense to override the user's setting if the connection is insecure

However, having 'lax' as the default option is useful since even though the absence of sameSite is interpreted as 'lax' by modern browsers, some user agents might not behave this way

Ref: https://web.dev/articles/samesite-cookies-explained#changes_to_the_default_behavior_without_samesite

@mcollina
Copy link
Member

The link to chrome blog is not working.

@Uzlopak @gurgunday what do you think the semversiness of this is? I'd err towards semver-major.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gurgunday
Copy link
Member Author

The link to chrome blog is not working.

Fixed

@mcollina mcollina added the semver-major Issue or PR that should land as semver major label Jan 25, 2024
@mcollina
Copy link
Member

After a skim through the article, I think this is semver-major.

@gurgunday gurgunday merged commit b3a2f4d into fastify:next Jan 26, 2024
19 checks passed
@gurgunday gurgunday deleted the same-site branch January 26, 2024 14:37
jsumners pushed a commit that referenced this pull request Jul 3, 2024
jsumners added a commit that referenced this pull request Jul 3, 2024
* set sameSite: lax by default (#277)

* update for Fastify v5 (#276)

* update for v5

* Update .github/workflows/ci.yml

Co-authored-by: Frazer Smith <[email protected]>
Signed-off-by: Gürgün Dayıoğlu <[email protected]>

* Revert "Update .github/workflows/ci.yml"

This reverts commit b7a3800.

* use replaceAll

* Revert "use replaceAll"

This reverts commit c691788.

---------

Signed-off-by: Gürgün Dayıoğlu <[email protected]>
Co-authored-by: Frazer Smith <[email protected]>

* set sameSite: lax by default (#277)

* update for Fastify v5 (#276)

* update for v5

* Update .github/workflows/ci.yml

Co-authored-by: Frazer Smith <[email protected]>
Signed-off-by: Gürgün Dayıoğlu <[email protected]>

* Revert "Update .github/workflows/ci.yml"

This reverts commit b7a3800.

* use replaceAll

* Revert "use replaceAll"

This reverts commit c691788.

---------

Signed-off-by: Gürgün Dayıoğlu <[email protected]>
Co-authored-by: Frazer Smith <[email protected]>

* update fastify deps

* Update .github/workflows/ci.yml

Co-authored-by: Gürgün Dayıoğlu <[email protected]>
Signed-off-by: Matteo Collina <[email protected]>

---------

Signed-off-by: Gürgün Dayıoğlu <[email protected]>
Signed-off-by: Matteo Collina <[email protected]>
Co-authored-by: Gürgün Dayıoğlu <[email protected]>
Co-authored-by: Frazer Smith <[email protected]>
Co-authored-by: Matteo Collina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major Issue or PR that should land as semver major
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants