From d58edc9edf7598e82e9f56aa1b0d0b82ef82a6dc Mon Sep 17 00:00:00 2001 From: Benni Mack <benni@typo3.org> Date: Mon, 27 Mar 2017 15:44:52 +0200 Subject: [PATCH] [TASK] Cleanup BE user authentication in Frontend The Frontend Controller (TSFE) does some initialization which should belong to the FrontendBackendUserAuthentication directly. Additionally, some further code cleanups are made for this area. Resolves: #80479 Releases: master Change-Id: I32d9c98bb511f1eaa1f56203c4678358107aa7bc Reviewed-on: https://review.typo3.org/52177 Tested-by: TYPO3com <no-reply@typo3.com> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Frank Naegler <frank.naegler@typo3.org> Tested-by: Frank Naegler <frank.naegler@typo3.org> --- .../FrontendBackendUserAuthentication.php | 46 +++++++++++-------- .../BackendUserAuthentication.php | 21 ++++----- .../TypoScriptFrontendController.php | 38 +++++++-------- 3 files changed, 53 insertions(+), 52 deletions(-) diff --git a/typo3/sysext/backend/Classes/FrontendBackendUserAuthentication.php b/typo3/sysext/backend/Classes/FrontendBackendUserAuthentication.php index d6b663446d8e..0717e13cca81 100644 --- a/typo3/sysext/backend/Classes/FrontendBackendUserAuthentication.php +++ b/typo3/sysext/backend/Classes/FrontendBackendUserAuthentication.php @@ -14,11 +14,13 @@ namespace TYPO3\CMS\Backend; * The TYPO3 project - inspiring people to share! */ +use TYPO3\CMS\Core\Authentication\BackendUserAuthentication; use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface; use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Database\Query\QueryBuilder; use TYPO3\CMS\Core\Database\Query\QueryHelper; use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction; +use TYPO3\CMS\Core\Type\Bitmask\Permission; use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Lang\LanguageService; @@ -26,7 +28,7 @@ use TYPO3\CMS\Lang\LanguageService; * TYPO3 backend user authentication in the TSFE frontend. * This includes mainly functions related to the Admin Panel */ -class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\BackendUserAuthentication +class FrontendBackendUserAuthentication extends BackendUserAuthentication { /** * Form field with login name. @@ -42,6 +44,14 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B */ public $formfield_uident = ''; + /** + * Formfield_status should be set to "". The value this->formfield_status is set to empty in order to + * disable login-attempts to the backend account through this script + * + * @var string + */ + public $formfield_status = ''; + /** * Decides if the writelog() function is called at login and logout. * @@ -142,8 +152,8 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B { return $this->extAdmEnabled && ( $this->adminPanel->isAdminModuleEnabled('edit') || - $GLOBALS['TSFE']->displayEditIcons == 1 || - $GLOBALS['TSFE']->displayFieldEditIcons == 1 + (int)$GLOBALS['TSFE']->displayEditIcons === 1 || + (int)$GLOBALS['TSFE']->displayFieldEditIcons === 1 ); } @@ -160,7 +170,7 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B /** * Determines whether the admin panel is enabled and visible. * - * @return bool Whether the admin panel is enabled and visible + * @return bool true if the admin panel is enabled and visible */ public function isAdminPanelVisible() { @@ -186,28 +196,29 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B } // Check IP if (trim($GLOBALS['TYPO3_CONF_VARS']['BE']['IPmaskList'])) { - $remoteAddress = GeneralUtility::getIndpEnv('REMOTE_ADDR'); - if (!GeneralUtility::cmpIP($remoteAddress, $GLOBALS['TYPO3_CONF_VARS']['BE']['IPmaskList'])) { + if (!GeneralUtility::cmpIP(GeneralUtility::getIndpEnv('REMOTE_ADDR'), $GLOBALS['TYPO3_CONF_VARS']['BE']['IPmaskList'])) { return false; } } - // Check SSL (https) - if ((bool)$GLOBALS['TYPO3_CONF_VARS']['BE']['lockSSL'] && !GeneralUtility::getIndpEnv('TYPO3_SSL')) { + // Check IP mask based on TSconfig + if (!$this->checkLockToIP()) { return false; } - // Finally a check from \TYPO3\CMS\Core\Authentication\BackendUserAuthentication::backendCheckLogin() - if ($this->isUserAllowedToLogin()) { - return true; - } else { + // Check SSL (https) + if ((bool)$GLOBALS['TYPO3_CONF_VARS']['BE']['lockSSL'] && !GeneralUtility::getIndpEnv('TYPO3_SSL')) { return false; } + // Finally a check as in BackendUserAuthentication::backendCheckLogin() + return $this->isUserAllowedToLogin(); } /** * Evaluates if the Backend User has read access to the input page record. * The evaluation is based on both read-permission and whether the page is found in one of the users webmounts. - * Only if both conditions are TRUE will the function return TRUE. + * Only if both conditions match, will the function return TRUE. + * * Read access means that previewing is allowed etc. + * * Used in \TYPO3\CMS\Frontend\Http\RequestHandler * * @param array $pageRec The page record to evaluate for @@ -215,7 +226,7 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B */ public function extPageReadAccess($pageRec) { - return $this->isInWebMount($pageRec['uid']) && $this->doesUserHaveAccess($pageRec, 1); + return $this->isInWebMount($pageRec['uid']) && $this->doesUserHaveAccess($pageRec, Permission::PAGE_SHOW); } /***************************************************** @@ -280,7 +291,7 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B * Returns the number of cached pages for a page id. * * @param int $pageId The page id. - * @return int The number of pages for this page in the table "cache_pages + * @return int The number of pages for this page in the "cache_pages" cache */ public function extGetNumberOfCachedPages($pageId) { @@ -299,10 +310,10 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B * Returns the label for key. If a translation for the language set in $this->uc['lang'] * is found that is returned, otherwise the default value. * If the global variable $LOCAL_LANG is NOT an array (yet) then this function loads - * the global $LOCAL_LANG array with the content of "sysext/lang/Resources/Private/Language/locallang_tsfe.xlf" + * the global $LOCAL_LANG array with the content of "EXT:lang/Resources/Private/Language/locallang_tsfe.xlf" * such that the values therein can be used for labels in the Admin Panel * - * @param string $key Key for a label in the $GLOBALS['LOCAL_LANG'] array of "sysext/lang/Resources/Private/Language/locallang_tsfe.xlf + * @param string $key Key for a label in the $GLOBALS['LOCAL_LANG'] array of "EXT:lang/Resources/Private/Language/locallang_tsfe.xlf * @return string The value for the $key */ public function extGetLL($key) @@ -313,7 +324,6 @@ class FrontendBackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\B $GLOBALS['LOCAL_LANG'] = []; } } - // Return the label string in the default backend output charset. return htmlspecialchars($this->getLanguageService()->getLL($key)); } diff --git a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php index 85dba0ea6fdf..7125d4d1336d 100644 --- a/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php +++ b/typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php @@ -36,7 +36,7 @@ use TYPO3\CMS\Core\Utility\GeneralUtility; * This class contains the configuration of the database fields used plus some * functions for the authentication process of backend users. */ -class BackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\AbstractUserAuthentication +class BackendUserAuthentication extends AbstractUserAuthentication { /** * Should be set to the usergroup-column (id-list) in the user-record @@ -589,12 +589,8 @@ class BackendUserAuthentication extends \TYPO3\CMS\Core\Authentication\AbstractU */ public function check($type, $value) { - if (isset($this->groupData[$type])) { - if ($this->isAdmin() || GeneralUtility::inList($this->groupData[$type], $value)) { - return true; - } - } - return false; + return isset($this->groupData[$type]) + && ($this->isAdmin() || GeneralUtility::inList($this->groupData[$type], $value)); } /** @@ -2480,21 +2476,20 @@ This is a dump of the failures: /** * If TYPO3_CONF_VARS['BE']['enabledBeUserIPLock'] is enabled and * an IP-list is found in the User TSconfig objString "options.lockToIP", - * then make an IP comparison with REMOTE_ADDR and return the outcome (TRUE/FALSE) + * then make an IP comparison with REMOTE_ADDR and check if the IP address matches * - * @return bool TRUE, if IP address validates OK (or no check is done at all) + * @return bool TRUE, if IP address validates OK (or no check is done at all because no restriction is set) */ public function checkLockToIP() { - $out = 1; + $isValid = true; if ($GLOBALS['TYPO3_CONF_VARS']['BE']['enabledBeUserIPLock']) { $IPList = $this->getTSConfigVal('options.lockToIP'); if (trim($IPList)) { - $baseIP = GeneralUtility::getIndpEnv('REMOTE_ADDR'); - $out = GeneralUtility::cmpIP($baseIP, $IPList); + $isValid = GeneralUtility::cmpIP(GeneralUtility::getIndpEnv('REMOTE_ADDR'), $IPList); } } - return $out; + return $isValid; } /** diff --git a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php index 6a25bf32e527..fcb0ec1c614f 100644 --- a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php +++ b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php @@ -1163,32 +1163,28 @@ class TypoScriptFrontendController GeneralUtility::callUserFunction($_funcRef, $_params, $this); } } - /** @var $BE_USER FrontendBackendUserAuthentication */ - $BE_USER = null; + $backendUserObject = null; // If the backend cookie is set, // we proceed and check if a backend user is logged in. if ($_COOKIE[BackendUserAuthentication::getCookieName()]) { $GLOBALS['TYPO3_MISC']['microtime_BE_USER_start'] = microtime(true); $this->getTimeTracker()->push('Back End user initialized', ''); - // @todo validate the comment below: is this necessary? if so, - // formfield_status should be set to "" in \TYPO3\CMS\Backend\FrontendBackendUserAuthentication - // which is a subclass of \TYPO3\CMS\Core\Authentication\BackendUserAuthentication - // ---- - // the value this->formfield_status is set to empty in order to - // disable login-attempts to the backend account through this script + $this->beUserLogin = false; // New backend user object - $BE_USER = GeneralUtility::makeInstance(FrontendBackendUserAuthentication::class); - // Object is initialized - $BE_USER->start(); - $BE_USER->unpack_uc(); - if (!empty($BE_USER->user['uid'])) { - $BE_USER->fetchGroupData(); - $this->beUserLogin = true; + $backendUserObject = GeneralUtility::makeInstance(FrontendBackendUserAuthentication::class); + $backendUserObject->start(); + $backendUserObject->unpack_uc(); + if (!empty($backendUserObject->user['uid'])) { + $backendUserObject->fetchGroupData(); } - // Unset the user initialization. - if (!$BE_USER->checkLockToIP() || !$BE_USER->checkBackendAccessSettingsFromInitPhp() || empty($BE_USER->user['uid'])) { - $BE_USER = null; - $this->beUserLogin = false; + // Unset the user initialization if any setting / restriction applies + if (!$backendUserObject->checkBackendAccessSettingsFromInitPhp()) { + $backendUserObject = null; + } elseif (!empty($backendUserObject->user['uid'])) { + // If the user is active now, let the controller know + $this->beUserLogin = true; + } else { + $backendUserObject = null; } $this->getTimeTracker()->pull(); $GLOBALS['TYPO3_MISC']['microtime_BE_USER_end'] = microtime(true); @@ -1196,13 +1192,13 @@ class TypoScriptFrontendController // POST BE_USER HOOK if (is_array($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tslib/index_ts.php']['postBeUser'])) { $_params = [ - 'BE_USER' => &$BE_USER + 'BE_USER' => &$backendUserObject ]; foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tslib/index_ts.php']['postBeUser'] as $_funcRef) { GeneralUtility::callUserFunction($_funcRef, $_params, $this); } } - return $BE_USER; + return $backendUserObject; } /** -- GitLab