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
Use appropriate SameSite values for cookies #15185
Conversation
@Findus23 @MichaelHeerklotz any chance you could give this PR a test as well? |
core/Cookie.php
Outdated
*/ | ||
private static function getSameSiteValueForBrowser($default) | ||
{ | ||
$sameSite = ucfirst($default); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just fyi might be good to use ucfirst(strtolower($default))
? Just in case...
core/Cookie.php
Outdated
@@ -153,7 +156,8 @@ protected function setCookie($Name, $Value, $Expires, $Path = '', $Domain = '', | |||
. (empty($Path) ? '' : '; path=' . $Path) | |||
. (empty($Domain) ? '' : '; domain=' . $Domain) | |||
. (!$Secure ? '' : '; secure') | |||
. (!$HTTPOnly ? '' : '; HttpOnly'); | |||
. (!$HTTPOnly ? '' : '; HttpOnly') | |||
. (!$sameSite ? '' : '; SameSite=' . $sameSite); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to be safe we maybe want to rawurlencode the samesite value... and just to be safe maybe also domain and path even though they are values coming from our system?
core/Tracker/IgnoreCookie.php
Outdated
@@ -61,7 +61,7 @@ public static function setIgnoreCookie() | |||
$ignoreCookie->delete(); | |||
} else { | |||
$ignoreCookie->set('ignore', '*'); | |||
$ignoreCookie->save(); | |||
$ignoreCookie->save('None'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we move this for now into this PR that handles the opt out? https://github.com/matomo-org/matomo/pull/15184/files
Part of #14395