ENH Add samesite attribute to cookies.

[GuySartorelli]

Sets a default samesite attribute on all cookies. Using the Lax value means that we're just setting an explicit value that is identical to the implicit value that was already being used.

This PR provides a way to configure both the default value for all cookies and for session cookies separately, as well as giving an extension hook if a specific value is desired for a given cookie.

The extension hook should be removed in the next major in favour of adding a new parameter like what was proposed in #9842 - I've avoided doing that here because the cookie backend is explicitly extensible (with an interface and docs and everything) so adding new parameters here would constitute a breaking change.

The following links may be useful background:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
https://www.php.net/manual/en/function.setcookie.php
https://www.php.net/manual/en/function.session-set-cookie-params.php

Parent issue:

• No support for SameSite cookie directive #9440

Related issue:

• Possible cookie issue (FluentLocale) tractorcow-farm/silverstripe-fluent#651

[GuySartorelli]

Leaving in draft for now because I still need to add unit tests.

[GuySartorelli]

Actually.... I don't think there is a way to test this... none of the unit tests current test against any of the properties of cookies other than the values and whether they're set at all. I don't think there's a way we can test if the cookie's samesite attribute is set correctly.

Marking as ready for review. But if anyone has ideas for how we could test this, that'd be awesome.

Edit: Steve has pointed out that it should be possible to test with a FunctionalTest - giving that a go.

[GuySartorelli]

After giving this a go it looks like I can't test cookie attributes in a FunctionalTest - the cookies are set using setcookie() directly, rather than adding them to the HTTPResponse object, which means we aren't fully including them in mock requests. The cookie values are being set in mock requests by pulling them out of the cookie jar and setting the $_COOKIE env variable. So we are mocking cookie values but not the other attributes in cookie headers.

Changing the way we add cookies to make them more fully mockable would definitely be a valuable enhancement but is out of scope for this PR. I've created an issue (#10338) for that.

[GuySartorelli]

@emteknetnz Requested changes made.

[emteknetnz]

Dismiss this stale review

Incorrect testing

[emteknetnz]

Test locally, works great. Thanks for implementing all the feedback!

[emteknetnz]

Just noticed there's a failing unit test - happy to merge once this is fixed

[GuySartorelli]

It turns out the $this->once() counts as an assertion (that the method will be called once).
Changed to $this->never() and removed the @doesNotPerformAssertions annotation.

[emteknetnz]

Merge on green