From 526d4c4d15761b58d0e8f83fa09a611a5e36c80d Mon Sep 17 00:00:00 2001 From: Guillaume Lajarige Date: Tue, 24 Jul 2018 14:26:51 +0000 Subject: [PATCH] =?UTF-8?q?N=C2=B01575=20Portal:=20Security=20hardening.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SVN:trunk[5968] --- .../browsebrickcontroller.class.inc.php | 18 +-- .../managebrickcontroller.class.inc.php | 29 ++-- .../objectcontroller.class.inc.php | 141 +++++++++--------- .../userprofilebrickcontroller.class.inc.php | 23 ++- .../requestmanipulatorhelper.class.inc.php | 119 +++++++++++++++ ...stmanipulatorserviceprovider.class.inc.php | 60 ++++++++ .../2.x/itop-portal-base/portal/web/index.php | 5 + 7 files changed, 291 insertions(+), 104 deletions(-) create mode 100644 datamodels/2.x/itop-portal-base/portal/src/helpers/requestmanipulatorhelper.class.inc.php create mode 100644 datamodels/2.x/itop-portal-base/portal/src/providers/requestmanipulatorserviceprovider.class.inc.php diff --git a/datamodels/2.x/itop-portal-base/portal/src/controllers/browsebrickcontroller.class.inc.php b/datamodels/2.x/itop-portal-base/portal/src/controllers/browsebrickcontroller.class.inc.php index 5d1b54585..ddb4a8eb9 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/controllers/browsebrickcontroller.class.inc.php +++ b/datamodels/2.x/itop-portal-base/portal/src/controllers/browsebrickcontroller.class.inc.php @@ -63,10 +63,10 @@ class BrowseBrickController extends BrickController // Getting current browse mode (First from router pamater, then default brick value) $sBrowseMode = (!empty($sBrowseMode)) ? $sBrowseMode : $oBrick->GetDefaultBrowseMode(); // Getting current dataloading mode (First from router parameter, then query parameter, then default brick value) - $sDataLoading = ($sDataLoading !== null) ? $sDataLoading : ( ($oRequest->query->get('sDataLoading') !== null) ? $oRequest->query->get('sDataLoading') : $oBrick->GetDataLoading() ); + $sDataLoading = ($sDataLoading !== null) ? $sDataLoading : $oApp['request_manipulator']->ReadParam('sDataLoading', $oBrick->GetDataLoading()); // Getting search value - $sSearchValue = $oRequest->get('sSearchValue', null); - if ($sSearchValue !== null) + $sSearchValue = $oApp['request_manipulator']->ReadParam('sSearchValue', ''); + if (!empty($sSearchValue)) { $sDataLoading = AbstractBrick::ENUM_DATA_LOADING_LAZY; } @@ -121,7 +121,7 @@ class BrowseBrickController extends BrickController // Adding search clause // Note : For know the search is naive and looks only for the exact match. It doesn't search for words separately - if ($sSearchValue !== null) + if (!empty($sSearchValue)) { // - Cleaning the search value by exploding and trimming spaces $aSearchValues = explode(' ', $sSearchValue); @@ -194,7 +194,7 @@ class BrowseBrickController extends BrickController { $aLevelsProperties[$aLevelsPropertiesKeys[$i]]['search']->SetSelectedClasses($aLevelsClasses); - if ($sSearchValue !== null) + if (!empty($sSearchValue)) { // Note : This could be way more simpler if we had a SetInternalParam($sParam, $value) verb $aQueryParams = $aLevelsProperties[$aLevelsPropertiesKeys[$i]]['search']->GetInternalParams(); @@ -228,8 +228,8 @@ class BrowseBrickController extends BrickController { case BrowseBrick::ENUM_BROWSE_MODE_LIST: // Retrieving parameters - $iPageNumber = (int) $oRequest->get('iPageNumber', 1); - $iListLength = (int) $oRequest->get('iListLength', BrowseBrick::DEFAULT_LIST_LENGTH); + $iPageNumber = (int) $oApp['request_manipulator']->ReadParam('iPageNumber', 1, FILTER_SANITIZE_NUMBER_INT); + $iListLength = (int) $oApp['request_manipulator']->ReadParam('iListLength', BrowseBrick::DEFAULT_LIST_LENGTH, FILTER_SANITIZE_NUMBER_INT); // Getting total records number $oCountSet = new DBObjectSet($oQuery); @@ -244,8 +244,8 @@ class BrowseBrickController extends BrickController case BrowseBrick::ENUM_BROWSE_MODE_TREE: case BrowseBrick::ENUM_BROWSE_MODE_MOSAIC: // Retrieving parameters - $sLevelAlias = $oRequest->get('sLevelAlias'); - $sNodeId = $oRequest->get('sNodeId'); + $sLevelAlias = $oApp['request_manipulator']->ReadParam('sLevelAlias', ''); + $sNodeId = $oApp['request_manipulator']->ReadParam('sNodeId', ''); // If no values for those parameters, we might be loading page in lazy mode for the first time, therefore the URL doesn't have those informations. if (empty($sLevelAlias)) diff --git a/datamodels/2.x/itop-portal-base/portal/src/controllers/managebrickcontroller.class.inc.php b/datamodels/2.x/itop-portal-base/portal/src/controllers/managebrickcontroller.class.inc.php index 7d6de768f..51a3cb83b 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/controllers/managebrickcontroller.class.inc.php +++ b/datamodels/2.x/itop-portal-base/portal/src/controllers/managebrickcontroller.class.inc.php @@ -57,7 +57,6 @@ class ManageBrickController extends BrickController * @param string $sBrickId * @param string $sGroupingTab * @param string $sDisplayMode - * @param string $sDataLoading * * @return Response * @@ -67,7 +66,7 @@ class ManageBrickController extends BrickController * @throws \MySQLException * @throws \OQLException */ - public function DisplayAction(Request $oRequest, Application $oApp, $sBrickId, $sGroupingTab, $sDisplayMode = null, $sDataLoading = null) + public function DisplayAction(Request $oRequest, Application $oApp, $sBrickId, $sGroupingTab, $sDisplayMode = null) { /** @var ManageBrick $oBrick */ $oBrick = ApplicationHelper::GetLoadedBrickFromId($oApp, $sBrickId); @@ -165,7 +164,7 @@ class ManageBrickController extends BrickController $oScopeHelper = $oApp['scope_validator']; $oScopeHelper->AddScopeToQuery($oQuery, $sClass); $aData = array(); - $this->ManageSearchValue($oRequest, $aData, $oQuery, $sClass); + $this->ManageSearchValue($oApp, $aData, $oQuery, $sClass); // Grouping tab if ($oBrick->HasGroupingTabs()) @@ -261,11 +260,11 @@ class ManageBrickController extends BrickController $bHasScope = true; // Getting current dataloading mode (First from router parameter, then query parameter, then default brick value) - $sDataLoading = ($oRequest->get('sDataLoading') !== null) ? $oRequest->get('sDataLoading') : $oBrick->GetDataLoading(); + $sDataLoading = $oApp['request_manipulator']->ReadParam('sDataLoading', $oBrick->GetDataLoading()); // - Retrieving the grouping areas to display - $sGroupingArea = $oRequest->get('sGroupingArea'); - if (!is_null($sGroupingArea)) + $sGroupingArea = $oApp['request_manipulator']->ReadParam('sGroupingArea', ''); + if (!empty($sGroupingArea)) { $bNeedDetails = true; } @@ -345,7 +344,7 @@ class ManageBrickController extends BrickController } // - Retrieving the current grouping tab to display if necessary and altering the query to do so - if ($sGroupingTab === null) + if (empty($sGroupingTab)) { if ($oBrick->HasGroupingTabs()) { @@ -366,7 +365,7 @@ class ManageBrickController extends BrickController } // - Adding search clause if necessary - $this->ManageSearchValue($oRequest, $aData, $oQuery, $sClass, $aColumnsAttrs); + $this->ManageSearchValue($oApp, $aData, $oQuery, $sClass, $aColumnsAttrs); // Preparing areas // - We need to retrieve distinct values for the grouping attribute @@ -416,7 +415,7 @@ class ManageBrickController extends BrickController } // - If specified or lazy loading, we truncate the $aGroupingAreasValues to keep only this one - if ($sGroupingArea !== null) + if (!empty($sGroupingArea)) { $aGroupingAreasValues = array($sGroupingArea => $aGroupingAreasValues[$sGroupingArea]); } @@ -472,8 +471,8 @@ class ManageBrickController extends BrickController if ($sDataLoading === AbstractBrick::ENUM_DATA_LOADING_LAZY) { // Retrieving parameters - $iPageNumber = (int)$oRequest->get('iPageNumber', 1); - $iListLength = (int)$oRequest->get('iListLength', ManageBrick::DEFAULT_LIST_LENGTH); + $iPageNumber = (int)$oApp['request_manipulator']->ReadParam('iPageNumber', 1, FILTER_SANITIZE_NUMBER_INT); + $iListLength = (int)$oApp['request_manipulator']->ReadParam('iListLength', ManageBrick::DEFAULT_LIST_LENGTH, FILTER_SANITIZE_NUMBER_INT); // Getting total records number $oCountSet = new DBObjectSet($oQuery); @@ -743,7 +742,7 @@ class ManageBrickController extends BrickController } /** - * @param Request $oRequest + * @param Application $oApp * @param array $aData * @param DBSearch $oQuery * @param string $sClass @@ -752,14 +751,14 @@ class ManageBrickController extends BrickController * @throws \Exception * @throws \CoreException */ - protected function ManageSearchValue(Request $oRequest, &$aData, DBSearch &$oQuery, $sClass, $aColumnsAttrs) + protected function ManageSearchValue(Application $oApp, &$aData, DBSearch &$oQuery, $sClass, $aColumnsAttrs) { // Getting search value - $sSearchValue = $oRequest->get('sSearchValue', null); + $sSearchValue = $oApp['request_manipulator']->ReadParam('sSearchValue', ''); // - Adding search clause if necessary // Note : This is a very naive search at the moment - if ($sSearchValue !== null) + if (!empty($sSearchValue)) { // Putting only valid attributes as one can define attributes of leaf classes in the brick definition (), but at this stage we are working on the abstract class. // Note: This won't fix everything as the search will not be looking in all fields. diff --git a/datamodels/2.x/itop-portal-base/portal/src/controllers/objectcontroller.class.inc.php b/datamodels/2.x/itop-portal-base/portal/src/controllers/objectcontroller.class.inc.php index e80891d5f..717168f79 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/controllers/objectcontroller.class.inc.php +++ b/datamodels/2.x/itop-portal-base/portal/src/controllers/objectcontroller.class.inc.php @@ -61,6 +61,8 @@ class ObjectController extends AbstractController const ENUM_MODE_VIEW = 'view'; const ENUM_MODE_EDIT = 'edit'; const ENUM_MODE_CREATE = 'create'; + + const DEFAULT_PAGE_NUMBER = 1; const DEFAULT_LIST_LENGTH = 10; /** @@ -103,6 +105,8 @@ class ObjectController extends AbstractController $oApp->abort(404, Dict::S('UI:ObjectDoesNotExist')); } + $sOperation = $oApp['request_manipulator']->ReadParam('operation', ''); + $aData = array('sMode' => 'view'); $aData['form'] = $this->HandleForm($oRequest, $oApp, $aData['sMode'], $sObjectClass, $sObjectId); $aData['form']['title'] = Dict::Format('Brick:Portal:Object:Form:View:Title', MetaModel::GetName($sObjectClass), $oObject->GetName()); @@ -123,7 +127,7 @@ class ObjectController extends AbstractController if ($oRequest->isXmlHttpRequest()) { // We have to check whether the 'operation' parameter is defined or not in order to know if the form is required via ajax (to be displayed as a modal dialog) or if it's a lifecycle call from a existing form. - if ($oRequest->request->get('operation') === null) + if (empty($sOperation)) { $oResponse = $oApp['twig']->render('itop-portal-base/portal/src/views/bricks/object/modal.html.twig', $aData); } @@ -135,8 +139,8 @@ class ObjectController extends AbstractController else { // Adding brick if it was passed - $sBrickId = $oRequest->get('sBrickId'); - if ($sBrickId !== null) + $sBrickId = $oApp['request_manipulator']->ReadParam('sBrickId', ''); + if (!empty($sBrickId)) { $oBrick = ApplicationHelper::GetLoadedBrickFromId($oApp, $sBrickId); if ($oBrick !== null) @@ -191,6 +195,8 @@ class ObjectController extends AbstractController $oApp->abort(404, Dict::S('UI:ObjectDoesNotExist')); } + $sOperation = $oApp['request_manipulator']->ReadParam('operation', ''); + $aData = array('sMode' => 'edit'); $aData['form'] = $this->HandleForm($oRequest, $oApp, $aData['sMode'], $sObjectClass, $sObjectId); $aData['form']['title'] = Dict::Format('Brick:Portal:Object:Form:Edit:Title', MetaModel::GetName($sObjectClass), $aData['form']['object_name']); @@ -199,7 +205,7 @@ class ObjectController extends AbstractController if ($oRequest->isXmlHttpRequest()) { // We have to check whether the 'operation' parameter is defined or not in order to know if the form is required via ajax (to be displayed as a modal dialog) or if it's a lifecycle call from a existing form. - if ($oRequest->request->get('operation') === null) + if (empty($sOperation)) { $oResponse = $oApp['twig']->render('itop-portal-base/portal/src/views/bricks/object/modal.html.twig', $aData); } @@ -211,8 +217,8 @@ class ObjectController extends AbstractController else { // Adding brick if it was passed - $sBrickId = $oRequest->get('sBrickId'); - if ($sBrickId !== null) + $sBrickId = $oApp['request_manipulator']->ReadParam('sBrickId', ''); + if (!empty($sBrickId)) { $oBrick = ApplicationHelper::GetLoadedBrickFromId($oApp, $sBrickId); if ($oBrick !== null) @@ -249,6 +255,8 @@ class ObjectController extends AbstractController $oApp->abort(404, Dict::S('UI:ObjectDoesNotExist')); } + $sOperation = $oApp['request_manipulator']->ReadParam('operation', ''); + $aData = array('sMode' => 'create'); $aData['form'] = $this->HandleForm($oRequest, $oApp, $aData['sMode'], $sObjectClass); $aData['form']['title'] = Dict::Format('Brick:Portal:Object:Form:Create:Title', MetaModel::GetName($sObjectClass)); @@ -257,7 +265,7 @@ class ObjectController extends AbstractController if ($oRequest->isXmlHttpRequest()) { // We have to check whether the 'operation' parameter is defined or not in order to know if the form is required via ajax (to be displayed as a modal dialog) or if it's a lifecycle call from a existing form. - if ($oRequest->request->get('operation') === null) + if (empty($sOperation)) { $oResponse = $oApp['twig']->render('itop-portal-base/portal/src/views/bricks/object/modal.html.twig', $aData); } @@ -269,8 +277,8 @@ class ObjectController extends AbstractController else { // Adding brick if it was passed - $sBrickId = $oRequest->get('sBrickId'); - if ($sBrickId !== null) + $sBrickId = $oApp['request_manipulator']->ReadParam('sBrickId', ''); + if (!empty($sBrickId)) { $oBrick = ApplicationHelper::GetLoadedBrickFromId($oApp, $sBrickId); if ($oBrick !== null) @@ -381,7 +389,7 @@ class ObjectController extends AbstractController } // Retrieving request parameters - $sOperation = $oRequest->request->get('operation'); + $sOperation = $oApp['request_manipulator']->ReadParam('operation', ''); // Retrieving form properties $aStimuliForms = ApplicationHelper::GetLoadedFormFromClass($oApp, $sObjectClass, 'apply_stimulus'); @@ -416,7 +424,7 @@ class ObjectController extends AbstractController // TODO : This is a ugly patch to avoid showing a modal with a readonly form to the user as it would prevent user from finishing the transition. // Instead, we apply the stimulus directly here and then go to the edited object. - if ($sOperation === null) + if (empty($sOperation)) { if (isset($aData['form']['editable_fields_count']) && $aData['form']['editable_fields_count'] === 0) { @@ -424,7 +432,7 @@ class ObjectController extends AbstractController $oSubRequest = $oRequest; $oSubRequest->request->set('operation', 'submit'); - $oSubRequest->request->set('stimulus_code', null); + $oSubRequest->request->set('stimulus_code', ''); $aData = array('sMode' => 'apply_stimulus'); $aData['form'] = $this->HandleForm($oSubRequest, $oApp, $aData['sMode'], $sObjectClass, $sObjectId, $aFormProperties); @@ -439,7 +447,7 @@ class ObjectController extends AbstractController if ($oRequest->isXmlHttpRequest()) { // We have to check whether the 'operation' parameter is defined or not in order to know if the form is required via ajax (to be displayed as a modal dialog) or if it's a lifecycle call from a existing form. - if ($sOperation === null) + if (empty($sOperation)) { $oResponse = $oApp['twig']->render('itop-portal-base/portal/src/views/bricks/object/modal.html.twig', $aData); } @@ -481,9 +489,8 @@ class ObjectController extends AbstractController public static function HandleForm(Request $oRequest, Application $oApp, $sMode, $sObjectClass, $sObjectId = null, $aFormProperties = null) { $aFormData = array(); - $oRequestParams = $oRequest->request; - $sOperation = $oRequestParams->get('operation'); - $bModal = ($oRequest->isXmlHttpRequest() && ($oRequest->request->get('operation') === null) ); + $sOperation = $oApp['request_manipulator']->ReadParam('operation', ''); + $bModal = ($oRequest->isXmlHttpRequest() && empty($sOperation)); // - Retrieve form properties if ($aFormProperties === null) @@ -492,14 +499,14 @@ class ObjectController extends AbstractController } // - Create and - if ($sOperation === null) + if (empty($sOperation)) { // Retrieving action rules // // Note : The action rules must be a base64-encoded JSON object, this is just so users are tempted to changes values. // But it would not be a security issue as it only presets values in the form. - $sActionRulesToken = $oRequest->get('ar_token'); - $aActionRules = ($sActionRulesToken !== null) ? ContextManipulatorHelper::DecodeRulesToken($sActionRulesToken) : array(); + $sActionRulesToken = $oApp['request_manipulator']->ReadParam('ar_token', ''); + $aActionRules = (!empty($sActionRulesToken)) ? ContextManipulatorHelper::DecodeRulesToken($sActionRulesToken) : array(); // Preparing object if ($sObjectId === null) @@ -572,9 +579,11 @@ class ObjectController extends AbstractController } else { - $aPrefillFormParam = array('user' => $_SESSION["auth_user"], + $aPrefillFormParam = array( + 'user' => $_SESSION["auth_user"], 'origin' => 'portal', - 'stimulus' => $oRequestParams->get('apply_stimulus')['code']); + 'stimulus' => $oApp['request_manipulator']->ReadParam('apply_stimulus', null)['code'], + ); $oObject->PrefillForm('state_change', $aPrefillFormParam); } @@ -612,9 +621,9 @@ class ObjectController extends AbstractController else { // Update / Submit / Cancel - $sFormManagerClass = $oRequestParams->get('formmanager_class'); - $sFormManagerData = $oRequestParams->get('formmanager_data'); - if ($sFormManagerClass === null || $sFormManagerData === null) + $sFormManagerClass = $oApp['request_manipulator']->ReadParam('formmanager_class', '', FILTER_UNSAFE_RAW); + $sFormManagerData = $oApp['request_manipulator']->ReadParam('formmanager_data', '', FILTER_UNSAFE_RAW); + if ( empty($sFormManagerClass) || empty($sFormManagerData) ) { IssueLog::Error(__METHOD__ . ' at line ' . __LINE__ . ' : Parameters formmanager_class and formamanager_data must be defined.'); $oApp->abort(500, 'Parameters formmanager_class and formmanager_data must be defined.'); @@ -636,13 +645,13 @@ class ObjectController extends AbstractController { case 'submit': // Applying modification to object - $aFormData['validation'] = $oFormManager->OnSubmit(array('currentValues' => $oRequestParams->get('current_values'), 'attachmentIds' => $oRequest->get('attachment_ids'), 'formProperties' => $aFormProperties, 'applyStimulus' => $oRequestParams->get('apply_stimulus'))); + $aFormData['validation'] = $oFormManager->OnSubmit(array('currentValues' => $oApp['request_manipulator']->ReadParam('current_values', array(), FILTER_UNSAFE_RAW), 'attachmentIds' => $oApp['request_manipulator']->ReadParam('attachment_ids', array(), FILTER_UNSAFE_RAW), 'formProperties' => $aFormProperties, 'applyStimulus' => $oApp['request_manipulator']->ReadParam('apply_stimulus', null))); if ($aFormData['validation']['valid'] === true) { // Note : We don't use $sObjectId there as it can be null if we are creating a new one. Instead we use the id from the created object once it has been seralized // Check if stimulus has to be applied - $sStimulusCode = ($oRequestParams->get('stimulus_code') !== null && $oRequestParams->get('stimulus_code') !== '') ? $oRequestParams->get('stimulus_code') : null; - if ($sStimulusCode !== null) + $sStimulusCode = $oApp['request_manipulator']->ReadParam('stimulus_code', ''); + if (!empty($sStimulusCode)) { $aFormData['validation']['redirection'] = array( 'url' => $oApp['url_generator']->generate('p_object_apply_stimulus', array('sObjectClass' => $sObjectClass, 'sObjectId' => $oFormManager->GetObject()->GetKey(), 'sStimulusCode' => $sStimulusCode)), @@ -650,17 +659,17 @@ class ObjectController extends AbstractController ); } // Otherwise, we show the object if there is no default - else - { +// else +// { // $aFormData['validation']['redirection'] = array( // 'alternative_url' => $oApp['url_generator']->generate('p_object_edit', array('sObjectClass' => $sObjectClass, 'sObjectId' => $oFormManager->GetObject()->GetKey())) // ); - } +// } } break; case 'update': - $oFormManager->OnUpdate(array('currentValues' => $oRequestParams->get('current_values'), 'formProperties' => $aFormProperties)); + $oFormManager->OnUpdate(array('currentValues' => $oApp['request_manipulator']->ReadParam('current_values', array(), FILTER_UNSAFE_RAW), 'formProperties' => $aFormProperties)); break; case 'cancel': @@ -679,11 +688,11 @@ class ObjectController extends AbstractController // Preparing fields list regarding the operation if ($sOperation === 'update') { - $aRequestedFields = $oRequestParams->get('requested_fields'); - $sFormPath = $oRequestParams->get('form_path'); + $aRequestedFields = $oApp['request_manipulator']->ReadParam('requested_fields', array(), FILTER_UNSAFE_RAW); + $sFormPath = $oApp['request_manipulator']->ReadParam('form_path', ''); // Checking if the update was on a subform, if so we need to make the rendering for that part only - if ($sFormPath !== null && $sFormPath !== $oFormManager->GetForm()->GetId()) + if ( !empty($sFormPath) && $sFormPath !== $oFormManager->GetForm()->GetId() ) { $oSubForm = $oFormManager->GetForm()->FindSubForm($sFormPath); $oSubFormRenderer = new BsFormRenderer($oSubForm); @@ -774,8 +783,8 @@ class ObjectController extends AbstractController // // Note : The action rules must be a base64-encoded JSON object, this is just so users are tempted to changes values. // But it would not be a security issue as it only presets values in the form. - $sActionRulesToken = $oRequest->get('ar_token'); - $aActionRules = ($sActionRulesToken !== null) ? ContextManipulatorHelper::DecodeRulesToken($sActionRulesToken) : array(); + $sActionRulesToken = $oApp['request_manipulator']->ReadParam('ar_token', ''); + $aActionRules = (!empty($sActionRulesToken)) ? ContextManipulatorHelper::DecodeRulesToken($sActionRulesToken) : array(); // Preparing object $oApp['context_manipulator']->PrepareObject($aActionRules, $oHostObject); } @@ -783,7 +792,7 @@ class ObjectController extends AbstractController // Updating host object with form data / values $sFormManagerClass = $aRequestContent['formmanager_class']; $sFormManagerData = $aRequestContent['formmanager_data']; - if ($sFormManagerClass !== null && $sFormManagerData !== null) + if (!empty($sFormManagerClass) && !empty($sFormManagerData)) { $oFormManager = $sFormManagerClass::FromJSON($sFormManagerData); $oFormManager->SetApplication($oApp); @@ -902,7 +911,7 @@ class ObjectController extends AbstractController 'sTargetAttCode' => $sTargetAttCode, 'sHostObjectClass' => $sHostObjectClass, 'sHostObjectId' => $sHostObjectId, - 'sActionRulesToken' => $oRequest->get('ar_token') + 'sActionRulesToken' => $oApp['request_manipulator']->ReadParam('ar_token', ''), ); // Checking security layers @@ -925,16 +934,15 @@ class ObjectController extends AbstractController // // Note : The action rules must be a base64-encoded JSON object, this is just so users are tempted to changes values. // But it would not be a security issue as it only presets values in the form. - $aActionRules = ($aData['sActionRulesToken'] !== null) ? ContextManipulatorHelper::DecodeRulesToken($aData['sActionRulesToken']) : array(); + $aActionRules = !empty($aData['sActionRulesToken']) ? ContextManipulatorHelper::DecodeRulesToken($aData['sActionRulesToken']) : array(); // Preparing object $oApp['context_manipulator']->PrepareObject($aActionRules, $oHostObject); } // Updating host object with form data / values - $oRequestParams = $oRequest->request; - $sFormManagerClass = $oRequestParams->get('formmanager_class'); - $sFormManagerData = $oRequestParams->get('formmanager_data'); - if ($sFormManagerClass !== null && $sFormManagerData !== null) + $sFormManagerClass = $oApp['request_manipulator']->ReadParam('formmanager_class', '', FILTER_UNSAFE_RAW); + $sFormManagerData = $oApp['request_manipulator']->ReadParam('formmanager_data', '', FILTER_UNSAFE_RAW); + if ( !empty($sFormManagerClass) && !empty($sFormManagerData) ) { $oFormManager = $sFormManagerClass::FromJSON($sFormManagerData); $oFormManager->SetApplication($oApp); @@ -950,18 +958,18 @@ class ObjectController extends AbstractController } // Updating host object - $oFormManager->OnUpdate(array('currentValues' => $oRequestParams->get('current_values'))); + $oFormManager->OnUpdate(array('currentValues' => $oApp['request_manipulator']->ReadParam('current_values', array(), FILTER_UNSAFE_RAW))); $oHostObject = $oFormManager->GetObject(); } // Retrieving request parameters - $iPageNumber = ($oRequest->get('iPageNumber') !== null) ? $oRequest->get('iPageNumber') : 1; - $iListLength = ($oRequest->get('iListLength') !== null) ? $oRequest->get('iListLength') : static::DEFAULT_LIST_LENGTH; - $bInitalPass = ($oRequest->get('draw') === null) ? true : false; - $sQuery = $oRequest->get('sSearchValue'); - $sFormPath = $oRequest->get('sFormPath'); - $sFieldId = $oRequest->get('sFieldId'); - $aObjectIdsToIgnore = $oRequest->get('aObjectIdsToIgnore'); + $iPageNumber = $oApp['request_manipulator']->ReadParam('iPageNumber', static::DEFAULT_PAGE_NUMBER, FILTER_SANITIZE_NUMBER_INT); + $iListLength = $oApp['request_manipulator']->ReadParam('iListLength', static::DEFAULT_LIST_LENGTH, FILTER_SANITIZE_NUMBER_INT); + $bInitalPass = $oApp['request_manipulator']->HasParam('draw') ? false : true; + $sQuery = $oApp['request_manipulator']->ReadParam('sSearchValue', ''); + $sFormPath = $oApp['request_manipulator']->ReadParam('sFormPath', ''); + $sFieldId = $oApp['request_manipulator']->ReadParam('sFieldId', ''); + $aObjectIdsToIgnore = $oApp['request_manipulator']->ReadParam('aObjectIdsToIgnore', null, FILTER_UNSAFE_RAW); // Building search query // - Retrieving target object class from attcode @@ -1042,7 +1050,7 @@ class ObjectController extends AbstractController // - Adding query condition $aInternalParams['this'] = $oHostObject; - if ($sQuery !== null) + if (!empty($sQuery)) { $oFullExpr = null; for ($i = 0; $i < count($aAttCodes); $i++) @@ -1217,9 +1225,9 @@ class ObjectController extends AbstractController } // Retrieving ormDocument's host object - $sObjectClass = $oRequest->get('sObjectClass'); - $sObjectId = $oRequest->get('sObjectId'); - $sObjectField = $oRequest->get('sObjectField'); + $sObjectClass = $oApp['request_manipulator']->ReadParam('sObjectClass', ''); + $sObjectId = $oApp['request_manipulator']->ReadParam('sObjectId', ''); + $sObjectField = $oApp['request_manipulator']->ReadParam('sObjectField', ''); // When reaching to an Attachment, we have to check security on its host object instead of the Attachment itself if($sObjectClass === 'Attachment') @@ -1259,8 +1267,7 @@ class ObjectController extends AbstractController } else { - $sCache = $oRequest->get('cache'); - $iCacheSec = ($sCache !== null) ? (int) $sCache : 0; + $iCacheSec = $oApp['request_manipulator']->ReadParam('cache', 0, FILTER_SANITIZE_NUMBER_INT); } $aHeaders = array(); @@ -1308,16 +1315,16 @@ class ObjectController extends AbstractController // Retrieving sOperation from request only if it wasn't forced (determined by the route) if ($sOperation === null) { - $sOperation = $oRequest->get('operation'); + $sOperation = $oApp['request_manipulator']->ReadParam('operation', null); } switch ($sOperation) { case 'add': - $sFieldName = $oRequest->get('field_name'); - $sObjectClass = $oRequest->get('object_class'); - $sTempId = $oRequest->get('temp_id'); + $sFieldName = $oApp['request_manipulator']->ReadParam('field_name', ''); + $sObjectClass = $oApp['request_manipulator']->ReadParam('object_class', ''); + $sTempId = $oApp['request_manipulator']->ReadParam('temp_id', ''); - if (($sObjectClass === null) || ($sTempId === null)) + if (empty($sObjectClass) || empty($sTempId)) { $aData['error'] = Dict::Format('UI:Error:2ParametersMissing', 'object_class', 'temp_id'); } @@ -1356,7 +1363,7 @@ class ObjectController extends AbstractController // - Route $aRouteParams = array( 'sObjectClass' => 'Attachment', - 'sObjectId' => $oRequest->get('sAttachmentId'), + 'sObjectId' => $oApp['request_manipulator']->ReadParam('sAttachmentId', null), 'sObjectField' => 'contents', ); $sRedirectRoute = $oApp['url_generator']->generate('p_object_document_download', $aRouteParams); @@ -1395,10 +1402,10 @@ class ObjectController extends AbstractController $aData = array(); // Retrieving parameters - $sObjectClass = $oRequest->Get('sObjectClass'); - $aObjectIds = $oRequest->Get('aObjectIds'); - $aObjectAttCodes = $oRequest->Get('aObjectAttCodes'); - if ($sObjectClass === null || $aObjectIds === null || $aObjectAttCodes === null) + $sObjectClass = $oApp['request_manipulator']->ReadParam('sObjectClass', ''); + $aObjectIds = $oApp['request_manipulator']->ReadParam('aObjectIds', array(), FILTER_UNSAFE_RAW); + $aObjectAttCodes = $oApp['request_manipulator']->ReadParam('aObjectAttCodes', array(), FILTER_UNSAFE_RAW); + if ( empty($sObjectClass) || empty($aObjectIds) || empty($aObjectAttCodes) ) { IssueLog::Info(__METHOD__ . ' at line ' . __LINE__ . ' : sObjectClass, aObjectIds and aObjectAttCodes expected, "' . $sObjectClass . '", "' . implode('/', $aObjectIds) . '" given.'); $oApp->abort(500, 'Invalid request data, some informations are missing'); diff --git a/datamodels/2.x/itop-portal-base/portal/src/controllers/userprofilebrickcontroller.class.inc.php b/datamodels/2.x/itop-portal-base/portal/src/controllers/userprofilebrickcontroller.class.inc.php index fb2048136..ef5c140ea 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/controllers/userprofilebrickcontroller.class.inc.php +++ b/datamodels/2.x/itop-portal-base/portal/src/controllers/userprofilebrickcontroller.class.inc.php @@ -86,7 +86,7 @@ class UserProfileBrickController extends BrickController // If this is ajax call, we are just submiting preferences or password forms if ($oRequest->isXmlHttpRequest()) { - $aCurrentValues = $oRequest->request->get('current_values'); + $aCurrentValues = $oApp['request_manipulator']->ReadParam('current_values', array(), FILTER_UNSAFE_RAW); $sFormType = $aCurrentValues['form_type']; if ($sFormType === PreferencesFormManager::FORM_TYPE) { @@ -142,10 +142,9 @@ class UserProfileBrickController extends BrickController public function HandlePreferencesForm(Request $oRequest, Application $oApp, $sFormMode) { $aFormData = array(); - $oRequestParams = $oRequest->request; // Handling form - $sOperation = $oRequestParams->get('operation'); + $sOperation = $oApp['request_manipulator']->ReadParam('operation', null); // - Create if ($sOperation === null) { @@ -165,8 +164,8 @@ class UserProfileBrickController extends BrickController // - Submit else if ($sOperation === 'submit') { - $sFormManagerClass = $oRequestParams->get('formmanager_class'); - $sFormManagerData = $oRequestParams->get('formmanager_data'); + $sFormManagerClass = $oApp['request_manipulator']->ReadParam('formmanager_class', null, FILTER_UNSAFE_RAW); + $sFormManagerData = $oApp['request_manipulator']->ReadParam('formmanager_data', null, FILTER_UNSAFE_RAW); if ($sFormManagerClass === null || $sFormManagerData === null) { IssueLog::Error(__METHOD__ . ' at line ' . __LINE__ . ' : Parameters formmanager_class and formamanager_data must be defined.'); @@ -176,7 +175,7 @@ class UserProfileBrickController extends BrickController // Rebuilding manager from json $oFormManager = $sFormManagerClass::FromJSON($sFormManagerData); // Applying modification to object - $aFormData['validation'] = $oFormManager->OnSubmit(array('currentValues' => $oRequestParams->get('current_values'))); + $aFormData['validation'] = $oFormManager->OnSubmit(array('currentValues' => $oApp['request_manipulator']->ReadParam('current_values', array(), FILTER_UNSAFE_RAW))); // Reloading page only if preferences were changed if (($aFormData['validation']['valid'] === true) && !empty($aFormData['validation']['messages']['success'])) { @@ -216,10 +215,9 @@ class UserProfileBrickController extends BrickController public function HandlePasswordForm(Request $oRequest, Application $oApp) { $aFormData = array(); - $oRequestParams = $oRequest->request; // Handling form - $sOperation = $oRequestParams->get('operation'); + $sOperation = $oApp['request_manipulator']->ReadParam('operation', null); // - Create if ($sOperation === null) { @@ -234,8 +232,8 @@ class UserProfileBrickController extends BrickController // - Submit else if ($sOperation === 'submit') { - $sFormManagerClass = $oRequestParams->get('formmanager_class'); - $sFormManagerData = $oRequestParams->get('formmanager_data'); + $sFormManagerClass = $oApp['request_manipulator']->ReadParam('formmanager_class', null, FILTER_UNSAFE_RAW); + $sFormManagerData = $oApp['request_manipulator']->ReadParam('formmanager_data', null, FILTER_UNSAFE_RAW); if ($sFormManagerClass === null || $sFormManagerData === null) { IssueLog::Error(__METHOD__ . ' at line ' . __LINE__ . ' : Parameters formmanager_class and formamanager_data must be defined.'); @@ -245,7 +243,7 @@ class UserProfileBrickController extends BrickController // Rebuilding manager from json $oFormManager = $sFormManagerClass::FromJSON($sFormManagerData); // Applying modification to object - $aFormData['validation'] = $oFormManager->OnSubmit(array('currentValues' => $oRequestParams->get('current_values'))); + $aFormData['validation'] = $oFormManager->OnSubmit(array('currentValues' => $oApp['request_manipulator']->ReadParam('current_values', array(), FILTER_UNSAFE_RAW))); } else { @@ -280,11 +278,10 @@ class UserProfileBrickController extends BrickController public function HandlePictureForm(Request $oRequest, Application $oApp, $sFormMode) { $aFormData = array(); - $oRequestParams = $oRequest->request; $sPictureAttCode = 'picture'; // Handling form - $sOperation = $oRequestParams->get('operation'); + $sOperation = $oApp['request_manipulator']->ReadParam('operation', null); // - No operation specified if ($sOperation === null) { diff --git a/datamodels/2.x/itop-portal-base/portal/src/helpers/requestmanipulatorhelper.class.inc.php b/datamodels/2.x/itop-portal-base/portal/src/helpers/requestmanipulatorhelper.class.inc.php new file mode 100644 index 000000000..ffac1c943 --- /dev/null +++ b/datamodels/2.x/itop-portal-base/portal/src/helpers/requestmanipulatorhelper.class.inc.php @@ -0,0 +1,119 @@ + + */ + +namespace Combodo\iTop\Portal\Helper; + +use Symfony\Component\HttpFoundation\RequestStack; + +/** + * RequestManipulatorHelper class + * + * Handle basic requests manipulation. + * + * @author Guillaume Lajarige + * @since 2.5.1 + */ +class RequestManipulatorHelper +{ + /** @var \Symfony\Component\HttpFoundation\RequestStack $oRequestStack */ + protected $oRequestStack; + + /** + * RequestManipulatorHelper constructor. + * + * @param \Symfony\Component\HttpFoundation\RequestStack $oRequestStack + */ + public function __construct(RequestStack &$oRequestStack) + { + $this->oRequestStack = $oRequestStack; + } + + /** + * @return \Symfony\Component\HttpFoundation\Request + */ + public function GetCurrentRequest() + { + return $this->oRequestStack->getCurrentRequest(); + } + + /** + * Returns if the request has a $sKey parameter. + * This looks in the GET arguments first, then PATH and finally the POST data. + * + * @param string $sKey + * + * @return bool + */ + public function HasParam($sKey) + { + if ($this->GetCurrentRequest()->query->has($sKey)) + { + return true; + } + + if ($this->GetCurrentRequest()->attributes->has($sKey)) + { + return true; + } + + if ($this->GetCurrentRequest()->request->has($sKey)) + { + return true; + } + + return false; + } + + /** + * Returns the $sKey parameter from the request filtered with $iFilter. + * This looks in the GET arguments first, then the PATH and finally the POST data. + * + * Note: It is inspired by the \Symfony\Component\HttpFoundation\ParameterBag::filter() function and was necessary as we sometimes have parameters that can be either in the GET/PATH/POST arguments and need to be filtered. Silex only offer the possibility to filter parameter from a single ParameterBag, so we created this helper. + * + * @param string $sKey + * @param mixed $default + * @param int $iFilter Default is FILTER_SANITIZE_STRING + * + * @return mixed|null + * + * @since 2.5.1 + */ + public function ReadParam($sKey, $default = null, $iFilter = FILTER_SANITIZE_STRING) + { + if ($this->GetCurrentRequest()->query->has($sKey)) + { + return $this->GetCurrentRequest()->query->filter($sKey, $default, $iFilter); + } + + if ($this->GetCurrentRequest()->attributes->has($sKey)) + { + return $this->GetCurrentRequest()->attributes->filter($sKey, $default, $iFilter); + } + + if ($this->GetCurrentRequest()->request->has($sKey)) + { + return $this->GetCurrentRequest()->request->filter($sKey, $default, $iFilter); + } + + return $default; + } + +} diff --git a/datamodels/2.x/itop-portal-base/portal/src/providers/requestmanipulatorserviceprovider.class.inc.php b/datamodels/2.x/itop-portal-base/portal/src/providers/requestmanipulatorserviceprovider.class.inc.php new file mode 100644 index 000000000..cfbd1982f --- /dev/null +++ b/datamodels/2.x/itop-portal-base/portal/src/providers/requestmanipulatorserviceprovider.class.inc.php @@ -0,0 +1,60 @@ + + */ + +namespace Combodo\iTop\Portal\Provider; + +use Pimple\Container; +use Pimple\ServiceProviderInterface; +use Combodo\iTop\Portal\Helper\RequestManipulatorHelper; + +/** + * RequestManipulatorHelper service provider + * + * @author Guillaume Lajarige + * @since 2.5.1 + */ +class RequestManipulatorServiceProvider implements ServiceProviderInterface +{ + + /** + * @param \Pimple\Container $oApp + */ + public function register(Container $oApp) + { + $oApp['request_manipulator'] = function ($oApp) + { + $oApp->flush(); + + $oRequestManipulatorHelper = new RequestManipulatorHelper($oApp['request_stack']); + + return $oRequestManipulatorHelper; + }; + } + + /** + * @param \Pimple\Container $oApp + */ + public function boot(Container $oApp) + { + + } + +} diff --git a/datamodels/2.x/itop-portal-base/portal/web/index.php b/datamodels/2.x/itop-portal-base/portal/web/index.php index f084b8c5f..c635eadf5 100644 --- a/datamodels/2.x/itop-portal-base/portal/web/index.php +++ b/datamodels/2.x/itop-portal-base/portal/web/index.php @@ -36,6 +36,8 @@ require_once __DIR__ . '/../src/providers/urlgeneratorserviceprovider.class.inc. require_once __DIR__ . '/../src/helpers/urlgeneratorhelper.class.inc.php'; require_once __DIR__ . '/../src/providers/contextmanipulatorserviceprovider.class.inc.php'; require_once __DIR__ . '/../src/helpers/contextmanipulatorhelper.class.inc.php'; +require_once __DIR__ . '/../src/providers/requestmanipulatorserviceprovider.class.inc.php'; +require_once __DIR__ . '/../src/helpers/requestmanipulatorhelper.class.inc.php'; require_once __DIR__ . '/../src/providers/scopevalidatorserviceprovider.class.inc.php'; require_once __DIR__ . '/../src/helpers/scopevalidatorhelper.class.inc.php'; require_once __DIR__ . '/../src/providers/lifecyclevalidatorserviceprovider.class.inc.php'; @@ -99,6 +101,9 @@ $oApp->before(function(Symfony\Component\HttpFoundation\Request $oRequest, Silex die(Dict::S('Portal:ErrorNoContactForThisUser')); } + // Register request manipulator now that the request has been created. + $oApp->register(new Combodo\iTop\Portal\Provider\RequestManipulatorServiceProvider()); + // Enable archived data utils::InitArchiveMode();