@beger opened this Pull Request on October 1st 2020

With this change you can now set up your JS tracking code like this:

_paq.push(['setCookieSameSite', 'None']);

This will set the SameSite attribute for all cookies set by Matomo to the value used above. The default is still SameSite='Lax' if setCookieSameSite is not used.

Implements the enhancement discussed in https://github.com/matomo-org/matomo/issues/16161.

@tsteur commented on October 1st 2020 Member

@beger thanks for creating this PR! I was wondering if we could simplify it and set automatically SameSite=None when isSecure is set? I'm suggesting this because None only works when also Secure is set see eg https://web.dev/samesite-cookies-explained/#changes-to-the-default-behavior-without-samesite AFAIK

@beger commented on October 1st 2020

@tsteur I think this may not be the right solution for every use case: you still may want to enforce a different SameSite policy even if your connection is secure. But I think it probably would make sense to do it the other way around and set isSecure to true automatically if SameSite is set to None. What do you think?

@tsteur commented on October 1st 2020 Member

@beger that could work. And ideally it was to throw an error and/or log a message if this is called on HTTP ?

@tsteur commented on October 26th 2020 Member

@beger be great if you could look at my last comment and let us know if you're still planning to work on this PR?

@beger commented on October 27th 2020

Hello @tsteur,
yes I'm still planning to implement this and also include your suggestion. Due to high workload on my side I'll probably get to it sometime during November.

@fealXX commented on October 28th 2020

Hi @beger, we're currently trying to use the code of your PR as a fix until a release happens and found a Problem:
If you're in a scenario where your tracking-domain is my.matomo.org, you're working on othersite.org and set your Cookiedomain to .matomo.org isPossibleToSetCookieOnDomain will fail. We need to set configCookieIsSecure and configCookieSameSite in line 4349 too.
function isPossibleToSetCookieOnDomain(domainToTest) { var valueToSet = 'testvalue'; setCookie('test', valueToSet, 10000, null, domainToTest, configCookieIsSecure, configCookieSameSite); [...]

Which leads me to another issue: This check depends on the order of operations of setCookieDomain, setSecureCookie and setCookieSameSite. If you first set the CookieDomain, then the SameSite/Secure attributes, the check fails. If you first set Secure and Samesite, the Check works fine. Maybe the cookie configuration should be moved into a single paq-Setting, along the lines of this, while keeping the old syntax for backwards compatibility:
_paq.push(['setCookieConfig', 'my.matomo.org', 'None', true]);

I'd be happy to help implementing this, if possible.

@tsteur commented on November 4th 2020 Member

fyi if you are using one tracker using and init this trackker using _paq.push() then Matomo should make sure to call the methods in the right order as the order is defined in var applyFirst =.

To make it fully independent would probably require quite a few more changes as it is already a general problem in Matomo which could therefore also be fixed in a separate PR later. We should document it though at least that eg setCookiePath should be called before setCookieDomain if the tracker is being initialised manually.

Adding it to isPossibleToSetCookieOnDomain be indeed good and respect settings there too.

@beger commented on November 17th 2020

Thank you @fealXX and @tsteur for implementing and reviewing this in https://github.com/matomo-org/matomo/pull/16724. I think this PR can now be closed.

@tsteur commented on November 17th 2020 Member

Thanks for getting this work started @beger and contributing to this feature, very appreciated 👍

This Pull Request was closed on November 17th 2020
Powered by GitHub Issue Mirror