diff --git a/addons/userrights/userrightsmatrix.class.inc.php b/addons/userrights/userrightsmatrix.class.inc.php index f9b8d5c19..7c7e9ca9e 100644 --- a/addons/userrights/userrightsmatrix.class.inc.php +++ b/addons/userrights/userrightsmatrix.class.inc.php @@ -134,7 +134,7 @@ class UserRightsMatrix extends UserRightsAddOnAPI $iChangeId = $oChange->DBInsert(); // Now record the admin user object - $iUserId = $oUser->DBInsertTrackedNoReload($oChange); + $iUserId = $oUser->DBInsertTrackedNoReload($oChange, true /* skip security */); $this->SetupUser($iUserId, true); return true; } diff --git a/addons/userrights/userrightsprofile.class.inc.php b/addons/userrights/userrightsprofile.class.inc.php index c469e2218..f5b61d845 100644 --- a/addons/userrights/userrightsprofile.class.inc.php +++ b/addons/userrights/userrightsprofile.class.inc.php @@ -436,7 +436,7 @@ class UserRightsProfile extends UserRightsAddOnAPI $oOrg->Set('code', 'SOMECODE'); // $oOrg->Set('status', 'implementation'); //$oOrg->Set('parent_id', xxx); - $iOrgId = $oOrg->DBInsertTrackedNoReload($oChange); + $iOrgId = $oOrg->DBInsertTrackedNoReload($oChange, true /* skip security */); $oContact = new Person(); $oContact->Set('name', 'My last name'); @@ -447,14 +447,14 @@ class UserRightsProfile extends UserRightsAddOnAPI //$oContact->Set('phone', ''); //$oContact->Set('location_id', $iLocationId); //$oContact->Set('employee_number', ''); - $iContactId = $oContact->DBInsertTrackedNoReload($oChange); + $iContactId = $oContact->DBInsertTrackedNoReload($oChange, true /* skip security */); $oUser = new UserLocal(); $oUser->Set('login', $sAdminUser); $oUser->Set('password', $sAdminPwd); $oUser->Set('contactid', $iContactId); $oUser->Set('language', $sLanguage); // Language was chosen during the installation - $iUserId = $oUser->DBInsertTrackedNoReload($oChange); + $iUserId = $oUser->DBInsertTrackedNoReload($oChange, true /* skip security */); // Add this user to the very specific 'admin' profile $oAdminProfile = MetaModel::GetObjectFromOQL("SELECT URP_Profiles WHERE name = :name", array('name' => ADMIN_PROFILE_NAME), true /*all data*/); @@ -464,7 +464,7 @@ class UserRightsProfile extends UserRightsAddOnAPI $oUserProfile->Set('userid', $iUserId); $oUserProfile->Set('profileid', $oAdminProfile->GetKey()); $oUserProfile->Set('reason', 'By definition, the administrator must have the administrator profile'); - $oUserProfile->DBInsertTrackedNoReload($oChange); + $oUserProfile->DBInsertTrackedNoReload($oChange, true /* skip security */); } return true; } diff --git a/addons/userrights/userrightsprojection.class.inc.php b/addons/userrights/userrightsprojection.class.inc.php index 3705a201c..5c4d9f79a 100644 --- a/addons/userrights/userrightsprojection.class.inc.php +++ b/addons/userrights/userrightsprojection.class.inc.php @@ -30,21 +30,20 @@ class UserRightsBaseClass extends cmdbAbstractObject { // Whenever something changes, reload the privileges - public function DBInsertTracked(CMDBChange $oChange) + // Whenever something changes, reload the privileges + + protected function AfterInsert() { - parent::DBInsertTracked($oChange); UserRights::FlushPrivileges(); } - public function DBUpdateTracked(CMDBChange $oChange) + protected function AfterUpdate() { - parent::DBUpdateTracked($oChange); UserRights::FlushPrivileges(); } - public function DBDeleteTracked(CMDBChange $oChange) + protected function AfterDelete() { - parent::DBDeleteTracked($oChange); UserRights::FlushPrivileges(); } } @@ -601,7 +600,7 @@ class UserRightsProjection extends UserRightsAddOnAPI $oOrg->Set('code', 'SOMECODE'); // $oOrg->Set('status', 'implementation'); //$oOrg->Set('parent_id', xxx); - $iOrgId = $oOrg->DBInsertTrackedNoReload($oChange); + $iOrgId = $oOrg->DBInsertTrackedNoReload($oChange, true /* skip strong security */); // Location : optional //$oLocation = new bizLocation(); @@ -623,21 +622,21 @@ class UserRightsProjection extends UserRightsAddOnAPI //$oContact->Set('phone', ''); //$oContact->Set('location_id', $iLocationId); //$oContact->Set('employee_number', ''); - $iContactId = $oContact->DBInsertTrackedNoReload($oChange); + $iContactId = $oContact->DBInsertTrackedNoReload($oChange, true /* skip security */); $oUser = new UserLocal(); $oUser->Set('login', $sAdminUser); $oUser->Set('password', $sAdminPwd); $oUser->Set('contactid', $iContactId); $oUser->Set('language', $sLanguage); // Language was chosen during the installation - $iUserId = $oUser->DBInsertTrackedNoReload($oChange); + $iUserId = $oUser->DBInsertTrackedNoReload($oChange, true /* skip security */); // Add this user to the very specific 'admin' profile $oUserProfile = new URP_UserProfile(); $oUserProfile->Set('userid', $iUserId); $oUserProfile->Set('profileid', ADMIN_PROFILE_ID); $oUserProfile->Set('reason', 'By definition, the administrator must have the administrator profile'); - $oUserProfile->DBInsertTrackedNoReload($oChange); + $oUserProfile->DBInsertTrackedNoReload($oChange, true /* skip security */); return true; } diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php index 030756e63..d81da14f1 100644 --- a/application/cmdbabstract.class.inc.php +++ b/application/cmdbabstract.class.inc.php @@ -411,6 +411,16 @@ abstract class cmdbAbstractObject extends CMDBObject $sZListName = isset($aExtraParams['zlist']) ? ($aExtraParams['zlist']) : 'list'; $aList = self::FlattenZList(MetaModel::GetZListItems($sClassName, $sZListName)); $aList = array_merge($aList, $aExtraFields); + // Filter the list to removed linked set since we are not able to display them here + foreach($aList as $index => $sAttCode) + { + $oAttDef = MetaModel::GetAttributeDef($sClassName, $sAttCode); + if ($oAttDef instanceof AttributeLinkedSet) + { + // Removed from the display list + unset($aList[$index]); + } + } if (!empty($sLinkageAttribute)) { // The set to display is in fact a set of links between the object specified in the $sLinkageAttribute diff --git a/application/displayblock.class.inc.php b/application/displayblock.class.inc.php index 9bb120041..211e48792 100644 --- a/application/displayblock.class.inc.php +++ b/application/displayblock.class.inc.php @@ -483,8 +483,7 @@ class DisplayBlock $bDisplayMenu = isset($aExtraParams['menu']) ? $aExtraParams['menu'] == true : true; if ($bDisplayMenu) { - if ((UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY) == UR_ALLOWED_YES) - && !MetaModel::IsReadOnlyClass($sClass)) + if ((UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY) == UR_ALLOWED_YES)) { $oAppContext = new ApplicationContext(); $sParams = $oAppContext->GetForLink(); @@ -526,8 +525,7 @@ class DisplayBlock $bDisplayMenu = isset($this->m_aParams['menu']) ? $this->m_aParams['menu'] == true : true; if ($bDisplayMenu) { - if ((UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY) == UR_ALLOWED_YES) - && (!MetaModel::IsReadOnlyClass($sClass))) + if ((UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY) == UR_ALLOWED_YES)) { $oAppContext = new ApplicationContext(); $sParams = $oAppContext->GetForLink(); @@ -579,8 +577,7 @@ class DisplayBlock break; case 'modify': - if ((UserRights::IsActionAllowed($this->m_oSet->GetClass(), UR_ACTION_MODIFY, $this->m_oSet) == UR_ALLOWED_YES) - && !MetaModel::IsReadOnlyClass($this->m_oSet->GetClass())) + if ((UserRights::IsActionAllowed($this->m_oSet->GetClass(), UR_ACTION_MODIFY, $this->m_oSet) == UR_ALLOWED_YES)) { while($oObj = $this->m_oSet->Fetch()) { @@ -849,17 +846,17 @@ class MenuBlock extends DisplayBlock { case 0: // No object in the set, the only possible action is "new" - $bIsModifyAllowed = (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY) == UR_ALLOWED_YES) && !MetaModel::IsReadOnlyClass($sClass); + $bIsModifyAllowed = (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY) == UR_ALLOWED_YES); if ($bIsModifyAllowed) { $aActions[] = array ('label' => Dict::S('UI:Menu:New'), 'url' => "../page/$sUIPage?operation=new&class=$sClass&$sContext{$sDefault}"); } break; case 1: $oObj = $oSet->Fetch(); $id = $oObj->GetKey(); - $bIsModifyAllowed = (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet) == UR_ALLOWED_YES) && !MetaModel::IsReadOnlyClass($sClass); - $bIsDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, $oSet) && !MetaModel::IsReadOnlyClass($sClass); - $bIsBulkModifyAllowed = (!MetaModel::IsAbstract($sClass)) && UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, $oSet) && !MetaModel::IsReadOnlyClass($sClass); - $bIsBulkDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, $oSet) && !MetaModel::IsReadOnlyClass($sClass); + $bIsModifyAllowed = (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet) == UR_ALLOWED_YES); + $bIsDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, $oSet); + $bIsBulkModifyAllowed = (!MetaModel::IsAbstract($sClass)) && UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, $oSet); + $bIsBulkDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, $oSet); // Just one object in the set, possible actions are "new / clone / modify and delete" if (isset($aExtraParams['link_attr'])) { @@ -912,16 +909,16 @@ class MenuBlock extends DisplayBlock default: // Check rights // New / Modify - $bIsModifyAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet) && !MetaModel::IsReadOnlyClass($sClass); - $bIsBulkModifyAllowed = (!MetaModel::IsAbstract($sClass)) && UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, $oSet) && !MetaModel::IsReadOnlyClass($sClass); - $bIsBulkDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, $oSet) && !MetaModel::IsReadOnlyClass($sClass); + $bIsModifyAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet); + $bIsBulkModifyAllowed = (!MetaModel::IsAbstract($sClass)) && UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_MODIFY, $oSet); + $bIsBulkDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, $oSet); if (isset($aExtraParams['link_attr'])) { $id = $aExtraParams['object_id']; $sTargetAttr = $aExtraParams['target_attr']; $oAttDef = MetaModel::GetAttributeDef($sClass, $sTargetAttr); $sTargetClass = $oAttDef->GetTargetClass(); - $bIsDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, $oSet) && !MetaModel::IsReadOnlyClass($sClass); + $bIsDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, $oSet); if ($bIsModifyAllowed) { $aActions[] = array ('label' => Dict::S('UI:Menu:Add'), 'url' => "../pages/$sUIPage?operation=modify_links&class=$sClass&link_attr=".$aExtraParams['link_attr']."&target_class=$sTargetClass&id=$id&addObjects=true&$sContext"); } //if ($bIsBulkModifyAllowed) { $aActions[] = array ('label' => 'Add...', 'url' => "../pages/$sUIPage?operation=modify_links&class=$sClass&linkage=".$aExtraParams['linkage']."&id=$id&addObjects=true&$sContext"); } if ($bIsBulkModifyAllowed) { $aActions[] = array ('label' => Dict::S('UI:Menu:Manage'), 'url' => "../pages/$sUIPage?operation=modify_links&class=$sClass&link_attr=".$aExtraParams['link_attr']."&target_class=$sTargetClass&id=$id&sContext"); } diff --git a/application/user.preferences.class.inc.php b/application/user.preferences.class.inc.php index 873c1055d..5c922fb42 100644 --- a/application/user.preferences.class.inc.php +++ b/application/user.preferences.class.inc.php @@ -143,7 +143,7 @@ class appUserPreferences extends DBObject { $aParams = array ( - "category" => "gui,alwaysreadable", + "category" => "gui", "key_type" => "autoincrement", "name_attcode" => "userid", "state_attcode" => "", diff --git a/core/cmdbchange.class.inc.php b/core/cmdbchange.class.inc.php index 73dcb79e2..4f7a0c80c 100644 --- a/core/cmdbchange.class.inc.php +++ b/core/cmdbchange.class.inc.php @@ -49,11 +49,6 @@ class CMDBChange extends DBObject MetaModel::Init_AddAttribute(new AttributeDateTime("date", array("allowed_values"=>null, "sql"=>"date", "default_value"=>"", "is_null_allowed"=>false, "depends_on"=>array()))); MetaModel::Init_AddAttribute(new AttributeString("userinfo", array("allowed_values"=>null, "sql"=>"userinfo", "default_value"=>null, "is_null_allowed"=>true, "depends_on"=>array()))); } - - static public function IsReadOnly() - { - return true; - } } ?> diff --git a/core/cmdbchangeop.class.inc.php b/core/cmdbchangeop.class.inc.php index 5d2be978d..d78870295 100644 --- a/core/cmdbchangeop.class.inc.php +++ b/core/cmdbchangeop.class.inc.php @@ -57,11 +57,6 @@ class CMDBChangeOp extends DBObject MetaModel::Init_SetZListItems('list', array('change', 'date', 'userinfo')); // Attributes to be displayed for the complete details } - static public function IsReadOnly() - { - return true; - } - /** * Describe (as a text string) the modifications corresponding to this change */ diff --git a/core/cmdbobject.class.inc.php b/core/cmdbobject.class.inc.php index d722a9eef..9ae600f4c 100644 --- a/core/cmdbobject.class.inc.php +++ b/core/cmdbobject.class.inc.php @@ -217,6 +217,35 @@ abstract class CMDBObject extends DBObject } } + /** + * Helper to ultimately check user rights before writing (Insert, Update or Delete) + * The check should never fail, because the UI should prevent from such a usage + * Anyhow, if the user has found a workaround... the security gets enforced here + */ + protected function CheckUserRights($bSkipStrongSecurity, $iActionCode) + { + if (is_null($bSkipStrongSecurity)) + { + // This is temporary + // We have implemented this safety net right before releasing iTop 1.0 + // and we decided that it was too risky to activate it + // Anyhow, users willing to have a very strong security could set + // skip_strong_security = 0, in the config file + $bSkipStrongSecurity = utils::GetConfig()->Get('skip_strong_security'); + } + if (!$bSkipStrongSecurity) + { + $sClass = get_class($this); + $oSet = DBObjectSet::FromObject($this); + if (!UserRights::IsActionAllowed($sClass, $iActionCode, $oSet)) + { + // Intrusion detected + throw new SecurityException('You are not allowed to modify objects of class: '.$sClass); + } + } + } + + public function DBInsert() { if(!is_object(self::$m_oCurrChange)) @@ -226,16 +255,20 @@ abstract class CMDBObject extends DBObject return $this->DBInsertTracked_Internal(); } - public function DBInsertTracked(CMDBChange $oChange) + public function DBInsertTracked(CMDBChange $oChange, $bSkipStrongSecurity = null) { + $this->CheckUserRights($bSkipStrongSecurity, UR_ACTION_MODIFY); + self::$m_oCurrChange = $oChange; $ret = $this->DBInsertTracked_Internal(); self::$m_oCurrChange = null; return $ret; } - public function DBInsertTrackedNoReload(CMDBChange $oChange) + public function DBInsertTrackedNoReload(CMDBChange $oChange, $bSkipStrongSecurity = null) { + $this->CheckUserRights($bSkipStrongSecurity, UR_ACTION_MODIFY); + self::$m_oCurrChange = $oChange; $ret = $this->DBInsertTracked_Internal(true); self::$m_oCurrChange = null; @@ -290,8 +323,10 @@ abstract class CMDBObject extends DBObject return $this->DBUpdateTracked_internal(); } - public function DBUpdateTracked(CMDBChange $oChange) + public function DBUpdateTracked(CMDBChange $oChange, $bSkipStrongSecurity = null) { + $this->CheckUserRights($bSkipStrongSecurity, UR_ACTION_MODIFY); + self::$m_oCurrChange = $oChange; $this->DBUpdateTracked_Internal(); self::$m_oCurrChange = null; @@ -323,8 +358,10 @@ abstract class CMDBObject extends DBObject return $this->DBDeleteTracked_Internal(); } - public function DBDeleteTracked(CMDBChange $oChange) + public function DBDeleteTracked(CMDBChange $oChange, $bSkipStrongSecurity = null) { + $this->CheckUserRights($bSkipStrongSecurity, UR_ACTION_DELETE); + self::$m_oCurrChange = $oChange; $this->DBDeleteTracked_Internal(); self::$m_oCurrChange = null; diff --git a/core/config.class.inc.php b/core/config.class.inc.php index 2fbca2351..8a50c276c 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -94,6 +94,14 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ), + 'skip_strong_security' => array( + 'type' => 'bool', + 'description' => 'Disable strong security - TEMPORY: this flag should be removed when we are more confident in the recent change in security', + 'default' => true, + 'value' => true, + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ), 'graphviz_path' => array( 'type' => 'string', 'description' => 'Path to the Graphviz "dot" executable for graphing objects lifecycle', diff --git a/core/dbobject.class.php b/core/dbobject.class.php index c999ceb62..b59515d51 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -83,11 +83,6 @@ abstract class DBObject } // Read-only <=> Written once (archive) - static public function IsReadOnly() - { - return false; - } - public function RegisterAsDirty() { // While the object may be written to the DB, it is NOT possible to reload it diff --git a/core/event.class.inc.php b/core/event.class.inc.php index d24b580ee..f49ef6bce 100644 --- a/core/event.class.inc.php +++ b/core/event.class.inc.php @@ -31,7 +31,7 @@ class Event extends cmdbAbstractObject { $aParams = array ( - "category" => "core/cmdb", + "category" => "core/cmdb,view_in_gui", "key_type" => "autoincrement", "name_attcode" => "", "state_attcode" => "", @@ -54,11 +54,6 @@ class Event extends cmdbAbstractObject // MetaModel::Init_SetZListItems('standard_search', array('name')); // Criteria of the std search form // MetaModel::Init_SetZListItems('advanced_search', array('name')); // Criteria of the advanced search form } - - static public function IsReadOnly() - { - return true; - } } class EventNotification extends Event @@ -67,7 +62,7 @@ class EventNotification extends Event { $aParams = array ( - "category" => "core/cmdb", + "category" => "core/cmdb,view_in_gui", "key_type" => "autoincrement", "name_attcode" => "", "state_attcode" => "", @@ -99,7 +94,7 @@ class EventNotificationEmail extends EventNotification { $aParams = array ( - "category" => "core/cmdb", + "category" => "core/cmdb,view_in_gui", "key_type" => "autoincrement", "name_attcode" => "", "state_attcode" => "", @@ -135,7 +130,7 @@ class EventIssue extends Event { $aParams = array ( - "category" => "core/cmdb", + "category" => "core/cmdb,view_in_gui", "key_type" => "autoincrement", "name_attcode" => "", "state_attcode" => "", @@ -234,7 +229,7 @@ class EventWebService extends Event { $aParams = array ( - "category" => "core/cmdb", + "category" => "core/cmdb,view_in_gui", "key_type" => "autoincrement", "name_attcode" => "", "state_attcode" => "", diff --git a/core/metamodel.class.php b/core/metamodel.class.php index ce66476d4..47981b076 100644 --- a/core/metamodel.class.php +++ b/core/metamodel.class.php @@ -267,12 +267,6 @@ abstract class MetaModel return self::GetParentPersistentClass($sClass); } - static public function IsReadOnlyClass($sClass) - { - $bReadOnly = call_user_func(array($sClass, 'IsReadOnly')); - return $bReadOnly; - } - final static public function GetName($sClass) { self::_check_subclass($sClass); diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php index 0f63306c4..9c0cead28 100644 --- a/core/userrights.class.inc.php +++ b/core/userrights.class.inc.php @@ -522,9 +522,6 @@ class UserRights public static function GetSelectFilter($sClass) { - // Need to load some records before the login is performed (user preferences) - if (MetaModel::HasCategory($sClass, 'alwaysreadable')) return true; - // When initializing, we need to let everything pass trough if (!self::CheckLogin()) return true; @@ -532,13 +529,14 @@ class UserRights // Portal users actions are limited by the portal page... if (self::IsPortalUser()) return true; - // this module is forbidden for non admins.... BUT I NEED IT HERE TO DETERMINE USER RIGHTS - if (MetaModel::HasCategory($sClass, 'addon/userrights')) return true; - - // the rest is allowed (#@# to be improved) - if (!MetaModel::HasCategory($sClass, 'bizmodel')) return true; - - return self::$m_oAddOn->GetSelectFilter(self::$m_oUser, $sClass); + if (MetaModel::HasCategory($sClass, 'bizmodel')) + { + return self::$m_oAddOn->GetSelectFilter(self::$m_oUser, $sClass); + } + else + { + return true; + } } public static function IsActionAllowed($sClass, $iActionCode, /*dbObjectSet*/ $oInstanceSet = null, $oUser = null) @@ -548,22 +546,27 @@ class UserRights if (self::IsAdministrator($oUser)) return true; - - // #@# Temporary????? - // The read access is controlled in MetaModel::MakeSelectQuery() - if ($iActionCode == UR_ACTION_READ) return true; - - // this module is forbidden for non admins - if (MetaModel::HasCategory($sClass, 'addon/userrights')) return false; - - // the rest is allowed (#@# to be improved) - if (!MetaModel::HasCategory($sClass, 'bizmodel')) return true; - - if (is_null($oUser)) + if (MetaModel::HasCategory($sClass, 'bizmodel')) { - $oUser = self::$m_oUser; + // #@# Temporary????? + // The read access is controlled in MetaModel::MakeSelectQuery() + if ($iActionCode == UR_ACTION_READ) return true; + + if (is_null($oUser)) + { + $oUser = self::$m_oUser; + } + return self::$m_oAddOn->IsActionAllowed($oUser, $sClass, $iActionCode, $oInstanceSet); + } + elseif(($iActionCode == UR_ACTION_READ) && MetaModel::HasCategory($sClass, 'view_in_gui')) + { + return true; + } + else + { + // Other classes could be edited/listed by the administrators + return false; } - return self::$m_oAddOn->IsActionAllowed($oUser, $sClass, $iActionCode, $oInstanceSet); } public static function IsStimulusAllowed($sClass, $sStimulusCode, /*dbObjectSet*/ $oInstanceSet = null, $oUser = null) @@ -573,17 +576,23 @@ class UserRights if (self::IsAdministrator($oUser)) return true; - // this module is forbidden for non admins - if (MetaModel::HasCategory($sClass, 'addon/userrights')) return false; - - // the rest is allowed (#@# to be improved) - if (!MetaModel::HasCategory($sClass, 'bizmodel')) return true; - - if (is_null($oUser)) + if (MetaModel::HasCategory($sClass, 'bizmodel')) { - $oUser = self::$m_oUser; + if (is_null($oUser)) + { + $oUser = self::$m_oUser; + } + return self::$m_oAddOn->IsStimulusAllowed($oUser, $sClass, $sStimulusCode, $oInstanceSet); + } + elseif(($iActionCode == UR_ACTION_READ) && MetaModel::HasCategory($sClass, 'view_in_gui')) + { + return true; + } + else + { + // Other classes could be edited/listed by the administrators + return false; } - return self::$m_oAddOn->IsStimulusAllowed($oUser, $sClass, $sStimulusCode, $oInstanceSet); } public static function IsActionAllowedOnAttribute($sClass, $sAttCode, $iActionCode, /*dbObjectSet*/ $oInstanceSet = null, $oUser = null) diff --git a/dictionaries/dictionary.itop.ui.php b/dictionaries/dictionary.itop.ui.php index 6e536929c..769dd54a9 100644 --- a/dictionaries/dictionary.itop.ui.php +++ b/dictionaries/dictionary.itop.ui.php @@ -655,6 +655,7 @@ Dict::Add('EN US', 'English', 'English', array( 'UI:Apply_Stimulus_On_Object_In_State_ToTarget_State' => 'Applying %1$s on object: %2$s in state %3$s to target state: %4$s.', 'UI:ObjectCouldNotBeWritten' => 'The object could not be written: %1$s', 'UI:PageTitle:FatalError' => 'iTop - Fatal Error', + 'UI:SystemIntrusion' => 'Access denied. You have trying to perform an operation that is not allowed for you.', 'UI:FatalErrorMessage' => 'Fatal error, iTop cannot continue.', 'UI:Error_Details' => 'Error: %1$s.', diff --git a/dictionaries/es_cr.dictionary.itop.ui.php b/dictionaries/es_cr.dictionary.itop.ui.php index a3bc810b1..983a83910 100644 --- a/dictionaries/es_cr.dictionary.itop.ui.php +++ b/dictionaries/es_cr.dictionary.itop.ui.php @@ -662,6 +662,7 @@ Dict::Add('ES CR', 'Spanish', 'Español, Castellano', array( 'UI:ObjectCouldNotBeWritten' => 'The object could not be written: %1$s', 'UI:PageTitle:FatalError' => 'iTop - Fatal Error', 'UI:FatalErrorMessage' => 'Fatal error, iTop cannot continue.', + 'UI:SystemIntrusion' => 'Access denied. You have trying to perform an operation that is not allowed for you.', 'UI:Error_Details' => 'Error: %1$s.', 'UI:PageTitle:ClassProjections' => 'iTop user management - class projections', diff --git a/dictionaries/fr.dictionary.itop.ui.php b/dictionaries/fr.dictionary.itop.ui.php index 896fbf2ee..142e894d0 100644 --- a/dictionaries/fr.dictionary.itop.ui.php +++ b/dictionaries/fr.dictionary.itop.ui.php @@ -659,6 +659,7 @@ Dict::Add('FR FR', 'French', 'Français', array( 'UI:ObjectCouldNotBeWritten' => 'L\'objet ne peut pas être enregistré: %1$s', 'UI:PageTitle:FatalError' => 'iTop - Erreur Fatale', 'UI:FatalErrorMessage' => 'Erreur fatale, iTop ne peut pas continuer.', + 'UI:SystemIntrusion' => 'Accès non autorisé. Vous êtes en train de d\'effectuer une opération qui ne vous est pas permise.', 'UI:Error_Details' => 'Erreur: %1$s.', 'UI:PageTitle:ClassProjections' => 'iTop gestion des utilisateurs - projections des classes', diff --git a/modules/authent-local/model.authent-local.php b/modules/authent-local/model.authent-local.php index 1631395d7..172d7abc2 100644 --- a/modules/authent-local/model.authent-local.php +++ b/modules/authent-local/model.authent-local.php @@ -103,7 +103,7 @@ class UserLocal extends UserInternal } $oChange->Set("userinfo", $sUserString); $oChange->DBInsert(); - $this->DBUpdateTracked($oChange); + $this->DBUpdateTracked($oChange, true); return true; } return false; diff --git a/pages/UI.php b/pages/UI.php index 26d02c9f5..0c6c6b89c 100644 --- a/pages/UI.php +++ b/pages/UI.php @@ -48,7 +48,7 @@ function DeleteObjects(WebPage $oP, $sClass, $aObjects, $bDeleteConfirmed) foreach ($aDeletes as $iId => $aData) { $oToDelete = $aData['to_delete']; - $bDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, DBObjectSet::FromObject($oToDelete)) && !MetaModel::IsReadOnlyClass($sClass); + $bDeleteAllowed = UserRights::IsActionAllowed($sClass, UR_ACTION_DELETE, DBObjectSet::FromObject($oToDelete)); $aTotalDeletedObjs[$sRemoteClass][$iId]['auto_delete'] = $aData['auto_delete']; if (!$bDeleteAllowed) { @@ -117,11 +117,11 @@ function DeleteObjects(WebPage $oP, $sClass, $aObjects, $bDeleteConfirmed) // Security - do not allow the user to force a forbidden delete by the mean of page arguments... if ($bFoundStopper) { - throw new SecurityException(Dict::S('UI:Error:NotEnoughRightsToDelete')); + throw new CoreException(Dict::S('UI:Error:NotEnoughRightsToDelete')); } if ($bFoundManual) { - throw new SecurityException(Dict::S('UI:Error:CannotDeleteBecauseOfDepencies')); + throw new CoreException(Dict::S('UI:Error:CannotDeleteBecauseOfDepencies')); } // Prepare the change reporting @@ -525,17 +525,24 @@ try { throw new ApplicationException(Dict::Format('UI:Error:2ParametersMissing', 'class', 'id')); } - $oObj = MetaModel::GetObject($sClass, $id, false); - if ($oObj != null) - { - $oP->set_title(Dict::Format('UI:DetailsPageTitle', $oObj->GetName(), $sClassLabel)); - $oObj->DisplayDetails($oP); - } - else + + $oObj = MetaModel::GetObject($sClass, $id, false /* MustBeFound */); + if (is_null($oObj)) { $oP->set_title(Dict::S('UI:ErrorPageTitle')); $oP->P(Dict::S('UI:ObjectDoesNotExist')); } + else + { + // The object could be listed, check if it is actually allowed to view it + $oSet = CMDBObjectSet::FromObject($oObj); + if (UserRights::IsActionAllowed($sClass, UR_ACTION_READ, $oSet) == UR_ALLOWED_NO) + { + throw new SecurityException('User not allowed to view this object', array('class' => $sClass, 'id' => $id)); + } + $oP->set_title(Dict::Format('UI:DetailsPageTitle', $oObj->GetName(), $sClassLabel)); + $oObj->DisplayDetails($oP); + } break; case 'search_oql': @@ -724,17 +731,20 @@ try throw new ApplicationException(Dict::Format('UI:Error:2ParametersMissing', 'class', 'id')); } // Check if the user can modify this object - $oSearch = new DBObjectSearch($sClass); - $oSearch->AddCondition('id', $id, '='); - $oSet = new CMDBObjectSet($oSearch); - if ($oSet->Count() > 0) + $oObj = MetaModel::GetObject($sClass, $id, false /* MustBeFound */); + if (is_null($oObj)) { - $oObj = $oSet->Fetch(); + $oP->set_title(Dict::S('UI:ErrorPageTitle')); + $oP->P(Dict::S('UI:ObjectDoesNotExist')); } - - $bIsModifiedAllowed = (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet) == UR_ALLOWED_YES) && !MetaModel::IsReadOnlyClass($sClass); - if( ($oObj != null) && $bIsModifiedAllowed ) + else { + // The object could be read - check if it is allowed to modify it + $oSet = CMDBObjectSet::FromObject($oObj); + if (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet) == UR_ALLOWED_NO) + { + throw new SecurityException('User not allowed to modify this object', array('class' => $sClass, 'id' => $id)); + } // Note: code duplicated to the case 'apply_modify' when a data integrity issue has been found $oP->set_title(Dict::Format('UI:ModificationPageTitle_Object_Class', $oObj->GetName(), $sClassLabel)); $oP->add("
\n"); @@ -745,11 +755,6 @@ try $oObj->DisplayModifyForm($oP); $oP->add("
\n"); } - else - { - $oP->set_title(Dict::S('UI:ErrorPageTitle')); - $oP->P(Dict::S('UI:ObjectDoesNotExist')); - } break; case 'clone': @@ -769,7 +774,7 @@ try $oObjToClone = $oSet->Fetch(); } - $bIsModifiedAllowed = (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet) == UR_ALLOWED_YES) && !MetaModel::IsReadOnlyClass($sClass); + $bIsModifiedAllowed = (UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, $oSet) == UR_ALLOWED_YES); if( ($oObjToClone != null) && ($bIsModifiedAllowed)) { $oP->add_linked_script("../js/json.js"); @@ -1034,7 +1039,7 @@ try { $aObjects[] = MetaModel::GetObject($sClass, $iId); } - if (MetaModel::IsReadOnlyClass($sClass) || !UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, DBObjectSet::FromArray($sClass, $aObjects))) + if (!UserRights::IsActionAllowed($sClass, UR_ACTION_BULK_DELETE, DBObjectSet::FromArray($sClass, $aObjects))) { throw new SecurityException(Dict::S('UI:Error:BulkDeleteNotAllowedOn_Class'), $sClass); } @@ -1049,7 +1054,7 @@ try $id = utils::ReadParam('id', ''); $oObj = MetaModel::GetObject($sClass, $id); - if (MetaModel::IsReadOnlyClass($sClass) || !UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, DBObjectSet::FromObject($oObj))) + if (!UserRights::IsActionAllowed($sClass, UR_ACTION_MODIFY, DBObjectSet::FromObject($oObj))) { throw new SecurityException(Dict::S('UI:Error:DeleteNotAllowedOn_Class'), $sClass); } @@ -1504,7 +1509,14 @@ catch(CoreException $e) { require_once('../setup/setuppage.class.inc.php'); $oP = new SetupWebPage(Dict::S('UI:PageTitle:FatalError')); - $oP->add("

".Dict::S('UI:FatalErrorMessage')."

\n"); + if ($e instanceof SecurityException) + { + $oP->add("

".Dict::S('UI:SystemIntrusion')."

\n"); + } + else + { + $oP->add("

".Dict::S('UI:FatalErrorMessage')."

\n"); + } $oP->error(Dict::Format('UI:Error_Details', $e->getHtmlDesc())); $oP->output(); diff --git a/webservices/check_sla_for_tickets.php b/webservices/check_sla_for_tickets.php index d1ceab550..0a992c4ba 100644 --- a/webservices/check_sla_for_tickets.php +++ b/webservices/check_sla_for_tickets.php @@ -38,7 +38,7 @@ while ($oToEscalate = $oSet->Fetch()) { $oToEscalate->ApplyStimulus('ev_timeout'); //$oToEscalate->Set('tto_escalation_deadline', null); - $oToEscalate->DBUpdateTracked($oMyChange); + $oToEscalate->DBUpdateTracked($oMyChange, true); echo "

ticket ".$oToEscalate->Get('ref')." reached TTO ESCALATION deadline

\n"; } @@ -47,7 +47,7 @@ while ($oToEscalate = $oSet->Fetch()) { $oToEscalate->ApplyStimulus('ev_timeout'); //$oToEscalate->Set('ttr_escalation_deadline', null); - $oToEscalate->DBUpdateTracked($oMyChange); + $oToEscalate->DBUpdateTracked($oMyChange, true); echo "

ticket ".$oToEscalate->Get('ref')." reached TTR ESCALATION deadline

\n"; } @@ -56,7 +56,7 @@ while ($oToEscalate = $oSet->Fetch()) { $oToEscalate->ApplyStimulus('ev_close'); //$oToEscalate->Set('closure_deadline', null); - $oToEscalate->DBUpdateTracked($oMyChange); + $oToEscalate->DBUpdateTracked($oMyChange, true); echo "

ticket ".$oToEscalate->Get('ref')." reached closure deadline

\n"; }