Contrib.social

Older PHP versions set SameSite attribute on insecure session cookie

Open on Drupal.org →
Created on 25 May 2022, over 3 years ago
Updated 12 June 2023, over 2 years ago

Problem/Motivation

On PHP versions earlier than 7.3, Drupal 7.79 adds the SameSite attribute to the path of the secure session cookie. If $conf['https'] equals TRUE, Drupal copies the parameters of the secure session cookie to an insecure session cookie, including the SameSite attribute in the path. Because the SameSite attribute is only supposed to be set on secure cookies, some browsers, including Chrome and Firefox, reject the insecure session cookie. This causes a duplicate record in the session table. Drupal writes to the session with both cookies, but reads the session with only the secure cookie. As a result,

  • Users initially see an anonymous user page after login.
  • Status messages set after login are never displayed.

Steps to reproduce

  1. Set $conf['https'] = TRUE; in settings.php.
  2. Log in over HTTPS.

Proposed resolution

On older versions of PHP, strip the SameSite attribute from the path if the samesite option is FALSE.

🐛 Bug report
Status

Needs work

Version

7.0 ⚰️

Component
Bootstrap  →

Last updated about 2 months ago

No maintainer
Created by

🇺🇸United States darren oh Lakeland, Florida

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Merge request !2317Fix older PHP versions setting SameSite attribute on insecure session cookie → (Open) created by darren oh
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    Not currently mergeable.
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    Not currently mergeable.
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    Not currently mergeable.
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    Not currently mergeable.
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    Not currently mergeable.
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    Not currently mergeable.
  • Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    Not currently mergeable.
  • Comment over 2 years ago →
    🇸🇰Slovakia poker10

    MR is not mergeable anymore, as there were recent changes in drupal_setcookie() function.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    2,149 pass
  • Status changed to RTBC over 2 years ago
  • Comment over 2 years ago →
    🇺🇸United States darren oh Lakeland, Florida
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    2,149 pass
  • Status changed to Needs review over 2 years ago
  • Comment over 2 years ago →
    🇸🇰Slovakia poker10

    Thanks for working on this. I have tested this on PHP 8.1 and if I set $conf['https'] = TRUE;, $conf['samesite_cookie_value'] = 'None'; and login via HTTPS, my Chrome browser still complains about the insecure session cookie with SameSite attribute. Does this problem really affect only PHP versions earlier than 7.3?

    Cookies with SameSite attribute are tested heavily in SessionTestCase and I think we should add a test for this - just to check if the SameSite attribute is present only in the secure cookie. See the uploaded test-only patch. I have not tested that patch on all PHP/SSL combinations.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 2 years ago
    2,149 pass
  • Status changed to Needs work over 2 years ago
  • Comment over 2 years ago →
    🇸🇰Slovakia poker10

    I have added this issue to the list of potential candidates for the next release - but we need to verify the point raised in #7 and add a test, so that we can compare test-only and regular patch results to see PHP versions affected by this (and confirm that the patch is correct). Thanks!

  • Pipeline finished with Success
    about 1 year ago
    Total: 236s
    #220128
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024