From 8e2c15ab914ccf29ce7e328de1826b99aa1dd839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Garc=C3=ADa=20G=C3=B3mez?= Date: Fri, 22 Oct 2021 16:42:09 +0200 Subject: [PATCH] Improved form token generation and validation. --- Core/Base/Controller.php | 5 +- Core/Controller/AdminPlugins.php | 49 ++-- Core/Controller/DocumentStitcher.php | 2 +- Core/Controller/EditApiKey.php | 49 ++-- Core/Controller/EditRole.php | 42 ++-- .../Lib/ExtendedController/BaseController.php | 34 ++- .../BusinessDocumentController.php | 52 ++--- Core/Lib/ExtendedController/DocFilesTrait.php | 80 +++---- .../ExtendedController/PanelController.php | 62 ++--- Core/Lib/MultiRequestProtection.php | 57 +++-- Core/View/AdminPlugins.html.twig | 214 +++++++++--------- Core/View/DocumentStitcher.html.twig | 2 +- .../Master/BusinessDocumentView.html.twig | 2 +- Core/View/Master/EditListView.html.twig | 4 +- Core/View/Master/EditListViewInLine.html.twig | 4 +- Core/View/Master/EditView.html.twig | 4 +- Core/View/Master/GridView.html.twig | 2 +- Core/View/Master/ListView.html.twig | 2 +- Core/View/Tab/ApiAccess.html.twig | 2 +- Core/View/Tab/DocFiles.html.twig | 4 +- Core/View/Tab/RoleAccess.html.twig | 2 +- 21 files changed, 351 insertions(+), 323 deletions(-) diff --git a/Core/Base/Controller.php b/Core/Base/Controller.php index 6ad5e69eea..94c3c475ea 100644 --- a/Core/Base/Controller.php +++ b/Core/Base/Controller.php @@ -123,7 +123,7 @@ public function __construct(string $className, string $uri = '') $this->className = $className; $this->dataBase = new DataBase(); $this->empresa = new Empresa(); - $this->multiRequestProtection = new MultiRequestProtection($className); + $this->multiRequestProtection = new MultiRequestProtection(); $this->request = Request::createFromGlobals(); $this->template = $this->className . '.html.twig'; $this->uri = $uri; @@ -201,6 +201,9 @@ public function privateCore(&$response, $user, $permissions) // Select the default company for the user $this->empresa = Empresas::get($this->user->idempresa); + // add the user to the token generation seed + $this->multiRequestProtection->addSeed($user->nick); + // Have this user a default page? $defaultPage = $this->request->query->get('defaultPage', ''); if ($defaultPage === 'TRUE') { diff --git a/Core/Controller/AdminPlugins.php b/Core/Controller/AdminPlugins.php index 652d3291c0..2fc74554fc 100644 --- a/Core/Controller/AdminPlugins.php +++ b/Core/Controller/AdminPlugins.php @@ -16,12 +16,13 @@ * You should have received a copy of the GNU Lesser General Public License * along with this program. If not, see . */ + namespace FacturaScripts\Core\Controller; use FacturaScripts\Core\Base; use FacturaScripts\Dinamic\Model\User; -use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\File\UploadedFile; +use Symfony\Component\HttpFoundation\Response; /** * AdminPlugins. @@ -41,10 +42,9 @@ class AdminPlugins extends Base\Controller public $pluginManager; /** - * * @return array */ - public function getAllPlugins() + public function getAllPlugins(): array { $downloadTools = new Base\DownloadTools(); $json = json_decode($downloadTools->getContents(self::PLUGIN_LIST_URL, 3), true); @@ -54,7 +54,7 @@ public function getAllPlugins() $list = []; foreach ($json as $item) { - /// plugin is already installed? + // plugin is already installed? $item['installed'] = false; foreach ($this->getPlugins() as $plug) { if ($plug['name'] == $item['name']) { @@ -101,14 +101,14 @@ public function getPageData() public function getPlugins() { $installedPlugins = $this->pluginManager->installedPlugins(); - if (!defined('FS_HIDDEN_PLUGINS')) { + if (false === defined('FS_HIDDEN_PLUGINS')) { return $installedPlugins; } - /// exclude hidden plugins - $hiddenPlugins = \explode(',', \FS_HIDDEN_PLUGINS); + // exclude hidden plugins + $hiddenPlugins = explode(',', FS_HIDDEN_PLUGINS); foreach ($installedPlugins as $key => $plugin) { - if (\in_array($plugin['name'], $hiddenPlugins, false)) { + if (in_array($plugin['name'], $hiddenPlugins, false)) { unset($installedPlugins[$key]); } } @@ -118,9 +118,9 @@ public function getPlugins() /** * Runs the controller's private logic. * - * @param Response $response - * @param User $user - * @param Base\ControllerPermissions $permissions + * @param Response $response + * @param User $user + * @param Base\ControllerPermissions $permissions */ public function privateCore(&$response, $user, $permissions) { @@ -169,7 +169,7 @@ private function enablePlugin($pluginName) /** * Execute main actions. - * + * * @param string $action */ private function execAction($action) @@ -198,8 +198,8 @@ private function execAction($action) break; default: - if (\FS_DEBUG) { - /// On debug mode, always deploy the contents of Dinamic. + if (FS_DEBUG) { + // On debug mode, always deploy the contents of Dinamic. $this->pluginManager->deploy(true, true); $this->toolBox()->cache()->clear(); } @@ -232,6 +232,25 @@ private function removePlugin($pluginName) */ private function uploadPlugin($uploadFiles) { + // check user permissions + if (false === $this->permissions->allowUpdate) { + $this->toolBox()->i18nLog()->warning('not-allowed-update'); + return; + } + + // valid request? + $token = $this->request->request->get('multireqtoken', ''); + if (empty($token) || false === $this->multiRequestProtection->validate($token)) { + $this->toolBox()->i18nLog()->warning('invalid-request'); + return; + } + + // duplicated request? + if ($this->multiRequestProtection->tokenExist($token)) { + $this->toolBox()->i18nLog()->warning('duplicated-request'); + return; + } + foreach ($uploadFiles as $uploadFile) { if (false === $uploadFile->isValid()) { $this->toolBox()->log()->error($uploadFile->getErrorMessage()); @@ -244,7 +263,7 @@ private function uploadPlugin($uploadFiles) } $this->pluginManager->install($uploadFile->getPathname(), $uploadFile->getClientOriginalName()); - \unlink($uploadFile->getPathname()); + unlink($uploadFile->getPathname()); } if ($this->pluginManager->deploymentRequired()) { diff --git a/Core/Controller/DocumentStitcher.php b/Core/Controller/DocumentStitcher.php index e60682c347..30167e1c3c 100644 --- a/Core/Controller/DocumentStitcher.php +++ b/Core/Controller/DocumentStitcher.php @@ -119,7 +119,7 @@ public function privateCore(&$response, $user, $permissions) if ($status) { // validate form request? $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { + if (empty($token) || false === $this->multiRequestProtection->validate($token)) { $this->toolBox()->i18nLog()->warning('invalid-request'); return; } diff --git a/Core/Controller/EditApiKey.php b/Core/Controller/EditApiKey.php index 91b5e413bb..afc481e07c 100644 --- a/Core/Controller/EditApiKey.php +++ b/Core/Controller/EditApiKey.php @@ -16,9 +16,9 @@ * You should have received a copy of the GNU Lesser General Public License * along with this program. If not, see . */ + namespace FacturaScripts\Core\Controller; -use Exception; use FacturaScripts\Core\Base\DataBase\DataBaseWhere; use FacturaScripts\Core\Lib\ExtendedController\BaseView; use FacturaScripts\Core\Lib\ExtendedController\EditController; @@ -34,7 +34,6 @@ class EditApiKey extends EditController { /** - * * @return array */ public function getAccessRules(): array @@ -63,7 +62,7 @@ public function getAccessRules(): array /** * Returns the model name. - * + * * @return string */ public function getModelClassName() @@ -96,7 +95,6 @@ protected function createViews() } /** - * * @param string $viewName */ protected function createViewsAccess(string $viewName = 'ApiAccess') @@ -105,29 +103,36 @@ protected function createViewsAccess(string $viewName = 'ApiAccess') } /** - * * @return bool */ protected function editRulesAction(): bool { + // check user permissions + if (false === $this->permissions->allowUpdate) { + $this->toolBox()->i18nLog()->warning('not-allowed-update'); + return true; + } elseif (false === $this->validateFormToken()) { + return true; + } + $allowGet = $this->request->request->get('allowget'); $allowPut = $this->request->request->get('allowput'); $allowPost = $this->request->request->get('allowpost'); $allowDelete = $this->request->request->get('allowdelete'); - /// update current access rules + // update current access rules $accessModel = new ApiAccess(); $where = [new DataBaseWhere('idapikey', $this->request->query->get('code'))]; $rules = $accessModel->all($where, [], 0, 0); foreach ($rules as $access) { - $access->allowget = \is_array($allowGet) && \in_array($access->resource, $allowGet); - $access->allowput = \is_array($allowPut) && \in_array($access->resource, $allowPut); - $access->allowpost = \is_array($allowPost) && \in_array($access->resource, $allowPost); - $access->allowdelete = \is_array($allowDelete) && \in_array($access->resource, $allowDelete); + $access->allowget = is_array($allowGet) && in_array($access->resource, $allowGet); + $access->allowput = is_array($allowPut) && in_array($access->resource, $allowPut); + $access->allowpost = is_array($allowPost) && in_array($access->resource, $allowPost); + $access->allowdelete = is_array($allowDelete) && in_array($access->resource, $allowDelete); $access->save(); } - /// add new rules + // add new rules foreach ($allowGet as $resource) { $found = false; foreach ($rules as $rule) { @@ -140,14 +145,14 @@ protected function editRulesAction(): bool continue; } - /// add + // add $newAccess = new ApiAccess(); $newAccess->idapikey = $this->request->query->get('code'); $newAccess->resource = $resource; - $newAccess->allowget = \is_array($allowGet) && \in_array($resource, $allowGet); - $newAccess->allowput = \is_array($allowPut) && \in_array($resource, $allowPut); - $newAccess->allowpost = \is_array($allowPost) && \in_array($resource, $allowPost); - $newAccess->allowdelete = \is_array($allowDelete) && \in_array($resource, $allowDelete); + $newAccess->allowget = is_array($allowGet) && in_array($resource, $allowGet); + $newAccess->allowput = is_array($allowPut) && in_array($resource, $allowPut); + $newAccess->allowpost = is_array($allowPost) && in_array($resource, $allowPost); + $newAccess->allowdelete = is_array($allowDelete) && in_array($resource, $allowDelete); $newAccess->save(); } @@ -183,29 +188,29 @@ protected function getResources(): array { $resources = []; - $path = \FS_FOLDER . DIRECTORY_SEPARATOR . 'Dinamic' . DIRECTORY_SEPARATOR . 'Lib' . DIRECTORY_SEPARATOR . 'API'; - foreach (\scandir($path, SCANDIR_SORT_NONE) as $resource) { - if (\substr($resource, -4) !== '.php') { + $path = FS_FOLDER . DIRECTORY_SEPARATOR . 'Dinamic' . DIRECTORY_SEPARATOR . 'Lib' . DIRECTORY_SEPARATOR . 'API'; + foreach (scandir($path, SCANDIR_SORT_NONE) as $resource) { + if (substr($resource, -4) !== '.php') { continue; } $class = substr('\\FacturaScripts\\Dinamic\\Lib\\API\\' . $resource, 0, -4); $APIClass = new $class($this->response, $this->request, []); - if (isset($APIClass) && \method_exists($APIClass, 'getResources')) { + if (isset($APIClass) && method_exists($APIClass, 'getResources')) { foreach ($APIClass->getResources() as $name => $data) { $resources[] = $name; } } } - \sort($resources); + sort($resources); return $resources; } /** * Load view data. * - * @param string $viewName + * @param string $viewName * @param BaseView $view */ protected function loadData($viewName, $view) diff --git a/Core/Controller/EditRole.php b/Core/Controller/EditRole.php index e594da3b76..adfbe17bcc 100644 --- a/Core/Controller/EditRole.php +++ b/Core/Controller/EditRole.php @@ -16,6 +16,7 @@ * You should have received a copy of the GNU Lesser General Public License * along with this program. If not, see . */ + namespace FacturaScripts\Core\Controller; use FacturaScripts\Core\Base\DataBase\DataBaseWhere; @@ -34,7 +35,6 @@ class EditRole extends EditController { /** - * * @return array */ public function getAccessRules(): array @@ -64,7 +64,6 @@ public function getAccessRules(): array } /** - * * @return string */ public function getModelClassName() @@ -98,7 +97,6 @@ protected function createViews() } /** - * * @param string $viewName */ protected function createViewsAccess(string $viewName = 'RoleAccess') @@ -107,7 +105,6 @@ protected function createViewsAccess(string $viewName = 'RoleAccess') } /** - * * @param string $viewName */ protected function createViewsUsers(string $viewName = 'EditRoleUser') @@ -115,40 +112,47 @@ protected function createViewsUsers(string $viewName = 'EditRoleUser') $this->addEditListView($viewName, 'RoleUser', 'users', 'fas fa-address-card'); $this->views[$viewName]->setInLine(true); - /// Disable column + // Disable column $this->views[$viewName]->disableColumn('role', true); } /** - * * @return bool */ protected function editRulesAction(): bool { + // check user permissions + if (false === $this->permissions->allowUpdate) { + $this->toolBox()->i18nLog()->warning('not-allowed-update'); + return true; + } elseif (false === $this->validateFormToken()) { + return true; + } + $show = $this->request->request->get('show'); $onlyOwner = $this->request->request->get('onlyOwner'); $update = $this->request->request->get('update'); $delete = $this->request->request->get('delete'); - /// update or delete current access rules + // update or delete current access rules $roleAccessModel = new RoleAccess(); $where = [new DataBaseWhere('codrole', $this->request->query->get('code'))]; $rules = $roleAccessModel->all($where, [], 0, 0); foreach ($rules as $roleAccess) { - /// delete rule? - if (false === \is_array($show) || false === \in_array($roleAccess->pagename, $show)) { + // delete rule? + if (false === is_array($show) || false === in_array($roleAccess->pagename, $show)) { $roleAccess->delete(); continue; } - /// update - $roleAccess->onlyownerdata = \is_array($onlyOwner) && \in_array($roleAccess->pagename, $onlyOwner); - $roleAccess->allowupdate = \is_array($update) && \in_array($roleAccess->pagename, $update); - $roleAccess->allowdelete = \is_array($delete) && \in_array($roleAccess->pagename, $delete); + // update + $roleAccess->onlyownerdata = is_array($onlyOwner) && in_array($roleAccess->pagename, $onlyOwner); + $roleAccess->allowupdate = is_array($update) && in_array($roleAccess->pagename, $update); + $roleAccess->allowdelete = is_array($delete) && in_array($roleAccess->pagename, $delete); $roleAccess->save(); } - /// add new rules + // add new rules foreach ($show as $pageName) { $found = false; foreach ($rules as $rule) { @@ -161,13 +165,13 @@ protected function editRulesAction(): bool continue; } - /// add + // add $newRoleAccess = new RoleAccess(); $newRoleAccess->codrole = $this->request->query->get('code'); $newRoleAccess->pagename = $pageName; - $newRoleAccess->onlyownerdata = \is_array($onlyOwner) && \in_array($pageName, $onlyOwner); - $newRoleAccess->allowupdate = \is_array($update) && \in_array($pageName, $update); - $newRoleAccess->allowdelete = \is_array($delete) && \in_array($pageName, $delete); + $newRoleAccess->onlyownerdata = is_array($onlyOwner) && in_array($pageName, $onlyOwner); + $newRoleAccess->allowupdate = is_array($update) && in_array($pageName, $update); + $newRoleAccess->allowdelete = is_array($delete) && in_array($pageName, $delete); $newRoleAccess->save(); } @@ -207,7 +211,7 @@ protected function getAllPages() /** * Load view data * - * @param string $viewName + * @param string $viewName * @param BaseView $view */ protected function loadData($viewName, $view) diff --git a/Core/Lib/ExtendedController/BaseController.php b/Core/Lib/ExtendedController/BaseController.php index cdb86973fb..e782b4c755 100644 --- a/Core/Lib/ExtendedController/BaseController.php +++ b/Core/Lib/ExtendedController/BaseController.php @@ -280,18 +280,7 @@ protected function deleteAction() if (false === $this->permissions->allowDelete || false === $this->views[$this->active]->settings['btnDelete']) { $this->toolBox()->i18nLog()->warning('not-allowed-delete'); return false; - } - - // valid request? - $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { - $this->toolBox()->i18nLog()->warning('invalid-request'); - return false; - } - - // duplicated request? - if ($this->multiRequestProtection->tokenExist($token)) { - $this->toolBox()->i18nLog()->warning('duplicated-request'); + } elseif (false === $this->validateFormToken()) { return false; } @@ -407,4 +396,25 @@ protected function requestGet(array $keys): array } return $result; } + + /** + * @return bool + */ + protected function validateFormToken(): bool + { + // valid request? + $token = $this->request->request->get('multireqtoken', ''); + if (empty($token) || false === $this->multiRequestProtection->validate($token)) { + $this->toolBox()->i18nLog()->warning('invalid-request'); + return false; + } + + // duplicated request? + if ($this->multiRequestProtection->tokenExist($token)) { + $this->toolBox()->i18nLog()->warning('duplicated-request'); + return false; + } + + return true; + } } diff --git a/Core/Lib/ExtendedController/BusinessDocumentController.php b/Core/Lib/ExtendedController/BusinessDocumentController.php index 813a1f96cb..b7d64f6e36 100644 --- a/Core/Lib/ExtendedController/BusinessDocumentController.php +++ b/Core/Lib/ExtendedController/BusinessDocumentController.php @@ -82,23 +82,23 @@ public function __construct(string $className, string $uri = '') */ protected function createViews() { - /// tabs on top + // tabs on top $this->setTabsPosition('top'); - /// document tab + // document tab $fullModelName = self::MODEL_NAMESPACE . $this->getModelClassName(); $view = new BusinessDocumentView($this->getLineXMLView(), 'new', $fullModelName); $this->addCustomView($view->getViewName(), $view); $this->setSettings($view->getViewName(), 'btnPrint', true); - /// edit tab + // edit tab $viewName = 'Edit' . $this->getModelClassName(); $this->addEditView($viewName, $this->getModelClassName(), 'detail', 'fas fa-edit'); - /// disable delete button + // disable delete button $this->setSettings($viewName, 'btnDelete', false); - /// files tab + // files tab $this->createViewDocFiles(); } @@ -198,10 +198,10 @@ protected function loadData($viewName, $view) break; } - /// data not found? + // data not found? $view->loadData($code); - /// User can access to data? + // User can access to data? if (false === $this->checkOwnerData($view->model)) { $this->setTemplate('Error/AccessDenied'); break; @@ -233,17 +233,17 @@ protected function recalculateDocumentAction(): bool { $this->setTemplate(false); - /// loads model + // loads model $data = $this->getBusinessFormData(); $merged = array_merge($data['custom'], $data['final'], $data['form'], $data['subject']); $this->views[$this->active]->loadFromData($merged); - /// update subject data? + // update subject data? if (false === $this->views[$this->active]->model->exists()) { $this->views[$this->active]->model->updateSubject(); } - /// recalculate + // recalculate $result = $this->documentTools->recalculateForm($this->views[$this->active]->model, $data['lines']); $this->response->setContent($result); return false; @@ -262,30 +262,30 @@ protected function saveDocumentAction(): bool return false; } - /// valid request? + // valid request? $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { + if (empty($token) || false === $this->multiRequestProtection->validate($token)) { $this->response->setContent($this->toolBox()->i18n()->trans('invalid-request')); return false; } - /// duplicated request? + // duplicated request? if ($this->multiRequestProtection->tokenExist($token)) { $this->response->setContent($this->toolBox()->i18n()->trans('duplicated-request')); return false; } - /// loads model + // loads model $data = $this->getBusinessFormData(); $this->views[$this->active]->model->setAuthor($this->user); $this->views[$this->active]->loadFromData($data['form']); $this->views[$this->active]->lines = $this->views[$this->active]->model->getLines(); - /// save + // save $result = $this->saveDocumentResult($this->views[$this->active], $data); $this->response->setContent($result); - /// event finish + // event finish $this->views[$this->active]->model->pipe('finish'); return false; } @@ -301,7 +301,7 @@ protected function saveDocumentError(string $message): string $message .= "\n" . $msg['message']; } - /// undo transaction + // undo transaction $this->dataBase->rollback(); return $message; @@ -315,19 +315,19 @@ protected function saveDocumentError(string $message): string */ protected function saveDocumentResult(BusinessDocumentView &$view, array &$data): string { - /// start transaction + // start transaction $this->dataBase->beginTransaction(); - /// sets subjects + // sets subjects $result = $this->setSubject($view, $data['subject']); if ('OK' !== $result) { return $this->saveDocumentError($result); } - /// custom data fields + // custom data fields $view->model->loadFromData($data['custom']); if ($view->model->save() && $this->saveLines($view, $data['lines'])) { - /// final data fields + // final data fields $view->model->loadFromData($data['final']); $this->documentTools->recalculate($view->model); @@ -353,7 +353,7 @@ protected function saveLines(BusinessDocumentView &$view, array &$newLines): boo return true; } - /// remove or modify old lines + // remove or modify old lines foreach ($view->lines as $oldLine) { $found = false; foreach ($newLines as $newLine) { @@ -374,7 +374,7 @@ protected function saveLines(BusinessDocumentView &$view, array &$newLines): boo } } - /// add new lines + // add new lines foreach (array_reverse($newLines) as $fLine) { if ($fLine['idlinea']) { continue; @@ -396,12 +396,12 @@ protected function subjectChangedAction(): bool { $this->setTemplate(false); - /// loads model + // loads model $data = $this->getBusinessFormData(); $merged = array_merge($data['custom'], $data['final'], $data['form'], $data['subject']); $this->views[$this->active]->loadFromData($merged); - /// update subject data? + // update subject data? if (false === $this->views[$this->active]->model->exists()) { $this->views[$this->active]->model->updateSubject(); } @@ -420,7 +420,7 @@ protected function subjectChangedAction(): bool */ protected function updateLine($oldLine, array $newLine): bool { - /// reload line data from database to get last changes + // reload line data from database to get last changes $oldLine->loadFromCode($oldLine->primaryColumnValue()); $oldLine->loadFromData($newLine, ['actualizastock']); diff --git a/Core/Lib/ExtendedController/DocFilesTrait.php b/Core/Lib/ExtendedController/DocFilesTrait.php index 171ec98dba..3a57f2e284 100644 --- a/Core/Lib/ExtendedController/DocFilesTrait.php +++ b/Core/Lib/ExtendedController/DocFilesTrait.php @@ -35,7 +35,6 @@ trait DocFilesTrait abstract protected function addHtmlView(string $viewName, string $fileName, string $modelName, string $viewTitle, string $viewIcon = 'fab fa-html5'); /** - * * @return bool */ private function addFileAction(): bool @@ -43,18 +42,7 @@ private function addFileAction(): bool if (false === $this->permissions->allowUpdate) { ToolBox::i18nLog()->warning('not-allowed-modify'); return true; - } - - /// valid request? - $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { - ToolBox::i18nLog()->warning('invalid-request'); - return true; - } - - /// duplicated request? - if ($this->multiRequestProtection->tokenExist($token)) { - ToolBox::i18nLog()->warning('duplicated-request'); + } elseif (false === $this->validateFileActionToken()) { return true; } @@ -85,8 +73,8 @@ private function addFileAction(): bool } /** - * * @param string $viewName + * @param string $template */ protected function createViewDocFiles(string $viewName = 'docfiles', string $template = 'Tab/DocFiles') { @@ -94,7 +82,6 @@ protected function createViewDocFiles(string $viewName = 'docfiles', string $tem } /** - * * @return bool */ private function deleteFileAction(): bool @@ -102,18 +89,7 @@ private function deleteFileAction(): bool if (false === $this->permissions->allowDelete) { ToolBox::i18nLog()->warning('not-allowed-delete'); return true; - } - - /// valid request? - $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { - ToolBox::i18nLog()->warning('invalid-request'); - return true; - } - - /// duplicated request? - if ($this->multiRequestProtection->tokenExist($token)) { - ToolBox::i18nLog()->warning('duplicated-request'); + } elseif (false === $this->validateFileActionToken()) { return true; } @@ -130,7 +106,6 @@ private function deleteFileAction(): bool } /** - * * @return bool */ private function editFileAction(): bool @@ -138,18 +113,7 @@ private function editFileAction(): bool if (false === $this->permissions->allowUpdate) { ToolBox::i18nLog()->warning('not-allowed-modify'); return true; - } - - /// valid request? - $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { - ToolBox::i18nLog()->warning('invalid-request'); - return true; - } - - /// duplicated request? - if ($this->multiRequestProtection->tokenExist($token)) { - ToolBox::i18nLog()->warning('duplicated-request'); + } elseif (false === $this->validateFileActionToken()) { return true; } @@ -165,7 +129,6 @@ private function editFileAction(): bool } /** - * * @param BaseView $view * @param string $model * @param string $modelid @@ -180,7 +143,6 @@ private function loadDataDocFiles($view, $model, $modelid) } /** - * * @return bool */ private function unlinkFileAction(): bool @@ -188,18 +150,7 @@ private function unlinkFileAction(): bool if (false === $this->permissions->allowUpdate) { ToolBox::i18nLog()->warning('not-allowed-modify'); return true; - } - - /// valid request? - $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { - ToolBox::i18nLog()->warning('invalid-request'); - return true; - } - - /// duplicated request? - if ($this->multiRequestProtection->tokenExist($token)) { - ToolBox::i18nLog()->warning('duplicated-request'); + } elseif (false === $this->validateFileActionToken()) { return true; } @@ -212,4 +163,25 @@ private function unlinkFileAction(): bool ToolBox::i18nLog()->notice('record-updated-correctly'); return true; } + + /** + * @return bool + */ + private function validateFileActionToken(): bool + { + // valid request? + $token = $this->request->request->get('multireqtoken', ''); + if (empty($token) || false === $this->multiRequestProtection->validate($token)) { + ToolBox::i18nLog()->warning('invalid-request'); + return false; + } + + // duplicated request? + if ($this->multiRequestProtection->tokenExist($token)) { + ToolBox::i18nLog()->warning('duplicated-request'); + return false; + } + + return true; + } } diff --git a/Core/Lib/ExtendedController/PanelController.php b/Core/Lib/ExtendedController/PanelController.php index a628bff2da..1d95d46867 100644 --- a/Core/Lib/ExtendedController/PanelController.php +++ b/Core/Lib/ExtendedController/PanelController.php @@ -77,24 +77,24 @@ public function privateCore(&$response, $user, $permissions) { parent::privateCore($response, $user, $permissions); - /// Get any operations that have to be performed + // Get any operations that have to be performed $action = $this->request->request->get('action', $this->request->query->get('action', '')); - /// Runs operations before reading data + // Runs operations before reading data if ($this->execPreviousAction($action) === false || $this->pipe('execPreviousAction', $action) === false) { return; } - /// Load the data for each view + // Load the data for each view $mainViewName = $this->getMainViewName(); foreach ($this->views as $viewName => $view) { - /// disable views if main view has no data + // disable views if main view has no data if ($viewName != $mainViewName && false === $this->hasData) { $this->setSettings($viewName, 'active', false); } if (false === $view->settings['active']) { - /// exclude inactive views + // exclude inactive views continue; } elseif ($this->active == $viewName) { $view->processFormData($this->request, 'load'); @@ -110,7 +110,7 @@ public function privateCore(&$response, $user, $permissions) } } - /// General operations with the loaded data + // General operations with the loaded data $this->execAfterAction($action); $this->pipe('execAfterAction', $action); } @@ -235,21 +235,10 @@ protected function addListView(string $viewName, string $modelName, string $view */ protected function editAction() { - if (!$this->permissions->allowUpdate) { + if (false === $this->permissions->allowUpdate) { $this->toolBox()->i18nLog()->warning('not-allowed-modify'); return false; - } - - // valid request? - $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { - $this->toolBox()->i18nLog()->warning('invalid-request'); - return false; - } - - // duplicated request? - if ($this->multiRequestProtection->tokenExist($token)) { - $this->toolBox()->i18nLog()->warning('duplicated-request'); + } elseif (false === $this->validateFormToken()) { return false; } @@ -332,7 +321,7 @@ protected function execPreviousAction($action) case 'insert': if ($this->insertAction() || !empty($this->views[$this->active]->model->primaryColumnValue())) { - /// wee need to clear model in these scenarios + // we need to clear model in these scenarios $this->views[$this->active]->model->clear(); } break; @@ -362,18 +351,7 @@ protected function insertAction() if (false === $this->permissions->allowUpdate) { $this->toolBox()->i18nLog()->warning('not-allowed-modify'); return false; - } - - // valid request? - $token = $this->request->request->get('multireqtoken', ''); - if (empty($token) || false === $this->multiRequestProtection->validate($token, $this->user->logkey)) { - $this->toolBox()->i18nLog()->warning('invalid-request'); - return false; - } - - // duplicated request? - if ($this->multiRequestProtection->tokenExist($token)) { - $this->toolBox()->i18nLog()->warning('duplicated-request'); + } elseif (false === $this->validateFormToken()) { return false; } @@ -385,19 +363,19 @@ protected function insertAction() } // save in database - if ($this->views[$this->active]->model->save()) { - /// redir to new model url only if this is the first view - if ($this->active === $this->getMainViewName()) { - $this->redirect($this->views[$this->active]->model->url() . '&action=save-ok'); - } + if (false === $this->views[$this->active]->model->save()) { + $this->toolBox()->i18nLog()->error('record-save-error'); + return false; + } - $this->views[$this->active]->newCode = $this->views[$this->active]->model->primaryColumnValue(); - $this->toolBox()->i18nLog()->notice('record-updated-correctly'); - return true; + // redirect to new model url only if this is the first view + if ($this->active === $this->getMainViewName()) { + $this->redirect($this->views[$this->active]->model->url() . '&action=save-ok'); } - $this->toolBox()->i18nLog()->error('record-save-error'); - return false; + $this->views[$this->active]->newCode = $this->views[$this->active]->model->primaryColumnValue(); + $this->toolBox()->i18nLog()->notice('record-updated-correctly'); + return true; } /** diff --git a/Core/Lib/MultiRequestProtection.php b/Core/Lib/MultiRequestProtection.php index f606e5e623..2f9b309587 100644 --- a/Core/Lib/MultiRequestProtection.php +++ b/Core/Lib/MultiRequestProtection.php @@ -31,7 +31,9 @@ class MultiRequestProtection { const CACHE_KEY = 'MultiRequestProtection'; + const MAX_TOKEN_AGE = 4; const MAX_TOKENS = 500; + const RANDOM_STRING_LENGTH = 6; /** * @var Cache @@ -41,24 +43,37 @@ class MultiRequestProtection /** * @var string */ - protected $controllerName; + protected $seed; - public function __construct(string $controllerName) + public function __construct() { $this->cache = new Cache(); - $this->controllerName = $controllerName; + + // something unique in each installation + $this->seed = PHP_VERSION . __FILE__ . FS_DB_NAME . FS_DB_PASS . FS_CACHE_PREFIX; } /** - * Generates a random token for this code. - * - * @param string $code + * @param string $seed + */ + public function addSeed(string $seed) + { + $this->seed .= $seed; + } + + /** + * Generates a random token. * * @return string */ - public function newToken(string $code = ''): string + public function newToken(): string { - return sha1($this->controllerName . $code) . '|' . $this->getRandomStr(); + // something that changes every hour + $num = intval(date('YmdH')) + strlen($this->seed); + + // combine and generate the token + $value = $this->seed . $num; + return sha1($value) . '|' . $this->getRandomStr(); } /** @@ -81,16 +96,28 @@ public function tokenExist(string $token): bool /** * @param string $token - * @param string $code * * @return bool */ - public function validate(string $token, string $code = ''): bool + public function validate(string $token): bool { - $newTokenParts = explode('|', $this->newToken($code)); $tokenParts = explode('|', $token); + if (count($tokenParts) != 2) { + // invalid token format + // the random part can be incremented in javascript so there is no fixed length + return false; + } + + // check all valid tokens roots + $num = intval(date('YmdH')) + strlen($this->seed); + $valid = [sha1($this->seed . $num)]; + for ($hour = 1; $hour <= self::MAX_TOKEN_AGE; $hour++) { + $time = strtotime('-' . $hour . ' hours'); + $altNum = intval(date('YmdH', $time)) + strlen($this->seed); + $valid[] = sha1($this->seed . $altNum); + } - return count($tokenParts) === 2 && $tokenParts[0] === $newTokenParts[0]; + return in_array($tokenParts[0], $valid); } /** @@ -99,7 +126,7 @@ public function validate(string $token, string $code = ''): bool protected function getRandomStr(): string { $chars = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; - return substr(str_shuffle($chars), 0, 5); + return substr(str_shuffle($chars), 0, self::RANDOM_STRING_LENGTH); } /** @@ -113,7 +140,7 @@ protected function getTokens(): array return $tokens; } - /// reduce tokens + // reduce tokens return array_slice($tokens, -10); } @@ -128,7 +155,7 @@ protected function saveToken(string $token): bool { $tokens = $this->getTokens(); - /// save new token + // save new token $tokens[] = $token; return $this->cache->set(self::CACHE_KEY, $tokens); } diff --git a/Core/View/AdminPlugins.html.twig b/Core/View/AdminPlugins.html.twig index 5a9d233544..642c91eb8d 100644 --- a/Core/View/AdminPlugins.html.twig +++ b/Core/View/AdminPlugins.html.twig @@ -1,21 +1,21 @@ {# - /** - * This file is part of FacturaScripts - * Copyright (C) 2017-2020 Carlos Garcia Gomez - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program. If not, see http://www.gnu.org/licenses/. - */ +/** + * This file is part of FacturaScripts + * Copyright (C) 2017-2021 Carlos Garcia Gomez + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ #} {% extends "Master/MenuTemplate.html.twig" %} @@ -29,21 +29,25 @@
- + {% if fsc.getPageData()['name'] == fsc.user.homepage %} - + {% else %} - + {% endif %}
{% if not constant('FS_DISABLE_ADD_PLUGINS') %} - @@ -101,13 +105,15 @@ {# Modal for add new plugins #} {% if not constant('FS_DISABLE_ADD_PLUGINS') %}
- + +