Improved security

git-svn-id: http://svn.code.sf.net/p/itop/code/trunk@809 a333f486-631f-4898-b8df-5754b55c2be0

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;
 	}

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;
 	}

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;
 	}
 

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

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"); }

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" => "",

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;
-	}
 }
 
 ?>

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
 	 */	 

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;

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',

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

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" => "",

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);

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;
 
+		if (MetaModel::HasCategory($sClass, 'bizmodel'))
+		{
+			// #@# Temporary?????
+			// The read access is controlled in MetaModel::MakeSelectQuery()
+			if ($iActionCode == UR_ACTION_READ) 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 (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'))
 		{
-			$oUser = self::$m_oUser;
+			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)

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.',
 

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',

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',

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;

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,16 +525,23 @@ try
 			{
 				throw new ApplicationException(Dict::Format('UI:Error:2ParametersMissing', 'class', 'id'));
 			}
-			$oObj = MetaModel::GetObject($sClass, $id, false);
-			if ($oObj != null)
+
+			$oObj = MetaModel::GetObject($sClass, $id, false /* MustBeFound */);
+			if (is_null($oObj))
 			{
-				$oP->set_title(Dict::Format('UI:DetailsPageTitle', $oObj->GetName(), $sClassLabel));
-				$oObj->DisplayDetails($oP);
+				$oP->set_title(Dict::S('UI:ErrorPageTitle'));
+				$oP->P(Dict::S('UI:ObjectDoesNotExist'));
 			}
 			else
 			{
-				$oP->set_title(Dict::S('UI:ErrorPageTitle'));
-				$oP->P(Dict::S('UI:ObjectDoesNotExist'));
+				// 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;
 	
@@ -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("<div class=\"page_header\">\n");
@@ -745,11 +755,6 @@ try
 				$oObj->DisplayModifyForm($oP);
 				$oP->add("</div>\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("<h1>".Dict::S('UI:FatalErrorMessage')."</h1>\n");	
+	if ($e instanceof SecurityException)
+	{
+		$oP->add("<h1>".Dict::S('UI:SystemIntrusion')."</h1>\n");
+	}
+	else
+	{
+		$oP->add("<h1>".Dict::S('UI:FatalErrorMessage')."</h1>\n");
+	}	
 	$oP->error(Dict::Format('UI:Error_Details', $e->getHtmlDesc()));	
 	$oP->output();
 

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 "<p>ticket ".$oToEscalate->Get('ref')." reached TTO ESCALATION deadline</p>\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 "<p>ticket ".$oToEscalate->Get('ref')." reached TTR ESCALATION deadline</p>\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 "<p>ticket ".$oToEscalate->Get('ref')." reached closure deadline</p>\n";
 }