Merge branch 'B7634SessionTimeoutZuKurz' into 'master'

B7634 session timeout zu kurz

See merge request !125

extension/Documentation/Manual.rst

@@ -642,7 +642,8 @@ FE-User: Session timeout seconds
 
 There is no timeout for website users who are not logged in (but typically those users don't have access to protected content).
 
-For logged in users, the default timeout is 1800 seconds. These timeout only affects QFQ related content and can be
+For logged in users, the default timeout is the php.ini settings for `session.cookie_lifetime` and `session.gc_maxlifetime`
+(minimum of both). These timeout only affects QFQ related content and can be
 specified a) globally (QFQ configuration) and b) specific per Form.
 
 The maximum timeout depends on the minimal value of php.ini `session.cookie_lifetime` and `session.gc_maxlifetime`.

extension/Source/core/Constants.php

@@ -22,7 +22,6 @@ const QFQ_TEMP_SOURCE = '.temp.source';
 
 const MAX_LENGTH_IPV6 = 45;
 const LENGTH_HEX_COLOR = 6; // 'ffeedd'
-const SESSION_LIFETIME_SECONDS = 86400;
 const SESSION_NAME = 'qfq';
 const SESSION_FE_USER_UID = 'feUserUid';
 const SESSION_FE_USER = 'feUser';
@@ -580,7 +579,7 @@ const SYSTEM_RECORD_LOCK_TIMEOUT_SECONDS = 'recordLockTimeoutSeconds';
 const SYSTEM_RECORD_LOCK_TIMEOUT_SECONDS_DEFAULT = 900; // 15 mins
 
 const SYSTEM_SESSION_TIMEOUT_SECONDS = 'sessionTimeoutSeconds';
-const SYSTEM_SESSION_TIMEOUT_SECONDS_DEFAULT = 1800; // 30 mins
+const SYSTEM_SESSION_TIMEOUT_SECONDS_DEFAULT = 86400; // 30 mins
 
 const SYSTEM_COOKIE_LIFETIME = 259200; // 3 days. Should be more than  SYSTEM_SESSION_TIMEOUT_SECONDS_DEFAULT, in case the user setup's a higher value.
 
@@ -1542,9 +1541,9 @@ const HTML2PDF_PAGEID = 'id';
 const HTML2PDF_PARAM_GET = 'paramGet';
 const HTML2PDF_URL_PRINT = 'urlPrint';
 
-const SESSION_COOKIE_PREFEIX = 'qfq.cookie.'; // temporary 'cookie file' to forward `fe_user` and `qfq` session.
+const SESSION_COOKIE_PREFIX = 'qfq.cookie.'; // temporary 'cookie file' to forward `fe_user` and `qfq` session.
 const SESSION_LAST_ACTIVITY = 'lastActivity';
-const SESSION_LAST_FE_COOKIE = 'lastFeCookie';
+const SESSION_LAST_COOKIE_FE = 'lastCookieFe';
 
 // Class: LINK
 const PARAM_DELIMITER = '|';

extension/Source/core/QuickFormQuery.php

@@ -168,7 +168,8 @@ class QuickFormQuery {
 
         $this->store = Store::getInstance($bodytext, $phpUnit);
 
-        if (Session::getAndDestroyFlagFeUserHasChanged()) {
+        // If an FE user logs out and a different user logs in (same browser session) - the old values has to be destroyed!
+        if (Session::getAndDestroyFlagFeUserHasChanged() || Session::checkSessionExpired($timeout)) {
             $this->store->unsetStore(STORE_USER);
         }
 
@@ -364,6 +365,7 @@ class QuickFormQuery {
             }
         }
 
+        // Session Expire happens quite late, cause it can be configured per form.
         Session::checkSessionExpired($this->formSpec[F_SESSION_TIMEOUT_SECONDS]);
 
         if ($formName !== false) {
@@ -1437,6 +1439,7 @@ class QuickFormQuery {
      */
     private function doReport() {
 
+        // Session Expire happens quite late, cause it can be configured per form.
         Session::checkSessionExpired($this->store->getVar(SYSTEM_SESSION_TIMEOUT_SECONDS, STORE_SYSTEM));
 
         $report = new Report($this->t3data, $this->evaluate, $this->phpUnit);

extension/Source/core/helper/SessionCookie.php

@@ -43,7 +43,7 @@ class SessionCookie {
         $path = $urlParts['path'];
 
         // $_COOKIES[]
-        if (false === ($this->pathFileNameCookie = tempnam(sys_get_temp_dir(), SESSION_COOKIE_PREFEIX))) {
+        if (false === ($this->pathFileNameCookie = tempnam(sys_get_temp_dir(), SESSION_COOKIE_PREFIX))) {
             throw new CodeException('Error creating output file.', ERROR_IO_CREATE_FILE);
         }
 

extension/Source/core/store/Config.php

@@ -149,7 +149,7 @@ class Config {
         self::checkForAttack($config);
 
         // Copy values to detect custom settings later
-        $config[F_FE_DATA_PATTERN_ERROR_SYSTEM]=$config[F_FE_DATA_PATTERN_ERROR];
+        $config[F_FE_DATA_PATTERN_ERROR_SYSTEM] = $config[F_FE_DATA_PATTERN_ERROR];
 
         return $config;
     }
@@ -346,7 +346,7 @@ class Config {
             SYSTEM_DB_UPDATE => SYSTEM_DB_UPDATE_AUTO,
             SYSTEM_RECORD_LOCK_TIMEOUT_SECONDS => SYSTEM_RECORD_LOCK_TIMEOUT_SECONDS_DEFAULT,
 
-            SYSTEM_SESSION_TIMEOUT_SECONDS => SYSTEM_SESSION_TIMEOUT_SECONDS_DEFAULT,
+            SYSTEM_SESSION_TIMEOUT_SECONDS => self::getPhpSessionTimeout(),
 
             SYSTEM_DOCUMENTATION_QFQ => SYSTEM_DOCUMENTATION_QFQ_URL,
             SYSTEM_ENTER_AS_SUBMIT => 1,
@@ -431,9 +431,7 @@ class Config {
      */
     public static function checkSessionTimeout($timeout) {
 
-        if (ini_get('session.gc_maxlifetime') < $timeout ||
-            ini_get('session.cookie_lifetime') < $timeout
-        ) {
+        if (self::getPhpSessionTimeout() < $timeout) {
             throw new qfq\UserFormException ("The specified timeout of $timeout seconds is higher than the PHP config 'session.gc_maxlifetime' ("
                 . ini_get('session.gc_maxlifetime') . ") and/or 'session.cookie_lifetime' ("
                 . ini_get('session.cookie_lifetime') . ")");
@@ -444,4 +442,26 @@ class Config {
 
         }
     }
+
+    /**
+     * Get the minimum of 'session.cookie_lifetime' and 'session.gc_maxlifetime'
+     * A zero means unlimited and will be limited to one day.
+     *
+     * @return int|string
+     */
+    private static function getPhpSessionTimeout() {
+        $timeout = ini_get('session.cookie_lifetime');
+        $gc_maxlifetime = ini_get('session.gc_maxlifetime');
+
+        if ($timeout == 0) {
+            $timeout = 86400;
+        }
+
+        if ($gc_maxlifetime == 0) {
+            // Just to set a limit
+            $timeout = 86400;
+        }
+
+        return $timeout > $gc_maxlifetime ? $gc_maxlifetime : $timeout;
+    }
 }
\ No newline at end of file

extension/Source/core/store/Session.php

@@ -8,12 +8,15 @@
 
 namespace qfq;
 
+require_once(__DIR__ . '/../typo3/Misc.php');
+
 
 /**
  * Class Session
  * @package qfq
  */
-class Session {
+class Session
+{
 
     private static $instance = null;
     private static $phpUnit = null;
@@ -21,7 +24,7 @@ class Session {
     private static $sessionId = null;
     private static $sessionOpen = false;
     private static $lastActivity = false;
-    private static $flagFeUserChanged = false;
+    private static $flagChangedCookieFe = false;
 
     /**
      * @param bool|false $phpUnit
@@ -115,15 +118,14 @@ class Session {
      */
     public static function destroy() {
 
+        if (isset($_COOKIE[SESSION_NAME])) {
+            unset($_COOKIE[SESSION_NAME]);
+            setcookie(SESSION_NAME, '', time() - 86400, '/'); // empty value and old timestamp
+        }
+
         session_destroy();
         $_SESSION = array();
 
-        //TODO: FE User ausloggen bei Attack - funktioniert so nicht - vermutlich sollte ein T3 Funktion aufgerufen werden!
-//        session_name('fe_typo_user');
-//        session_start();
-//        session_destroy();
-//        $_SESSION = array();
-
     }
 
     /**
@@ -144,7 +146,10 @@ class Session {
 
     /**
      * Check if the feUserUid is stored in the session (even with 'false' which indicates not logged in user).
-     *   If not, clear the session and save the feUser, feUserUid in the session.
+     *   If not,
+     *    - clear the session
+     *    - save the feUser, feUserUid in the session.
+     *
      * Check if the recent logged in feUserUid is equal to the one stored in session: If different, invalidate (clear)
      * the session and save the new feUser, feUserUid in the session. If isset($GLOBALS["TSFE"]), than we're in a T3
      * environment, else we are called as API classes and need to fake feUser / feUserUid from previous stored session.
@@ -162,49 +167,43 @@ class Session {
 
         if (isset($GLOBALS["TSFE"])) {
             // if no one is logged in: 0
-            $feUidLoggedIn = isset($GLOBALS["TSFE"]->fe_user->user["uid"]) ? $GLOBALS["TSFE"]->fe_user->user["uid"] : false;
-            $feUserSession = isset($GLOBALS["TSFE"]->fe_user->user["username"]) ? $GLOBALS["TSFE"]->fe_user->user["username"] : false;
-            $feUserGroup = isset($GLOBALS["TSFE"]->fe_user->user["usergroup"]) ? $GLOBALS["TSFE"]->fe_user->user["usergroup"] : false;
-            $beUser = isset($GLOBALS["BE_USER"]->user["username"]) ? $GLOBALS["BE_USER"]->user["username"] : false;
-
-            $cookieFeUser = ($_COOKIE['fe_typo_user']) ?? false;
-            if ($cookieFeUser !== self::get(SESSION_LAST_FE_COOKIE)) {
-                self::$flagFeUserChanged = true; // Set the flag that the FE_USER User has changed
+            $feUidLoggedIn = $GLOBALS["TSFE"]->fe_user->user["uid"] ?? false;
+            $feUserSession = $GLOBALS["TSFE"]->fe_user->user["username"] ?? false;
+            $feUserGroup = $GLOBALS["TSFE"]->fe_user->user["usergroup"] ?? false;
+            $beUser = $GLOBALS["BE_USER"]->user["username"] ?? false;
+
+            // Cookie identifier
+            $cookieFe = ($_COOKIE['fe_typo_user']) ?? false;
+            if ($cookieFe !== self::get(SESSION_LAST_COOKIE_FE)) {
+                self::$flagChangedCookieFe = true; // Set the flag that the FE_USER User has changed
+                // Update SESSION_LAST_FE_COOKIE
+                self::set(SESSION_LAST_COOKIE_FE, $cookieFe);
             }
 
             // Manage Custom Session Timeout for logged in users
             if (isset($GLOBALS["TSFE"]->fe_user->user["username"]) && isset($_COOKIE['fe_typo_user'])) {
 
-                if ($_COOKIE['fe_typo_user'] == self::get(SESSION_LAST_FE_COOKIE)) {
-                    self::$lastActivity = self::get(SESSION_LAST_ACTIVITY); // ok, still the same user is logged in: get the last activity timestamp.
-                } else {
-                    // New user: remember user
+                if (self::$flagChangedCookieFe) {
+                    // New user: start timeout timer
                     self::$lastActivity = time();
+                } else {
+                    // ok, still the same user is logged in: get the last activity timestamp to compare later against timeout.
+                    self::$lastActivity = self::get(SESSION_LAST_ACTIVITY);
                 }
             }
 
-            // Update SESSION_LAST_FE_COOKIE if cookie has changed.
-            if (self::$flagFeUserChanged) {
-                self::set(SESSION_LAST_FE_COOKIE, $cookieFeUser);
-            }
-
         } else {
             // If we are called through API there is no T3 environment. Assume nothing has changed, and fake the following check to always 'no change'.
             $feUidLoggedIn = $feUserUidSession;
         }
 
         if ($feUidLoggedIn != $feUserUidSession) {
-            // destroy existing session store
-//            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);
             Session::set(SESSION_FE_USER, $feUserSession);
             Session::set(SESSION_FE_USER_GROUP, $feUserGroup);
             Session::set(SESSION_BE_USER, $beUser);
-
-//            throw new UserFormException("FYI: Session has been cleared. Reload this page. ".
-//                "feUserUidSession:'$feUserUidSession',  feUserSession:'$feUserSession' isset(TSFE):'" . isset($GLOBALS["TSFE"]) ? 'true' : 'false' );
         }
     }
 
@@ -317,9 +316,9 @@ class Session {
         }
 
         if (time() - self::$lastActivity > $timeout) {
-            throw new UserFormException(json_encode(
-                [ERROR_MESSAGE_TO_USER => 'Your session is expired. Please logout and login again.',
-                    ERROR_MESSAGE_SUPPORT => "lastActivity:" . self::$lastActivity . ' Timeout:' . $timeout]), ERROR_SESSION_EXPIRED);
+
+            Misc::feLogOff();
+            self::destroy();
         }
     }
 
@@ -330,9 +329,9 @@ class Session {
      */
     public static function getAndDestroyFlagFeUserHasChanged() {
 
-        $flag = self::$flagFeUserChanged;
+        $flag = self::$flagChangedCookieFe;
 
-        self::$flagFeUserChanged = false;
+        self::$flagChangedCookieFe = false;
 
         return $flag;
     }

extension/Source/core/store/Store.php

@@ -260,9 +260,7 @@ class Store {
             if (defined('PHPUNIT_QFQ')) {
                 $cwd = getcwd();
                 $pos = strpos($cwd, '/typo3conf/');
-                if ($pos == false) {
-                    throw new CodeException("Directory component '/typo3conf/' not found in '$cwd'", 1);
-                }
+
                 // this means phpUnit.
                 $config[SYSTEM_SITE_PATH] = substr($cwd, 0, $pos);
                 $config[SYSTEM_EXT_PATH] = $config[SYSTEM_SITE_PATH] . $relExtDir;
@@ -387,7 +385,7 @@ class Store {
      */
     public static function setStore(array $dataArray, $store, $flagOverwrite = false) {
 
-        // Check valid Storename
+        // Check valid store name
         if (!isset(self::$sanitizeStore)) {
             throw new UserFormException("Unknown Store: $store", ERROR_UNNOWN_STORE);
         }

extension/Source/core/typo3/Misc.php

+<?php
+/**
+ * Created by PhpStorm.
+ * User: crose
+ * Date: 16.02.19
+ * Time: 18:44
+ */
+
+namespace qfq;
+
+use qfq;
+
+class Misc
+{
+    public static function feLogOff()
+    {
+        $GLOBALS['TSFE']->fe_user->logoff();
+    }
+}
\ No newline at end of file

extension/Tests/unit/core/database/AbstractDatabaseTest.php

@@ -68,14 +68,6 @@ abstract class AbstractDatabaseTest extends TestCase {
         $this->sip = new Sip(true);
         $this->sip->sipUniqId('badcaffee1234');
 
-        // SWITCH to TestDB
-//        $dbNamePhpUnit = $this->store->getVar('dbNamePhpUnit', STORE_SYSTEM . STORE_EMPTY);
-//        if ($dbNamePhpUnit == '') {
-//            $dbNamePhpUnit = $this->store->getVar('DB_1_NAME', STORE_SYSTEM) . '_phpunit';
-//            $this->store->setVar('dbNamePhpUnit', $dbNamePhpUnit, STORE_SYSTEM);
-//        }
-//        $this->store->setVar('DB_1_NAME', $dbNamePhpUnit, STORE_SYSTEM);
-//        $dbName = $this->store->getVar('DB_NAME_TEST', STORE_SYSTEM);
         $dbName = $this->store->getVar('DB_1_NAME', STORE_SYSTEM);
 
         if ($dbName == '') {

extension/Tests/unit/core/store/StoreTest.php

@@ -353,7 +353,6 @@ class StoreTest extends TestCase {
             SYSTEM_SECURITY_ATTACK_DELAY => 5,
             SYSTEM_SECURITY_SHOW_MESSAGE => '0',
             SYSTEM_SECURITY_GET_MAX_LENGTH => 50,
-            SYSTEM_SESSION_TIMEOUT_SECONDS => SYSTEM_SESSION_TIMEOUT_SECONDS_DEFAULT,
             SYSTEM_RECORD_LOCK_TIMEOUT_SECONDS => SYSTEM_RECORD_LOCK_TIMEOUT_SECONDS_DEFAULT,
             SYSTEM_ENTER_AS_SUBMIT => 1,
             SYSTEM_EDIT_FORM_PAGE => 'form',
@@ -439,7 +438,7 @@ EOT;
 
         # The following won't be checked by content, cause they will change on different installations.
 //        foreach ([ SYSTEM_SQL_LOG, SYSTEM_MAIL_LOG, SYSTEM_SITE_PATH, SYSTEM_EXT_PATH, SYSTEM_SEND_E_MAIL] as $key) {
-        foreach ([ SYSTEM_SITE_PATH, SYSTEM_EXT_PATH, SYSTEM_SEND_E_MAIL] as $key) {
+        foreach ([SYSTEM_SITE_PATH, SYSTEM_EXT_PATH, SYSTEM_SEND_E_MAIL, SYSTEM_SESSION_TIMEOUT_SECONDS] as $key) {
             $this->assertTrue(isset($config[$key]), "Missing default value for '$key' "  );
             unset ($config[$key]);
         }

extension/ext_conf_template.txt

@@ -103,8 +103,8 @@ securityShowMessage = true
 # cat=security/security; type=string; label='GET'-Parameter max length:Default is '50'. GET vars longer than 'x' character triggers an `attack-detected`.
 securityGetMaxLength = 50
 
-# cat=security/security; type=string; label=Session Timeout in seconds:Default is '1800'. After inactivity of 'x' seconds, the user has to relogin.
-sessionTimeoutSeconds = 1800
+# cat=security/security; type=string; label=Session Timeout in seconds:Default is empty to take the php.ini system value (minimum of  'session.cookie_lifetime' and 'session.gc_maxlifetime').
+sessionTimeoutSeconds =