clerk / javascript

Official Javascript repository for Clerk authentication
https://clerk.com
MIT License
958 stars 212 forks source link

fix(clerk-js): Set localhost SameSite=None cookies as Secure #3604

Closed anagstef closed 5 hours ago

anagstef commented 2 weeks ago

Description

This PR removes the specific fix we did a while ago for Cypress. The real reason our cookies where getting deleted in Cypress Chrome is because Chrome requires Secure when setting SameSite=None cookies.

Safari needs a bit of a different handling because it does not consider localhost to be a secure context, so in Safari localhost cookies cannot have the Secure attribute.

So the new logic about the Secure attribute on cookies goes as follows:

Checklist

Type of change

changeset-bot[bot] commented 2 weeks ago

🦋 Changeset detected

Latest commit: 802fd8f897d2b0311f70fdf696146485b910de6c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | ----------------------- | ----- | | @clerk/clerk-js | Patch | | @clerk/astro | Patch | | @clerk/chrome-extension | Patch | | @clerk/clerk-expo | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

anagstef commented 2 weeks ago

!snapshot

clerk-cookie commented 2 weeks ago
Hey @anagstef - the snapshot version command generated the following package versions: Package Version
@clerk/chrome-extension 1.1.0-snapshot.v1f5b5e4
@clerk/clerk-js 5.7.2-snapshot.v1f5b5e4
@clerk/elements 0.9.0-snapshot.v1f5b5e4
@clerk/clerk-expo 1.2.3-snapshot.v1f5b5e4
gatsby-plugin-clerk 5.0.0-beta.45
@clerk/localizations 2.4.6-snapshot.v1f5b5e4
@clerk/ui 0.1.3-snapshot.v1f5b5e4

Tip: Use the snippet copy button below to quickly install the required packages. @clerk/chrome-extension

npm i @clerk/chrome-extension@1.1.0-snapshot.v1f5b5e4 --save-exact

@clerk/clerk-js

npm i @clerk/clerk-js@5.7.2-snapshot.v1f5b5e4 --save-exact

@clerk/elements

npm i @clerk/elements@0.9.0-snapshot.v1f5b5e4 --save-exact

@clerk/clerk-expo

npm i @clerk/clerk-expo@1.2.3-snapshot.v1f5b5e4 --save-exact

gatsby-plugin-clerk

npm i gatsby-plugin-clerk@5.0.0-beta.45 --save-exact

@clerk/localizations

npm i @clerk/localizations@2.4.6-snapshot.v1f5b5e4 --save-exact

@clerk/ui

npm i @clerk/ui@0.1.3-snapshot.v1f5b5e4 --save-exact
panteliselef commented 2 weeks ago

So it seems this PR reverts #3245 and proposes another solution

anagstef commented 2 weeks ago

@panteliselef Exactly! This PR removes the hacky Cypress fix and proposes a more holistic solution for Secure when on localhost, which also fixes the Cypress issues, even if the cookies are set with SameSite=None.