diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php
index f55087dcb..b6a6ebb2c 100644
--- a/core/attributedef.class.inc.php
+++ b/core/attributedef.class.inc.php
@@ -1690,7 +1690,7 @@ class AttributeLinkedSet extends AttributeDefinition
{
if ($sObjClass == $this->GetLinkedClass())
{
- // Simplify the output if the exact class could be determined implicitely
+ // Simplify the output if the exact class could be determined implicitely
continue;
}
}
@@ -2012,7 +2012,7 @@ class AttributeLinkedSet extends AttributeDefinition
{
if ($sObjClass == $this->GetLinkedClass())
{
- // Simplify the output if the exact class could be determined implicitely
+ // Simplify the output if the exact class could be determined implicitely
continue;
}
}
@@ -2417,7 +2417,7 @@ class AttributeDBFieldVoid extends AttributeDefinition
return false;
}
- //
+ //
protected function ScalarToSQL($value)
{
return $value;
@@ -4592,7 +4592,13 @@ class AttributeCaseLog extends AttributeLongText
{
if (strlen($proposedValue) > 0)
{
- $oCaseLog->AddLogEntry($proposedValue);
+ //N°5135 - add impersonation information in caselog
+ if (UserRights::IsImpersonated()){
+ $sOnBehalfOf = $sUserString = Dict::Format('UI:Archive_User_OnBehalfOf_User', UserRights::GetRealUserFriendlyName(), UserRights::GetUserFriendlyName());
+ $oCaseLog->AddLogEntry($proposedValue, $sOnBehalfOf, UserRights::GetConnectedUserId());
+ } else {
+ $oCaseLog->AddLogEntry($proposedValue);
+ }
}
}
$ret = $oCaseLog;
@@ -5399,7 +5405,7 @@ class AttributeEnum extends AttributeString
{
if (is_null($sValue))
{
- // Unless a specific label is defined for the null value of this enum, use a generic "undefined" label
+ // Unless a specific label is defined for the null value of this enum, use a generic "undefined" label
$sLabel = Dict::S('Class:'.$this->GetHostClass().'/Attribute:'.$this->GetCode().'/Value:'.$sValue,
Dict::S('Enum:Undefined'));
}
@@ -5421,7 +5427,7 @@ class AttributeEnum extends AttributeString
{
if (is_null($sValue))
{
- // Unless a specific label is defined for the null value of this enum, use a generic "undefined" label
+ // Unless a specific label is defined for the null value of this enum, use a generic "undefined" label
$sDescription = Dict::S('Class:'.$this->GetHostClass().'/Attribute:'.$this->GetCode().'/Value:'.$sValue.'+',
Dict::S('Enum:Undefined'));
}
@@ -7218,7 +7224,7 @@ class AttributeExternalField extends AttributeDefinition
protected function GetSQLCol($bFullSpec = false)
{
- // throw new CoreException("external attribute: does it make any sense to request its type ?");
+ // throw new CoreException("external attribute: does it make any sense to request its type ?");
$oExtAttDef = $this->GetExtAttDef();
return $oExtAttDef->GetSQLCol($bFullSpec);
@@ -9266,7 +9272,7 @@ class AttributeSubItem extends AttributeDefinition
return $res;
}
- //
+ //
// protected function ScalarToSQL($value) {return $value;} // format value as a valuable SQL literal (quoted outside)
public function FromSQLToValue($aCols, $sPrefix = '')
diff --git a/core/cmdbobject.class.inc.php b/core/cmdbobject.class.inc.php
index ea9329ba3..8e6a83603 100644
--- a/core/cmdbobject.class.inc.php
+++ b/core/cmdbobject.class.inc.php
@@ -3,7 +3,7 @@
//
// This file is part of iTop.
//
-// iTop is free software; you can redistribute it and/or modify
+// iTop is free software; you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
@@ -214,7 +214,12 @@ abstract class CMDBObject extends DBObject
if (is_null(self::$m_sInfo)) {
return CMDBChange::GetCurrentUserName();
} else {
- return self::$m_sInfo;
+ //N°5135 - add impersonation information in activity log/current cmdb change
+ if (UserRights::IsImpersonated()){
+ return sprintf("%s (%s)", CMDBChange::GetCurrentUserName(), self::$m_sInfo);
+ } else {
+ return self::$m_sInfo;
+ }
}
}
@@ -227,7 +232,10 @@ abstract class CMDBObject extends DBObject
*/
protected static function GetTrackUserId()
{
- if (is_null(self::$m_sUserId))
+ if (is_null(self::$m_sUserId)
+ //N°5135 - indicate impersonation inside changelogs
+ && (false === UserRights::IsImpersonated())
+ )
{
return CMDBChange::GetCurrentUserId();
}
@@ -236,10 +244,10 @@ abstract class CMDBObject extends DBObject
return self::$m_sUserId;
}
}
-
+
/**
* Get the 'origin' information (defaulting to 'interactive')
- */
+ */
protected static function GetTrackOrigin()
{
if (is_null(self::$m_sOrigin))
@@ -268,7 +276,7 @@ abstract class CMDBObject extends DBObject
* @since 2.7.7 3.0.2 3.1.0 N°3717 {@see CMDBChange} **will be persisted later** in {@see \CMDBChangeOp::OnInsert} (was done previously directly here)
* This will avoid creating in DB CMDBChange lines without any corresponding CMDBChangeOp
*/
- protected static function CreateChange()
+ public static function CreateChange()
{
self::$m_oCurrChange = MetaModel::NewObject("CMDBChange");
self::$m_oCurrChange->Set("date", time());
@@ -640,7 +648,7 @@ abstract class CMDBObject extends DBObject
{
return;
}
-
+
$ret = parent::DBUpdate();
return $ret;
}
@@ -725,11 +733,11 @@ abstract class CMDBObject extends DBObject
class CMDBObjectSet extends DBObjectSet
{
// this is the public interface (?)
-
+
// I have to define those constructors here... :-(
// just to get the right object class in return.
// I have to think again to those things: maybe it will work fine if a have a constructor define here (?)
-
+
static public function FromScratch($sClass)
{
$oFilter = new DBObjectSearch($sClass);
@@ -738,7 +746,7 @@ class CMDBObjectSet extends DBObjectSet
// NOTE: THIS DOES NOT WORK IF m_bLoaded is private in the base class (and you will not get any error message)
$oRetSet->m_bLoaded = true; // no DB load
return $oRetSet;
- }
+ }
// create an object set ex nihilo
// input = array of objects
@@ -747,7 +755,7 @@ class CMDBObjectSet extends DBObjectSet
$oRetSet = self::FromScratch($sClass);
$oRetSet->AddObjectArray($aObjects, $sClass);
return $oRetSet;
- }
+ }
static public function FromArrayAssoc($aClasses, $aObjects)
{
diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php
index adfa4cb18..204f6fcec 100644
--- a/core/userrights.class.inc.php
+++ b/core/userrights.class.inc.php
@@ -19,7 +19,7 @@ define('UR_ACTION_CREATE', 7); // Instantiate an object
define('UR_ACTION_APPLICATION_DEFINED', 10000); // Application specific actions (CSV import, View schema...)
/**
- * User management module API
+ * User management module API
*
* @package iTopORM
*/
@@ -139,7 +139,7 @@ abstract class UserRightsAddOnAPI
$oExpression = new FieldExpression($sAttCode, $sClass);
$oFilter = new DBObjectSearch($sClass);
$oListExpr = ListExpression::FromScalars($aAllowedOrgs);
-
+
$oCondition = new BinaryExpression($oExpression, 'IN', $oListExpr);
$oFilter->AddConditionExpression($oCondition);
@@ -156,7 +156,7 @@ abstract class UserRightsAddOnAPI
$oShareSearch = new DBObjectSearch('SharedObject');
$oOrgField = new FieldExpression('org_id', 'SharedObject');
$oShareSearch->AddConditionExpression(new BinaryExpression($oOrgField, 'IN', $oListExpr));
-
+
$oSearchSharers = new DBObjectSearch('Organization');
$oSearchSharers->AllowAllData();
$oSearchSharers->AddCondition_ReferencedBy($oShareSearch, 'sharing_org_id');
@@ -172,16 +172,16 @@ abstract class UserRightsAddOnAPI
$oFilter->MergeConditionExpression(new BinaryExpression($oExpression, 'IN', $oSharersList));
}
}
-
+
$aShareProperties = SharedObject::GetSharedClassProperties($sClass);
if ($aShareProperties)
{
$sShareClass = $aShareProperties['share_class'];
$sShareAttCode = $aShareProperties['attcode'];
-
+
$oSearchShares = new DBObjectSearch($sShareClass);
$oSearchShares->AllowAllData();
-
+
$sHierarchicalKeyCode = MetaModel::IsHierarchicalClass('Organization');
$oOrgField = new FieldExpression('org_id', $sShareClass);
$oSearchShares->AddConditionExpression(new BinaryExpression($oOrgField, 'IN', $oListExpr));
@@ -236,7 +236,7 @@ abstract class User extends cmdbAbstractObject
MetaModel::Init_AddAttribute(new AttributeApplicationLanguage("language", array("sql"=>"language", "default_value"=>"EN US", "is_null_allowed"=>false, "depends_on"=>array())));
MetaModel::Init_AddAttribute(new AttributeEnum("status", array("allowed_values" => new ValueSetEnum('enabled,disabled'), "sql"=>"status", "default_value"=>"enabled", "is_null_allowed"=>false, "depends_on"=>array())));
-
+
MetaModel::Init_AddAttribute(new AttributeLinkedSetIndirect("profile_list", array("linked_class"=>"URP_UserProfile", "ext_key_to_me"=>"userid", "ext_key_to_remote"=>"profileid", "allowed_values"=>null, "count_min"=>1, "count_max"=>0, "depends_on"=>array())));
MetaModel::Init_AddAttribute(new AttributeLinkedSetIndirect("allowed_org_list", array("linked_class"=>"URP_UserOrg", "ext_key_to_me"=>"userid", "ext_key_to_remote"=>"allowed_org_id", "allowed_values"=>null, "count_min"=>1, "count_max"=>0, "depends_on"=>array())));
@@ -504,7 +504,7 @@ abstract class User extends cmdbAbstractObject
return ''.Dict::S('UI:UserManagement:ActionAllowed:No').'';
}
}
-
+
function DoShowGrantSumary($oPage, $sClassCategory)
{
if (UserRights::IsAdministrator($this))
@@ -536,7 +536,7 @@ abstract class User extends cmdbAbstractObject
{
$sStimuli = ''.Dict::S('UI:UserManagement:NoLifeCycleApplicable').'';
}
-
+
$aDisplayData[] = array(
'class' => MetaModel::GetName($sClass),
'read' => $this->GetGrantAsHtml($sClass, UR_ACTION_READ),
@@ -550,7 +550,7 @@ abstract class User extends cmdbAbstractObject
}
$oKPI->ComputeAndReport('Computation of user rights');
-
+
$aDisplayConfig = array();
$aDisplayConfig['class'] = array('label' => Dict::S('UI:UserManagement:Class'), 'description' => Dict::S('UI:UserManagement:Class+'));
$aDisplayConfig['read'] = array('label' => Dict::S('UI:UserManagement:Action:Read'), 'description' => Dict::S('UI:UserManagement:Action:Read+'));
@@ -585,7 +585,7 @@ abstract class User extends cmdbAbstractObject
}
}
-
+
public function CheckToDelete(&$oDeletionPlan)
{
if (MetaModel::GetConfig()->Get('demo_mode'))
@@ -597,8 +597,8 @@ abstract class User extends cmdbAbstractObject
return false;
}
return parent::CheckToDelete($oDeletionPlan);
- }
-
+ }
+
protected function DBDeleteSingleObject()
{
if (MetaModel::GetConfig()->Get('demo_mode'))
@@ -633,7 +633,7 @@ abstract class User extends cmdbAbstractObject
*/
abstract class UserInternal extends User
{
- // Nothing special, just a base class to categorize this type of authenticated users
+ // Nothing special, just a base class to categorize this type of authenticated users
public static function Init()
{
$aParams = array
@@ -662,15 +662,15 @@ abstract class UserInternal extends User
/**
* Use with care!
- */
+ */
public function SetPassword($sNewPassword)
{
}
/**
- * The email recipient is the person who is allowed to regain control when the password gets lost
+ * The email recipient is the person who is allowed to regain control when the password gets lost
* Throws an exception if the feature cannot be available
- */
+ */
public function GetResetPasswordEmail()
{
if (!MetaModel::IsValidAttCode(get_class($this), 'contactid'))
@@ -703,7 +703,7 @@ abstract class UserInternal extends User
}
/**
- * Self register extension
+ * Self register extension
*
* @package iTopORM
*/
@@ -719,10 +719,10 @@ interface iSelfRegister
* @return bool true if the user is a valid one, false otherwise
*/
public static function CheckCredentialsAndCreateUser($sName, $sPassword, $sLoginMode, $sAuthentication);
-
+
/**
* Called after the user has been authenticated and found in iTop. This method can
- * Update the user's definition on the fly (profiles...) to keep it in sync with an external source
+ * Update the user's definition on the fly (profiles...) to keep it in sync with an external source
* @param User $oUser The user to update/synchronize
* @param string $sLoginMode The login mode used (cas|form|basic|url)
* @param string $sAuthentication The authentication method used
@@ -732,7 +732,7 @@ interface iSelfRegister
}
/**
- * User management core API
+ * User management core API
*
* @package iTopORM
*/
@@ -830,7 +830,7 @@ class UserRights
else
{
return true;
- }
+ }
}
/**
@@ -1053,6 +1053,11 @@ class UserRights
Dict::SetUserLanguage(self::GetUserLanguage());
Session::Set('impersonate_user', $sLogin);
self::_ResetSessionCache();
+
+ //N°5135 - Impersonate: history of changes versus log entries
+ //track impersonation inside changelogs
+ CMDBObject::SetTrackUserId(null);
+ CMDBObject::CreateChange();
}
}
return $bRet;
@@ -1066,9 +1071,15 @@ class UserRights
if (!is_null(self::$m_oRealUser))
{
self::$m_oUser = self::$m_oRealUser;
+ //N°5135 - fix IsImpersonated() after calling Deimpersonate()
+ self::$m_oRealUser = null;
Dict::SetUserLanguage(self::GetUserLanguage());
Session::Unset('impersonate_user');
self::_ResetSessionCache();
+
+ //N°5135 - Impersonate: history of changes versus log entries
+ //stop tracking impersonation inside changelogs
+ CMDBObject::CreateChange();
}
}
@@ -1136,7 +1147,7 @@ class UserRights
if (is_null(self::$m_oUser))
{
return 'EN US';
-
+
}
else
{
@@ -1483,7 +1494,7 @@ class UserRights
{
if (!self::IsLoggedIn())
{
- //throw new UserRightException('No user logged in', array());
+ //throw new UserRightException('No user logged in', array());
return false;
}
return true;
@@ -1871,7 +1882,7 @@ class UserRights
case 'internal':
$sBaseClass = 'UserInternal';
break;
-
+
default:
echo "sAuthentication = $sAuthentication
\n";
assert(false); // should never happen
@@ -1936,7 +1947,7 @@ class UserRights
Session::Unset('profile_list');
Session::Unset('archive_allowed');
}
-
+
/**
* Fake error handler to silently discard fatal errors
* @param int $iErrNo
@@ -1968,7 +1979,7 @@ class ActionChecker
var $iActionCode;
var $iAllowedCount = null;
var $aAllowedIDs = null;
-
+
public function __construct(DBSearch $oFilter, $iActionCode)
{
$this->oFilter = $oFilter;
@@ -1976,7 +1987,7 @@ class ActionChecker
$this->iAllowedCount = null;
$this->aAllowedIDs = null;
}
-
+
/**
* returns the number of objects for which the action is allowed
* @return integer The number of "allowed" objects 0..N
@@ -1986,7 +1997,7 @@ class ActionChecker
if ($this->iAllowedCount == null) $this->CheckObjects();
return $this->iAllowedCount;
}
-
+
/**
* If IsAllowed returned UR_ALLOWED_DEPENDS, this methods returns
* an array of ObjKey => Status (true|false)
@@ -1997,7 +2008,7 @@ class ActionChecker
if ($this->aAllowedIDs == null) $this->IsAllowed();
return $this->aAllowedIDs;
}
-
+
/**
* Check if the speficied stimulus is allowed for the set of objects
* @return UR_ALLOWED_YES, UR_ALLOWED_NO or UR_ALLOWED_DEPENDS
@@ -2048,7 +2059,7 @@ class ActionChecker
class StimulusChecker extends ActionChecker
{
var $sState = null;
-
+
public function __construct(DBSearch $oFilter, $sState, $iStimulusCode)
{
parent::__construct($oFilter, $iStimulusCode);
@@ -2063,7 +2074,7 @@ class StimulusChecker extends ActionChecker
{
$sClass = $this->oFilter->GetClass();
if (MetaModel::IsAbstract($sClass)) return UR_ALLOWED_NO; // Safeguard, not implemented if the base class of the set is abstract !
-
+
$oSet = new DBObjectSet($this->oFilter);
$iActionAllowed = UserRights::IsStimulusAllowed($sClass, $this->iActionCode, $oSet);
if ($iActionAllowed == UR_ALLOWED_NO)
@@ -2075,7 +2086,7 @@ class StimulusChecker extends ActionChecker
{
// Hmmm, may not be needed right now because we limit the "multiple" action to object in
// the same state... may be useful later on if we want to extend this behavior...
-
+
// Check for each object if the action is allowed or not
$this->aAllowedIDs = array();
$oSet->Rewind();
@@ -2100,15 +2111,15 @@ class StimulusChecker extends ActionChecker
$this->aAllowedIDs[$oObj->GetKey()] = true;
$this->iState = $oObj->GetState();
$this->iAllowedCount++;
- }
+ }
}
else
{
- $this->aAllowedIDs[$oObj->GetKey()] = false;
- }
+ $this->aAllowedIDs[$oObj->GetKey()] = false;
+ }
}
}
-
+
if ($this->iAllowedCount == $oSet->Count())
{
$iActionAllowed = UR_ALLOWED_YES;
@@ -2120,9 +2131,9 @@ class StimulusChecker extends ActionChecker
return $iActionAllowed;
}
-
+
public function GetState()
{
return $this->iState;
- }
+ }
}
diff --git a/test/core/CMDBObjectTest.php b/test/core/CMDBObjectTest.php
index 237b51ca4..fd2368fc0 100644
--- a/test/core/CMDBObjectTest.php
+++ b/test/core/CMDBObjectTest.php
@@ -5,6 +5,7 @@ namespace Combodo\iTop\Test\UnitTest\Core;
use CMDBObject;
use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use Exception;
use MetaModel;
/**
@@ -21,6 +22,17 @@ use MetaModel;
*/
class CMDBObjectTest extends ItopDataTestCase
{
+ private $sAdminLogin;
+ private $sImpersonatedLogin;
+
+ /**
+ * @throws Exception
+ */
+ protected function setUp(): void
+ {
+ parent::setUp();
+ }
+
/**
* @covers CMDBObject::SetCurrentChange
*/
@@ -72,4 +84,111 @@ class CMDBObjectTest extends ItopDataTestCase
CMDBObject::SetCurrentChange($oInitialCurrentChange);
CMDBObject::SetTrackInfo($sInitialTrackInfo);
}
-}
\ No newline at end of file
+
+ public function CurrentChangeUnderImpersonationProvider(){
+ return [
+ 'no track info' => [ 'sTrackInfo' => null ],
+ 'track info from approvalbase' => [
+ 'sTrackInfo' => 'ImpersonatedSurName ImpersonatedName Approved on behalf of YYY',
+ 'sExpectedChangeLogWhenImpersonation' => "AdminSurName AdminName on behalf of ImpersonatedSurName ImpersonatedName (ImpersonatedSurName ImpersonatedName Approved on behalf of YYY)",
+ ],
+ ];
+ }
+
+ /**
+ * @covers CMDBObject::SetCurrentChange
+ * @since 3.0.1 N°5135 - Impersonate: history of changes versus log entries
+ *
+ * @dataProvider CurrentChangeUnderImpersonationProvider
+ */
+ public function testCurrentChangeUnderImpersonation($sTrackInfo=null, $sExpectedChangeLogWhenImpersonation=null) {
+ $this->CreateTestOrganization();
+
+ $sUid = date('dmYHis');
+ $sAdminLogin = "admin-user-".$sUid;
+ $sImpersonatedLogin = "impersonated-user-".$sUid;
+
+ $iAdminUserId = $this->CreateUserForImpersonation($sAdminLogin, 'Administrator', 'AdminName', 'AdminSurName');
+ $this->CreateUserForImpersonation($sImpersonatedLogin, 'Configuration Manager', 'ImpersonatedName', 'ImpersonatedSurName');
+
+ $_SESSION = [];
+ \UserRights::Login($sAdminLogin);
+
+ // save initial conditions
+ $oInitialCurrentChange = CMDBObject::GetCurrentChange();
+ $sInitialTrackInfo = CMDBObject::GetTrackInfo();
+
+ // reset current change
+ CMDBObject::SetCurrentChange(null);
+
+ if (is_null($sTrackInfo)){
+ CMDBObject::SetTrackInfo(null);
+ } else {
+ CMDBObject::SetTrackInfo($sTrackInfo);
+ }
+
+ $this->CreateSimpleObject();
+ if (is_null($sTrackInfo)){
+ self::assertEquals("AdminSurName AdminName", CMDBObject::GetCurrentChange()->Get('userinfo'),
+ 'TrackInfo : no impersonation');
+ } else {
+ self::assertEquals($sTrackInfo, CMDBObject::GetCurrentChange()->Get('userinfo'),
+ 'TrackInfo : no impersonation');
+ }
+ self::assertEquals($iAdminUserId, CMDBObject::GetCurrentChange()->Get('user_id'),
+ 'TrackInfo : admin userid');
+
+ \UserRights::Impersonate($sImpersonatedLogin);
+ $this->CreateSimpleObject();
+
+ if (is_null($sExpectedChangeLogWhenImpersonation)){
+ self::assertEquals("AdminSurName AdminName on behalf of ImpersonatedSurName ImpersonatedName", CMDBObject::GetCurrentChange()->Get('userinfo'),
+ 'TrackInfo : impersonation');
+ } else {
+ self::assertEquals($sExpectedChangeLogWhenImpersonation, CMDBObject::GetCurrentChange()->Get('userinfo'),
+ 'TrackInfo : impersonation');
+ }
+
+ self::assertEquals(null, CMDBObject::GetCurrentChange()->Get('user_id'),
+ 'TrackInfo : no userid to force userinfo being displayed on UI caselog side');
+
+ \UserRights::Deimpersonate();
+ $this->CreateSimpleObject();
+ if (is_null($sTrackInfo)){
+ self::assertEquals("AdminSurName AdminName", CMDBObject::GetCurrentChange()->Get('userinfo'),
+ 'TrackInfo : no impersonation');
+ } else {
+ self::assertEquals($sTrackInfo, CMDBObject::GetCurrentChange()->Get('userinfo'),
+ 'TrackInfo : no impersonation');
+ }
+ self::assertEquals($iAdminUserId, CMDBObject::GetCurrentChange()->Get('user_id'),
+ 'TrackInfo : admin userid');
+
+ // restore initial conditions
+ CMDBObject::SetCurrentChange($oInitialCurrentChange);
+ CMDBObject::SetTrackInfo($sInitialTrackInfo);
+ }
+
+ private function CreateSimpleObject(){
+ /** @var \DocumentWeb $oTestObject */
+ $oTestObject = MetaModel::NewObject('DocumentWeb');
+ $oTestObject->Set('name', 'PHPUnit test');
+ $oTestObject->Set('org_id', $this->getTestOrgId() );
+ $oTestObject->Set('url', 'https://www.combodo.com');
+ $oTestObject->DBWrite();
+ }
+
+ private function CreateUserForImpersonation($sLogin, $sProfileName, $sName, $sSurname): int {
+ /** @var \Person $oPerson */
+ $oPerson = $this->createObject('Person', array(
+ 'name' => $sName,
+ 'first_name' => $sSurname,
+ 'org_id' => $this->getTestOrgId(),
+ ));
+
+ $oProfile = \MetaModel::GetObjectFromOQL("SELECT URP_Profiles WHERE name = :name", array('name' => $sProfileName), true);
+ $oUser = $this->CreateUser($sLogin, $oProfile->GetKey(), "1234567Azert@", $oPerson->GetKey());
+
+ return $oUser->GetKey();
+ }
+}