[TASK] Use ServerRequestInterface in LoginController

Replace _GP, getIndpEnv usages Resolves: #84274 Releases: master Change-Id: Ic90a83214b0c7cb8d4334f5b72c581968f193766 Reviewed-on: https://review.typo3.org/56176 Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: TYPO3com <no-reply@typo3.com>
TYPO3 · Mar 15, 2018 · 0340961 · 0340961
1 parent 649b99d
commit 0340961
Show file tree

Hide file tree

Showing 3 changed files with 127 additions and 36 deletions.
diff --git a/typo3/sysext/backend/Classes/Controller/LoginController.php b/typo3/sysext/backend/Classes/Controller/LoginController.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Backend\Controller;
 
 /*
@@ -21,13 +22,15 @@
 use TYPO3\CMS\Backend\Exception;
 use TYPO3\CMS\Backend\LoginProvider\LoginProviderInterface;
 use TYPO3\CMS\Backend\Routing\UriBuilder;
+use TYPO3\CMS\Backend\Template\DocumentTemplate;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
 use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
 use TYPO3\CMS\Core\Configuration\ExtensionConfiguration;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\FormProtection\BackendFormProtection;
 use TYPO3\CMS\Core\FormProtection\FormProtectionFactory;
 use TYPO3\CMS\Core\Http\HtmlResponse;
+use TYPO3\CMS\Core\Localization\LanguageService;
 use TYPO3\CMS\Core\Localization\Locales;
 use TYPO3\CMS\Core\Page\PageRenderer;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -96,22 +99,26 @@ class LoginController implements LoggerAwareInterface
      */
     public function __construct()
     {
+        // @deprecated since v9, will be obsolete in v10
+        $request = $GLOBALS['TYPO3_REQUEST'];
+        $parsedBody = $request->getParsedBody();
+        $queryParams = $request->getQueryParams();
         $this->validateAndSortLoginProviders();
 
         // We need a PHP session session for most login levels
         session_start();
-        $this->redirectUrl = GeneralUtility::sanitizeLocalUrl(GeneralUtility::_GP('redirect_url'));
-        $this->loginProviderIdentifier = $this->detectLoginProvider();
+        $this->redirectUrl = GeneralUtility::sanitizeLocalUrl($parsedBody['redirect_url'] ?? $queryParams['redirect_url'] ?? null);
+        $this->loginProviderIdentifier = $this->detectLoginProvider($request);
 
-        $this->loginRefresh = (bool)GeneralUtility::_GP('loginRefresh');
+        $this->loginRefresh = (bool)($parsedBody['loginRefresh'] ?? $queryParams['loginRefresh'] ?? false);
         // Value of "Login" button. If set, the login button was pressed.
-        $this->submitValue = GeneralUtility::_GP('commandLI');
-
+        $this->submitValue = $parsedBody['commandLI'] ?? $queryParams['commandLI'] ?? null;
         // Try to get the preferred browser language
         /** @var Locales $locales */
         $locales = GeneralUtility::makeInstance(Locales::class);
+        $httpAcceptLanguage = $request->getServerParams()['HTTP_ACCEPT_LANGUAGE'];
         $preferredBrowserLanguage = $locales
-            ->getPreferredClientLanguage(GeneralUtility::getIndpEnv('HTTP_ACCEPT_LANGUAGE'));
+            ->getPreferredClientLanguage($httpAcceptLanguage);
 
         // If we found a $preferredBrowserLanguage and it is not the default language and no be_user is logged in
         // initialize $this->getLanguageService() again with $preferredBrowserLanguage
@@ -131,7 +138,7 @@ public function __construct()
         }
 
         // If "L" is "OUT", then any logged in is logged out. If redirect_url is given, we redirect to it
-        if (GeneralUtility::_GP('L') === 'OUT' && is_object($this->getBackendUserAuthentication())) {
+        if (($parsedBody['L'] ?? $queryParams['L'] ?? null) === 'OUT' && is_object($this->getBackendUserAuthentication())) {
             $this->getBackendUserAuthentication()->logoff();
             HttpUtility::redirect($this->redirectUrl);
         }
@@ -148,24 +155,38 @@ public function __construct()
      */
     public function formAction(ServerRequestInterface $request): ResponseInterface
     {
-        return new HtmlResponse($this->main());
+        return new HtmlResponse($this->createLoginLogoutForm($request));
     }
 
     /**
      * Main function - creating the login/logout form
      *
      * @throws Exception
      * @return string The content to output
+     * @deprecated since v9, will be removed in v10
      */
-    public function main()
+    public function main(): string
+    {
+        trigger_error('Method main() will be replaced by protected method createLoginLogoutForm() in v10. Do not call from other extension', E_USER_DEPRECATED);
+        return $this->createLoginLogoutForm($GLOBALS['TYPO3_REQUEST']);
+    }
+
+    /**
+     * Main function - creating the login/logout form
+     *
+     * @param ServerRequestInterface $request
+     * @return string $content
+     * @throws Exception
+     */
+    protected function createLoginLogoutForm(ServerRequestInterface $request): string
     {
         /** @var $pageRenderer PageRenderer */
         $pageRenderer = GeneralUtility::makeInstance(PageRenderer::class);
         $pageRenderer->loadRequireJsModule('TYPO3/CMS/Backend/Login');
 
         // Checking, if we should make a redirect.
         // Might set JavaScript in the header to close window.
-        $this->checkRedirect();
+        $this->checkRedirect($request);
 
         // Extension Configuration
         $extConf = GeneralUtility::makeInstance(ExtensionConfiguration::class)->get('backend');
@@ -235,7 +256,7 @@ public function main()
         $formType = empty($this->getBackendUserAuthentication()->user['uid']) ? 'LoginForm' : 'LogoutForm';
         $this->view->assignMultiple([
             'backendUser' => $this->getBackendUserAuthentication()->user,
-            'hasLoginError' => $this->isLoginInProgress(),
+            'hasLoginError' => $this->isLoginInProgress($request),
             'formType' => $formType,
             'logo' => $logo,
             'images' => [
@@ -250,7 +271,7 @@ public function main()
         ]);
 
         // Initialize interface selectors:
-        $this->makeInterfaceSelectorBox();
+        $this->makeInterfaceSelector($request);
 
         /** @var LoginProviderInterface $loginProvider */
         $loginProvider = GeneralUtility::makeInstance($this->loginProviders[$this->loginProviderIdentifier]['provider']);
@@ -272,14 +293,15 @@ public function main()
      *   a) if either the login is just done (isLoginInProgress) or
      *   b) a loginRefresh is done
      *
+     * @param ServerRequestInterface $request
      * @throws \RuntimeException
      * @throws \UnexpectedValueException
      */
-    protected function checkRedirect()
+    protected function checkRedirect(ServerRequestInterface $request): void
     {
         if (
             empty($this->getBackendUserAuthentication()->user['uid'])
-            && ($this->isLoginInProgress() || !$this->loginRefresh)
+            && ($this->isLoginInProgress($request) || !$this->loginRefresh)
         ) {
             return;
         }
@@ -305,18 +327,17 @@ protected function checkRedirect()
         $redirectToUrl = (string)$this->getBackendUserAuthentication()->getTSConfigVal('auth.BE.redirectToURL');
         if (empty($redirectToUrl)) {
             // Based on the interface we set the redirect script
-            switch (GeneralUtility::_GP('interface')) {
+            $parsedBody = $request->getParsedBody();
+            $queryParams = $request->getQueryParams();
+            $interface = $parsedBody['interface'] ?? $queryParams['interface'] ?? '';
+            switch ($interface) {
                 case 'frontend':
-                    $interface = 'frontend';
                     $this->redirectToURL = '../';
                     break;
                 case 'backend':
-                    $interface = 'backend';
                     $uriBuilder = GeneralUtility::makeInstance(UriBuilder::class);
                     $this->redirectToURL = (string)$uriBuilder->buildUriFromRoute('main');
                     break;
-                default:
-                    $interface = '';
             }
         } else {
             $this->redirectToURL = $redirectToUrl;
@@ -346,12 +367,24 @@ protected function checkRedirect()
     }
 
     /**
-     * Making interface selector:
+     * Making interface selector
+     *
+     * @deprecated since v9, will be removed in v10
+     */
+    public function makeInterfaceSelectorBox(): void
+    {
+        trigger_error('Method makeInterfaceSelectorBox() will be replaced by protected method makeInterfaceSelector() in v10. Do not call from other extension', E_USER_DEPRECATED);
+        $this->makeInterfaceSelector($GLOBALS['TYPO3_REQUEST']);
+    }
+
+    /**
+     * Making interface selector
+     * @param ServerRequestInterface $request
      */
-    public function makeInterfaceSelectorBox()
+    protected function makeInterfaceSelector(ServerRequestInterface $request): void
     {
         // If interfaces are defined AND no input redirect URL in GET vars:
-        if ($GLOBALS['TYPO3_CONF_VARS']['BE']['interfaces'] && ($this->isLoginInProgress() || !$this->redirectUrl)) {
+        if ($GLOBALS['TYPO3_CONF_VARS']['BE']['interfaces'] && ($this->isLoginInProgress($request) || !$this->redirectUrl)) {
             $parts = GeneralUtility::trimExplode(',', $GLOBALS['TYPO3_CONF_VARS']['BE']['interfaces']);
             if (count($parts) > 1) {
                 // Only if more than one interface is defined we will show the selector
@@ -385,7 +418,7 @@ public function makeInterfaceSelectorBox()
      *
      * @return array An array of login news.
      */
-    protected function getSystemNews()
+    protected function getSystemNews(): array
     {
         $systemNewsTable = 'sys_news';
         $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
@@ -416,7 +449,7 @@ protected function getSystemNews()
      * @return string Returns the filename of $filename if valid, otherwise blank string.
      * @internal
      */
-    private function getUriForFileName($filename)
+    private function getUriForFileName($filename): string
     {
         // Check if it's already a URL
         if (preg_match('/^(https?:)?\/\//', $filename)) {
@@ -433,11 +466,14 @@ private function getUriForFileName($filename)
     /**
      * Checks if login credentials are currently submitted
      *
+     * @param ServerRequestInterface $request
      * @return bool
      */
-    protected function isLoginInProgress()
+    protected function isLoginInProgress(ServerRequestInterface $request): bool
     {
-        $username = GeneralUtility::_GP('username');
+        $parsedBody = $request->getParsedBody();
+        $queryParams = $request->getQueryParams();
+        $username = $parsedBody['username'] ?? $queryParams['username'] ?? null;
         return !empty($username) || !empty($this->submitValue);
     }
 
@@ -497,11 +533,14 @@ protected function validateAndSortLoginProviders()
      * Detect the login provider, get from request or choose the
      * first one as default
      *
+     * @param ServerRequestInterface $request
      * @return string
      */
-    protected function detectLoginProvider()
+    protected function detectLoginProvider(ServerRequestInterface $request): string
     {
-        $loginProvider = GeneralUtility::_GP('loginProvider');
+        $parsedBody = $request->getParsedBody();
+        $queryParams = $request->getQueryParams();
+        $loginProvider = $parsedBody['loginProvider'] ?? $queryParams['loginProvider'] ?? '';
         if ((empty($loginProvider) || !isset($this->loginProviders[$loginProvider])) && !empty($_COOKIE['be_lastLoginProvider'])) {
             $loginProvider = $_COOKIE['be_lastLoginProvider'];
         }
@@ -510,9 +549,11 @@ protected function detectLoginProvider()
             $loginProvider = key($this->loginProviders);
         }
         // Use the secure option when the current request is served by a secure connection:
-        $cookieSecure = (bool)$GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieSecure'] && GeneralUtility::getIndpEnv('TYPO3_SSL');
-        setcookie('be_lastLoginProvider', $loginProvider, $GLOBALS['EXEC_TIME'] + 7776000, null, null, $cookieSecure, true); // 90 days
-        return $loginProvider;
+        $normalizedParams = $request->getAttribute('normalizedParams');
+        $isHttps = $normalizedParams->isHttps();
+        $cookieSecure = (bool)$GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieSecure'] && $isHttps;
+        setcookie('be_lastLoginProvider', (string)$loginProvider, $GLOBALS['EXEC_TIME'] + 7776000, '', '', $cookieSecure, true); // 90 days
+        return (string)$loginProvider;
     }
 
     /**
@@ -526,27 +567,27 @@ public function getLoginProviderIdentifier()
     /**
      * Returns LanguageService
      *
-     * @return \TYPO3\CMS\Core\Localization\LanguageService
+     * @return LanguageService
      */
-    protected function getLanguageService()
+    protected function getLanguageService(): LanguageService
     {
         return $GLOBALS['LANG'];
     }
 
     /**
      * @return BackendUserAuthentication
      */
-    protected function getBackendUserAuthentication()
+    protected function getBackendUserAuthentication(): BackendUserAuthentication
     {
         return $GLOBALS['BE_USER'];
     }
 
     /**
      * Returns an instance of DocumentTemplate
      *
-     * @return \TYPO3\CMS\Backend\Template\DocumentTemplate
+     * @return DocumentTemplate
      */
-    protected function getDocumentTemplate()
+    protected function getDocumentTemplate(): DocumentTemplate
     {
         return $GLOBALS['TBE_TEMPLATE'];
     }

diff --git a/...log/master/Deprecation-84274-ProtectedMethodsAndPropertiesInLoginController.rst b/...log/master/Deprecation-84274-ProtectedMethodsAndPropertiesInLoginController.rst
@@ -0,0 +1,43 @@
+.. include:: ../../Includes.txt
+
+================================================================================
+Deprecation: #84274 - Protected methods and properties in LoginController
+================================================================================
+
+See :issue:`84274`
+
+Description
+===========
+
+This file is about third party usage (consumer that call the class as well as
+signals or hooks depending on it) of :php:`TYPO3\CMS\Backend\Controller\LoginController`.
+
+All methods not used as entry points by :php:`TYPO3\CMS\Backend\Http\RouteDispatcher` will be
+removed or set to protected in v10 and throw deprecation warnings if used from a third party:
+
+* [not scanned] :php:`main()`
+* :php:`makeInterfaceSelectorBox()`
+
+
+Impact
+======
+
+Calling above method on an instance of
+:php:`LoginController` will throw a deprecation warning in v9 and a PHP fatal in v10.
+
+
+Affected Installations
+======================
+
+The extension scanner will find all usages, but may also find some false positives.  In general all extensions
+that set properties or call methods except :php:`formAction()` are affected.
+
+
+Migration
+=========
+
+In general, extensions should not instantiate and re-use controllers of the core. Existing
+usages should be rewritten to be free of calls like these.
+
+
+.. index:: Backend, PHP-API, PartiallyScanned
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
@@ -1934,4 +1934,11 @@
             'Deprecation-84295-UseServerRequestInterfaceInFileEditFileController.rst',
         ],
     ],
+    'TYPO3\CMS\Backend\Controller\LoginController->makeInterfaceSelectorBox' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-84274-ProtectedMethodsAndPropertiesInLoginController.rst',
+        ],
+    ],
 ];