Set samesite cookie attribute for PHP sessions

Symfony Framework emulates the behavior of PHP < 7.3.

@nullkernel,
We have a similar case on one of our web apps (Builded with D7), We have tested your patch and it works perfectly.
Hoping it will be accepted!

Thanks !

Thanks for the patch!

It was really helpful in getting us on the right track. We were experiencing some issues with a POST-redirect back to our CMS from a payment provider as the example given in the description.

However some clients is incompatible with SameSite=None as explained here: https://www.chromium.org/updates/same-site/incompatible-clients

To sum up there are two main issues:

1. All browsers on iOS 12 + Safari and Embedded browser MacOS 10.14: Will treat cookies marked with `SameSite=None` as if they were marked `SameSite=Strict`
2. Chrome 51 to Chrome 66 (inclusive on both ends) + UC browser on Android prior to version 12.13.2 will reject cookies with SameSite=None.

Looking at our current web-analytics it turns out that issue 1. would hit around 6-7% of our users.

We have therefore modified the patch from #5 adding some user agents sniffing to avoid setting SameSite=None in devices effected by issue 1.

I'm attaching patch here in case someone will find it useful.

All regex used in the patch are from the example code in https://www.chromium.org/updates/same-site/incompatible-clients

Thanks for the updated patch!

This is certainly an improvement over our patch and we will use that instead.

I've experimented a little with PHP >= 7.3 support.

Looks like we have to use the new option array signature for PHP's setcookie function to support SameSite. This is only supported in >= 7.3 so we will also have to check PHP version when setting cookie.

Attaching patch with my experiment that adds new drupal_set_session_cookie() function that checks PHP version.

I have checked for the case where cookie is not secure but SameSite=None. Not sure if this is the correct way to handle this. Perhaps it would be better to enfore secure if SameSite=None? Not even sure if we should handle this at all.

An alternative approach would be to use something like this library instead for setting session cookie:
https://github.com/ovunctukenmez/SameSiteCookieSetter/blob/master/SameSi...

It also includes the compatibility checks.

Also a minor correction in the patch: I've changed the code in drupal_session_can_send_samesite_cookie_attribute() to return TRUE if SameSite is something other than None. I believe we should still try to set SameSite in other cases since only None is the issue. Also changed to only check for None instead of the other attributes.

Attaching new patch with minor correction: use 'expires' instead of 'expire' in cookie params array.

I'd be opposed to the idea of browser sniffing as it is quite fragile to implement in real world, and the added complexity to Drupal core.

As of now, samesite is supported in ~92 browsers, and PHP 7.2 is security fixes-only mode. We would be adding too much code to core to only support a small minority of users.

It is possible to add SameSite support without any changes in Drupal. SameSite cookies will not completely prevent CSRF, and would only be an added feature that I don't is worth the hassle of maintaining bespoke code to add support for sites using PHP 7.2 or lower, and those who cannot bother to edit a PHP.ini setting. This is a setting that can be changed even in shared hosts.

I'm with @nullkernel on this one. We've been significantly impacted by this on our E-commerce site and are desperate for a solution that will resolve this. We have tried setting session.cookie_samesite to None in our settings.php file for PHP >= 7.3 and that helped some users but there are still others experiencing the issue and I'm hopeful that one of these patches will resolve the issue altogether for us. I'm going to try testing the last patch.

I tested the last patch drupal-3170525-11-samesite_cookie_attribute_support_for_legacy_php_versions.patch by @simonholt83 and I see where the SameSite cookie is set to "None" as expected. I was able to go through with a test payment on our development e-commerce site and everything seemed okay. The only thing I noticed was that once I logged out, I got the following message displayed:

Warning: setcookie(): Unrecognized key 'lifetime' found in the options array in drupal_set_session_cookie() (line 580 of /path/to/drupal-root/includes/session.inc).

@nullkernel I tried your patch in comment #8 and it applied properly and sets the SameSite cookie to "None" as expected (which is good), however, we still seem to be encountering users who are getting the Page not found or Access Denied after payment error in a related Drupal Commerce Issue # 3169405. And these are in recent versions of Google Chrome too with UA String "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.75 Safari/537.36". Any other ideas?

Hi @nullkernel,

We cleared the Drupal sessions table after we applied the patch and are still seeing users experiencing the issue. I don't think our users are taking too long on the payment page either.

The only way I've been able to replicate the issue on my computer is by not setting SameSite=None or by setting it to Lax. Once I have your patch applied or set session.cookie_samesite in my settings.php then it works as expected. So I'm really not sure what is happening on those users Mobile phones or Computers. The only other thing I'm wondering about is if those users having the issue in Chrome, have some sort of Ad Blocker or are in Incognito mode or something else that is blocking 3rd-party cookies.

I'm really at a loss as to why so many users are still having the issue. 🤷🏽‍♂️😩

I appreciate this issue thread - but I am not sure I understand potential fixes.

My current site is D7 with PHP 7.3 and Commerce Kickstart.

We are starting to see users being redirected to 404 after completing their purchase (using third-party vendor to process credit card). It appears that the order is completed successfully and our order confirmation email is sent. But the user is now logged out of the site.

Since I am using PHP 7.3, I figured I'd test (locally with DevDesktop) updating the php.ini field with session.cookie_samesite = "None"
This setting prevented me from logging into the site via /user/login.

So I tried session.cookie_samesite = "Lax"
I could login and create an order, test the payment, but was redirected to 404.

In all test cases, I used the latest version of Chrome (86.0.4240.80), started with a clean browser, and deleted any user session data from the database.

Is it my understanding that this issue is related specifically to newer versions of Chrome on either Mac or PC? Are there other browsers affected specifically (when using PHP 7.3)?

Should I put the session.cookie_samesite line in the settings.php file rather than php.ini? If so, what should that line be exactly?

I may be wrong, but I would not need the patch solution since I'm using PHP 7.3. Please confirm. Thanks!

@thalemn: I believe when you put it in your php.ini, it should have worked. But you can also try putting the following in your settings.php file:

ini_set('session.cookie_samesite', 'None');

Or you can try the patch that @nullkernel created in comment #8.

Hi, all

I updated the patch from comment #8 and added a new variable where we could check custom value for SameSIte attribute and set this variable in your custom code.

@nullkernel, I was using your patch exclusively when I made the comment #18 and #20.

I think I'm narrowing down why some users are still having this issue on my site though despite applying the patch by @nullkernel. I am using Drupal Commerce 7.x and am now noticing that if a user selects a product and adds it to their cart before they login, that when they do proceed to their cart and get prompted to login (and then they login) that somehow the SameSite cookie value gets overwritten from "None" to being empty. Any ideas why that would happen or where to fix that?

I think if I can figure that part out then it will resolve this issue for good.

In patch #11 (interdiff in #12) I was trying to use the new $options array in PHP's setcookie function to set SameSite, when using PHP 7.3+. But the tests still fails, so it needs some more work.

In pactch #8 we take an advantage of a bug in PHP's setcookie function, where you can add SameSite to the $path argument, but this bug should have been fixed in PHP 7.3. See this answer on stack for more info: https://stackoverflow.com/a/51128675.

So to support settingt SameSite in PHP 7.3+ looks like we need to use the $options array argument. And since this new argument isn't supported in versions lower than 7.3 we still need to use the bug in older versions.

An alternative to using user agent sniffing to handle incompatible clients, would be to set two cookies like explained here: https://web.dev/samesite-cookie-recipes/#handling-incompatible-clients

I may be wrong, but I would not need the patch solution since I'm using PHP 7.3. Please confirm. Thanks!

@thalemn

You can use the new php.ini directive in PHP 7.3 to set samesite, but then i guess it would be enforced for all clients and you won't be able to handle incompatible clients. This might be an acceptable solution for you.

@simonholt83, I tried your patch again (#11) and it solves my issue in comment #26. It seems what was also happening for me when I only used patch #8 was then when a user session is regenerated, it updated the Session cookie without SameSite="None" and so those users were still being affected until I applied your patch which takes care of cookies set during session regeneration.

As it relates to why tests might be failing for your patch, I think that its because the following warning is being generated by PHP:

Warning: setcookie(): Unrecognized key 'lifetime' found in the options array in drupal_set_session_cookie() (line 580 of /path/to/drupal-root/includes/session.inc).

And that has to do with the fact that your $params array references a 'lifetime' key, however, it seems that the key really should have been 'expires' when used with setcookie(). I think if you fix that in the patch, it should then pass the tests.

Really appreciate all the thought and effort that's gone into this!

However, I share @Ayesh's concerns (#13) about putting browser-sniffing code into core's session handling.

As @cilefen pointed out (#2), Symfony emulates PHP's samesite functionality for older versions of PHP, but that doesn't include doing User Agent sniffing to try and tailor behaviour for browsers that lag behind on this functionality.

I think it would be good for D7 core to add the samesite cookie emulation for older PHP versions (at present D7 tries to support versions all the way back to PHP 5.3, although how much longer that's feasible remains to be seen - see: #3155190: [meta] Should Drupal 7 drop support for older PHP versions?).

As for catering for User Agents with poor or non-existent support for samesite though, I'd argue that ought to be handled by contrib. I would prefer to add a hook into D7 core which would allow contrib or custom code to alter core's behaviour when it sets session cookies, rather than add browser-sniffing code into core itself.

Having a contrib module handle the incompatible browsers would allow sites to decide if they need this functionality, and also allow that module to update the browser-sniffing code and release more frequently than core.

Setting to NW on that basis; I'd like to get this into the next core release (see #3179845: [meta] Priorities for 2020-12-02 bugfix release of Drupal 7.76 / 7.77) but in order to do so, I'd like to see the approach changed to emulating the basic functionality for older PHP and providing a hook to allow contrib/custom code to alter core's functionality.

Here's a quick first pass at a patch which adds a drupal_setcookie() wrapper, from which we can call drupal_alter().

We'd want to add tests and API documentation for the new alter hook, but initially let's ensure that tests still pass in the various PHP versions.

I'm not certain about passing the cookie name within $context etc.. but that can all be tweaked.

If this works, it should allow the samesite cookie attribute handling to be done from a contrib (/custom) code - including emulation for older PHP versions to the browser sniffing for uncooperative clients... unless I'm missing something?

I'd be happy to consider samesite emulation for older PHP versions within core, but would prefer to leave the browser sniffing to contrib.

No interdiff as this is a completely different approach.

The approach in #31 needs a bit of a rethink; the earlier patches used session_set_cookie_params() in drupal_session_start() and without that we won't be affecting session cookies in many cases when a user logs in.

Apologies if I'm retreading the same ground you've already covered, but this obviously helps me review the changes we're making as I gain a better understanding.

I'll post another patch ASAP which will probably be a bit more like #8 / #11.

This patch has a tidier implementation of hook_cookie_alter() (should that be "hook_session_cookie_alter"? perhaps).

The emulation of samesite for PHP < 7.3 is added in. It feels like there's some duplication and it may be possible to refactor.

I've added an implementation of the new hook to the user module using the samesite_cookie_attribute_value variable for older versions of PHP.

I think this is getting closer to what I had in mind:

• Supporting the samesite attribute on session cookies for older versions of PHP.
• Provide a new hook which would allow contrib / custom code to e.g. do browser sniffing to decide how to handle samesite for browsers with poor support.

We'd still want to:

• Add the new variable to default.settings.php with an explanation of what it's for.
• Add API docs for the new hook.
• Add tests.

I'd like to see if tests pass with this patch first (noting that tests passed for #31 although it didn't really do what was intended).

This patch adds an entry to default.settings.php, some API docs, and a couple of tests.

The tests can be a little fragile as they're checking for the session cookie in a 'Set-Cookie' header but \DrupalWebTestCase::drupalGetHeader() will return the first header that matches. If - for example - you're using XDEBUG and that's creating another Set-Cookie header, the assertion might fail.

We could iterate through all matching headers checking them for the session cookie if this looks like it's going to be a problem.

With any luck the tests should account for PHP versions before and after the introduction of the samesite functionality in PHP 7.3

I also renamed the hook to hook_session_cookie_alter() and the variable to samesite_cookie_emulation_value as it only comes into play when PHP won't handle the samesite attribute itself.

After considerable fiddling around I've had to resort to doing an ini_set() outside of any function in session_test.module because I don't think there's a hook that runs early enough to affect the session cookie settings in order to test them. I think this is okay to do in this very specific situation, and is simpler than other options I considered such as hijacking the session_inc variable.

We may still want to improve the assertions which check the session cookies, per #34 - as mentioned if there are other cookies involved the session cookie might not be the first thing that's returned by $this->drupalGetHeader('Set-Cookie', TRUE).

With any luck the tests should now work with the newer PHP versions that have native support for the SameSite attribute.

Last patch for now; this is just a tweak to the tests so that we set SameSite to Strict before trying to change it to Lax via hook_session_cookie_alter().

I'm thinking that Strict is the least likely setting that any host the tests are running on will already have set up, so it's the best one to check we can change settings to and from.

Added to the test coverage so we simulate "None", "Lax" and "Strict" via php settings or the emulation thereof.

Plus test various settings transitions via hook_session_cookie_alter(). I don't think we need to cover every possible combination, but this should ensure that we're not passing tests just because the system running the tests is set up a certain way.

1. +++ b/includes/session.inc
   @@ -284,6 +284,20 @@ function drupal_session_start() {
   +    $name = $value = NULL;
   +    drupal_alter('session_cookie', $name, $value, $params);
   +    if ($params !== $original_params) {
   

   It might be that this is called really early in the bootstrap process and needs hook_boot() under certain circumstances.

   Maybe just a note to make.

2. +++ b/includes/session.inc
   @@ -488,7 +526,15 @@ function _drupal_session_delete_cookie($name, $secure = NULL) {
   +      'httponly' => $params['httponly'],
   +      'samesite' => isset($params['samesite']) ? $params['samesite'] : NULL
   +    );
   

   The samesite should be in an if () ...

   to not set NULL keys.

3. +++ b/includes/session.inc
   @@ -556,3 +602,33 @@ function drupal_save_session($status = NULL) {
   +function drupal_setcookie($name, $value, $options) {
   +  drupal_alter('session_cookie', $name, $value, $options);
   +  if (\PHP_VERSION_ID < 70300) {
   +    // samesite is not supported by PHP < 7.3 so emulate it.
   

   Probably should be in bootstrap.inc and also generally the alter hook should be:

   `hook_drupal_set_cookie_alter()`

Fixes per Fabian's review in #38.

I've moved drupal_setcookie() to common.inc as opposed to bootstrap.inc as I think the former is generally the home for "wrappers or custom implementations of PHP functions".

If this passes tests, my only remaining reservation would be whether the new alter hook is always safe to call so early in the bootstrap (per #38 1.)

Is there anything we can do to reduce the risk of it causing problems? I'm not sure I understand exactly what Fabian meant by "needs hook_boot()" ?

No interdiff I'm afraid as the patch needed a reroll (due to changes in default.settings.php)

Oops, it'd help to rename the implementation of the new alter hook too.

Erm, that'd apply to both implementations of the new hook (missed the one in the test module).

I'm sorry for drupal that cant fix this major bug that interrupt whole site functionality.
because chrome is most popular browser today and with this bug we cant sell product and create eCommerce website.
after this long time no valid and official solution...

@Fabianx and I have discussed this. As mentioned in #39 it seems risky to introduce a new hook which is potentially called so early. One risk is that caches can become polluted with incorrect / incomplete information etc.. and core's existing tests are unlikely to reveal a problem like that.

In order to get this feature added, we should remove the new hook implementation (initially at least).

If sites want to customise the behaviour (e.g. using browser sniffing) that could happen from settings.php via the variable system / $conf.

Anyone desperate to see progress on this issue could create a new patch along those lines; I'll get to it eventually if nobody else does, but I have several other plates spinning at the moment.

+  if (\PHP_VERSION_ID < 70300) {
+    $options['samesite'] = empty($options['samesite']) ? variable_get('samesite_cookie_emulation_value', 'None') : $options['samesite'];
+  }

I don't understand why you'd want different PHP version to behave differently, by default.

From what I can gather this has the effect of:
- PHP less than 7.3 defaults to 'None'
- PHP greater than or equal to 7.3 defaults to '', which behaves like 'Strict' (in Chrome at least)

Wouldn't we be better defaulting to the same thing, either:
- 'None', because it behaves the same as it's always has and sites that want to can override with $conf
- 'Strict', because it's "better" by default and sites using payment gateways or whatnot can override with $conf

Also should the $conf override apply to all cookies, or only session cookies.
I'm thinking if we go with 'None' as the default it's fine for all cookies, whereas if we go for 'Strict' as the default we should only add a $conf override for session cookies, and contrib modules can decide what they want to do for themselves.

I'm leaning towards default 'None' everywhere, what does anyone else think?

Here is an attempted patch that tries that then, i.e.

• Removes alter hook
• Defaults to 'None' for all PHP versions
• Allows $conf to override default
• Applies $conf override to all cookies, not only sessions, presuming they use drupal_setcookie()

Todo:

• Add tests
• Change PHP_VERSION_ID to \PHP_VERSION_ID

Done:

• Fixed failing test, due to copy-paste typo
• Change PHP_VERSION_ID to \PHP_VERSION_ID

Todo:

• Add tests

Thanks for picking this up.

I'm attaching an interdiff between 3170525-43.patch and 3170525-49.patch although a lot of the change is removing tests which mostly looked at the alter hook which is also being removed.

I don't understand why you'd want different PHP version to behave differently, by default.

I think the idea was that on newer versions of PHP the default value for samesite is handled by PHP and can be controlled through PHP ini settings.

It may well be simpler to just use one setting / value for all versions though.

If we're now going to say that if you want to alter that value (e.g. with browser sniffing) that'll have to happen via $conf, I think that should probably be outlined in default.settings.php

I want to refresh my memory on this one a bit and have a proper look at the latest patch, but this is looking pretty good, thanks!

..and yes, we'll want tests to cover the revised functionality.

Here's a set of very basic tests to check that setting the samesite_cookie_value variable to valid values is reflected in the attributes of the session cookie.

I'd like to add to this so that we check what happens when e.g. newer PHP versions have a given ini value for session.cookie_samesite and we set the variable on top of that. I believe the variable overrides the PHP ini setting, but would be good to confirm that with tests. I think some of the previous session_test.module code can be recycled for that.

We could also add a test where the variable is not set to check Drupal's default, which should be "None" per:

function drupal_setcookie($name, $value, $options) {
  $options['samesite'] = empty($options['samesite']) ? variable_get('samesite_cookie_value', 'None') : $options['samesite'];

(Should that use a constant for the default?)

We could also add test coverage for the case where a samesite option is passed to drupal_setcookie() which differs from the default / variable.

I think these additional tests cover the other scenarios - they:

• Check that the default value for the SameSite cookie attribute is 'None'.
• Check that (in PHP 7.3 and later) the PHP session.cookie-samesite setting is not altered by Drupal unless the samesite_cookie_value variable is set.
• Check that the samesite_cookie_value variable overrides any PHP session.cookie-samesite setting (in PHP 7.3 and later).

The relevant tests should be skipped for PHP versions before 7.3

Do we need to test anything else?

Noticed a couple of stray copy-pasta comments in the tests which I'll remove.

Also, testSameSiteCookieAttributeNoneDefault() may not work if the testing system has its own session.cookie-samesite setting which differs from 'None'.

Do we need to test the value after logout?

Do we need to test the value after logout?

So checking the set-cookie header when Drupal tells the browser to delete the (session) cookie? Hmm, I'm not sure whether we'd expect that to have the SameSite attribute or not - would need to check.

It looks to me like the PHP setting (in PHP 7.3 and above) is not being overridden by the variable, which will also need to be looked at a bit more closely.

I think what the tests for PHP 7.3 and above reveal makes sense.

We're doing:

    $params = $original_params = session_get_cookie_params();
    $params['samesite'] = empty($params['samesite']) ? variable_get('samesite_cookie_value', 'None') : $params['samesite'];

So if there's no explicit setting for SameSite in the PHP settings (the default in PHP is an empty value which "means that no SameSite cookie attribute will be set") then the variable - or Drupal's default of "None" - is used.

If PHP has been set up with an actual value for session.cookie-samesite that'll be used, and the variable / Drupal default are ignored.

That's why testSamesiteCookieOverrideLaxToStrict() is failing.

If we're happy that the behaviour is as it should be, we'll just need to change that test so that it instead verifies the explicit PHP setting is not overridden, and document the way this works properly in default.settings.php

I think the behaviour seems okay, but one problem is that this issue was originally filed because of the inconsistencies in browser treatment of the SameSite attribute. If the idea was/is to be able to override the attribute via $conf (e.g. some code in - or an include from - settings.php that does browser sniffing) that's not going to work for PHP 7.3 and later if a PHP ini setting is in place.

Any thoughts?

If I understood correctly you're saying the behaviour is currently such that:

- Neither $conf nor session.cookie-samesite set, default to None
- $conf is set, but session.cookie-samesite isn't, use $conf value
- $conf isn't set, but session.cookie-samesite is, use session.cookie-samesite value
- Both $conf and session.cookie-samesite are set, use session.cookie-samesite value

I think the flexibility of being able to override the last combination is valuable, so should be changed to:

- Both $conf and session.cookie-samesite are set, use $conf value

So checking the set-cookie header when Drupal tells the browser to delete the (session) cookie? Hmm, I'm not sure whether we'd expect that to have the SameSite attribute or not - would need to check.

Yes I would expect it to have a value, the test could basically be:
- Anonymous user, is samesite set to 'None'
- Login, is samesite still set to 'None'
- Logout, is samesite still set to 'None'

The reason I mention this, is because when I was moving a few things around in the code I came across the following situation:
- Anonymous user, samesite was set to 'None'
- Login, samesite was still set to 'None'
- Logout, samesite changed to ''

So it's basically to ensure this doesn't occur.

I think this now has all the tests we've added so far passing (checked with local drupalci using PHP 7.4)

So need to verify that other PHP versions pass tests too, and that these tests really represent the functionality that we're after (e.g. compare them to #57 and think through all the permutations we want to handle).

Not yet added any testing of cookie attributes on logging out.

Adding a test for cookie attributes on logout.

Important question here is how would this change affect existing sites (e.g. assuming we put only an example $conf in default.settings.php)?

I'm thinking sites would see a change in that their session cookies would suddenly get SameSite=None from the default. If so, that may not be a desirable change. It would at least need a Change Record.

@nullkernel, @simonholt83 and others that worked on this early on... does the current patch address your use case? The idea would be that any browser-snifffing code would need to be called from settings.php to adjust the samesite_cookie_value variable in $conf.

Important question here is how would this change affect existing sites (e.g. assuming we put only an example $conf in default.settings.php)?

I'm thinking sites would see a change in that their session cookies would suddenly get SameSite=None from the default. If so, that may not be a desirable change.

So as I see it the possible scenarios for existing sites are:
- PHP pre-7.3, before patch has the value of '', which behaves like 'None' on most browsers and older versions of Chrome, but like 'Strict' on newer version of Chrome, this is one of the behaviours we're attempting to resolve to ensure consistency across browsers, after the patch they will all behave like 'None' by default.
- PHP 7.3+ with session.cookie-samesite not set, basically the same as above.
- PHP 7.3+ with session.cookie-samesite set, $conf will be commented out, so it'll use the session.cookie-samesite value as expected.

I actually think all that is fine and no-one should notice any difference, except that newer versions of Chrome now work as expected out of the box.

+++ b/includes/common.inc
@@ -8571,3 +8571,33 @@ function drupal_get_filetransfer_info() {
+function drupal_setcookie($name, $value, $options) {
+  $options['samesite'] = variable_get('samesite_cookie_value', !empty($options['samesite']) ? $options['samesite'] : 'None');
+  if (\PHP_VERSION_ID >= 70300) {

This logic needs to be moved to the call-site of the function, because else a caller might not get what they expect.

---

Overall the patch looks good, but we should talk about order of precedence for the samesite:

- 1. Explicitly given to the drupal_set_cookie function [IMHO if we add a new function it should be side-effect free]
- 2. Explicit $conf setting
- 3. PHP INI setting
- 4. None

is what I think makes the most sense.

Overall the logic should be something like:

if (!isset($options['samesite'])) {
  // Calculate it in order of 2-4 to provide a sensibly default, can be disabled by setting samesite in options to FALSE
}

// It can be disabled by setting to FALSE, but PHP wants it to be unset in $options.
if (empty($options['samesite'])) {
  unset($options['samesite']);
}

// Set cookie with samesite

Current patch #59 works well for me (PHP 5.6)

The patch has come a long way :) Thanks for the good work.

@mcdruid
An alter hook would have been easier, but I think we can make it work with browser sniffing in settings.php and the new patch.

I think this is pretty much what @Fabianx and I discussed (#61).

The new helper function is maybe not strictly necessary but hopefully makes the logic clear - the precedence is as outlined in #61 and spelled out in comments.

I've added a test for explicitly disabling the samesite attribute (by setting the variable to FALSE in $conf).

Tests are passing for me locally on PHP 7.2 but let's see what drupalci thinks with all the other PHP versions we support...

Oops, yeah we'll want to remove the samesite_ini element from $options.

I added that so that we can differentiate between a param passed explicitly to drupal_setcookie() and the PHP default which comes from session_get_cookie_params().

Looks good. We will set samesite variable FALSE for incompatible browsers then.

Another situation where the ability to disable samesite might be beneficial is local dev without https. If I remember correctly secure cookies are enforced with samesite.

Thanks, I'm going to take that as RTBC :)

+++ b/includes/session.inc
@@ -284,6 +284,19 @@ function drupal_session_start() {
+    $params = $original_params = session_get_cookie_params();
+    $params['samesite'] = variable_get('samesite_cookie_value', !empty($params['samesite']) ? $params['samesite'] : 'None');
+    if ($params !== $original_params) {
+      if (\PHP_VERSION_ID >= 70300) {
+        session_set_cookie_params($params);
+      }
+      else {
+        // Emulate samesite support in older PHP versions.
+        $params['path'] .= empty($params['samesite']) ? '' : '; SameSite=' . $params['samesite'];
+        session_set_cookie_params($params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly']);
+      }
+    }

Is this necessary?

Would this not lead in circumstances to setting a path twice and ending up with ';SameSite=;SameSite='

?

The rest looks good.

Good catch; I don't think that's necessary... checking that tests agree.

Couple of tweaks to make in comments - can be done on commit:

+++ b/modules/simpletest/tests/session_test.module
@@ -64,6 +64,19 @@ function session_test_menu() {
+ * case we want simulate a given session.cookie_samesite setting in php.ini or

we want *to* simulate

+++ b/sites/default/default.settings.php
@@ -716,3 +716,22 @@ $conf['file_scan_ignore_directories'] = array(
+ * @see drupal_session_start()

remove reference to drupal_session_start()

Oh, looks like we do need to process the cookie params in drupal_session_start() if we want to be able to override them.

In order to avoid duplicating code I'd prefer to reuse the helper function(s) there, but common.inc hasn't always been included that early.

So I've reluctantly moved the helpers into session.inc which is probably appropriate most of the time - possible exception being if drupal_setcookie() is being used for something other than session cookies. The helpers could go into bootstrap.inc instead?

I've added a simple check to try and avoid adding the SameSite hack to $options['path'] more than once. I suppose this doesn't cater for the case where we may have a different value for samesite than has already been added? Is that likely / possible?

Tests pass again for me in PHP 7.2

I think that could happen if the path already had samesite emulation from the ini and we try to override with different value from config.

In that case I think the correct is to use the existing samesite as the patch does now and ignore the new value from config.

Another concern: I see that we set "secure = FALSE" when setting insecure session cookie. That could give some issues with cookies being rejected by the browser if samesite=None.

With this extra tweak to pass the result of session_get_cookie_params() to the helper(s) as samesite_ini I am still seeing a test fail for testSameSiteCookieAttributeNoneDefault with PHP 7.3

I think that may be because that test is explicitly setting session.cookie_samesite to an empty value via ini_set and that's disabling the SameSite attribute completely as opposed to allowing Drupal to set the default of None.

We need to work out if it's actually legitimate to set an empty value like that, and if so decide what the behaviour should then be. I'm not sure that removing the attribute is correct in this case.

...waiting for that test to re-run, I think it'll fail though.

https://www.php.net/manual/en/session.configuration.php#ini.session.cook... says:

An empty value means that no SameSite cookie attribute will be set.

So it's legitimate to test for this php ini setting. However, testSameSiteCookieAttributeNoneDefault currently expects the Drupal default of "None" to be applied, which won't be the case with our stated order of precedence:

 * 1) A value explicitly passed to drupal_setcookie()
 * 2) A value set in $conf['samesite_cookie_value']
 * 3) The setting from php ini
 * 4) The default of 'None'

In this case, 3 is correctly being applied over 4.

So I think the test is now wrong as it is applied for PHP 7.3 and later.

This patch changes the tests so that we're checking "None" is the default, and separately checking that setting an empty value in php ini "means that no SameSite cookie attribute will be set".

testSameSiteCookieAttributeNoneDefault is failing for PHP 7.3 because I think the default php setting is an empty value which means the attribute is omitted. IIUC that default is up to the distribution / packager of PHP though so it's probably not wise to rely on it being set a certain way.

So although it doesn't seem ideal, this patch explicitly sets session.cookie_samesite to None for PHP 7.3 and above for that default test.

...and the patch.

#73

I've added a simple check to try and avoid adding the SameSite hack to $options['path'] more than once. I suppose this doesn't cater for the case where we may have a different value for samesite than has already been added? Is that likely / possible?

#75

I think that could happen if the path already had samesite emulation from the ini and we try to override with different value from config.

The emulation only happens for < PHP 7.3 and there is no ini for those versions, so I think we're okay to simply ensure we don't add the hack more than once.

@simonholt83's other comment in #75:

I see that we set "secure = FALSE" when setting insecure session cookie. That could give some issues with cookies being rejected by the browser if samesite=None.

See: https://web.dev/samesite-cookies-explained/#changes-to-the-default-behav...

...which links to: https://tools.ietf.org/html/draft-west-cookie-incrementalism-00

So yes, it looks like browsers are moving in the direction of mandating that:

Cookies with SameSite=None must also specify Secure

Hmm. Thanks for bringing that up.

I wonder if we should reconsider the default based on that; i.e. have the default be FALSE / no SameSite attribute is set?

What does D8/9 do?

Confirmed that with SameSite=None (being emulated and set as default with PHP 7.2) I am unable to login over plain http in a chromium-based browser because the cookie is not (and cannot be) marked as Secure:

The same problem happens in PHP 7.4 if we set $conf['samesite_cookie_value'] = 'None'; (or with session.cookie_samesite set to "None" in php.ini).

We'll have to think about how to approach this.

Perhaps it's up to site owners not to set up impossible configs (e.g. plain http logins and SameSite=None), but it'd certainly be ungood to have a default that breaks session cookies (for the majority of current browsers) on non-https sites for older PHP versions.

This changes the default to FALSE (i.e. no samesite attribute), which I think is the safest, least disruptive option.

Other than that, the order of precedence should be unchanged (see e.g. #78).

Most of the changes in the patch are to the tests and comments in default.settings.php

This is passing in PHP 7.2 and 7.4 for me, let's see if the bot agrees.

This changes the default to FALSE (i.e. no samesite attribute)

Why? This surely this reintroduces the inconsistency that I mentioned previously!

i.e.

So as I see it the possible scenarios for existing sites are:
- PHP pre-7.3, before patch has the value of '', which behaves like 'None' on most browsers and older versions of Chrome, but like 'Strict' on newer version of Chrome, this is one of the behaviours we're attempting to resolve to ensure consistency across browsers, after the patch they will all behave like 'None' by default.
- PHP 7.3+ with session.cookie-samesite not set, basically the same as above.
- PHP 7.3+ with session.cookie-samesite set, $conf will be commented out, so it'll use the session.cookie-samesite value as expected.

I actually think all that is fine and no-one should notice any difference, except that newer versions of Chrome now work as expected out of the box.

You're basically saying it should be broken out of the box, that doesn't make any sense.
It's always behaved like 'None' until Chrome broke things, why change it now?

IIUC chromium-based browsers (perhaps somewhere between 2/3 and 3/4 of all browsers) have already implemented this behaviour:

• Cookies without a SameSite attribute will be treated as SameSite=Lax.
• Cookies with SameSite=None must also specify Secure, meaning they require a secure context.

https://web.dev/samesite-cookies-explained/#changes-to-the-default-behav...

FF has implemented the same behaviour as an experimental feature which is not yet on by default, but likely will be eventually.

If we default to SameSite=None D7's session cookies will no longer be accepted by (chromium-based) browsers unless they also are also marked as Secure. So any site that does login over plain http (however inadvisable that is) will be broken; it will no longer be possible to login in the majority of browsers. I tested this in #84.

There's an argument to say we should default to "Lax". See e.g. #3150614: Set SameSite on session cookies for D9.

However, I don't think we can justify defaulting to "None" if that would break login on what I am guessing would be a substantial number of sites.

Defaulting to FALSE / no attribute means no unexpected regression for sites when they update.

We are, however, providing the facility for sites to configure the attribute via either PHP ini or the $conf/variable system. Sites can conditionally change the attribute from settings.php if they need to.

Couple of small tweaks which could be fixed on commit if there are no further changes:

+++ b/includes/session.inc
@@ -556,3 +612,59 @@ function drupal_save_session($status = NULL) {
+ * @return
+ *   An associative array which may have any of the keys expires, path, domain,
+ *   secure, httponly, samesite and samesite_ini
+ */
+function _drupal_cookie_params($options) {

remove samesite_ini as this function won't return that element

+++ b/modules/simpletest/tests/session.test
@@ -244,6 +244,167 @@ class SessionTestCase extends DrupalWebTestCase {
+      $user = $this->drupalCreateUser(array('access content'));                   
+      // Send our own login POST so that we can pass a custom header to trigger
+      // session_test.module to call ini_set('session.cookie_samesite', $value)
+      $headers[] = 'X-Session-Cookie-Ini-Set: Lax';                               
+      $edit = array(                                                              
+        'name' => $user->name,                                                    
+        'pass' => $user->pass_raw,                                                
+      );                                                                          
+      $this->drupalPost('user', $edit, t('Log in'), array(), $headers);           
+    }                                                                             
+    $this->assertTrue(TRUE, ($debug = $this->drupalGetHeader('Set-Cookie', TRUE)));

some copypasta/debug to be removed here (first and last lines)

+++ b/modules/simpletest/tests/session.test
@@ -244,6 +244,167 @@ class SessionTestCase extends DrupalWebTestCase {
+    $this->assertTrue(preg_match('/SameSite=Lax/i', $this->drupalGetHeader('Set-Cookie', TRUE)), 'Session cookie is set as SameSite=Strict.');
...
+  /**

message doesn't match the assertion (should say "Lax")

So is the suggestion that the default is something like this?

if (drupal_is_https()) {
  return 'None';
}
else {
  return FALSE; // no SameSite attribute
}

As for whether the default is Lax or None, the majority of browsers are now behaving as if it were Lax if there is no attribute. That in itself is an argument for Lax to reduce unexpected disruption. As you mention @nullkernel, we should...

prefer not breaking functionality for existing sites

Trying out handling the default like this:

/**
 * Determine the value for the samesite cookie attribute, in the following order
 * of precedence:
 *
 * 1) A value explicitly passed to drupal_setcookie()
 * 2) A value set in $conf['samesite_cookie_value']
 * 3) The setting from php ini
 * 4) The default of None or FALSE (no attribute) if the cookie is not Secure
 *
 * @param $options
 *   An associative array as passed to drupal_setcookie().
 * @return
 *   The value for the samesite cookie attribute.
 */
function _drupal_samesite_cookie($options) {
  if (isset($options['samesite'])) {
    return $options['samesite'];
  }
  $override = variable_get('samesite_cookie_value', NULL);
  if ($override !== NULL) {
    return $override;
  }
  if (isset($options['samesite_ini'])) {
    return $options['samesite_ini'];
  }
  return empty($options['secure']) ? FALSE : 'None';
}

That seems to work.

We'll need to add tests to cover both scenarios.

Would that address concerns about defaulting to no attribute (and leaving browsers to do their currently somewhat inconsistent thing) whilst avoiding breaking login on sites that don't enforce https?

Fixes the small issues from #88.

The default for samesite is handled per #91, so 'None' for Secure cookies and FALSE / no attribute for cookies under plain http.

There's a new test for https / Secure cookies defaulting to 'None'.

The default only applies to PHP versions before 7.3 as any ini setting takes precedence (see #78) over the default, and PHP 7.3 and later always have an ini setting even if that is empty (/ no attribute) which is the default in most cases.

We can only speculate about the motivations of the Chromium team, and whether other browsers will inevitably follow suit per https://tools.ietf.org/html/draft-west-cookie-incrementalism-00

There are a couple of issues I'd like to focus on in order to get this committed for the next release:

1) What should Drupal's default be for the samesite attribute?

#92 uses 'None' as long as the Secure attribute is also going to be set. The attribute will be omitted for non-secure cookies.

I am happy to stick with this; there is an argument to say it represents a change from current behaviour for the majority browser (family), but that's only been the case since summer 2020 when chromium started to default to 'Lax'.

2) With this order of precedence for the setting:

 * 1) A value explicitly passed to drupal_setcookie()
 * 2) A value set in $conf['samesite_cookie_value']
 * 3) The setting from php ini
 * 4) The default of None or FALSE (no attribute) if the cookie is not Secure

...the default is only ever applied for PHP versions without native support for session.cookie_samesite (so PHP 7.2 and earlier).

This is because the ini setting will be applied even if it's set as "no value" (meaning the attribute will be omitted). No value seems to be the PHP default in most cases AFAICS.

I think that's okay. The php ini setting can be changed as required, and if that's not possible the conf/variable is available.

Overall I'm happy with the functionality implemented in #92.

• It provides emulation for the samesite attribute in older versions of PHP and facilitates the use of the native support in recent versions.
• It provides at least one viable way for sites to configure their samesite behaviour, conditionally (e.g. using browser detection) if appropriate.
• It should avoid breaking sites using earlier PHP versions which don't enforce https for login (we can discuss how ill-advised that is, but I'm fairly sure it would be a non-zero number of sites that would be affected).

Anything else to be considered here or are we RTBC and ready for @Fabianx to review as Framework Manager?

Would that address concerns about defaulting to no attribute (and leaving browsers to do their currently somewhat inconsistent thing) whilst avoiding breaking login on sites that don't enforce https?

I think so, thanks for listening.

Anything else to be considered here or are we RTBC and ready for @Fabianx to review as Framework Manager?

My only comment for now is that this seems a little unclear:
4) The default of None or FALSE (no attribute) if the cookie is not Secure

Maybe:
4) The default of None if the cookie is Secure, or FALSE (no attribute) if the cookie is not Secure

Or even something like:
4) A value of None if the cookie is Secure
5) The default of FALSE (no attribute), which will use inconsistent browser specific behaviours (not recommended)

...the default is only ever applied for PHP versions without native support for session.cookie_samesite (so PHP 7.2 and earlier)

Hmm, I don't think this is true, I was seeing '' on PHP 7.4

Thanks for the feedback.

Yup, perhaps we could make the comment about the two alternatives for the default more clear.

> ...the default is only ever applied for PHP versions without native support for session.cookie_samesite (so PHP 7.2 and earlier)

Hmm, I don't think this is true, I was seeing '' on PHP 7.4

I think that if you don't set the variable in PHP 7.3 or 7.4 you'll get whatever is set in php.ini (which is often no attribute because session.cookie_samesite is set to an empty value), and Drupal's default never comes into play.

Will look closer at that if you're seeing something different.

RTBC - with two changes:

Let's just move all three new helper functions to bootstrap.

- common.inc calling a session.inc function that has nothing to do with sessions was weird
- I think someone would want to call drupal_set_cookie without wanting to include common.inc

In the same vein we can remove the samesite_ini and just add another call to session_get_cookie_params().:

+++ b/includes/session.inc
@@ -488,7 +534,17 @@ function _drupal_session_delete_cookie($name, $secure = NULL) {
+    if (isset($params['samesite'])) {
+      $options['samesite_ini'] = $params['samesite'];
+    }

@@ -556,3 +612,59 @@ function drupal_save_session($status = NULL) {
+  if (isset($options['samesite_ini'])) {
+    return $options['samesite_ini'];
+  }

Move this to the _drupal_samesite_cookie function as:

$ini_options = session_get_cookie_params();

if (isset($ini_options['samesite']) {
  return $ini_options['samesite'];
}

at the same point.

Then we can also simplify the remaining case, before drupal_session_start():

- Just unset() the $options['samesite'] always and let the helper do the rest.

Thanks for sharing that @nullkernel. #229825: backport "$_COOKIE['has_js'] must die" patch to 7.x is on the Drupal 7 to do list #3192080: [meta] Priorities for 2021-04-07 release of Drupal 7, perhaps a good reason to prioritize getting rid of the has_js cookie altogether?

Testing changes per review in #97

Oops - interdiffs are not patches.

This will need a Change Record (/ Notice).

The failing tests have reminded me why we did the dance with the samesite_ini element in $options.

Without doing that, we pick up $options['samesite'] from the calls to session_get_cookie_params() in the session.inc functions (even although by default it's often set to an empty value).

That being the case, we don't know whether this is a param that was explicitly passed to drupal_setcookie() or whether it came from ini (meaning session_get_cookie_params()).

That means that the variable won't override the ini setting in the helper (we never get beyond 1 in the list):

/**
 * Determine the value for the samesite cookie attribute, in the following order
 * of precedence:
 *
 * 1) A value explicitly passed to drupal_setcookie()
 * 2) A value set in $conf['samesite_cookie_value']
 * 3) The setting from php ini
 * 4) The default of None, or FALSE (no attribute) if the cookie is not Secure
 *
 * @param $options
 *   An associative array as passed to drupal_setcookie().
 * @return
 *   The value for the samesite cookie attribute.
 */
function _drupal_samesite_cookie($options) {
  if (isset($options['samesite'])) {
    return $options['samesite'];
  }
  $override = variable_get('samesite_cookie_value', NULL);
  if ($override !== NULL) {
    return $override;
  }
  if (isset($options['samesite_ini'])) {
    return $options['samesite_ini'];
  }
  return empty($options['secure']) ? FALSE : 'None';
}

So I think we need to go back to using samesite_ini to differentiate ini settings, allowing them to be 3 in the order of precedence.

Here's a patch which moves all 3 functions to bootstrap, but leaves the samesite_ini implementation in place.

Interdiff is against #92 as not sure a comparison with #100 is useful.

#100 would have worked, but it needs this line from the last patch:

+    // Apply any overrides to the session cookie params.
+    $params = $original_params = session_get_cookie_params();
+    if (isset($params['samesite'])) {
+      unset($params['samesite']);
+    }

to work correctly.

After this change and if tests pass, this can go in.

Well spotted - with this small change, tests pass for me with PHP 7.4

...let's check everything else.

Committed, but we still need a change record.

I've applied this patch to a local D7 site that was having problems with Chrome loosing the session during Paypal Payflow Pro checkouts. With the patch applied as-is, and no changes to settings.php, both Safari and Chrome can now use the site's functionality, including checking out with Payflow Pro (yay!).

However, if I set samesite_cookie_value to "None", as is recommended, it causes problems with Chrome - forms don't work and throw "The form has been modified" errors. The solution was to comment out $conf['samesite_cookie_value'] so it works as-is.

@DamienMcKenna on the site(s) that had problems with the variable, what was:

• the scheme in use (http/https)?
• the PHP version?

On versions of PHP before 7.3 the difference between setting the variable to 'None' and not setting the variable would be that the default would be 'None' where the cookie will also have the Secure attribute (which should mean https), but it'd be empty if not (i.e. http / no Secure attribute). If you set the variable, Drupal will use that value unconditionally (same with later versions of PHP which have the session.cookie_samesite ini setting).

Could you maybe try this in settings.php?

$conf['samesite_cookie_value'] = drupal_is_https() ? 'None' : '';

That should yield the same result as not setting the variable at all, but if it makes the problem you encountered go away it will have helped us identify the cause.

Depending on what we find, this could be useful context for the CR. I may also tweak the comments in the bootstrap code and default.settings.php a little.

That one site uses HTTPS only, $conf['https'] is set to TRUE, and it uses PHP 7.4.

I think for sites using PHP 7.3 and later I'd use the ini setting if possible, although as noted that'll be applied unconditionally. Hopefully not a problem if everything's always https.

I'm not sure what difference it would make here but the $conf['https'] = TRUE; setting is quite misleading; that enables "mixed mode" whereby:

you use the same session over both HTTP and HTTPS -- but with two cookies where the HTTPS cookie is sent over HTTPS only.

https://www.drupal.org/https-information#drupal-config

You may well know all that already, but I think a lot of people misunderstand what this does (and it was removed in D8).

@DamienMcKenna I'd be happy to have a look at the misbehaving site if you want to share details in slack (although sounds like I'd need to login and submit forms etc..?) I'd like to understand what's going on if using the variable is causing a problem (especially if the ini setting does not).

Ok, some further testing.

I set the variable to 'None' and removed the 'https' setting.

I then attempted three different checkouts:

• Anonymous visitor from a freshly loaded browser: checkout fails.
• Authenticated visitor: checkout succeeds.
• Authenticated visitor, logs out then does an anonymous order: checkout succeeds.

If you have some time over the next week I'd appreciate it if you'd ping me on Slack and I can share credentials to the site. Thank you.

Just to follow-up here, it seems the problems I was seeing were because I still had $conf['https'] = TRUE; in my settings.php - I thought I'd removed it but I edited the wrong file by mistake.

Once that line was removed from the correct settings.php file the site's checkout appeared to work correctly, am just waiting for my client to confirm the fix.

Huge thanks to mcdruid for taking a half hour of his day to work through this with me.

Added a draft CR.

Change record: https://www.drupal.org/node/3207213

We could use some reviews on the same for D9 #3150614: Set SameSite on session cookies. Thank you!

We had some issues with redirecting back to our site after a user makes a payment via third-party website.
We have updated D7 core to 7.79 and use PHP 7.4

I have added the following to the settings.php file:

$conf['samesite_cookie_value'] = 'None';
$conf['https'] = TRUE;

I have installed the samesite_cookie module with the following settings:
SameSite attribute value to send: (no override)
Checked Enable legacy browser workaround

These settings appear to work with the following browsers (all latest versions) and OS:
Safari/Mac
Chrome/Mac and PC
Firefox/Mac and PC
Edge/PC

Can someone please confirm my settings are correct, or if I should change them.

$conf['https'] = TRUE;

...is poorly named - that's actually enabling "mixed mode" for https whereby Drupal will use a pair of session cookies only one of which will be for secure requests.

See: https://www.drupal.org/https-information#drupal-config

I don't think most sites want or need this.

I would try removing it if you can.

@thalemn: please see my the comments above - as @mcdruid says the 'https' setting is badly named, you should make sure to NOT set it to TRUE on a site as it breaks Chrome.

Thank you @mcdruid and @DamienMcKenna for your help.

Just to clarify, my site uses D7 Commerce and sends a user to a third-party site to process payment, then back to D7 site when payment is completed. After upgrading the site to Drupal 7.79, users would create an order using Chrome, go to third-party site to process payment, then come back to D7 site as logged out (not expected) and the order status set to pending (funds captured by third-party site), the expected result would be a completed order with the user logged in to see the order confirmation page. This created a terrible situation for our customers!

So I then realized that I need to do something with the configuration of the SameSite cookie attributes. I installed the samesite_cookie module and configured SameSite: NONE and enabled legacy browser fix. This did not fix the issue described above (user logged out and order status pending when using Chrome).

So I abandoned the samesite_cookie module and tried adding configuration only to the settings.php file. First I applied the $conf['samesite_cookie_value'] = 'None'; attribute and tested. This resulted in our pre-Core 7.79 update issue with users placing an order with Chrome, and after using the third-party site to process the payment, they returned to Drupal 404 page and were logged out (the order did process as completed, but the user did not get that message).

So that's when I added $conf['https'] = TRUE; to the settings.php file. And voila, everything appeared to work as expected. Created orders in Drupal Commerce, directed to third-party site for payment, redirected back to Drupal with a completed order page and still logged in.

But when I check the console logs, I am seeing messages that the SameSite cookie is not secure. I (perhaps foolishly) thought the https setting would correct this.

I guess I need help understanding how to configure my D7 commerce site now that I've updated the core to 7.79. Any further assistance is much appreciated.

@nullkernel thanks for the information. Much appreciated!

My site forces the user to use https, so there's never any none secure traffic on the site. That's why I'm confused about this SameSite cookie attribute. If I set it to NONE (either in your module or in the settings.php file), that should resolve the issue, right?

I haven't found a specific issue related to your module, just the SameSite cookie attributes in general.

Again, it appears to happen primarily with users who use Chrome (Mac or PC), and sometimes with Safari.

On my production site, I have reverted to using core 7.78 and when users complete an order (with Chrome) they are redirected to a 404 page. The order processes and is complete, but the user is logged out of the site and land on the 404 page.

I am doing all of my testing on my development site. Both prod and dev force https, so we can rule this out.

The issue is present when a user leaves the Drupal commerce site to process their payment on a third-party site, then return to the Drupal site when the payment is complete. When they return to Drupal (using Core 7.78), they are either logged out and presented with a 404 page (order is processed and complete), or (using Core 7.79) they are logged out and presented the home page (order is pending, not complete).

@nullkernel

Thanks for your continued help and advice.

I am now using your module for testing the SameSite cookie attributes (no attributes in settings.php). I have switched the values from (no override), None, Lax, and Strict. And after processing orders (using Chrome Dev tools Console with Issues tab and Include third-party cookie issues checked), I can see the SSESS cookie attributes changing. Cool. The SSESS cookie never is SESS.

So when I start with a clean Chrome browser session (Guest people account), I can successfully complete an order with all results as expected. There are a handful of cookies with the reply and complete requests (leaving the third-party site and going back to Drupal), that have undefined SameSite attributes (but are all secure). Only the NO_CACHE cookies are not secure and have no attribute. The one Facebook Pixel cookie is not secure and Lax.

So I think I will give this a go on production. We have an excellent CS team that will let me know if there are issues.

Thanks again for all of your help!

@nullkernel

Update this morning: I restarted my Macbook Air, confirmed that Chrome was the latest release, and did a test order.

After using the third-party site to process the payment, I was redirected back to Drupal. This time I was logged out and presented with the home page. The order is processed as pending, with no messages sent.

Using the Chrome dev tools, I can see that the SSESS cookie value is deleted (reply cookies), and that the SSESS cookie was secure and NONE. So what do you think is happening here? And if I place a second order, it will process as expected. But restarting my computer causes the issue again.

@nullkernel,

I disabled and uninstalled your module and set the cookie attribute via settings.php file: $conf['samesite_cookie_value'] = 'NONE';

Same results - SSESS cookie is deleted after submitting payment on third-party website and redirecting back to Drupal without completing the order (and I am logged out of Drupal and presented with the homepage). I am not familiar with how this works, but from what I can tell, when the third-party site 'replies', that's when the cookie is deleted.

So I think your module is working as expected since I get the same results either way.

I am starting to suspect that the third-party website is somehow causing the SSESS cookie to be deleted. What is baffling is that this issue only occurs the first time I place an order after rebooting my machine. If I place a second order, it processes as expected. (I use CCleaner to clean my machine before a reboot).

I appreciate your help.

@thalemn: Try using Chrome's web inspector to review the request when the initial form is submitted, check to see if you have two SetCookie headers, one with the "Secure" option and one without. I had this problem on a D7 site, every time the visitor used Chrome to do the checkout when they returned back the session had disappeared. It turned out to be the $conf['HTTPS'] option that caused the problem, removing that option fixed it and visitors using Chrome didn't have any problems.

@DamienMcKenna,

I'll check again, but I don't have $conf['HTTPS'] option in my settings.php file. Thanks for the suggestion.

OK, I see only one SSESS cookie in the request header (secure, None).

But in the response header, I see one SSESS cookie (secure, NONE), which has a value of deleted. I'm pretty sure this is causing the issue, but I don't know why. My settings.php file has $conf['samesite_cookie_value'] = 'NONE'; (using all caps for the attribute). The request header attribute is None (not all caps). Maybe the attribute is set as None somewhere else?

Thanks for you help!

Update: I decided to try something related to roles and permissions. I have been testing the orders using Role A (minimal permissions). I decided to try Role B (administrator with maximum permissions) - the order processed and completed as expected. So I am thinking that we have a permission setting that is blocking/deleting the Session Cookie when returning to the website from the third-party payment site.

To test something, I restored the database from this morning's backup, then ran drush rr and drush cc all. Voila, every order processes as expected now. I didn't touch permissions, and using Role A (minimal permissions), the orders process and complete successfully. I have no idea why drush rr would fix the issues, but it did.

Hello,

i still have an problem with session cookies on D 7.79.

When i set Samesite Cookie to None as described i have no more errors when redirection from paypal as long as i´m a anonymous user.

When i fill my cart an whant to login in during checkout i get an error that my session id is a duplicate entry in db and my cart is gone. is there something i miss in my config?

only on chrome browser at the moment - every other browser is working normal.

thanks for your help!

Our situation is very similar.

I added the following to the .htaccess file (in addition to the SameSite Cookie set to None as described):

<If "%{HTTP_USER_AGENT} !~ /(iPhone; CPU iPhone OS 1[0-3]|iPad; CPU OS 1[0-3]|iPod touch; CPU iPhone OS 1[0-3]|Macintosh; Intel Mac OS X.*Version\x2F1[0-3].*Safari|Macintosh;.*Mac OS X 10_14.* AppleWebKit.*Version\x2F1[0-3].*Safari)/i">
    Header edit Set-Cookie ^(.*)$ $1;SameSite=None;Secure
</If>

Tested successfully on dev site. Now testing in production.

@thalemn and @cwarts Just out of curiosity, what version of php are you using?

7.4

7.3

Out of curiosity: do you operate your sites behind a reverse proxy? In this case Drupal might be unable to detect that the original request came in via HTTPS and thus the Secure cookie attribute is not added to the session cookie automatically.

You can try to enforce the Secure cookie attribute by enabling the PHP ini setting session.cookie_secure for all requests by adding the following line to settings.php:

ini_set('session.cookie_secure', TRUE);

Also instead of setting $conf['samesite_cookie_value'] I'd rather recommend setting the session.cookie_samesite attribute as PHP ini setting for consistency reasons (unless for sites on PHP<7.3 of course).

Note that all of that is only working for sites which are HTTPS-only (HTTP to HTTPS redirect is okay though).

@znerol i´m using aegir for my site hosting so maybe there is also something to do.

i changed my php.ini settings as u explained in your comment before but the problem is the same.

when i try to login i got the error in log

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'xxx' for key 'PRIMARY': UPDATE {sessions} SET sid=:db_update_placeholder_0,

and my cart is empty.

@thalemn did you find a way to get this working?

thank you.

@cwarts
Update: No we do not have a working solution. I thought the .htaccess file code I previously posted would fix the issue, but it didn't.
I think we should reopen this issue status to not fixed.

Setup can be complicated depending on the deployment and the sites requirements. I'll try to walk you through this.

Lax or None?

First step is to determine whether you need SameSite=Lax or SameSide=None.

• If your payment provider uses a GET request to redirect users back to your site, then choose SameSite=Lax. This option results in better protection for session cookies.
• If your payment provider uses a POST request to redirect users back to your site, then you need SameSite=None. This is also the correct option if you have multiple providers and they use a mix of both GET and POST return URLs.

Easy: SameSite=Lax

Setup and testing are quite easy.

For PHP>=7.3

Add ini_set('session.cookie_samesite', 'Lax'); to your settings.php preferably near the other session related settings.

For PHP<7.3

Add $conf['samesite_cookie_value'] = 'Lax'; to your settings.php.

The effects of this change can be tested easily even in environments lacking HTTPS support (which is most probably the case on a dev-setup). I do not know of any brower incompatibilities (please correct me if I'm wrong).

Hard: SameSite=None

First try to upgrade your payment process such that it is compatibly with SameSite=Lax. This can be done with many payment providers by switching to Instant Payment Notifications (IPN). In that case, the payment service provider will communicate any changes of the payment status back to the site using a server-to-server request. In such a scenario it is not necessary anymore to include payment details in the return URL and thus that call can be downgraded to a GET request and thus will be compatible with SameSite=Lax.

Setup and testing is a bit more complicated. This is because:

Cookies with SameSite=None must now also specify the Secure attribute (they require a secure context/HTTPS).

See: MDN.

This means that before trying to add SameSite attribute you need to ensure that the site is available via HTTPS and the site sets its session cookies with the Secure attribute. It follows that this cannot be tested anymore on the typical local dev-setup (unless you run HTTPS on the dev-machine of course).

Pull up developer tools and inspect the session cookie of your site. If the Secure attribute is not present, then stop here and fix that first. Make sure you do not enable mixed-mode.

If everything is okay, then configure the SameSite cookie attribute like this:

For PHP>=7.3

Add ini_set('session.cookie_samesite', 'None') to your settings.php preferrably near the other session related settings.

For PHP<7.3

Add $conf['samesite_cookie_value'] = 'None'; to your settings.php.

If that works, then check whether your audience is using buggy clients. If that is the case, then you can try @nullkernel SameSite Cookie module which includes browser sniffing code. A working HTTPS setup and a properly flagged Secure session is of course still required.

SameSite=Strict why not?

For completeness, if you specify SameSite=Strict, then every time somebody clicks on a link to your site, they will end up there without a session. Even if they've been logged in before (or when they had a cart before). Thus this option is really only useful for static sites which do not allow any end-user interaction.

Still have issues?

If you have problems following this steps or if your site is behaving in unexpected ways, then please consider opening a support request. Also, in order to check whether the problem is site-specific or deployment-specific, consider deploying a fresh Drupal standard installation and try to reproduce the problem there.

Thanks a lot for the great explanation. I think it'll be very useful for anyone facing a similar issue.

Supose you have an e-commerce site with multiple payment methods, one redirects the user using a GET request after payment and the other redirects the user using a POST request.

Is there a solution in that case? Could the 2 payment methods work or it's only possible to make one of them to work?

> You'd need to use SameSite=None for that case. It's less restrictive than Lax.

Correct. I updated that. Also added a recommendation to try switching to a payment flow which does work with SameSite=Lax first.

Hello everybody,

on weekend i have time to try some configurations. i ended up with this two lines in my settings.php

$conf['samesite_cookie_value'] = 'None';
$conf['https'] = FALSE;

i make several orders with anonymous and logged in user - all with paypal wps - and everything is working now.

i updated my live sites to this configuration and will have a look on it the next days.

thanks everybody for your help and hints on this.

I've been doing some testing using BrowserStack.com and I have had issues using Edge on Windows 10.
My Drupal 7 site is using PHP 7.4, Core 7.79.
The screenshot shows that the Request Session cookie is SameSite: None, and secure.
After completing the order, the Response Session cookie is deleted (reply) (SameSite: None and secure)
Any ideas why this is happening? Let me know if you need more information about this issue.

The issue has been resolved by upgrading the simplesamlphp library to 1.19.0. This upgrade provides a means to set the SameSite cookie attribute to none (and secure). This appears to resolve our issues with a cookie being deleted after processing the payment and returning to the Drupal 7 Commerce site.

The recent conversation above is helpful. However, our setup is not related to payments, but rather our proxy system. We have a proxy server set up to serve some of the paths at our domain from a Drupal 7 instance, while serving other paths from a D9 instance. If a user is logged into D7, then visits a proxied D9 page, the session is removed and they can no longer edit the site. I tried adding $conf['samesite_cookie_value'] = 'Lax';, but that didn't resolve the issue.

Given that the session cookie gets clobbered when the user hits the D9 page, as opposed to when the user returns to a D7 path, it could be that there is something we need to do in D9 codebase to prevent it. Any suggestions would be greatly appreciated.