Set SameSite on session cookies

"Undefined" or "not set"?

"Undefined" in this context seems to imply something random could happen. I would prefer "not set" or "no value".

Fixed coding standards and remarks from comments #77-80 in the MR. Here is a backport of the latest iteration to D9.5.

Test out the patch from #81. it help out the SameSite cookie problem for me. +1 for RTBC

Last test failure was unrelated. Rebased on latest 10.1.x.

Unfortunately, #81 does not apply to Drupal 9.5.3.

Nitpick. This issue is for Drupal 10 which means we can use PHP 8.1 syntax here.

- $valid = in_array($samesite, ['Lax', 'Strict'], TRUE) || ($samesite === 'None' && $request_object->isSecure());
+ $valid = match ($samesite) {'Lax', 'Strict' => TRUE, 'None' =>  $request_object->isSecure(), default => FALSE};

The latter could be split into multiple lines.

• Addressed remark from #85, yay for match expressions!
• The requirement check was accidentally nested inside the PHP version check, fixed that.
• Updated documentation.
• Attaching new D9.5.x patch to address #84

Thanks!

OK the test coverage looks neat and the docs are fine. Looks like a lot of browsers, if not all, are defaulting to 'Lax' rather than none, so this shouldn't be disruptive, but better to get this into 10.1.x early.

I'm slightly weary about offering 'Strict' given Berdir's comments that it would break everything, but... maybe there's a use case and we probably want to allow configuration between 'Lax' and 'None' anyway.

I added a release note since this changes scaffold files.

The change record looks incorrect.

I made some edits to https://www.drupal.org/node/3275352 → . Have a look.

I think the change record was correct before. Commit a3bddae4 doesn't change the default - neither for existing nor for new sites. In order to change the default value for the session.cookie_samesite, it is necessary to set the container parameter in core/core.services.yml. The commit only touches sites/default/default.services.yml.

The following change is necessary in order to influence the default behavior (for new and existing sites):

diff --git a/core/core.services.yml b/core/core.services.yml
index 1c5d787c57..3beafcd932 100644
--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -9,6 +9,7 @@ parameters:
     gc_divisor: 100
     gc_maxlifetime: 200000
     cookie_lifetime: 2000000
+    cookie_samesite: Lax
     sid_length: 48
     sid_bits_per_character: 6
   twig.config:

I'll reverted the changes but it says things like "Drupal core will begin to warn if the session cookie SameSite is not set to Lax or Strict" which I do not think are true.

Drupal core will begin to warn if the session cookie SameSite is not set to Lax or Strict

That's in fact what happens. php core/scripts/drupal quick-start, then navigate to /admin/reports/status#warning, this is what I get:

I've updated the CR with the correct way to fix the warning.

Again, "Drupal core will begin to warn if the session cookie SameSite is not set to Lax or Strict" is untrue. I've looked at the commit. "None" is an allowed value under one condition.

@znerol - it's a bit trickier than that... completely new sites where sites/default/services.yml gets created from sites/default/default.services.yml will have it set to Lax and will not have a warning.

I do think we need a quick follow-up though. I think we should add cookie_samesite: Lax to core.services.yml in it's session.storage.options so that quickstart sites have the correct default.

@znerol - it's a bit trickier than that... completely new sites where sites/default/services.yml gets created from sites/default/default.services.yml will have it set to Lax and will not have a warning.

I tried the web installer as well as the quick-start script. Neither of them copies default.services.yml to services.yml. Both of them will copy default.settings.php to settings.php though. So the install behavior is different for services.yml and settings.php. I wonder whether we should fix that inconsistency.

So the install behavior is different for services.yml and settings.php.

It looks like this has a purpose, see 📌 Stop creating services.yml by default Fixed .

Let's do #97 here, I was also under the mistaken impression that default.services.yml kicks in for newly installed sites without manual copying.

Reverted so all the changes can go back in togher.

I added the #97's suggestion, please review.

📌 Stop creating services.yml by default Fixed makes this really tricky. Because it means sites don't have a sites/default/services.yml - that means that the whole "existing sites can stay as they are and new sites get the new value" assumption won't work.

That said, I think the current approach is fine given https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/Sam... - it says

Cookies without SameSite default to SameSite=Lax

so defaulting to Lax makes sense as that is what most modern browsers are doing anyway.

I think the CR will need to document how to unset it in case someone is relying on the current behaviour and we need to call this out in the release notes.

Thanks @rpayanm, the SameSite attribute is now present on the session cookie for all sites. Going to revert the CR to @cilefen version and add instructions on how to modify the default.

Rerolled patch against 9.4

Rerolled patch against 9.4.10

Previous patch for 9.4 had patch for sites/default/default.services.yml which is scaffolded

Automatically closed - issue fixed for 2 weeks with no activity.