[TASK] Install Tool: Remove authentication from backend context 60/53860/7
authorBenni Mack <benni@typo3.org>
Fri, 1 Sep 2017 07:00:10 +0000 (09:00 +0200)
committerAndreas Fernandez <typo3@scripting-base.de>
Thu, 7 Sep 2017 14:40:17 +0000 (16:40 +0200)
Currently calling the install tool modules from within the Backend does a
simple redirect with adding GET variables.

That's the reason why you need to re-authenticate again, and the context
is handed over as a query parameter, which is simply not needed at all.

Now, the redirect is removed, as the Backend entrypoint / request handler
handles the authentication of the backend user, and the standalone entry
point deals with the install tool password etc.

The context parameter is now detected by the entry point (!) as well,
allowing to get rid of quite some code.

There are some more consequences:
- Calling the install tool from the backend does not validate if you configuration
is set up (= recovery necessary) -> since you're already in the backend we guess
you're fine anyway.
- Redirect functionality is almost not needed anymore in the regular request handler
- routeParameters concept was removed again (which was introduced a couple of weeks ago)

Additionally, the contextService could be replaced at a later stage with just
a string.

Resolves: #82306
Releases: master
Change-Id: If7e4ddfaccf46cf93448d06c0ba9af81d5b9494c
Reviewed-on: https://review.typo3.org/53860
Reviewed-by: Romain Canon <romain.hydrocanon@gmail.com>
Tested-by: Romain Canon <romain.hydrocanon@gmail.com>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Andreas Fernandez <typo3@scripting-base.de>
Tested-by: Andreas Fernandez <typo3@scripting-base.de>
typo3/sysext/backend/Classes/Http/BackendModuleRequestHandler.php
typo3/sysext/core/Tests/Acceptance/Backend/Install/InstallModuleCest.php [deleted file]
typo3/sysext/install/Classes/Controller/AbstractController.php
typo3/sysext/install/Classes/Controller/Action/AbstractAction.php
typo3/sysext/install/Classes/Controller/Action/ActionInterface.php
typo3/sysext/install/Classes/Controller/AjaxController.php
typo3/sysext/install/Classes/Controller/BackendModuleController.php
typo3/sysext/install/Classes/Controller/StepController.php
typo3/sysext/install/Classes/Controller/ToolController.php
typo3/sysext/install/Classes/Service/ContextService.php
typo3/sysext/install/ext_tables.php

index 64c6df3..38c5d07 100644 (file)
@@ -164,14 +164,6 @@ class BackendModuleRequestHandler implements RequestHandlerInterface
         if (isset($moduleConfiguration['routeTarget'])) {
             $dispatcher = GeneralUtility::makeInstance(Dispatcher::class);
             $this->request = $this->request->withAttribute('target', $moduleConfiguration['routeTarget']);
-            // @internal routeParameters are a helper construct for the install tool only.
-            // @todo: remove this, after sub-actions in install tool can be addressed directly
-            if (!empty($moduleConfiguration['routeParameters'])) {
-                $this->request = $this->request->withQueryParams(array_merge_recursive(
-                    $this->request->getQueryParams(),
-                    $moduleConfiguration['routeParameters']
-                ));
-            }
             $response = $dispatcher->dispatch($this->request, $response);
         } else {
             // extbase module
diff --git a/typo3/sysext/core/Tests/Acceptance/Backend/Install/InstallModuleCest.php b/typo3/sysext/core/Tests/Acceptance/Backend/Install/InstallModuleCest.php
deleted file mode 100644 (file)
index b6fbff4..0000000
+++ /dev/null
@@ -1,112 +0,0 @@
-<?php
-namespace TYPO3\CMS\Core\Tests\Acceptance\Backend\Language;
-
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-use TYPO3\TestingFramework\Core\Acceptance\Step\Backend\Admin;
-
-/**
- * Install Module tests
- */
-class InstallModuleCest
-{
-    /**
-     * @var string
-     */
-    protected $password = '';
-
-    /**
-     * @param Admin $I
-     */
-    public function _before(Admin $I)
-    {
-        $this->password = getenv('typo3InstallToolPassword');
-
-        $I->useExistingSession();
-        // Ensure main content frame is fully loaded, otherwise there are load-race-conditions
-        $I->switchToIFrame('list_frame');
-        $I->waitForText('Web Content Management System');
-        $I->switchToIFrame();
-
-        $I->see('Maintenance');
-        $I->click('Maintenance');
-
-        // switch to content iframe
-        $I->switchToIFrame('list_frame');
-    }
-
-    /**
-     * @param Admin $I
-     */
-    public function unlockAndLockInstallTool(Admin $I)
-    {
-        $I->wantTo('Check the Install Tool unlock and lock functions.');
-
-        // @todo probably there is a better solution skipping the test
-        if (empty($this->password)) {
-            $I->comment('Skip this test.');
-        } else {
-            $I->amGoingTo('unlock the install tool');
-            $I->waitForElement('#t3-install-form-unlock');
-            $I->see('The Install Tool is locked');
-            $I->see('Unlock the Install Tool');
-            $I->click('//button[@value="enableInstallTool"]');
-            $I->waitForElement('#t3-install-outer');
-            $I->see('Password');
-            $I->see('Login');
-
-            $I->amGoingTo('lock the install tool');
-            $I->see('Lock Install Tool again');
-            $I->click('Lock Install Tool again');
-            $I->see('The Install Tool is locked');
-        }
-    }
-
-    /**
-     * @param Admin $I
-     */
-    public function loginToInstallTool(Admin $I)
-    {
-        $I->wantTo('Check the Install Tool Login with wrong and right passwords.');
-
-        // @todo probably there is a better solution skipping the test
-        if (empty($this->password)) {
-            $I->comment('Skip this test.');
-        } else {
-            $I->amGoingTo('unlock the install tool');
-            $I->waitForElement('#t3-install-form-unlock');
-            $I->see('The Install Tool is locked');
-            $I->see('Unlock the Install Tool');
-            $I->click('//button[@value="enableInstallTool"]');
-            $I->waitForElement('#t3-install-outer');
-
-            $I->amGoingTo('login to install tool with wrong password');
-            $I->fillField('#t3-install-form-password', 'wrong_' . $this->password);
-            $I->click('//button[@type="submit"]');
-            $I->waitForElement('//div[@class="t3js-message typo3-message alert alert-danger"]');
-            $I->see('Login failed');
-            $I->see('Given password does not match the install tool login password.');
-            $I->see('Calculated hash:');
-
-            $I->amGoingTo('login to install tool with right password');
-            $I->fillField('#t3-install-form-password', $this->password);
-            $I->click('//button[@type="submit"]');
-            $I->waitForElement('//body[@class="backend"]');
-            $I->see('Lock Install Tool');
-
-            $I->click('Lock Install Tool');
-            $I->see('The Install Tool is locked');
-        }
-    }
-}
index b4a0303..d7e95f5 100644 (file)
@@ -44,6 +44,7 @@ class AbstractController
         $action = GeneralUtility::makeInstance(LoginForm::class);
         $action->setController('common');
         $action->setAction('login');
+        $action->setContext($request->getAttribute('context', 'standalone'));
         $action->setToken($this->generateTokenForAction('login'));
         $action->setPostValues($request->getParsedBody()['install'] ?? []);
         if ($message) {
@@ -146,13 +147,6 @@ class AbstractController
         }
         $parameters[] = 'install[redirectCount]=' . $redirectCount;
 
-        // Add context parameter in case this script was called within backend scope
-        $context = 'install[context]=standalone';
-        if (isset($getPostValues['context']) && $getPostValues['context'] === 'backend') {
-            $context = 'install[context]=backend';
-        }
-        $parameters[] = $context;
-
         // Add controller parameter
         $controllerParameter = 'install[controller]=step';
         if ((isset($getPostValues['controller']) && $getPostValues['controller'] === 'tool')
index 0bbfde1..a1fcaa1 100644 (file)
@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Install\Controller\Action;
 
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Fluid\View\StandaloneView;
+use TYPO3\CMS\Install\Service\ContextService;
 
 /**
  * General purpose controller action helper methods and bootstrap
@@ -53,6 +54,11 @@ abstract class AbstractAction implements ActionInterface
     protected $messages = [];
 
     /**
+     * @var ContextService
+     */
+    protected $contextService;
+
+    /**
      * Handles the action
      *
      * @return string Rendered content
@@ -68,9 +74,6 @@ abstract class AbstractAction implements ActionInterface
      */
     protected function initializeHandle()
     {
-        // Context service distinguishes between standalone and backend context
-        $contextService = GeneralUtility::makeInstance(\TYPO3\CMS\Install\Service\ContextService::class);
-
         $viewRootPath = GeneralUtility::getFileAbsFileName('EXT:install/Resources/Private/');
         $controllerActionDirectoryName = ucfirst($this->controller);
         $mainTemplate = ucfirst($this->action);
@@ -85,8 +88,8 @@ abstract class AbstractAction implements ActionInterface
             ->assign('action', $this->action)
             ->assign('controller', $this->controller)
             ->assign('token', $this->token)
-            ->assign('context', $contextService->getContextString())
-            ->assign('contextService', $contextService)
+            ->assign('context', $this->contextService->getContextString())
+            ->assign('contextService', $this->contextService)
             ->assign('messages', $this->messages)
             ->assign('typo3Version', TYPO3_version)
             ->assign('siteName', $GLOBALS['TYPO3_CONF_VARS']['SYS']['sitename']);
@@ -152,17 +155,13 @@ abstract class AbstractAction implements ActionInterface
 
     /**
      * Context determines if the install tool is called within backend or standalone
+     * This method creates a context service that distinguishes between standalone and backend context
      *
-     * @return string Either 'standalone' or 'backend'
+     * @param $context string Either 'standalone' or 'backend'
      */
-    protected function getContext()
+    public function setContext($context)
     {
-        $context = 'standalone';
-        $formValues = GeneralUtility::_GP('install');
-        if (isset($formValues['context'])) {
-            $context = $formValues['context'] === 'backend' ? 'backend' : 'standalone';
-        }
-        return $context;
+        $this->contextService = GeneralUtility::makeInstance(\TYPO3\CMS\Install\Service\ContextService::class, $context);
     }
 
     /**
index 01f5e2f..c39096a 100644 (file)
@@ -49,6 +49,13 @@ interface ActionInterface
     public function setAction($action);
 
     /**
+     * Set the context name, can be "installer", "standalone" or "backend"
+     *
+     * @param string $context
+     */
+    public function setContext($context);
+
+    /**
      * Set POST values
      *
      * @param array $postValues List of values submitted via POST
index 5bca62a..b1d2316 100644 (file)
@@ -111,6 +111,7 @@ class AjaxController extends AbstractController
         }
         $toolAction->setController('ajax');
         $toolAction->setAction($action);
+        $toolAction->setContext($request->getAttribute('context', 'standalone'));
         $toolAction->setToken($this->generateTokenForAction($action));
         $toolAction->setPostValues($request->getParsedBody()['install'] ?? []);
         return $this->output($toolAction->handle());
index c5f0aba..e7351f5 100644 (file)
@@ -16,17 +16,9 @@ namespace TYPO3\CMS\Install\Controller;
 
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
-use TYPO3\CMS\Backend\Template\ModuleTemplate;
-use TYPO3\CMS\Core\FormProtection\FormProtectionFactory;
-use TYPO3\CMS\Core\Utility\GeneralUtility;
-use TYPO3\CMS\Fluid\View\StandaloneView;
-use TYPO3\CMS\Install\Service\EnableFileService;
 
 /**
- * Backend module controller
- *
- * Embeds in backend and only shows the 'enable install tool button' or redirects
- * to step installer if install tool is enabled.
+ * Backend module controller to dispatch to the main modules or to an AJAX request
  *
  * This is a classic backend module that does not interfere with other code
  * within the install tool, it can be seen as a facade around install tool just
@@ -35,62 +27,82 @@ use TYPO3\CMS\Install\Service\EnableFileService;
 class BackendModuleController
 {
     /**
-     * Index action shows install tool / step installer or redirect to action to enable install tool
+     * Renders the maintenance tool action (or AJAX, if it was specifically requested)
      *
      * @param ServerRequestInterface $request
      * @param ResponseInterface $response
      * @return ResponseInterface
-     * @throws \RuntimeException
      */
-    public function index(ServerRequestInterface $request, ResponseInterface $response)
+    public function maintenanceAction(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
     {
-        $enableFileService = GeneralUtility::makeInstance(EnableFileService::class);
-
-        $formProtection = FormProtectionFactory::get();
-
-        $targetUrl = 'install.php?install[context]=backend';
-        if (!empty($request->getQueryParams()['install']['action'])) {
-            $subAction = !empty($request->getQueryParams()['install']['action'])
-                ? $request->getQueryParams()['install']['action']
-                : '';
-            $targetUrl .= '&install[controller]=tool&install[action]=' . $subAction;
-        }
-
-        if ($enableFileService->checkInstallToolEnableFile()) {
-            // Install tool is open and valid, redirect to it
-            $response = $response
-                ->withStatus(303)
-                ->withHeader('Location', $targetUrl);
-        } elseif ($request->getMethod() === 'POST' && $request->getParsedBody()['action'] === 'enableInstallTool') {
-            // Request to open the install tool
-            $installToolEnableToken = $request->getParsedBody()['installToolEnableToken'];
-            if (!$formProtection->validateToken($installToolEnableToken, 'installTool')) {
-                throw new \RuntimeException('Given form token was not valid', 1369161225);
-            }
-            $enableFileService->createInstallToolEnableFile();
+        return $this->executeSpecificToolAction($request, 'maintenance');
+    }
 
-            // Install tool is open and valid, redirect to it
-            $response = $response
-                ->withStatus(303)
-                ->withHeader('Location', $targetUrl);
-        } else {
-            // Show the "create enable install tool" button
-            $token = $formProtection->generateToken('installTool');
+    /**
+     * Renders the settings tool action (or AJAX, if it was specifically requested)
+     *
+     * @param ServerRequestInterface $request
+     * @param ResponseInterface $response
+     * @return ResponseInterface
+     */
+    public function settingsAction(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
+    {
+        return $this->executeSpecificToolAction($request, 'settings');
+    }
 
-            $view = GeneralUtility::makeInstance(StandaloneView::class);
-            $view->setTemplatePathAndFilename(
-                GeneralUtility::getFileAbsFileName(
-                    'EXT:install/Resources/Private/Templates/BackendModule/ShowEnableInstallToolButton.html'
-                )
-            );
-            $view->assign('installToolEnableToken', $token);
+    /**
+     * Renders the upgrade tool action (or AJAX, if it was specifically requested)
+     *
+     * @param ServerRequestInterface $request
+     * @param ResponseInterface $response
+     * @return ResponseInterface
+     */
+    public function upgradeAction(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
+    {
+        return $this->executeSpecificToolAction($request, 'upgrade');
+    }
 
-            $moduleTemplate = GeneralUtility::makeInstance(ModuleTemplate::class);
-            $moduleTemplate->setContent($view->render());
+    /**
+     * Renders the environment tool action (or AJAX, if it was specifically requested)
+     *
+     * @param ServerRequestInterface $request
+     * @param ResponseInterface $response
+     * @return ResponseInterface
+     */
+    public function environmentAction(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface
+    {
+        return $this->executeSpecificToolAction($request, 'environment');
+    }
 
-            $response->getBody()->write($moduleTemplate->renderContent());
+    /**
+     * Sets the action inside the install tool to a specific action and calls the "toolcontroller" afterwards
+     *
+     * @param ServerRequestInterface $request
+     * @param $action
+     * @return ResponseInterface
+     */
+    protected function executeSpecificToolAction(ServerRequestInterface $request, $action): ResponseInterface
+    {
+        $request = $request->withAttribute('context', 'backend');
+        // Can be moved into one controller in my opinion now, or should go into a dispatcher that
+        // also deals with actions
+        if ($request->getQueryParams()['install']['controller'] === 'ajax') {
+            return $this->handleAjaxRequest($request);
         }
+        $queryParameters = $request->getQueryParams();
+        $queryParameters['install']['action'] = $action;
+        $request = $request->withQueryParams($queryParameters);
+        return (new ToolController())->execute($request);
+    }
 
-        return $response;
+    /**
+     * Calls the AJAX controller (if requested as "controller")
+     *
+     * @param ServerRequestInterface $request
+     * @return ResponseInterface
+     */
+    protected function handleAjaxRequest(ServerRequestInterface $request): ResponseInterface
+    {
+        return (new AjaxController())->execute($request);
     }
 }
index ac88063..311c0c7 100644 (file)
@@ -82,6 +82,7 @@ class StepController extends AbstractController
             $action = GeneralUtility::makeInstance(\TYPO3\CMS\Install\Controller\Action\Common\InstallToolDisabledAction::class);
             $action->setAction('installToolDisabled');
         }
+        $action->setContext('standalone');
         $action->setController('common');
         return $this->output($action->handle());
     }
@@ -97,6 +98,7 @@ class StepController extends AbstractController
         /** @var \TYPO3\CMS\Install\Controller\Action\ActionInterface $action */
         $action = GeneralUtility::makeInstance(\TYPO3\CMS\Install\Controller\Action\Common\InstallToolPasswordNotSetAction::class);
         $action->setController('common');
+        $action->setContext('standalone');
         $action->setAction('installToolPasswordNotSet');
         return $this->output($action->handle());
     }
@@ -117,6 +119,7 @@ class StepController extends AbstractController
             /** @var AbstractStepAction $stepAction */
             $stepAction = $this->getActionInstance($action);
             $stepAction->setAction($action);
+            $stepAction->setContext('standalone');
             $stepAction->setToken($this->generateTokenForAction($action));
             $stepAction->setPostValues($postValues);
             $messages = $stepAction->execute();
@@ -146,6 +149,7 @@ class StepController extends AbstractController
         $stepAction = $this->getActionInstance($action);
         $stepAction->setAction($action);
         $stepAction->setController('step');
+        $stepAction->setContext('standalone');
         $stepAction->setToken($this->generateTokenForAction($action));
         $stepAction->setPostValues($postValues);
 
@@ -238,6 +242,7 @@ class StepController extends AbstractController
                 $action->setStepsCounter($currentStep, $totalSteps);
             }
             $action->setController('step');
+            $action->setContext('standalone');
             $action->setAction('environmentAndFolders');
             if (!empty($errorMessagesFromExecute)) {
                 $action->setMessages($errorMessagesFromExecute);
index 7b8117b..205329d 100644 (file)
@@ -62,6 +62,7 @@ class ToolController extends AbstractController
         $toolAction->setController('tool');
         $toolAction->setAction($action);
         $toolAction->setToken($this->generateTokenForAction($action));
+        $toolAction->setContext($request->getAttribute('context', 'standalone'));
         $toolAction->setPostValues($request->getParsedBody()['install'] ?? []);
         return $this->output($toolAction->handle());
     }
index e1f256d..ca8d331 100644 (file)
@@ -26,13 +26,12 @@ class ContextService
 
     /**
      * Constructor, prepare the context information
+     *
+     * @param string $context
      */
-    public function __construct()
+    public function __construct($context)
     {
-        $formValues = \TYPO3\CMS\Core\Utility\GeneralUtility::_GP('install');
-        if (isset($formValues['context'])) {
-            $this->backendContext = ($formValues['context'] === 'backend');
-        }
+        $this->backendContext = ($context === 'backend');
     }
 
     /**
index 7b8b734..2b0b31d 100644 (file)
@@ -17,12 +17,7 @@ if (TYPO3_MODE === 'BE') {
         '',
         '',
         [
-            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::index',
-            'routeParameters' => [
-                'install' => [
-                    'action' => 'maintenance'
-                ]
-            ],
+            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::maintenanceAction',
             'access' => 'systemMaintainer',
             'name' => 'tools_toolsmaintenance',
             'iconIdentifier' => 'module-install-maintenance',
@@ -35,12 +30,7 @@ if (TYPO3_MODE === 'BE') {
         '',
         '',
         [
-            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::index',
-            'routeParameters' => [
-                'install' => [
-                    'action' => 'settings'
-                ]
-            ],
+            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::settingsAction',
             'access' => 'systemMaintainer',
             'name' => 'tools_toolssettings',
             'iconIdentifier' => 'module-install-settings',
@@ -53,12 +43,7 @@ if (TYPO3_MODE === 'BE') {
         '',
         '',
         [
-            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::index',
-            'routeParameters' => [
-                'install' => [
-                    'action' => 'upgrade'
-                ]
-            ],
+            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::upgradeAction',
             'access' => 'systemMaintainer',
             'name' => 'tools_toolsupgrade',
             'iconIdentifier' => 'module-install-upgrade',
@@ -71,12 +56,7 @@ if (TYPO3_MODE === 'BE') {
         '',
         '',
         [
-            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::index',
-            'routeParameters' => [
-                'install' => [
-                    'action' => 'environment'
-                ]
-            ],
+            'routeTarget' => \TYPO3\CMS\Install\Controller\BackendModuleController::class . '::environmentAction',
             'access' => 'systemMaintainer',
             'name' => 'tools_toolsenvironment',
             'iconIdentifier' => 'module-install-environment',