From 9e702899f2ca7394d096878826128ff7e45ff22b Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Thu, 17 Sep 2009 15:50:28 +0000 Subject: [PATCH] Fixed a security hole: any user was allowed to edit users and profiles, and therefore could give himself admin rights - now, only admins are allowed to do that Also added a debugging capability: user rights shown for any object class, on demand in URP_Users::DisplayBareRelations() SVN:trunk[178] --- .../userrightsprofile.class.inc.php | 49 +++++++++++++------ core/userrights.class.inc.php | 32 ++++++++++-- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/addons/userrights/userrightsprofile.class.inc.php b/addons/userrights/userrightsprofile.class.inc.php index aae8862c9..d325b10fb 100644 --- a/addons/userrights/userrightsprofile.class.inc.php +++ b/addons/userrights/userrightsprofile.class.inc.php @@ -78,7 +78,7 @@ class URP_Users extends UserRightsBaseClass } } - function DoShowGrantSumary($oPage) + function DoShowGrantSumary($oPage, $sClassCategory) { $iUserId = $this->GetKey(); if (UserRights::IsAdministrator($iUserId)) @@ -89,17 +89,25 @@ class URP_Users extends UserRightsBaseClass } $aDisplayData = array(); - foreach (MetaModel::GetClasses('bizmodel') as $sClass) + foreach (MetaModel::GetClasses($sClassCategory) as $sClass) { - $aStimuli = array(); - foreach (MetaModel::EnumStimuli($sClass) as $sStimulusCode => $oStimulus) + $aClassStimuli = MetaModel::EnumStimuli($sClass); + if (count($aClassStimuli) > 0) { - if (UserRights::IsStimulusAllowed($sClass, $sStimulusCode, null, $iUserId)) + $aStimuli = array(); + foreach ($aClassStimuli as $sStimulusCode => $oStimulus) { - $aStimuli[] = ''.htmlentities($oStimulus->Get('label')).''; + if (UserRights::IsStimulusAllowed($sClass, $sStimulusCode, null, $iUserId)) + { + $aStimuli[] = ''.htmlentities($oStimulus->Get('label')).''; + } } + $sStimuli = implode(', ', $aStimuli); + } + else + { + $sStimuli = 'n/a'; } - $sStimuli = implode(', ', $aStimuli); $aDisplayData[] = array( 'class' => MetaModel::GetName($sClass), @@ -131,7 +139,22 @@ class URP_Users extends UserRightsBaseClass $oPage->SetCurrentTabContainer('Related Objects'); $oPage->SetCurrentTab('Grants matrix'); - $this->DoShowGrantSumary($oPage); + $this->DoShowGrantSumary($oPage, 'bizmodel'); + + // debug + if (false) + { + $oPage->SetCurrentTab('More on user rigths (dev only)'); + $oPage->add("

User rights

\n"); + $this->DoShowGrantSumary($oPage, 'addon/userrights'); + $oPage->add("

Change log

\n"); + $this->DoShowGrantSumary($oPage, 'core/cmdb'); + $oPage->add("

Application

\n"); + $this->DoShowGrantSumary($oPage, 'application'); + $oPage->add("

GUI

\n"); + $this->DoShowGrantSumary($oPage, 'gui'); + + } } } @@ -1008,8 +1031,6 @@ exit; public function IsActionAllowed($iUserId, $sClass, $iActionCode, $oInstanceSet = null) { - if ($this->IsAdministrator($iUserId)) return true; - $oUser = $this->m_aUsers[$iUserId]; if (is_null($oInstanceSet)) @@ -1051,8 +1072,6 @@ exit; public function IsActionAllowedOnAttribute($iUserId, $sClass, $sAttCode, $iActionCode, $oInstanceSet = null) { - if ($this->IsAdministrator($iUserId)) return true; - $oUser = $this->m_aUsers[$iUserId]; if (is_null($oInstanceSet)) @@ -1134,8 +1153,6 @@ exit; public function IsStimulusAllowed($iUserId, $sClass, $sStimulusCode, $oInstanceSet = null) { - if ($this->IsAdministrator($iUserId)) return true; - $oUser = $this->m_aUsers[$iUserId]; // Note: this code is VERY close to the code of IsActionAllowed() @@ -1508,11 +1525,11 @@ class SetupITILProfiles } - protected static function DoCreateActionGrant($iProfile, $iAction, $sClass) + protected static function DoCreateActionGrant($iProfile, $iAction, $sClass, $bPermission = true) { $oNewObj = MetaModel::NewObject("URP_ActionGrant"); $oNewObj->Set('profileid', $iProfile); - $oNewObj->Set('permission', true); + $oNewObj->Set('permission', $bPermission); $oNewObj->Set('class', $sClass); $oNewObj->Set('action', self::$m_aActions[$iAction]); $iId = $oNewObj->DBInsertNoReload(); diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php index e6107fdc8..03a0dd2e7 100644 --- a/core/userrights.class.inc.php +++ b/core/userrights.class.inc.php @@ -206,16 +206,28 @@ class UserRights public static function GetFilter($sClass) { - if (!MetaModel::HasCategory($sClass, 'bizmodel')) return new DBObjectSearch($sClass); if (!self::CheckLogin()) return false; + if (self::IsAdministrator()) return new DBObjectSearch($sClass); + + // 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 new DBObjectSearch($sClass); return self::$m_oAddOn->GetFilter(self::$m_iUserId, $sClass); } public static function IsActionAllowed($sClass, $iActionCode, /*dbObjectSet*/ $oInstanceSet = null, $iUserId = null) { - if (!MetaModel::HasCategory($sClass, 'bizmodel')) return true; if (!self::CheckLogin()) return false; + if (self::IsAdministrator($iUserId)) 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($iUserId)) { @@ -229,8 +241,14 @@ class UserRights public static function IsStimulusAllowed($sClass, $sStimulusCode, /*dbObjectSet*/ $oInstanceSet = null, $iUserId = null) { - if (!MetaModel::HasCategory($sClass, 'bizmodel')) return true; if (!self::CheckLogin()) return false; + if (self::IsAdministrator($iUserId)) 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($iUserId)) { @@ -244,8 +262,14 @@ class UserRights public static function IsActionAllowedOnAttribute($sClass, $sAttCode, $iActionCode, /*dbObjectSet*/ $oInstanceSet = null, $iUserId = null) { - if (!MetaModel::HasCategory($sClass, 'bizmodel')) return true; if (!self::CheckLogin()) return false; + if (self::IsAdministrator($iUserId)) 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($iUserId)) {