Commit 1f5d27c2 authored by Carsten  Rose's avatar Carsten Rose
Browse files

B7634. Fixes #7634. Session Timeout zu kurz. The bug was probably not a bug,...

B7634. Fixes #7634. Session Timeout zu kurz. The bug was probably not a bug, but a too short timeout together with an misleading exception (logout and in - but reload the page also leads to valid page). The fix is 'take the system default (php.ini) and set that one'.
parent 18249401
Pipeline #1513 passed with stage
in 2 minutes and 17 seconds
......@@ -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`.
......
......@@ -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.
......@@ -1544,7 +1543,7 @@ const HTML2PDF_URL_PRINT = 'urlPrint';
const SESSION_COOKIE_PREFEIX = '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 = '|';
......
......@@ -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);
......
......@@ -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
......@@ -21,7 +21,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
......@@ -144,7 +144,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 +165,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,8 +314,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_TO_USER => 'Your session is expired.',
ERROR_MESSAGE_SUPPORT => "lastActivity:" . self::$lastActivity . ' Timeout:' . $timeout]), ERROR_SESSION_EXPIRED);
}
}
......@@ -330,9 +328,9 @@ class Session {
*/
public static function getAndDestroyFlagFeUserHasChanged() {
$flag = self::$flagFeUserChanged;
$flag = self::$flagChangedCookieFe;
self::$flagFeUserChanged = false;
self::$flagChangedCookieFe = false;
return $flag;
}
......
......@@ -387,7 +387,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);
}
......
......@@ -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]);
}
......
......@@ -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 =
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment