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

Issue with “sameSite“ attribute #620

Closed
yaconnn opened this issue Apr 16, 2020 · 6 comments
Closed

Issue with “sameSite“ attribute #620

yaconnn opened this issue Apr 16, 2020 · 6 comments

Comments

@yaconnn
Copy link

yaconnn commented Apr 16, 2020

In Firefox there is a warning hint popping up in the console constantly, stating that there would be an issue with the sameSite-Attribute: "Some cookies are misusing the recommended “sameSite“ attribute"

As far as I understood it, for future browser-versions it is required to define the Cookies' secure- and sameSite-Attributes. While this is easy to do, the problem is that this doesn't necessarily make the warning disappear. Whenever there is a Cookies.remove('name')-Function defined on the site, this would cause this error message to appear already. Even if the settings are defined correctly in the Cookies.set()-Function or if there isn't even any function defined to create a cookie at all.

Is there a solution, so the remove-Function wouldn't cause this error message anymore in FF?

@carhartl
Copy link
Member

Whenever there is a Cookies.remove('name')-Function defined on the site, this would cause this error message to appear already.

I can't reproduce this (Firefox 75.0 on macOS Catalina).

Screenshot 2020-04-22 at 17 55 18

Where do you see such a warning, and which version/OS of Firefox are you seeing this in? Can you provide a screenshot?

I suspect this warning comes from a different cookie..

@yaconnn
Copy link
Author

yaconnn commented Apr 23, 2020

The message appeared in Firefox 76.0b7 (Developer Edition) on Windows 10. But during the reproduction of the error, I just found out that this only appears to be an issue in this Developer Edition. I also reproduced the error on a macOS virtual machine, with macOS Mojave. There the warning also appears solely in the Developer Edition of Firefox but not in any other browser that I tested. That being said, it seems to be a very specific and in that sense currently not that important issue. I still created a fiddle to reproduce it (https://jsfiddle.net/4es6Lpka/). Attached are two screenshots from that fiddle which show the actual warning message (setting the Cookie before had no effect).

2020-04-23 03_05_43-Microsoft Edge

2020-04-23 03_27_49-Microsoft Edge

@carhartl
Copy link
Member

carhartl commented Apr 23, 2020

Thanks!

Ok, so, technically, removing a cookie is writing one (with an expiration date in the past). We’re currently not setting SameSite by default, so it defaults to “none”, and newer browsers start to require such a cookie to also have the Secure attribute. Hence the warning.

I could imagine that browsers will become smarter in that they don’t issue a warning for a cookie that just expired (I think they should).

@yaconnn Btw, in one of the screenshots, you’re using the api wrong — all cookie attributes go into a single object as argument.

So it’s not Cookies.set('foo', 'bar', { secure: true }, { sameSite: 'lax' }) but Cookies.set('foo', 'bar', { secure: true, sameSite: 'lax' }). In the former, the last argument is being ignored and you're creating a cookie without SameSite again.

@carhartl
Copy link
Member

carhartl commented Apr 23, 2020

One solution would be to specify sameSite: 'lax' in the remove() function - it doesn't really matter because the cookie is expired anyway. But that only works under the assumption that sameSite will not affect scope of the cookie similar to domain or path (otherwise we'd not be correctly deleting the cookie in question). I haven't tested this so far, but will do.

Another solution might be to make sameSite: 'lax' a default overall going forward (which might be a reasonable default anyway, but could also cause surprises).

@carhartl
Copy link
Member

Adding sameSite: 'lax' to the attributes in the remove function leaves removing intact and eliminates the warning.

@yaconnn
Copy link
Author

yaconnn commented Apr 23, 2020

Perfect, thanks for putting efforts into solving the issue and clearing me up about the correct usage of the API.

I just tried out your solution and it does solve it! :)

@yaconnn yaconnn closed this as completed Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants