From 4d25aee5587b167e1676c8e59147b13b18a9a02d Mon Sep 17 00:00:00 2001
From: Benjamin Mack <benni@typo3.org>
Date: Sat, 1 Feb 2014 13:48:13 +0100
Subject: [PATCH] [TASK] Only set FE user cookie if session data or user logged
in
Currently the FE session cookie is set on every request
and since 4.2 the sessionID is generated again on every
request unless the user is logged in. This is implemented
for avoiding the security problem of the
session fixation (see #19831).
If an installation does not use FE session cookies at all,
an option (TYPO3_CONF_VARS->FE->dontSetCookie)
never sets the cookie.
As the current behavior for non-logged-in FE calls
is not usable, the behaviour is changed to only set
the cookie if the user is logged in or the session data
is modified. The last example is helpful for websites
with e.g. a shopping cart on non-logged-in pages.
Currently, if an extension is trying to implement the
latter, the extension needs to hook or XCLASS the
FrontendUserAuthentication class to set the cookie
whenever needed.
Additionally, the security problem still exists if the
cookie is not set by TYPO3 itself, that's why the
cookie can only be set if there is a valid entry in
fe_user_sessions.
if using external caching (e.g. reverse proxies),
a "unneeded" cookie is always set currently,
which extensions like EXT:moc_varnish or
EXT:cachinfo mock to only set the cookie
if needed.
The attached patch removes the default-setting
of a cookie in the frontend, and only triggers
the setcookie() function when sessionData is
added or a user is logged-in.
Resolves: #55549
Releases: 6.2
Change-Id: If478bc00c2c55dda0cc38a898a1288098891671f
Reviewed-on: https://review.typo3.org/27230
Reviewed-by: Markus Klein
Tested-by: Markus Klein
Reviewed-by: Wouter Wolters
Tested-by: Wouter Wolters
Reviewed-by: Benjamin Mack
Tested-by: Benjamin Mack
---
NEWS.md | 14 +++++-
.../AbstractUserAuthentication.php | 44 ++++++++++++++++---
.../Configuration/DefaultConfiguration.php | 1 -
.../Controller/FrontendLoginController.php | 2 +-
.../FrontendUserAuthentication.php | 41 ++++++++++++++++-
.../TypoScriptFrontendController.php | 3 --
.../SilentConfigurationUpgradeService.php | 2 +
7 files changed, 93 insertions(+), 14 deletions(-)
diff --git a/NEWS.md b/NEWS.md
index 70b85b100ff7..a36b97df3332 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -146,6 +146,18 @@ The extension works with the same options as before.
Previously $row['cache_data'] was a serialized array. To avoid double serializing and unserializing,
from now on $row['cache_data'] is just reconstituted as array when fetching from cache.
+* Frontend Cookie now only set when needed, not set by default anymore
+
+The cookie "fe_typo_user" set in the frontend by each request, is now only
+being set if the session data is used via $TSFE->fe_user->setKey('ses')
+so it can be used for shopping baskets for non-logged-in users
+out-of-the-box without hacking the default behaviour of setting the
+cookie.
+The previous behaviour always set the "fe_typo_user" cookie, but changed
+the session ID on each request, until it was fixated by a user login.
+The superfluous option "dontSetCookie" is now ineffective as the cookie
+is not set anymore by default.
+
### Administration / Customization
* Content-length header (TypoScript setting config.enableContentLengthHeader)
@@ -191,4 +203,4 @@ Pages and content elements are now categorizable by default.
* New menu types
The "Special Menus" content element type now offers the possibility to display
-a list of categorized pages or content elements.
\ No newline at end of file
+a list of categorized pages or content elements.
diff --git a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
index 3ec45b9b7906..b97cc41614e8 100644
--- a/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
+++ b/typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
@@ -274,6 +274,7 @@ abstract class AbstractUserAuthentication {
// to come from $_COOKIE).
/**
* @todo Define visibility
+ * @deprecated since TYPO3 CMS 6.2, remove two versions later, use $this->isCookieSet() instead
*/
public $cookieId;
@@ -315,12 +316,19 @@ abstract class AbstractUserAuthentication {
*/
public $forceSetCookie = FALSE;
- // Will prevent the setting of the session cookie (takes precedence over forceSetCookie)
/**
+ * Will prevent the setting of the session cookie (takes precedence over forceSetCookie)
+ * @var bool
* @todo Define visibility
*/
public $dontSetCookie = FALSE;
+ /**
+ * is set to know on this current request if a cookie was set
+ * @var bool
+ */
+ protected $cookieWasSetOnCurrentRequest = FALSE;
+
// If set, the challenge value will be stored in a session as well so the
// server can check that is was not forged.
/**
@@ -390,10 +398,7 @@ abstract class AbstractUserAuthentication {
// $id is set to ses_id if cookie is present. Else set to FALSE, which will start a new session
$id = $this->getCookie($this->name);
$this->svConfig = $GLOBALS['TYPO3_CONF_VARS']['SVCONF']['auth'];
- // If we have a flash client, take the ID from the GP
- if (!$id && $GLOBALS['CLIENT']['BROWSER'] == 'flash') {
- $id = GeneralUtility::_GP($this->name);
- }
+
// If fallback to get mode....
if (!$id && $this->getFallBack && $this->get_name) {
$id = isset($_GET[$this->get_name]) ? GeneralUtility::_GET($this->get_name) : '';
@@ -402,7 +407,7 @@ abstract class AbstractUserAuthentication {
}
$mode = 'get';
}
- $this->cookieId = $id;
+
// If new session or client tries to fix session...
if (!$id || !$this->isExistingSessionRecord($id)) {
// New random session-$id is made
@@ -470,6 +475,7 @@ abstract class AbstractUserAuthentication {
* Sets the session cookie for the current disposal.
*
* @return void
+ * @throws \TYPO3\CMS\Core\Exception
*/
protected function setSessionCookie() {
$isSetSessionCookie = $this->isSetSessionCookie();
@@ -489,6 +495,7 @@ abstract class AbstractUserAuthentication {
// Do not set cookie if cookieSecure is set to "1" (force HTTPS) and no secure channel is used:
if ((int)$settings['cookieSecure'] !== 1 || GeneralUtility::getIndpEnv('TYPO3_SSL')) {
setcookie($this->name, $this->id, $cookieExpire, $cookiePath, $cookieDomain, $cookieSecure, $cookieHttpOnly);
+ $this->cookieWasSetOnCurrentRequest = TRUE;
} else {
throw new \TYPO3\CMS\Core\Exception('Cookie was not set since HTTPS was forced in $TYPO3_CONF_VARS[SYS][cookieSecure].', 1254325546);
}
@@ -543,6 +550,7 @@ abstract class AbstractUserAuthentication {
* @return string The value stored in the cookie
*/
protected function getCookie($cookieName) {
+ $cookieValue = '';
if (isset($_SERVER['HTTP_COOKIE'])) {
$cookies = GeneralUtility::trimExplode(';', $_SERVER['HTTP_COOKIE']);
foreach ($cookies as $cookie) {
@@ -980,6 +988,19 @@ abstract class AbstractUserAuthentication {
}
}
+ /**
+ * Empty / unset the cookie
+ *
+ * @param string $cookieName usually, this is $this->name
+ * @return void
+ */
+ public function removeCookie($cookieName) {
+ $cookieDomain = $this->getCookieDomain();
+ // If no cookie domain is set, use the base path
+ $cookiePath = $cookieDomain ? '/' : GeneralUtility::getIndpEnv('TYPO3_SITE_PATH');
+ setcookie($cookieName, NULL, -1, $cookiePath, $cookieDomain);
+ }
+
/**
* Determine whether there's an according session record to a given session_id
* in the database. Don't care if session record is still valid or not.
@@ -996,6 +1017,17 @@ abstract class AbstractUserAuthentication {
return $row[0] ? TRUE : FALSE;
}
+ /**
+ * Returns whether this request is going to set a cookie
+ * or a cookie was already found in the system
+ * replaces the old functionality for "$this->cookieId"
+ *
+ * @return boolean Returns TRUE if a cookie is set
+ */
+ public function isCookieSet() {
+ return $this->cookieWasSetOnCurrentRequest || $this->getCookie($this->name);
+ }
+
/*************************
*
* SQL Functions
diff --git a/typo3/sysext/core/Configuration/DefaultConfiguration.php b/typo3/sysext/core/Configuration/DefaultConfiguration.php
index dea48e1b2708..a9b829dabb67 100644
--- a/typo3/sysext/core/Configuration/DefaultConfiguration.php
+++ b/typo3/sysext/core/Configuration/DefaultConfiguration.php
@@ -686,7 +686,6 @@ return array(
'defaultTypoScript_constants.' => array(), // Lines of TS to include after a static template with the uid = the index in the array (Constants)
'defaultTypoScript_setup' => '', // String (textarea). Enter lines of default TypoScript, setup-field.
'defaultTypoScript_setup.' => array(), // Lines of TS to include after a static template with the uid = the index in the array (Setup)
- 'dontSetCookie' => FALSE, // Boolean: If set, the no cookies is attempted to be set in the front end. Of course no userlogins are possible either...
'additionalAbsRefPrefixDirectories' => '', // Enter additional directories to be prepended with absRefPrefix. Directories must be comma-separated. TYPO3 already prepends the following directories: media/, typo3conf/ext/, fileadmin/
'IPmaskMountGroups' => array( // This allows you to specify an array of IPmaskLists/fe_group-uids. If the REMOTE_ADDR of the user matches an IPmaskList, then the given fe_group is add to the gr_list. So this is an automatic mounting of a user-group. But no fe_user is logged in though! This feature is implemented for the default frontend user authentication and might not be implemented for alternative authentication services.
// array('IPmaskList_1','fe_group uid'), array('IPmaskList_2','fe_group uid')
diff --git a/typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php b/typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php
index 17ef8892b171..d39a72ca6700 100644
--- a/typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php
+++ b/typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php
@@ -180,7 +180,7 @@ class FrontendLoginController extends \TYPO3\CMS\Frontend\Plugin\AbstractPlugin
}
// Process the redirect
if (($this->logintype === 'login' || $this->logintype === 'logout') && $this->redirectUrl && !$this->noRedirect) {
- if (!$GLOBALS['TSFE']->fe_user->cookieId) {
+ if (!$GLOBALS['TSFE']->fe_user->isCookieSet()) {
$content .= $this->cObj->stdWrap($this->pi_getLL('cookie_warning', '', TRUE), $this->conf['cookieWarning_stdWrap.']);
} else {
// Add hook for extra processing before redirect
diff --git a/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php b/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php
index 8fa3b2538a54..95614355ef5c 100644
--- a/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php
+++ b/typo3/sysext/frontend/Classes/Authentication/FrontendUserAuthentication.php
@@ -113,6 +113,10 @@ class FrontendUserAuthentication extends \TYPO3\CMS\Core\Authentication\Abstract
* Default constructor.
*/
public function __construct() {
+ // Disable cookie by default, will be activated if saveSessionData() is called,
+ // a user is logging-in or an existing session is found
+ $this->dontSetCookie = TRUE;
+
$this->session_table = 'fe_sessions';
$this->name = self::getCookieName();
$this->get_name = 'ftu';
@@ -239,6 +243,19 @@ class FrontendUserAuthentication extends \TYPO3\CMS\Core\Authentication\Abstract
return $loginData;
}
+ /**
+ * Creates a user session record and returns its values.
+ * However, as the FE user cookie is normally not set, this has to be done
+ * before the parent class is doing the rest.
+ *
+ * @param array $tempuser User data array
+ * @return array The session data for the newly created session.
+ */
+ public function createUserSession($tempuser) {
+ $this->setSessionCookie();
+ return parent::createUserSession($tempuser);
+ }
+
/**
* Will select all fe_groups records that the current fe_user is member of - and which groups are also allowed in the current domain.
* It also accumulates the TSconfig for the fe_user/fe_groups in ->TSdataArray
@@ -396,6 +413,10 @@ class FrontendUserAuthentication extends \TYPO3\CMS\Core\Authentication\Abstract
if (empty($this->sesData)) {
// Remove session-data
$this->removeSessionData();
+ // Remove cookie if not logged in as the session data is removed as well
+ if (!empty($this->user['uid'])) {
+ $this->removeCookie($this->name);
+ }
} elseif ($this->sessionDataTimestamp === NULL) {
// Write new session-data
$insertFields = array(
@@ -405,6 +426,8 @@ class FrontendUserAuthentication extends \TYPO3\CMS\Core\Authentication\Abstract
);
$this->sessionDataTimestamp = $GLOBALS['EXEC_TIME'];
$GLOBALS['TYPO3_DB']->exec_INSERTquery('fe_session_data', $insertFields);
+ // Now set the cookie (= fix the session)
+ $this->setSessionCookie();
} else {
// Update session data
$updateFields = array(
@@ -426,6 +449,20 @@ class FrontendUserAuthentication extends \TYPO3\CMS\Core\Authentication\Abstract
$GLOBALS['TYPO3_DB']->exec_DELETEquery('fe_session_data', 'hash=' . $GLOBALS['TYPO3_DB']->fullQuoteStr($this->id, 'fe_session_data'));
}
+ /**
+ * Log out current user!
+ * Removes the current session record, sets the internal ->user array to a blank string
+ * Thereby the current user (if any) is effectively logged out!
+ * Additionally the cookie is removed
+ *
+ * @return void
+ */
+ public function logoff() {
+ parent::logoff();
+ // Remove the cookie on log-off
+ $this->removeCookie($this->name);
+ }
+
/**
* Executes the garbage collection of session data and session.
* The lifetime of session data is defined by $TYPO3_CONF_VARS['FE']['sessionDataLifetime'].
@@ -532,10 +569,10 @@ class FrontendUserAuthentication extends \TYPO3\CMS\Core\Authentication\Abstract
* @todo Define visibility
*/
public function record_registration($recs, $maxSizeOfSessionData = 0) {
- // Storing value ONLY if there is a confirmed cookie set (->cookieID),
+ // Storing value ONLY if there is a confirmed cookie set,
// otherwise a shellscript could easily be spamming the fe_sessions table
// with bogus content and thus bloat the database
- if (!$maxSizeOfSessionData || $this->cookieId) {
+ if (!$maxSizeOfSessionData || $this->isCookieSet()) {
if ($recs['clear_all']) {
$this->setKey('ses', 'recs', array());
}
diff --git a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
index 2ce01170f332..6b6d3c66c552 100644
--- a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
+++ b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
@@ -929,9 +929,6 @@ class TypoScriptFrontendController {
unset($cookieName);
}
}
- if ($this->TYPO3_CONF_VARS['FE']['dontSetCookie']) {
- $this->fe_user->dontSetCookie = 1;
- }
$this->fe_user->start();
$this->fe_user->unpack_uc('');
// Gets session data
diff --git a/typo3/sysext/install/Classes/Service/SilentConfigurationUpgradeService.php b/typo3/sysext/install/Classes/Service/SilentConfigurationUpgradeService.php
index f62e39810b73..1c34c57d8d56 100644
--- a/typo3/sysext/install/Classes/Service/SilentConfigurationUpgradeService.php
+++ b/typo3/sysext/install/Classes/Service/SilentConfigurationUpgradeService.php
@@ -86,6 +86,8 @@ class SilentConfigurationUpgradeService {
'FE/simulateStaticDocuments',
// #52786
'FE/logfile_dir',
+ // #55549
+ 'FE/dontSetCookie',
// #52011
'GFX/im_combine_filename',
// #52088
--
GitLab