Merge remote-tracking branch 'origin/support/3.0' into develop

This commit is contained in:
Eric Espie
2022-08-11 11:33:59 +02:00
4 changed files with 203 additions and 59 deletions

View File

@@ -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 = '')

View File

@@ -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)
{

View File

@@ -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 '<span style="background-color: #ffdddd;">'.Dict::S('UI:UserManagement:ActionAllowed:No').'</span>';
}
}
function DoShowGrantSumary($oPage, $sClassCategory)
{
if (UserRights::IsAdministrator($this))
@@ -536,7 +536,7 @@ abstract class User extends cmdbAbstractObject
{
$sStimuli = '<em title="'.Dict::S('UI:UserManagement:NoLifeCycleApplicable+').'">'.Dict::S('UI:UserManagement:NoLifeCycleApplicable').'</em>';
}
$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 "<p>sAuthentication = $sAuthentication</p>\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;
}
}
}

View File

@@ -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);
}
}
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();
}
}