[TASK] Cleanup BE user authentication in Frontend 77/52177/3
authorBenni Mack <benni@typo3.org>
Mon, 27 Mar 2017 13:44:52 +0000 (15:44 +0200)
committerFrank Naegler <frank.naegler@typo3.org>
Mon, 27 Mar 2017 18:56:06 +0000 (20:56 +0200)
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>
typo3/sysext/backend/Classes/FrontendBackendUserAuthentication.php
typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php
typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php

index d6b6634..0717e13 100644 (file)
@@ -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.
@@ -43,6 +45,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.
      *
      * @var bool
@@ -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));
     }
 
index 85dba0e..7125d4d 100644 (file)
@@ -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;
     }
 
     /**
index 6a25bf3..fcb0ec1 100644 (file)
@@ -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;
     }
 
     /**