SOLVED: MODX Session Cookie with SameSite

[josh_curtis]

Summary

I’m trying to get the MODX Session Cookie to have SameSite=Strict set.

What I’ve tried

I’ve tried modifying modx.class.php: both the setcookie call at 2606 and the session_start at 2351 but no dice. I’m using PHP 7.3 so I’m theoretically able use to the new associative array for options for both setcookie and sessionstart, but I haven’t been able to get it to take so far.

Environment

MODX 2.8 family using NGINX on MODX Cloud using Chrome’s developer tools > Application > Cookies information and looking for a check in the SameSite box in the grid.

[markh]

Relevant issue on github.

Likely needs to be added in _initSession, the session_set_cookie_params line (2597 in my install, but, replacing the options with an associative array. While you may be on 7.3, MODX also still supports older versions though, so that’s a bit tricky. So that needs a version check…

Last I checked there were also tricky things for SameSite related to browser compatibility, where some browsers may reject the cookie if it has the SameSite option, and others will reject it if it doesn’t… maybe that situation is better now?

[josh_curtis]

Thanks Mark - a bit embarrassed I didn’t manage to turn up that issue in my search.

Thanks for the pointer.

Yeah - would definitely need a version check. Needs to be done in headers before 7.3 based on info I found.

[josh_curtis]

I can’t figure out for the life of me what I’m doing wrong.

I’m just trying to prove I can influence the session/cookie by changing its name. Then I can be sure I’m working in the right place to try to influence the ‘samesite’ setting.

In my test site I’m updating modx.class.php at both the session_start call inside function startSession()
if (!session_start(array("name" => "TestName"))) {

and setcookie
setcookie("TestName", session_id(), $cookieExpiration, $cookiePath, $cookieDomain,

and I can’t get it to start a session/set a cookie with a different name thus far.

Seemingly no matter what I do in the code, the same “PHPSESSID”-named cookie reappears on the next (Incognito) page load. The session ID value is different every time, so it seems it’s a new cookie set.

If I give the session a name in the system settings at session_name, it works exactly as expected (changing the name to what I’ve provided on the next page load). But if I get even one step removed and hard code a name in the only spot in the entire code-base I can find that value being picked up:
$site_sessionname = $this->getOption('session_name', $options, '');
becomes
$site_sessionname = "TestName";
(at modx.class.php:2591 in the repo) - still no dice

I’m working from inside the manager so (I’m almost certain) I can confirm it’s the same site I’m editing AND loading so not anything that stupid going on.

Caching shouldn’t affect code in the class should it? (I’ve tried clearing cache a number of times, but not on every attempt I’ve made.)

[raffenberg]

If you don’t bother to alter core files, the following changes in modx.class.php allow the use of the samsite setting(since PHP 7.3.0). Added after line 2591 if (!empty($site_sessionname)) session_name($site_sessionname);

$cookieSamesite= $this->getOption('session_cookie_samesite', $options, 'Lax');
if(PHP_VERSION_ID < 70300) {
    session_set_cookie_params($cookieLifetime, $cookiePath, $cookieDomain, $cookieSecure, $cookieHttpOnly);
} else {
    session_set_cookie_params([
    'lifetime' => $cookieLifetime,
    'path' => $cookiePath,
    'domain' => $cookieDomain,
    'secure' => $cookieSecure,
    'httponly' => $cookieHttpOnly,
    'samesite' => $cookieSamesite
    ]);
}

Add a system-setting session_cookie_samesite with your desired setting Strict

[josh_curtis]

I’ve found the problem I reckon. Was in the right time and place all along.

Just discovered that when I save in the MODX Manager, that’s throwing a 422 which isn’t noticed by the MODX Manager as an error to report. Thought it was saving all along. Never was. Not once. FFS.

[josh_curtis]

Can confirm this works. For others playing along at home, just beware that this involves hacking the MODX core files. This is not recommended as every update you make to MODX will break this arrangement and any errors you make while doing so won’t end well for your site.

Buyer beware - but yes - this works. Thank you @raffenberg!

[markh]

Perhaps you could submit a pull request with that code @raffenberg?

[josh_curtis]

If not, I’ll have a go. Can we just get a ruling on what “support older versions of PHP” means?

Does it mean we need to keep feature parity so for the sub 7.3 users and therefore we’d need to include the sending of the relevant header from the relevant spot in the code base or is it admissible to run with this code and, say, mention in the description for the system setting “only works with PHP 7.3 and above”?

[raffenberg]

Please, if someone could submit a pull request, it will save me hours

[markh]

Judging from the source of all programming knowledge, the 2 ways <7.3 can set SameSite from PHP is by either setting a manual header() or a manual setcookie() call.

As we’re talking primarily about the MODX session cookie here, which is set automatically by PHP in the session_start call, we can’t do either of those without rewriting how sessions are created so I’d personally be okay with SameSite support requiring PHP 7.3+.

[raffenberg]

ok, it went faster than expected. i submitted a request

[josh_curtis]

Nice work @raffenberg

And thanks to Mark and Jason for the prodding and speedy reviewing.