From d42443697cf095a02b66cd601f1e3b15c2f30262 Mon Sep 17 00:00:00 2001 From: Denis Flaven Date: Mon, 9 Dec 2013 11:39:25 +0000 Subject: [PATCH] Security enhancements: - ensure that a user can ony see the details of the ticket she/he is allowed to see, even if the id is typed manually - add a define'd filter to filter the drop-down lists of the search form for searching closed tickets. SVN:trunk[3027] --- application/portalwebpage.class.inc.php | 64 +++++++++++++++++-- .../datamodel.itop-request-mgmt.xml | 2 + .../datamodel.itop-request-mgmt-itil.xml | 2 + .../datamodel.itop-request-mgmt.xml | 2 + portal/index.php | 9 +++ portal/readme.txt | 3 +- 6 files changed, 75 insertions(+), 7 deletions(-) diff --git a/application/portalwebpage.class.inc.php b/application/portalwebpage.class.inc.php index b6b00cb3d..57fa8bfdc 100644 --- a/application/portalwebpage.class.inc.php +++ b/application/portalwebpage.class.inc.php @@ -430,8 +430,7 @@ EOF } } - - protected function DisplaySearchField($sClass, $sAttSpec, $aExtraParams, $sPrefix, $sFieldName = null) + protected function DisplaySearchField($sClass, $sAttSpec, $aExtraParams, $sPrefix, $sFieldName = null, $aFilterParams = array()) { if (is_null($sFieldName)) { @@ -462,7 +461,7 @@ EOF { throw new Exception("Attribute specification '$sAttSpec', '$sAttCode' should be either a link set or an external key"); } - $this->DisplaySearchField($sTargetClass, $sSubSpec, $aExtraParams, $sPrefix, $sFieldName); + $this->DisplaySearchField($sTargetClass, $sSubSpec, $aExtraParams, $sPrefix, $sFieldName, $aFilterParams); } else { @@ -476,7 +475,22 @@ EOF if ($oAttDef->IsExternalKey()) { $sTargetClass = $oAttDef->GetTargetClass(); - $oAllowedValues = new DBObjectSet(new DBObjectSearch($sTargetClass)); + $sFilterDefName = 'PORTAL_TICKETS_SEARCH_FILTER_'.$sAttSpec; + if (defined($sFilterDefName)) + { + try + { + $oAllowedValues = new DBObjectSet(DBObjectSearch::FromOQL(constant($sFilterDefName)), array(), $aFilterParams); + } + catch(OQLException $e) + { + throw new Exception("Incorrect filter '$sFilterDefName' for attribute '$sAttcode': ".$e->getMessage()); + } + } + else + { + $oAllowedValues = new DBObjectSet(new DBObjectSearch($sTargetClass)); + } $iFieldSize = $oAttDef->GetMaxSize(); $iMaxComboLength = $oAttDef->GetMaximumComboLength(); @@ -533,9 +547,30 @@ EOF } } + /** + * Get The organization of the current user (i.e. the organization of its contact) + * @throws Exception + */ + function GetUserOrg() + { + $oOrg = null; + $iContactId = UserRights::GetContactId(); + $oContact = MetaModel::GetObject('Contact', $iContactId, false); // false => Can fail + if (is_object($oContact)) + { + $oOrg = MetaModel::GetObject('Organization', $oContact->Get('org_id'), false); // false => can fail + } + else + { + throw new Exception(Dict::S('Portal:ErrorNoContactForThisUser')); + } + return $oOrg; + } public function DisplaySearchForm($sClass, $aAttList, $aExtraParams, $sPrefix, $bClosed = true) { + $oUserOrg = $this->GetUserOrg(); + $aFilterParams = array('org_id' => $oUserOrg->GetKey(), 'contact_id' => UserRights::GetContactId()); $sCSSClass = ($bClosed) ? 'DrawerClosed' : ''; $this->add("
\n"); $this->add_ready_script( @@ -552,7 +587,7 @@ EOF foreach($aAttList as $sAttSpec) { //$oAppContext->Reset($sAttSpec); // Make sure the same parameter will not be passed twice - $this->DisplaySearchField($sClass, $sAttSpec, $aExtraParams, $sPrefix); + $this->DisplaySearchField($sClass, $sAttSpec, $aExtraParams, $sPrefix, null, $aFilterParams); } $this->add("

\n"); $this->add("

\n"); @@ -758,7 +793,24 @@ EOF } } - $oObj = MetaModel::GetObject($sClass, $iId, false); + $sOQL = "SELECT $sClass WHERE org_id = :org_id"; + $oSearch = DBObjectSearch::FromOQL($sOQL); + $iUser = UserRights::GetContactId(); + if ($iUser > 0 && !IsPowerUser()) + { + $oSearch->AddCondition('caller_id', $iUser); + } + $oSearch->AddCondition('id', $iId); + + $oContact = MetaModel::GetObject('Contact', $iUser, false); // false => Can fail + if (!is_object($oContact)) + { + throw new Exception(Dict::S('Portal:ErrorNoContactForThisUser')); + } + + $oSet = new DBObjectSet($oSearch, array(), array('org_id' => $oContact->Get('org_id'))); + + $oObj = $oSet->Fetch(); if (!is_object($oObj)) { throw new Exception("Could not find the object $sClass/$iId"); diff --git a/datamodels/1.x/itop-request-mgmt-1.0.0/datamodel.itop-request-mgmt.xml b/datamodels/1.x/itop-request-mgmt-1.0.0/datamodel.itop-request-mgmt.xml index ced2f4292..1e7ba8b9d 100644 --- a/datamodels/1.x/itop-request-mgmt-1.0.0/datamodel.itop-request-mgmt.xml +++ b/datamodels/1.x/itop-request-mgmt-1.0.0/datamodel.itop-request-mgmt.xml @@ -17,6 +17,8 @@ + + diff --git a/datamodels/2.x/itop-request-mgmt-itil/datamodel.itop-request-mgmt-itil.xml b/datamodels/2.x/itop-request-mgmt-itil/datamodel.itop-request-mgmt-itil.xml index dd577e7a6..f4d278ab9 100755 --- a/datamodels/2.x/itop-request-mgmt-itil/datamodel.itop-request-mgmt-itil.xml +++ b/datamodels/2.x/itop-request-mgmt-itil/datamodel.itop-request-mgmt-itil.xml @@ -24,6 +24,8 @@ + + diff --git a/datamodels/2.x/itop-request-mgmt/datamodel.itop-request-mgmt.xml b/datamodels/2.x/itop-request-mgmt/datamodel.itop-request-mgmt.xml index f8cbb6c6a..979ab40cf 100755 --- a/datamodels/2.x/itop-request-mgmt/datamodel.itop-request-mgmt.xml +++ b/datamodels/2.x/itop-request-mgmt/datamodel.itop-request-mgmt.xml @@ -17,6 +17,8 @@ + + diff --git a/portal/index.php b/portal/index.php index 2c24778b2..6b6b13cfb 100644 --- a/portal/index.php +++ b/portal/index.php @@ -790,6 +790,15 @@ function ListResolvedRequests(WebPage $oP) function ListClosedTickets(WebPage $oP) { $aAttSpecs = explode(',', PORTAL_TICKETS_SEARCH_CRITERIA); + // Remove the caller_id form the search criteria if the user is not a Portal Power User + // since the user is only allowed to see her/his own tickets + foreach($aAttSpecs as $idx => $sAttCode) + { + if (($sAttCode == 'caller_id') && !IsPowerUser()) + { + unset($aAttSpecs[$idx]); + } + } $aClasses = GetTicketClasses(); $sMainClass = reset($aClasses); $oP->DisplaySearchForm($sMainClass, $aAttSpecs, array('operation' => 'show_closed'), 'search_', false /* => not closed */); diff --git a/portal/readme.txt b/portal/readme.txt index 08108fe6b..ac3190873 100644 --- a/portal/readme.txt +++ b/portal/readme.txt @@ -19,7 +19,8 @@ PORTAL_VALIDATE_SERVICESUBCATEGORY_QUERY: OQL to check the service again (securi PORTAL_ALL_PARAMS: parameters that the wizard will kindly propagate through its pages (mixing should not be a problem, default value could be cleaned a little...) PORTAL_SET_TYPE_FROM: attribute of the class ServiceSubcategory determining the request type PORTAL_TYPE_TO_CLASS: optional mapping from the request types to ticket classes -PORTAL_TICKETS_SEARCH_CRITERIA: list of search criteria for closed tickets +PORTAL_TICKETS_SEARCH_CRITERIA: comma separated list of search criteria (attcodes) for closed tickets +PORTAL_TICKETS_SEARCH_FILTER_attcode: an OQL query to limit the list of values available in the search form (drop-down list). One define per entry in PORTAL_TICKETS_SEARCH_CRITERIA Caution: Hardcoded stuff