From 64ea5ccdf364427930960960bb77f62ba2d5a8bb Mon Sep 17 00:00:00 2001 From: Guite Date: Tue, 19 Mar 2019 13:39:55 +0100 Subject: [PATCH] fixed #3206 --- CHANGELOG-3.0.md | 1 + .../CoreBundle/Resources/config/services.yml | 10 - .../HookBundle/Controller/HookController.php | 39 ++- .../HookBundle/Resources/public/js/hookui.js | 20 +- .../Resources/views/Hook/edit.html.twig | 2 + .../Zikula/Core/Token/CsrfTokenHandler.php | 122 --------- src/lib/Zikula/Core/Token/Generator.php | 245 ------------------ .../Core/Token/Storage/SessionStorage.php | 106 -------- .../Core/Token/Storage/StorageInterface.php | 52 ---- src/lib/Zikula/Core/Token/Validator.php | 113 -------- .../Controller/ModuleController.php | 71 +++-- .../ExtensionsModule/Menu/MenuBuilder.php | 39 ++- .../Controller/MembershipController.php | 21 +- .../views/Membership/adminList.html.twig | 2 +- .../views/Membership/userlist.html.twig | 2 +- .../Controller/IdsLogController.php | 18 +- .../Resources/views/IdsLog/view.html.twig | 3 +- 17 files changed, 117 insertions(+), 749 deletions(-) delete mode 100644 src/lib/Zikula/Core/Token/CsrfTokenHandler.php delete mode 100644 src/lib/Zikula/Core/Token/Generator.php delete mode 100644 src/lib/Zikula/Core/Token/Storage/SessionStorage.php delete mode 100644 src/lib/Zikula/Core/Token/Storage/StorageInterface.php delete mode 100644 src/lib/Zikula/Core/Token/Validator.php diff --git a/CHANGELOG-3.0.md b/CHANGELOG-3.0.md index 0dca907e65..7f0abfc040 100644 --- a/CHANGELOG-3.0.md +++ b/CHANGELOG-3.0.md @@ -23,6 +23,7 @@ CHANGELOG - ZIKULA 3.0.x - `Zikula\UsersModule\ProfileModule\ProfileModuleInterface` requires a new method `getBundleName()` to be implemented. - `Zikula\BlocksModule\AbstractBlockHandler` is not container aware anymore. - `Zikula\ExtensionsModule\Entity\ExtensionEntity` has renamed `core_min` to `coreCompatibility` and removed `core_max` property (#3649). + - Removed all classes from the `Zikula\Core\Token` namespace. If you need custom csrf tokens use [isCsrfTokenValid()](https://symfony.com/doc/current/security/csrf.html#generating-and-checking-csrf-tokens-manually) instead (#3206). - The `Zikula\Bundle\HookBundle\ServiceIdTrait` trait has been removed. - Dropped vendors: - Removed afarkas/html5shiv diff --git a/src/lib/Zikula/Bundle/CoreBundle/Resources/config/services.yml b/src/lib/Zikula/Bundle/CoreBundle/Resources/config/services.yml index 749420909e..949ece7215 100644 --- a/src/lib/Zikula/Bundle/CoreBundle/Resources/config/services.yml +++ b/src/lib/Zikula/Bundle/CoreBundle/Resources/config/services.yml @@ -1,6 +1,3 @@ -parameters: - zikula_core.internal.token.max_life: 3600 - services: _defaults: autowire: true @@ -84,13 +81,6 @@ services: Michelf\: resource: '../../../../../../vendor/michelf/php-markdown/Michelf/*' - Zikula\Core\Token\Storage: '@Zikula\Core\Token\Storage\SessionStorage' - - Zikula\Core\Token\Generator: - arguments: - $secret: '_dummy' - $maxLifetime: '%zikula_core.internal.token.max_life%' - # public because BootstrapBundlesCommand accesses this using container Zikula\Bundle\CoreBundle\Bundle\Helper\BootstrapHelper: public: true diff --git a/src/lib/Zikula/Bundle/HookBundle/Controller/HookController.php b/src/lib/Zikula/Bundle/HookBundle/Controller/HookController.php index a894161843..2f749c5156 100644 --- a/src/lib/Zikula/Bundle/HookBundle/Controller/HookController.php +++ b/src/lib/Zikula/Bundle/HookBundle/Controller/HookController.php @@ -21,7 +21,6 @@ use Zikula\Bundle\HookBundle\Dispatcher\HookDispatcherInterface; use Zikula\Common\Translator\TranslatorInterface; use Zikula\Common\Translator\TranslatorTrait; -use Zikula\Core\Token\CsrfTokenHandler; use Zikula\ExtensionsModule\Entity\ExtensionEntity; use Zikula\ExtensionsModule\Entity\RepositoryInterface\ExtensionRepositoryInterface; use Zikula\PermissionsModule\Api\ApiInterface\PermissionApiInterface; @@ -281,7 +280,9 @@ public function toggleSubscribeAreaStatusAction( HookDispatcherInterface $dispatcher ) { $this->setTranslator($translator); - $this->checkAjaxToken(); + if (!$this->checkAjaxToken($request)) { + throw new AccessDeniedException(); + } // get subscriberarea from POST $subscriberArea = $request->request->get('subscriberarea', ''); @@ -364,7 +365,9 @@ public function changeProviderAreaOrderAction( HookCollectorInterface $collector ) { $this->setTranslator($this->get('translator.default')); - $this->checkAjaxToken(); + if (!$this->checkAjaxToken($request)) { + throw new AccessDeniedException(); + } // get subscriberarea from POST $subscriberarea = $request->request->get('subscriberarea', ''); @@ -393,38 +396,32 @@ public function changeProviderAreaOrderAction( // set sorting $this->get('hook_dispatcher')->setBindOrder($subscriberarea, $providerarea); - $ol_id = $request->request->get('ol_id', ''); - - return $this->json(['result' => true, 'ol_id' => $ol_id]); + return $this->json(['result' => true]); } /** * Check the CSRF token. - * Checks will fall back to $token check if automatic checking fails - * - * @param CsrfTokenHandler $tokenHandler - * @param string $token Token, default null * - * @throws AccessDeniedException If the CSFR token fails - * @throws \Exception if request is not an XmlHttpRequest + * @param Request $request * - * @return void + * @return boolean */ - private function checkAjaxToken(CsrfTokenHandler $tokenHandler, $token = null) + private function checkAjaxToken(Request $request) { - $currentRequest = $this->get('request_stack')->getCurrentRequest(); - if (!$currentRequest->isXmlHttpRequest()) { - throw new \Exception(); + if (!$request->isXmlHttpRequest()) { + return false; } $sessionName = $this->container->getParameter('zikula.session.name'); - $sessionId = $currentRequest->cookies->get($sessionName, null); + $sessionId = $request->cookies->get($sessionName, null); - if ($sessionId == $currentRequest->getSession()->getId()) { - return; + if ($sessionId != $request->getSession()->getId()) { + return false; } - $tokenHandler->validate($token); + if (!$this->isCsrfTokenValid('hook-ui', $request->request->get('token'))) { + return false; + } } private function getExtensionsCapableOf(HookCollectorInterface $collector, ExtensionRepositoryInterface $extensionRepository, $type) diff --git a/src/lib/Zikula/Bundle/HookBundle/Resources/public/js/hookui.js b/src/lib/Zikula/Bundle/HookBundle/Resources/public/js/hookui.js index 212cbd83a5..e57bae8981 100644 --- a/src/lib/Zikula/Bundle/HookBundle/Resources/public/js/hookui.js +++ b/src/lib/Zikula/Bundle/HookBundle/Resources/public/js/hookui.js @@ -143,14 +143,13 @@ var cloneDraggedItem = true; * @return none; */ subscriberAreaToggle = function(sarea, parea) { - var pars = { - subscriberarea: sarea, - providerarea: parea - }; - $.ajax({ url: Routing.generate('zikula_hook_hook_togglesubscribeareastatus'), - data: pars + data: { + token: $('#csrfToken').data('token'), + subscriberarea: sarea, + providerarea: parea + } }).done(function(data) { if (data.action == 'bind') { if (!appendItemBeforeResponse) { @@ -286,16 +285,15 @@ var cloneDraggedItem = true; providersAreas += '&providerarea[]=' + $($(this).attr('id') + '_a').val(); }); - var pars = 'ol_id=' + listId + - '&subscriberarea=' + subscriberArea + - providersAreas; + var parameters = 'subscriberarea=' + subscriberArea + providersAreas; + parameters += '&token=' + $('#csrfToken').data('token'); $.ajax({ url: Routing.generate('zikula_hook_hook_changeproviderareaorder'), - data: pars + data: parameters }).done(function(data) { // update new sort order - recolorListElements(data.ol_id, $('#' + data.ol_id).down(0).attr('id')); + recolorListElements(listId, $('#' + listId).down(0).attr('id')); }).fail(function(result) { alert(result.status + ': ' + result.statusText); }); diff --git a/src/lib/Zikula/Bundle/HookBundle/Resources/views/Hook/edit.html.twig b/src/lib/Zikula/Bundle/HookBundle/Resources/views/Hook/edit.html.twig index eecb0b0809..a180acf257 100644 --- a/src/lib/Zikula/Bundle/HookBundle/Resources/views/Hook/edit.html.twig +++ b/src/lib/Zikula/Bundle/HookBundle/Resources/views/Hook/edit.html.twig @@ -13,6 +13,8 @@ {{ pageSetVar('title', __('Hooks')) }} + +
{{ __('Drag items from the right column to the left column to connect the hooks. Only areas with the same category can be connected.') }}
{% if showBothPanels %}
diff --git a/src/lib/Zikula/Core/Token/CsrfTokenHandler.php b/src/lib/Zikula/Core/Token/CsrfTokenHandler.php deleted file mode 100644 index c64d5a0c93..0000000000 --- a/src/lib/Zikula/Core/Token/CsrfTokenHandler.php +++ /dev/null @@ -1,122 +0,0 @@ -generator = $generator; - $this->validator = $validator; - $this->requestStack = $requestStack; - $this->variableApi = $variableApi; - $this->session = $session; - } - - /** - * Validate a Csrf token. - * - * @param string $token The token, if not set, will pull from $_POST['csrftoken'] - * @param bool $invalidateSessionOnFailure - */ - public function validate($token = null, $invalidateSessionOnFailure = false) - { - $request = $this->requestStack->getCurrentRequest(); - - if (is_null($token)) { - $token = $request->request->get('csrftoken', false); - } - - if ($this->variableApi->getSystemVar('sessioncsrftokenonetime') && $this->validator->validate($token, false, false)) { - return true; - } - - if ($this->validator->validate($token)) { - return true; - } - - // validation failed - if ($invalidateSessionOnFailure) { - $this->session->invalidate(); - $this->session->set('session_expire', true); - } - throw new AccessDeniedException('Error! Something went wrong: security token validation failed. Go to the homepage.'); - } - - /** - * Generate a Csrf token. - * - * @param boolean $forceUnique Force a unique token regardless of system settings - * - * @return string - */ - public function generate($forceUnique = false) - { - if (!$forceUnique && $this->variableApi->getSystemVar('sessioncsrftokenonetime')) { - $storage = $this->generator->getStorage(); - $tokenId = $this->session->get('sessioncsrftokenid'); - $data = $storage->get($tokenId); - if (!$data) { - $this->generator->generate($this->generator->uniqueId(), time()); - $this->generator->save(); - $this->session->set('sessioncsrftokenid', $this->generator->getId()); - - return $this->generator->getToken(); - } - - return $data['token']; - } - - $this->generator->generate($this->generator->uniqueId(), time()); - $this->generator->save(); - - return $this->generator->getToken(); - } -} diff --git a/src/lib/Zikula/Core/Token/Generator.php b/src/lib/Zikula/Core/Token/Generator.php deleted file mode 100644 index ade6386075..0000000000 --- a/src/lib/Zikula/Core/Token/Generator.php +++ /dev/null @@ -1,245 +0,0 @@ -storage = $storage; - $this->secret = $secret; - $this->maxLifetime = $maxLifetime; - } - - /** - * Save token. - * - * @return void - */ - public function save() - { - $this->storage->save($this->id, $this->token, $this->timestamp); - $this->garbageCollection(); - } - - /** - * Delete token. - * - * @return void - */ - public function delete() - { - $this->storage->delete($this->token); - } - - /** - * Generate a unique ID. - * - * @return string - */ - public function uniqueId() - { - return uniqid('', true); - } - - /** - * Generate token based on ID. - * - * If tokens are intended to be one-time tokens then use a unique ID each - * time. They are validated with delete=true. If the are per-session, then - * they should be generated with the same unique ID and validated with - * delete = false leaving storage expire/GC to remove the token. - * - * @param string $id Token ID - * @param integer $timestamp Create with this timestamp (defaults null = now) - * - * @return Generator - */ - public function generate($id, $timestamp = null) - { - $this->id = $id; - $this->timestamp = is_null($timestamp) ? time() : $timestamp; - $this->hash = md5($this->id . $this->secret . $this->timestamp); - $this->token = base64_encode("{$this->id}:{$this->hash}:{$this->timestamp}"); - - return $this; - } - - /** - * Decode a token. - * - * @param string $token Token - * - * @return array - */ - public function decode($token) - { - return explode(':', base64_decode($token)); - } - - /** - * Gets storage driver. - * - * @return StorageInterface - */ - public function getStorage() - { - return $this->storage; - } - - /** - * Gets token ID. - * - * @return string - */ - public function getId() - { - return $this->id; - } - - /** - * Gets signing secret. - * - * @return string - */ - public function getSecret() - { - return $this->secret; - } - - /** - * Sets secret. - * - * @param string $secret - */ - public function setSecret($secret) - { - $this->secret = $secret; - } - - /** - * Gets generated token. - * - * @return string - */ - public function getToken() - { - return $this->token; - } - - /** - * Gets hash for of token. - * - * @return string - */ - public function getHash() - { - return $this->hash; - } - - /** - * Gets timestamp of token creation. - * - * @return integer - */ - public function getTimestamp() - { - return $this->timestamp; - } - - /** - * Runs garbage collection to clean up tokens that expire - * before the session expired. - * - * Generates a number between 1 and $probability and runs - * garbage collection if result is 1. - * - * @param integer $probability Defaults to 20, ie 1/20 = 5% - */ - public function garbageCollection($probability = 20) - { - if (1 === mt_rand(1, $probability)) { - $this->storage->gc($this->maxLifetime); - } - } - - /** - * Gets the max lifetime in seconds. - * - * @return integer - */ - public function getMaxLifetime() - { - return $this->maxLifetime; - } -} diff --git a/src/lib/Zikula/Core/Token/Storage/SessionStorage.php b/src/lib/Zikula/Core/Token/Storage/SessionStorage.php deleted file mode 100644 index 23faff462a..0000000000 --- a/src/lib/Zikula/Core/Token/Storage/SessionStorage.php +++ /dev/null @@ -1,106 +0,0 @@ -session = $session; - $this->key = $key; - } - - /** - * {@inheritdoc} - */ - public function get($id) - { - if (empty($id)) { - return false; - } - - $tokens = $this->session->get($this->key, []); - - if (!array_key_exists($id, $tokens)) { - return false; - } - - return $tokens[$id]; - } - - /** - * {@inheritdoc} - */ - public function save($id, $token, $timestamp) - { - $tokens = $this->session->get($this->key, []); - $tokens[$id] = [ - 'token' => $token, - 'time' => (int)$timestamp - ]; - $this->session->set($this->key, $tokens); - } - - /** - * {@inheritdoc} - */ - public function delete($id) - { - $tokens = $this->session->get($this->key, []); - if (array_key_exists($id, $tokens)) { - unset($tokens[$id]); - } - - $this->session->set($this->key, $tokens); - } - - /** - * {@inheritdoc} - */ - public function gc($lifetime) - { - $tokens = $this->session->get($this->key, []); - $now = time(); - foreach ($tokens as $key => $token) { - if ($now - (int)$token['time'] > $lifetime) { - unset($token[$key]); - } - } - - $this->session->set($this->key, $tokens); - } -} diff --git a/src/lib/Zikula/Core/Token/Storage/StorageInterface.php b/src/lib/Zikula/Core/Token/Storage/StorageInterface.php deleted file mode 100644 index 3ad7abb913..0000000000 --- a/src/lib/Zikula/Core/Token/Storage/StorageInterface.php +++ /dev/null @@ -1,52 +0,0 @@ -tokenGenerator = $tokenGenerator; - $this->storage = $tokenGenerator->getStorage(); - $this->secret = $tokenGenerator->getSecret(); - } - - /** - * Validate a token. - * - * Tokens should be deleted if they are generated as one-time tokens - * with a unique ID each time. If the are per-session, then they should be - * generated with the same unique ID and not deleted when validated here. - * - * @param string $token Token to validate - * @param boolean $delete Whether to delete the token if valid - * @param boolean $checkExpire Whether to check for token expiry - * - * @return boolean - */ - public function validate($token, $delete = true, $checkExpire = true) - { - if (!$token) { - return false; - } - - list($id, , $timestamp) = $this->tokenGenerator->decode($token); - $decoded = [ - 'id' => $id, - 'time' => $timestamp - ]; - - // Garbage collect the session. - $this->tokenGenerator->garbageCollection(); - - // Check if token ID exists first. - $stored = $this->storage->get($decoded['id']); - if (!$stored) { - return false; - } - - // Check if the token has been tampered with. - $duplicateToken = $this->tokenGenerator->generate($decoded['id'], $decoded['time'])->getToken(); - if ($stored['token'] !== $duplicateToken) { - $this->storage->delete($decoded['id']); - - return false; - } - - // Check if token has expired. - if ($checkExpire) { - $timeDiff = ((int)$decoded['time'] + $this->tokenGenerator->getMaxLifetime()) - time(); - if ($timeDiff < 0) { - $this->storage->delete($decoded['id']); - - return false; - } - } - - // All checked out, delete the token and return true. - if ($delete) { - $this->storage->delete($decoded['id']); - } - - return true; - } -} diff --git a/src/system/ExtensionsModule/Controller/ModuleController.php b/src/system/ExtensionsModule/Controller/ModuleController.php index 8bf82b33a8..f6e7380be8 100644 --- a/src/system/ExtensionsModule/Controller/ModuleController.php +++ b/src/system/ExtensionsModule/Controller/ModuleController.php @@ -31,7 +31,6 @@ use Zikula\Core\CoreEvents; use Zikula\Core\Event\GenericEvent; use Zikula\Core\Event\ModuleStateEvent; -use Zikula\Core\Token\CsrfTokenHandler; use Zikula\ExtensionsModule\Constant; use Zikula\ExtensionsModule\Entity\ExtensionDependencyEntity; use Zikula\ExtensionsModule\Entity\ExtensionEntity; @@ -137,13 +136,12 @@ public function viewModuleListAction( } /** - * @Route("/modules/activate/{id}/{csrftoken}", methods = {"GET"}, requirements={"id" = "^[1-9]\d*$"}) + * @Route("/modules/activate/{id}/{token}", methods = {"GET"}, requirements={"id" = "^[1-9]\d*$"}) * * Activate an extension * * @param integer $id - * @param CsrfTokenHandler $tokenHandler - * @param string $csrftoken + * @param string $token * @param ExtensionRepositoryInterface $extensionRepository * @param ExtensionStateHelper $extensionStateHelper * @param CacheClearer $cacheClearer @@ -152,8 +150,7 @@ public function viewModuleListAction( */ public function activateAction( $id, - CsrfTokenHandler $tokenHandler, - $csrftoken, + $token, ExtensionRepositoryInterface $extensionRepository, ExtensionStateHelper $extensionStateHelper, CacheClearer $cacheClearer @@ -162,7 +159,9 @@ public function activateAction( throw new AccessDeniedException(); } - $tokenHandler->validate($csrftoken); + if (!$this->isCsrfTokenValid('activate-extension', $token)) { + throw new AccessDeniedException(); + } $extension = $extensionRepository->find($id); if (Constant::STATE_NOTALLOWED == $extension->getState()) { @@ -178,13 +177,12 @@ public function activateAction( } /** - * @Route("/modules/deactivate/{id}/{csrftoken}", methods = {"GET"}, requirements={"id" = "^[1-9]\d*$"}) + * @Route("/modules/deactivate/{id}/{token}", methods = {"GET"}, requirements={"id" = "^[1-9]\d*$"}) * * Deactivate an extension * * @param integer $id - * @param CsrfTokenHandler $tokenHandler - * @param string $csrftoken + * @param string $token * @param ExtensionRepositoryInterface $extensionRepository * @param ExtensionStateHelper $extensionStateHelper * @param CacheClearer $cacheClearer @@ -193,8 +191,7 @@ public function activateAction( */ public function deactivateAction( $id, - CsrfTokenHandler $tokenHandler, - $csrftoken, + $token, ExtensionRepositoryInterface $extensionRepository, ExtensionStateHelper $extensionStateHelper, CacheClearer $cacheClearer @@ -203,7 +200,9 @@ public function deactivateAction( throw new AccessDeniedException(); } - $tokenHandler->validate($csrftoken); + if (!$this->isCsrfTokenValid('deactivate-extension', $token)) { + throw new AccessDeniedException(); + } $extension = $extensionRepository->find($id); if (ZikulaKernel::isCoreModule($extension->getName())) { @@ -301,14 +300,15 @@ public function compatibilityAction(ExtensionEntity $extension) } /** - * @Route("/install/{id}", requirements={"id" = "^[1-9]\d*$"}) + * @Route("/install/{id}/{token}", requirements={"id" = "^[1-9]\d*$"}) * @Theme("admin") * @Template("ZikulaExtensionsModule:Module:install.html.twig") * - * Initialise an extension. + * Install and initialise an extension. * * @param Request $request * @param ExtensionEntity $extension + * @param string $token * @param ExtensionRepositoryInterface $extensionRepository * @param ExtensionHelper $extensionHelper * @param ExtensionStateHelper $extensionStateHelper @@ -320,6 +320,7 @@ public function compatibilityAction(ExtensionEntity $extension) public function installAction( Request $request, ExtensionEntity $extension, + $token, ExtensionRepositoryInterface $extensionRepository, ExtensionHelper $extensionHelper, ExtensionStateHelper $extensionStateHelper, @@ -329,11 +330,17 @@ public function installAction( if (!$this->hasPermission('ZikulaExtensionsModule::', '::', ACCESS_ADMIN)) { throw new AccessDeniedException(); } + + $id = $extension->getId(); + if (!$this->isCsrfTokenValid('install-extension', $token)) { + throw new AccessDeniedException(); + } + if (!$this->get('kernel')->isBundle($extension->getName())) { - $extensionStateHelper->updateState($extension->getId(), Constant::STATE_TRANSITIONAL); + $extensionStateHelper->updateState($id, Constant::STATE_TRANSITIONAL); $cacheClearer->clear('symfony'); - return $this->redirectToRoute('zikulaextensionsmodule_module_install', ['id' => $extension->getId()]); + return $this->redirectToRoute('zikulaextensionsmodule_module_install', ['id' => $id]); } $unsatisfiedDependencies = $dependencyHelper->getUnsatisfiedExtensionDependencies($extension); $form = $this->createForm(ExtensionInstallType::class, [ @@ -363,17 +370,17 @@ public function installAction( } if ($extensionHelper->install($extension)) { $this->addFlash('status', $this->__f('Done! Installed %s.', ['%s' => $extension->getName()])); - $extensionsInstalled[] = $extension->getId(); + $extensionsInstalled[] = $id; $cacheClearer->clear('symfony'); return $this->redirectToRoute('zikulaextensionsmodule_module_postinstall', ['extensions' => json_encode($extensionsInstalled)]); } else { - $extensionStateHelper->updateState($extension->getId(), Constant::STATE_UNINITIALISED); + $extensionStateHelper->updateState($id, Constant::STATE_UNINITIALISED); $this->addFlash('error', $this->__f('Initialization of %s failed!', ['%s' => $extension->getName()])); } } if ($form->get('cancel')->isClicked()) { - $extensionStateHelper->updateState($extension->getId(), Constant::STATE_UNINITIALISED); + $extensionStateHelper->updateState($id, Constant::STATE_UNINITIALISED); $this->addFlash('status', $this->__('Operation cancelled.')); } @@ -383,7 +390,7 @@ public function installAction( return [ 'dependencies' => $unsatisfiedDependencies, 'extension' => $extension, - 'form' => $form->createView(), + 'form' => $form->createView() ]; } @@ -438,24 +445,25 @@ private function formatDependencyCheckboxArray(ExtensionRepositoryInterface $ext } /** - * @Route("/upgrade/{id}/{csrftoken}", requirements={"id" = "^[1-9]\d*$"}) + * @Route("/upgrade/{id}/{token}", requirements={"id" = "^[1-9]\d*$"}) * * Upgrade an extension. * * @param ExtensionEntity $extension - * @param CsrfTokenHandler $tokenHandler - * @param string $csrftoken + * @param string $token * @param ExtensionHelper $extensionHelper * * @return RedirectResponse */ - public function upgradeAction(ExtensionEntity $extension, CsrfTokenHandler $tokenHandler, $csrftoken, ExtensionHelper $extensionHelper) + public function upgradeAction(ExtensionEntity $extension, $token, ExtensionHelper $extensionHelper) { if (!$this->hasPermission('ZikulaExtensionsModule::', '::', ACCESS_ADMIN)) { throw new AccessDeniedException(); } - $tokenHandler->validate($csrftoken); + if (!$this->isCsrfTokenValid('upgrade-extension', $token)) { + throw new AccessDeniedException(); + } $result = $extensionHelper->upgrade($extension); if ($result) { @@ -468,7 +476,7 @@ public function upgradeAction(ExtensionEntity $extension, CsrfTokenHandler $toke } /** - * @Route("/uninstall/{id}", requirements={"id" = "^[1-9]\d*$"}) + * @Route("/uninstall/{id}/{token}", requirements={"id" = "^[1-9]\d*$"}) * @Theme("admin") * @Template("ZikulaExtensionsModule:Module:uninstall.html.twig") * @@ -476,6 +484,7 @@ public function upgradeAction(ExtensionEntity $extension, CsrfTokenHandler $toke * * @param Request $request * @param ExtensionEntity $extension + * @param string $token * @param BlockRepositoryInterface $blockRepository * @param ExtensionHelper $extensionHelper * @param ExtensionStateHelper $extensionStateHelper @@ -487,6 +496,7 @@ public function upgradeAction(ExtensionEntity $extension, CsrfTokenHandler $toke public function uninstallAction( Request $request, ExtensionEntity $extension, + $token, BlockRepositoryInterface $blockRepository, ExtensionHelper $extensionHelper, ExtensionStateHelper $extensionStateHelper, @@ -496,8 +506,13 @@ public function uninstallAction( if (!$this->hasPermission('ZikulaExtensionsModule::', '::', ACCESS_ADMIN)) { throw new AccessDeniedException(); } + + if (!$this->isCsrfTokenValid('uninstall-extension', $token)) { + throw new AccessDeniedException(); + } + if (Constant::STATE_MISSING == $extension->getState()) { - throw new \RuntimeException($this->__("Error! The requested extension cannot be uninstalled because its files are missing!")); + throw new \RuntimeException($this->__('Error! The requested extension cannot be uninstalled because its files are missing!')); } if (!$this->get('kernel')->isBundle($extension->getName())) { $extensionStateHelper->updateState($extension->getId(), Constant::STATE_TRANSITIONAL); diff --git a/src/system/ExtensionsModule/Menu/MenuBuilder.php b/src/system/ExtensionsModule/Menu/MenuBuilder.php index 6c97ff8be6..74d2f829c4 100644 --- a/src/system/ExtensionsModule/Menu/MenuBuilder.php +++ b/src/system/ExtensionsModule/Menu/MenuBuilder.php @@ -12,10 +12,10 @@ namespace Zikula\ExtensionsModule\Menu; use Knp\Menu\FactoryInterface; +use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface; use Zikula\Bundle\CoreBundle\HttpKernel\ZikulaKernel; use Zikula\Common\Translator\TranslatorInterface; use Zikula\Common\Translator\TranslatorTrait; -use Zikula\Core\Token\CsrfTokenHandler; use Zikula\ExtensionsModule\Constant; use Zikula\PermissionsModule\Api\ApiInterface\PermissionApiInterface; @@ -34,20 +34,20 @@ class MenuBuilder private $permissionApi; /** - * @var CsrfTokenHandler + * @var CsrfTokenManagerInterface */ - private $tokenHandler; + private $csrfTokenManager; public function __construct( TranslatorInterface $translator, FactoryInterface $factory, PermissionApiInterface $permissionApi, - CsrfTokenHandler $tokenHandler + CsrfTokenManagerInterface $csrfTokenManager ) { $this->setTranslator($translator); $this->factory = $factory; $this->permissionApi = $permissionApi; - $this->tokenHandler = $tokenHandler; + $this->csrfTokenManager = $csrfTokenManager; } public function createAdminMenu(array $options) @@ -60,28 +60,31 @@ public function createAdminMenu(array $options) return $menu; } - $csrfToken = $this->tokenHandler->generate(true); + $id = $extension->getId(); switch ($extension->getState()) { case Constant::STATE_ACTIVE: if (!ZikulaKernel::isCoreModule($extension->getName())) { + $csrfToken = $this->getCsrfToken('deactivate-extension'); $menu->addChild($this->__f('Deactivate %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_deactivate', - 'routeParameters' => ['id' => $extension->getId(), 'csrftoken' => $csrfToken], + 'routeParameters' => ['id' => $id, 'token' => $csrfToken] ])->setAttribute('icon', 'fa fa-minus-circle') ->setLinkAttribute('class', 'text-danger'); // or set style text-color #0c00 } break; case Constant::STATE_INACTIVE: + $csrfToken = $this->getCsrfToken('activate-extension'); $menu->addChild($this->__f('Activate %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_activate', - 'routeParameters' => ['id' => $extension->getId(), 'csrftoken' => $csrfToken], + 'routeParameters' => ['id' => $id, 'token' => $csrfToken] ])->setAttribute('icon', 'fa fa-plus-square') ->setLinkAttribute('class', 'text-success'); + $csrfToken = $this->getCsrfToken('uninstall-extension'); $menu->addChild($this->__f('Uninstall %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_uninstall', - 'routeParameters' => ['id' => $extension->getId()], + 'routeParameters' => ['id' => $id, 'token' => $csrfToken] ])->setAttribute('icon', 'fa fa-trash-o') ->setLinkAttribute('style', 'color:#c00'); break; @@ -89,9 +92,10 @@ public function createAdminMenu(array $options) // Nothing to do. break; case Constant::STATE_UPGRADED: + $csrfToken = $this->getCsrfToken('upgrade-extension'); $menu->addChild($this->__f('Upgrade %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_upgrade', - 'routeParameters' => ['id' => $extension->getId(), 'csrftoken' => $csrfToken], + 'routeParameters' => ['id' => $id, 'token' => $csrfToken] ])->setAttribute('icon', 'fa fa-refresh') ->setLinkAttribute('style', 'color:#00c'); break; @@ -100,24 +104,26 @@ public function createAdminMenu(array $options) // do not allow deletion of invalid modules if previously installed (#1278) break; case Constant::STATE_NOTALLOWED: + $csrfToken = $this->getCsrfToken('uninstall-extension'); $menu->addChild($this->__f('Remove %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_uninstall', - 'routeParameters' => ['id' => $extension->getId()], + 'routeParameters' => ['id' => $id, 'token' => $csrfToken] ])->setAttribute('icon', 'fa fa-trash-o') ->setLinkAttribute('style', 'color:#c00'); break; case Constant::STATE_UNINITIALISED: default: if ($extension->getState() < 10) { + $csrfToken = $this->getCsrfToken('install-extension'); $menu->addChild($this->__f('Install %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_install', - 'routeParameters' => ['id' => $extension->getId()], + 'routeParameters' => ['id' => $id, 'token' => $csrfToken] ])->setAttribute('icon', 'fa fa-cog') ->setLinkAttribute('class', 'text-success'); } else { $menu->addChild($this->__f('Core compatibility information: %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_compatibility', - 'routeParameters' => ['id' => $extension->getId()], + 'routeParameters' => ['id' => $id] ])->setAttribute('icon', 'fa fa-info-circle') ->setLinkAttribute('style', 'color:black'); } @@ -127,7 +133,7 @@ public function createAdminMenu(array $options) if (!in_array($extension->getState(), [Constant::STATE_UNINITIALISED, Constant::STATE_INVALID])) { $menu->addChild($this->__f('Edit %s', ['%s' => $extension->getDisplayname()]), [ 'route' => 'zikulaextensionsmodule_module_modify', - 'routeParameters' => ['id' => $extension->getId()], + 'routeParameters' => ['id' => $id] ])->setAttribute('icon', 'fa fa-wrench') ->setLinkAttribute('style', 'color:black'); } @@ -135,6 +141,11 @@ public function createAdminMenu(array $options) return $menu; } + private function getCsrfToken(string $tokenId): string + { + return $this->csrfTokenManager->getToken($tokenId)->getValue(); + } + public function setTranslator($translator) { $this->translator = $translator; diff --git a/src/system/GroupsModule/Controller/MembershipController.php b/src/system/GroupsModule/Controller/MembershipController.php index 598e946cac..75ba68b1d4 100644 --- a/src/system/GroupsModule/Controller/MembershipController.php +++ b/src/system/GroupsModule/Controller/MembershipController.php @@ -20,7 +20,6 @@ use Zikula\Core\Controller\AbstractController; use Zikula\Core\Event\GenericEvent; use Zikula\Core\Response\PlainResponse; -use Zikula\Core\Token\CsrfTokenHandler; use Zikula\ExtensionsModule\Api\ApiInterface\VariableApiInterface; use Zikula\GroupsModule\Entity\GroupEntity; use Zikula\GroupsModule\Entity\RepositoryInterface\GroupRepositoryInterface; @@ -89,14 +88,12 @@ public function listAction( * Display all members of a group to an admin * * @param GroupEntity $group - * @param CsrfTokenHandler $tokenHandler * @param string $letter the letter from the alpha filter * @param integer $startNum the start item number for the pager * @return array */ public function adminListAction( GroupEntity $group, - CsrfTokenHandler $tokenHandler, $letter = '*', $startNum = 0 ) { @@ -109,32 +106,31 @@ public function adminListAction( 'pager' => [ 'amountOfItems' => $group->getUsers()->count(), 'itemsPerPage' => $this->getVar('itemsperpage', 25) - ], - 'csrfToken' => $tokenHandler->generate() + ] ]; } /** - * @Route("/admin/add/{uid}/{gid}/{csrfToken}", requirements={"gid" = "^[1-9]\d*$", "uid" = "^[1-9]\d*$"}) + * @Route("/admin/add/{uid}/{gid}/{token}", requirements={"gid" = "^[1-9]\d*$", "uid" = "^[1-9]\d*$"}) * * Add user to a group. * * @param UserEntity $userEntity * @param GroupEntity $group - * @param CsrfTokenHandler $tokenHandler - * @param string $csrfToken + * @param string $token * @return RedirectResponse */ public function addAction( UserEntity $userEntity, GroupEntity $group, - CsrfTokenHandler $tokenHandler, - $csrfToken + $token ) { - $tokenHandler->validate($csrfToken); if (!$this->hasPermission('ZikulaGroupsModule::', $group->getGid() . '::', ACCESS_EDIT)) { throw new AccessDeniedException(); } + if (!$this->isCsrfTokenValid('membership-add', $token)) { + throw new AccessDeniedException(); + } if ($userEntity->getGroups()->contains($group)) { $this->addFlash('warning', $this->__('The selected user is already a member of this group.')); @@ -312,8 +308,7 @@ public function getUsersByFragmentAsTableAction(Request $request, UserRepository return $this->render('@ZikulaGroupsModule/Membership/userlist.html.twig', [ 'users' => $users, - 'gid' => $request->get('gid'), - 'csrfToken' => $request->get('csrfToken') + 'gid' => $request->request->get('gid') ], new PlainResponse()); } diff --git a/src/system/GroupsModule/Resources/views/Membership/adminList.html.twig b/src/system/GroupsModule/Resources/views/Membership/adminList.html.twig index 9559eb456c..934d76ae8e 100644 --- a/src/system/GroupsModule/Resources/views/Membership/adminList.html.twig +++ b/src/system/GroupsModule/Resources/views/Membership/adminList.html.twig @@ -43,7 +43,7 @@

{{ __('Seach by username') }}

- +
diff --git a/src/system/GroupsModule/Resources/views/Membership/userlist.html.twig b/src/system/GroupsModule/Resources/views/Membership/userlist.html.twig index a98c91292a..02dffe7d17 100644 --- a/src/system/GroupsModule/Resources/views/Membership/userlist.html.twig +++ b/src/system/GroupsModule/Resources/views/Membership/userlist.html.twig @@ -4,7 +4,7 @@ {{ user.uname }} {{ user.email }} - + {% else %} diff --git a/src/system/SecurityCenterModule/Controller/IdsLogController.php b/src/system/SecurityCenterModule/Controller/IdsLogController.php index 00c9fc734f..45a1eaad05 100644 --- a/src/system/SecurityCenterModule/Controller/IdsLogController.php +++ b/src/system/SecurityCenterModule/Controller/IdsLogController.php @@ -24,7 +24,6 @@ use Zikula\Component\SortableColumns\SortableColumns; use Zikula\Core\Controller\AbstractController; use Zikula\Core\Response\PlainResponse; -use Zikula\Core\Token\CsrfTokenHandler; use Zikula\SecurityCenterModule\Entity\Repository\IntrusionRepository; use Zikula\SecurityCenterModule\Form\Type\IdsLogExportType; use Zikula\SecurityCenterModule\Form\Type\IdsLogFilterType; @@ -45,14 +44,13 @@ class IdsLogController extends AbstractController * * @param Request $request * @param IntrusionRepository $repository - * @param CsrfTokenHandler $tokenHandler * @param RouterInterface $router * * @return Response symfony response object * * @throws AccessDeniedException Thrown if the user doesn't have admin access to the module */ - public function viewAction(Request $request, IntrusionRepository $repository, CsrfTokenHandler $tokenHandler, RouterInterface $router) + public function viewAction(Request $request, IntrusionRepository $repository, RouterInterface $router) { // Security check if (!$this->hasPermission('ZikulaSecurityCenterModule::', '::', ACCESS_EDIT)) { @@ -127,8 +125,7 @@ public function viewAction(Request $request, IntrusionRepository $repository, Cs 'pager' => [ 'amountOfItems' => $amountOfItems, 'itemsPerPage' => $pageSize - ], - 'csrftoken' => $tokenHandler->generate(true) + ] ]; return $templateParameters; @@ -298,24 +295,23 @@ public function purgeAction(Request $request, IntrusionRepository $repository) * * @param Request $request * @param IntrusionRepository $repository - * @param CsrfTokenHandler $tokenHandler * * @return RedirectResponse * * @throws \InvalidArgumentException Thrown if the object id is not numeric or if */ - public function deleteentryAction(Request $request, IntrusionRepository $repository, CsrfTokenHandler $tokenHandler) + public function deleteentryAction(Request $request, IntrusionRepository $repository) { - // Security check if (!$this->hasPermission('ZikulaSecurityCenterModule::', '::', ACCESS_DELETE)) { throw new AccessDeniedException(); } - // verify auth-key - $tokenHandler->validate($request->get('csrftoken')); + if (!$this->isCsrfTokenValid('delete-idsentry', $request->query->get('token'))) { + throw new AccessDeniedException(); + } // get parameters - $id = (int)$request->get('id', 0); + $id = (int)$request->query->get('id', 0); // sanity check if (!is_numeric($id)) { diff --git a/src/system/SecurityCenterModule/Resources/views/IdsLog/view.html.twig b/src/system/SecurityCenterModule/Resources/views/IdsLog/view.html.twig index d02eb3e5ab..dedf5b3d09 100644 --- a/src/system/SecurityCenterModule/Resources/views/IdsLog/view.html.twig +++ b/src/system/SecurityCenterModule/Resources/views/IdsLog/view.html.twig @@ -88,6 +88,7 @@ + {% set csrfToken = csrf_token('delete-idsentry') %} {% for event in logEntries %} {{ event.name }} @@ -109,7 +110,7 @@ {{ event.date|localizeddate('medium', 'none') }} - + {% endfor %}