Send all session cookie params when updating session expire time. #13869
Conversation
| $sessionCookieLifetime = Config::getInstance()->General['login_cookie_expire']; | ||
| setcookie(session_name(), session_id(), time() + $sessionCookieLifetime); | ||
| setcookie(session_name(), session_id(), time() + $sessionCookieLifetime, $sessionParams['path'], |
There was a problem hiding this comment.
interesting, wouldn't have thought this makes a difference. reading in docs eg for path
path= The default value is the current directory that the cookie is being set in.
but maybe the other defaults from session cookie params were not applied. Or does this work better in case user has defined a different session path when the cookie is created the first time?
I presume using the Cookie class here doesn't help much?
I'm seeing there is also @Config::getInstance()->Tracker['cookie_path'] which maybe needs to be respected?
There was a problem hiding this comment.
I think in the user's case the session cookie path was different from the default cookie path, but not completely sure.
I presume using the Cookie class here doesn't help much?
I don't think so. This resets the session cookie, and the session cookie is originally set by PHP, not through Matomo. So using Cookie would be using different logic altogether and might have some strange side effects?
I'm seeing there is also @config::getInstance()->Tracker['cookie_path'] which maybe needs to be respected?
This is for the session and the tracker doesn't use the session? Ie, the tracker cookie_path is for piwik_ignore cookies right?
There was a problem hiding this comment.
This is for the session and the tracker doesn't use the session? Ie, the tracker cookie_path is for piwik_ignore cookies right?
Yes, sorry wasn't looking right. So maybe it came from deprecating login_cookie_path? Does it make sense to apply it again? Or I see in SessionInitalizer it is still used. Getting bit confused re the paths and different usages etc.
There was a problem hiding this comment.
it's used in the deprecated SessionInitializer that creates an auth cookie, not in the core one.
We could default the session cookie path to the value of login_cookie_path in Piwik\Session. That's probably a good idea, since multiple apps could use the same PHP INI config?
There was a problem hiding this comment.
PHP might not know what path it is serving if behind a reverse proxy, so better to respect the given path.
| @@ -81,6 +81,8 @@ public static function start($options = false) | |||
| // the session data won't be deleted until the cookie expires. | |||
| @ini_set('session.gc_maxlifetime', $config->General['login_cookie_expire']); | |||
|
|
|||
| @ini_set('session.cookie_path', empty($config->General['login_cookie_path']) ? '/' : $config->General['login_cookie_path']); | |||
Refs #13852