From e3a0b0eb0ba3932d9f7fa642ec3f5107d18e0019 Mon Sep 17 00:00:00 2001 From: Benni Mack <benni@typo3.org> Date: Fri, 1 Sep 2017 09:00:10 +0200 Subject: [PATCH] [TASK] Install Tool: Remove authentication from backend context 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> --- .../Http/BackendModuleRequestHandler.php | 8 -- .../Backend/Install/InstallModuleCest.php | 112 ---------------- .../Classes/Controller/AbstractController.php | 8 +- .../Controller/Action/AbstractAction.php | 25 ++-- .../Controller/Action/ActionInterface.php | 7 + .../Classes/Controller/AjaxController.php | 1 + .../Controller/BackendModuleController.php | 120 ++++++++++-------- .../Classes/Controller/StepController.php | 5 + .../Classes/Controller/ToolController.php | 1 + .../Classes/Service/ContextService.php | 9 +- typo3/sysext/install/ext_tables.php | 28 +--- 11 files changed, 101 insertions(+), 223 deletions(-) delete mode 100644 typo3/sysext/core/Tests/Acceptance/Backend/Install/InstallModuleCest.php diff --git a/typo3/sysext/backend/Classes/Http/BackendModuleRequestHandler.php b/typo3/sysext/backend/Classes/Http/BackendModuleRequestHandler.php index 64c6df3ade56..38c5d073db07 100644 --- a/typo3/sysext/backend/Classes/Http/BackendModuleRequestHandler.php +++ b/typo3/sysext/backend/Classes/Http/BackendModuleRequestHandler.php @@ -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 index b6fbff433cac..000000000000 --- a/typo3/sysext/core/Tests/Acceptance/Backend/Install/InstallModuleCest.php +++ /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'); - } - } -} diff --git a/typo3/sysext/install/Classes/Controller/AbstractController.php b/typo3/sysext/install/Classes/Controller/AbstractController.php index b4a0303947b6..d7e95f57639f 100644 --- a/typo3/sysext/install/Classes/Controller/AbstractController.php +++ b/typo3/sysext/install/Classes/Controller/AbstractController.php @@ -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') diff --git a/typo3/sysext/install/Classes/Controller/Action/AbstractAction.php b/typo3/sysext/install/Classes/Controller/Action/AbstractAction.php index 0bbfde1ddc8c..a1fcaa1191a2 100644 --- a/typo3/sysext/install/Classes/Controller/Action/AbstractAction.php +++ b/typo3/sysext/install/Classes/Controller/Action/AbstractAction.php @@ -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 @@ -52,6 +53,11 @@ abstract class AbstractAction implements ActionInterface */ protected $messages = []; + /** + * @var ContextService + */ + protected $contextService; + /** * Handles the action * @@ -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); } /** diff --git a/typo3/sysext/install/Classes/Controller/Action/ActionInterface.php b/typo3/sysext/install/Classes/Controller/Action/ActionInterface.php index 01f5e2f72459..c39096ac527c 100644 --- a/typo3/sysext/install/Classes/Controller/Action/ActionInterface.php +++ b/typo3/sysext/install/Classes/Controller/Action/ActionInterface.php @@ -48,6 +48,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 * diff --git a/typo3/sysext/install/Classes/Controller/AjaxController.php b/typo3/sysext/install/Classes/Controller/AjaxController.php index 5bca62a4f4c4..b1d2316dff68 100644 --- a/typo3/sysext/install/Classes/Controller/AjaxController.php +++ b/typo3/sysext/install/Classes/Controller/AjaxController.php @@ -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()); diff --git a/typo3/sysext/install/Classes/Controller/BackendModuleController.php b/typo3/sysext/install/Classes/Controller/BackendModuleController.php index c5f0aba7be99..e7351f526c23 100644 --- a/typo3/sysext/install/Classes/Controller/BackendModuleController.php +++ b/typo3/sysext/install/Classes/Controller/BackendModuleController.php @@ -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); } } diff --git a/typo3/sysext/install/Classes/Controller/StepController.php b/typo3/sysext/install/Classes/Controller/StepController.php index ac8806373399..311c0c7e456c 100644 --- a/typo3/sysext/install/Classes/Controller/StepController.php +++ b/typo3/sysext/install/Classes/Controller/StepController.php @@ -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); diff --git a/typo3/sysext/install/Classes/Controller/ToolController.php b/typo3/sysext/install/Classes/Controller/ToolController.php index 7b8117bd320e..205329ddd7c9 100644 --- a/typo3/sysext/install/Classes/Controller/ToolController.php +++ b/typo3/sysext/install/Classes/Controller/ToolController.php @@ -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()); } diff --git a/typo3/sysext/install/Classes/Service/ContextService.php b/typo3/sysext/install/Classes/Service/ContextService.php index e1f256d7954c..ca8d3311ca56 100644 --- a/typo3/sysext/install/Classes/Service/ContextService.php +++ b/typo3/sysext/install/Classes/Service/ContextService.php @@ -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'); } /** diff --git a/typo3/sysext/install/ext_tables.php b/typo3/sysext/install/ext_tables.php index 7b8b734ca798..2b0b31d3baf0 100644 --- a/typo3/sysext/install/ext_tables.php +++ b/typo3/sysext/install/ext_tables.php @@ -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', -- GitLab