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

Some cookies are misusing the “SameSite“ attribute, so it won’t work as expected #762

Closed
thomasjsn opened this issue Jan 20, 2022 · 7 comments
Labels
question Something needs clarification. support Someone asking for support -> Should be moved to GitHub Discussions

Comments

@thomasjsn
Copy link

It doesn't look like the SameSite attribute is set on the cookies by Isso. I'm serving Isso on the same domain as the site, but do get the following warnings in Firefox debugger:

Cookie “22” has “SameSite” policy set to “Lax” because it is missing a “SameSite” attribute, and “SameSite=Lax” is the default value for this attribute. new
Cookie “isso-22” has “SameSite” policy set to “Lax” because it is missing a “SameSite” attribute, and “SameSite=Lax” is the default value for this attribute. embed.min.js:19:1600
Cookie “22” has “SameSite” policy set to “Lax” because it is missing a “SameSite” attribute, and “SameSite=Lax” is the default value for this attribute. new

Looking at the response headers:

{
  "name": "set-cookie",
  "value": "22=WzIyLCJkZmQ4YWViMzk2ZjYyNGE1N2VmYWJjMDJjOWRjMzkyZjhhNTVmNDBmIl0.Yeip9A.E6Rrzx8L5Kq8Sqp7M4_EED-Y70M; Expires=Thu, 20 Jan 2022 00:31:52 GMT; Max-Age=900; Path=/"
},
{
  "name": "x-set-cookie",
  "value": "isso-22=WzIyLCJkZmQ4YWViMzk2ZjYyNGE1N2VmYWJjMDJjOWRjMzkyZjhhNTVmNDBmIl0.Yeip9A.E6Rrzx8L5Kq8Sqp7M4_EED-Y70M; Expires=Thu, 20 Jan 2022 00:31:52 GMT; Max-Age=900; Path=/"
}

I've tried setting samesite = Strict in the config, but no change.

Suggestions?

@thomasjsn
Copy link
Author

Also, why both Set-Cookie and X-Set-Cookie?

@thomasjsn
Copy link
Author

thomasjsn commented Jan 20, 2022

I might add that this is not causing me problems (because I use the same domain), only warnings. But I suspect it will be problematic if Isso is served from another domain.

@thomasjsn
Copy link
Author

Confirm that if I append the SameSite property to the Set-Cookie header in my Nginx config, the warning for Set-Cookie goes away:

proxy_cookie_path / "/; Secure; SameSite=strict";

But this is not so easy to do for the X-Set-Cookie header.

@thomasjsn
Copy link
Author

thomasjsn commented Jan 20, 2022

Oh, I'm using the 0.12.5 release. Which doesn't include be538a2

Update: I can confirm that this commit does resolve the issue 👍

Any plans for a release which contains the latest changes, available in pip?

Sorry for the monologue :p

@ix5
Copy link
Member

ix5 commented Jan 29, 2022

Any plans for a release which contains the latest changes, available in pip?

That's up to @jelmer or @blatinier , they have access to pypi.

But I'm sure that something can be arranged. The maintainers (including me) are a bit hesitant to merge things or release new versions because we find it difficult to properly test changes, especially the front-end part.
See #755 and #754

So anyone stepping up and saying "yes, I'm willing to test new versions before they're released", and not just for their own setup (i.e. "yeah, my issue got fixed, kthxbye"), would be highly appreciated ;)

@ix5 ix5 closed this as completed Jan 29, 2022
@ix5 ix5 added question Something needs clarification. support Someone asking for support -> Should be moved to GitHub Discussions labels Jan 29, 2022
@Fryboyter
Copy link
Contributor

So anyone stepping up and saying "yes, I'm willing to test new versions before they're released", and not just for their own setup (i.e. "yeah, my issue got fixed, kthxbye"), would be highly appreciated ;)

How would that work out roughly? In principle, I would have no problem testing something at weekends or during the week when I have time. However, my knowledge of programming is very limited. Or rather, almost non-existent.

@ix5
Copy link
Member

ix5 commented Feb 4, 2022

How would that work out roughly? In principle, I would have no problem testing something at weekends or during the week when I have time. However, my knowledge of programming is very limited. Or rather, almost non-existent.

For this specific change, it'd be great to have someone test different browsers and browser versions to ensure cookies are set correctly. Especially interesting would be whether Isso is still compatible with tracking restrictions in privacy-focused browsers (or using add-ons such as uBlock Origin).

For testing things in general, having someone run the master branch or a checkout of a pending PR and just confirming that it doesn't break their setup would already be a great help. A typical test would include commenting on their (local test) site, commenting from a different browser, using the admin functionality, clearing cookies, ...


We really ought to compile a list of all the things that could be tested, even though it wouldn't necessarily be required to run all of them on every change.

Where things could get hairy would be complex setups like Sub-URI or Multiple Sites as specified in the docs.

In the long run, it'd be great to have more powerful testing tools and an easier way to test different setups, e.g. a docker-compose file which spins up isso + nginx instances with different configs automatically, and a way to run and instrument tests in that type of setup.

If someone happens to have experience in this regard or could point to a good example from another comparable(!) project, they're welcome to suggest that here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Something needs clarification. support Someone asking for support -> Should be moved to GitHub Discussions
Projects
None yet
Development

No branches or pull requests

3 participants