We're also seeing this problem. Rather than returning true
for all browser versions, what exactly should the conditional in should_use_samesite_none
be? The comment in that function implies that some browser versions do not support SameSite=None so it seems like always returning true
will cause some problems as it fixes others.
This is my guess on how the function should be patched but I'd love feedback:
private static function should_use_samesite_none(): bool {
// We only want None or no attribute at this point. When we have cookie handling compatible with Lax,
// we can look at checking a setting.
// Browser support for none is not consistent yet. There are known issues with Safari, and IE11.
// Things are stablising, however as they're not stable yet we will deal specifically with the version of chrome
// that introduces a default of lax, setting it to none for the current version of chrome (2 releases before the change).
// We also check you are using secure cookies and HTTPS because if you are not running over HTTPS
// then setting SameSite=None will cause your session cookie to be rejected.
if ((\core_useragent::check_browser_version('Chrome', '78') || \core_useragent::check_browser_version('Firefox', '96')) && is_moodle_cookie_secure()) {
return true;
}
return false;
}
Also, an aside, that function's documentation says it returns a string when you can even see from its signature it returns a boolean.