Bug #5668 / Broken SIP after login.

Session.php: After setting the QFQ cookie lifetime AND calling setcookie() after session_start(), the QFQ session remains. Also, skipping Session::clearAll() protects against broken SIP after logout!

extension/qfq/qfq/store/Session.php

@@ -23,23 +23,31 @@ class Session {
      * @throws CodeException
      */
     private function __construct($phpUnit = false) {
+
         if (self::$phpUnit !== null) {
             throw new CodeException("Try to set flag phpunit again - that should not happen.", ERROR_CODE_SHOULD_NOT_HAPPEN);
         }
 
         self::$phpUnit = $phpUnit;
+
         if (self::$phpUnit === true) {
             self::$sessionLocal = array();
         } else {
             ini_set('session.cookie_httponly', 1);
 
+            $lifetime = 86400; // one day
+
             $path = $this->getSitePath();
-            session_set_cookie_params(0, $path);
+            session_set_cookie_params($lifetime, $path);
+            $currentCookieParams = session_get_cookie_params();
 
             session_name(SESSION_NAME);
 
             session_start();
 
+            // Currently, setcookie() is only called to really extend the lifetime. All other parameter needs to be given again.
+            setcookie(SESSION_NAME, session_id(), time() + $lifetime, $path, $currentCookieParams['domain'], $currentCookieParams['secure'], true);
+
             self::$sessionId = session_id();
         }
 
@@ -62,6 +70,7 @@ class Session {
 
         $path = $_SERVER['SCRIPT_NAME'];
         $pos = strrpos($path, '/');
+
         if ($pos === false) {
             throw new CodeException("Broken _SERVER[SCRIPT_NAME]: $path", ERROR_SESSION_BROKEN_SCRIPT_PATH);
         }
@@ -97,6 +106,7 @@ class Session {
      * Destroy a session - this is only needed in case of attacks
      */
     public static function destroy() {
+
         session_destroy();
         $_SESSION = array();
 
@@ -116,6 +126,7 @@ class Session {
      *
      */
     public static function open() {
+
         if (self::$sessionOpen != true && self::$sessionId != null) {
             session_id(self::$sessionId);
             session_start();
@@ -148,9 +159,9 @@ class Session {
             $feUidLoggedIn = $feUserUidSession;
         }
 
-        if ($feUidLoggedIn !== $feUserUidSession) {
+        if ($feUidLoggedIn != $feUserUidSession) {
             // destroy existing session store
-            Session::clearAll();
+//            Session::clearAll(); // #5668 / Broken SIP after login - is it really a security improvement to destroy the SIP store in case the feUser changes? Probably not.
 
             // save new feUserUid, feUserName
             Session::set(SESSION_FE_USER_UID, $feUidLoggedIn);