6912- add test

Co-authored-by: Molkobain <lajarige.guillaume@free.fr>
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
Co-authored-by: Romain Quetiez <romain.quetiez@combodo.com>

application/loginwebpage.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.
@@ -38,7 +38,7 @@ class LoginWebPage extends NiceWebPage
 	const EXIT_PROMPT = 0;
 	const EXIT_HTTP_401 = 1;
 	const EXIT_RETURN = 2;
-	
+
 	const EXIT_CODE_OK = 0;
 	const EXIT_CODE_MISSINGLOGIN = 1;
 	const EXIT_CODE_MISSINGPASSWORD = 2;
@@ -80,7 +80,7 @@ class LoginWebPage extends NiceWebPage
 	}
 
 	protected static $m_sLoginFailedMessage = '';
-	
+
 	public function __construct($sTitle = null)
 	{
 		if ($sTitle === null) {
@@ -92,7 +92,7 @@ class LoginWebPage extends NiceWebPage
 		$this->no_cache();
 		$this->add_xframe_options();
 	}
-	
+
 	public function SetStyleSheet()
 	{
 		$this->add_linked_stylesheet(utils::GetAbsoluteUrlAppRoot().'css/login.css');
@@ -867,12 +867,12 @@ class LoginWebPage extends NiceWebPage
 	 * @api
 	 *
 	 * @param string $sAuthUser
-	 * @param Person $oPerson
+	 * @param Person|null $oPerson
 	 * @param array $aRequestedProfiles profiles to add to the new user
 	 *
 	 * @return \UserExternal|null
 	 */
-	public static function ProvisionUser($sAuthUser, $oPerson, $aRequestedProfiles)
+	public static function ProvisionUser(string $sAuthUser, ?Person $oPerson, array $aRequestedProfiles)
 	{
 		if (!MetaModel::IsValidClass('URP_Profiles'))
 		{
@@ -897,7 +897,9 @@ class LoginWebPage extends NiceWebPage
 			{
 				$oUser = MetaModel::NewObject('UserExternal');
 				$oUser->Set('login', $sAuthUser);
-				$oUser->Set('contactid', $oPerson->GetKey());
+				if (! is_null($oPerson)){
+					$oUser->Set('contactid', $oPerson->GetKey());
+				}
 				$oUser->Set('language', MetaModel::GetConfig()->GetDefaultLanguage());
 			}
 
@@ -949,7 +951,7 @@ class LoginWebPage extends NiceWebPage
 	 * Overridable: depending on the user, head toward a dedicated portal
 	 * @param string|null $sRequestedPortalId
 	 * @param int $iOnExit How to complete the call: redirect or return a code
-	 */	 
+	 */
 	protected static function ChangeLocation($sRequestedPortalId = null, $iOnExit = self::EXIT_PROMPT)
 	{
 		$ret = call_user_func(array(self::$sHandlerClass, 'Dispatch'), $sRequestedPortalId);
@@ -1010,9 +1012,9 @@ class LoginWebPage extends NiceWebPage
 	static function DoLoginEx($sRequestedPortalId = null, $bMustBeAdmin = false, $iOnExit = self::EXIT_PROMPT)
 	{
 		$operation = utils::ReadParam('loginop', '');
-	
+
 		$sMessage = self::HandleOperations($operation); // May exit directly
-	
+
 		$iRet = self::Login($iOnExit);
 		if ($iRet == self::EXIT_CODE_OK)
 		{
@@ -1042,7 +1044,7 @@ class LoginWebPage extends NiceWebPage
 		{
 			return $sMessage;
 		}
-	}	
+	}
 	protected static function HandleOperations($operation)
 	{
 		$sMessage = ''; // most of the operations never return, but some can return a message to be displayed
@@ -1156,11 +1158,11 @@ class LoginWebPage extends NiceWebPage
 		}
 		return $sMessage;
 	}
-	
+
 	protected static function Dispatch($sRequestedPortalId)
 	{
 		if ($sRequestedPortalId === null) return true; // allowed to any portal => return true
-		
+
 		$aPortalsConf = PortalDispatcherData::GetData();
 		$aDispatchers = array();
 		foreach($aPortalsConf as $sPortalId => $aConf)
@@ -1168,7 +1170,7 @@ class LoginWebPage extends NiceWebPage
 			$sHandlerClass = $aConf['handler'];
 			$aDispatchers[$sPortalId] = new $sHandlerClass($sPortalId);
 		}
-		
+
 		if (array_key_exists($sRequestedPortalId, $aDispatchers) && $aDispatchers[$sRequestedPortalId]->IsUserAllowed())
 		{
 			return true;

core/metamodel.class.php

@@ -1241,7 +1241,7 @@ abstract class MetaModel
 			}
 			$sTable = self::DBGetTable($sClass);
 
-			// Could be completed later with all the classes that are using a given table 
+			// Could be completed later with all the classes that are using a given table
 			if (!array_key_exists($sTable, $aTables)) {
 				$aTables[$sTable] = array();
 			}
@@ -3522,7 +3522,7 @@ abstract class MetaModel
 		}
 
 		// Set the "host class" as soon as possible, since HierarchicalKeys use it for their 'target class' as well
-		// and this needs to be know early (for Init_IsKnowClass 19 lines below)		
+		// and this needs to be know early (for Init_IsKnowClass 19 lines below)
 		$oAtt->SetHostClass($sTargetClass);
 
 		// Some attributes could refer to a class
@@ -3564,7 +3564,7 @@ abstract class MetaModel
 
 		self::$m_aAttribDefs[$sTargetClass][$oAtt->GetCode()] = $oAtt;
 		self::$m_aAttribOrigins[$sTargetClass][$oAtt->GetCode()] = $sTargetClass;
-		// Note: it looks redundant to put targetclass there, but a mix occurs when inheritance is used		
+		// Note: it looks redundant to put targetclass there, but a mix occurs when inheritance is used
 	}
 
 	/**
@@ -3764,7 +3764,7 @@ abstract class MetaModel
 		self::$m_aStimuli[$sTargetClass][$oStimulus->GetCode()] = $oStimulus;
 
 		// I wanted to simplify the syntax of the declaration of objects in the biz model
-		// Therefore, the reference to the host class is set there 
+		// Therefore, the reference to the host class is set there
 		$oStimulus->SetHostClass($sTargetClass);
 	}
 
@@ -4219,40 +4219,77 @@ abstract class MetaModel
 		}
 		else
 		{
-			$aCurrentUser = array();
-			$aCurrentContact = array();
+			$aCurrentUser = [];
+			$aCurrentContact = [];
 			foreach ($aExpectedArgs as $expression)
 			{
 				$aName = explode('->', $expression->GetName());
 				if ($aName[0] == 'current_contact_id') {
 					$aPlaceholders['current_contact_id'] = UserRights::GetContactId();
-				}
-				if ($aName[0] == 'current_user') {
+				} else if ($aName[0] == 'current_user') {
 					array_push($aCurrentUser, $aName[1]);
-				}
-				if ($aName[0] == 'current_contact') {
+				} else if ($aName[0] == 'current_contact') {
 					array_push($aCurrentContact, $aName[1]);
 				}
 			}
 			if (count($aCurrentUser) > 0) {
-				$oUser = UserRights::GetUserObject();
-				$aPlaceholders['current_user->object()'] = $oUser;
-				foreach ($aCurrentUser as $sField) {
-					$aPlaceholders['current_user->'.$sField] = $oUser->Get($sField);
-				}
+				static::FillObjectPlaceholders($aPlaceholders, 'current_user', UserRights::GetUserObject(), $aCurrentUser);
 			}
 			if (count($aCurrentContact) > 0) {
-				$oPerson = UserRights::GetContactObject();
-				$aPlaceholders['current_contact->object()'] = $oPerson;
-				foreach ($aCurrentContact as $sField) {
-					$aPlaceholders['current_contact->'.$sField] = $oPerson->Get($sField);
-				}
+				static::FillObjectPlaceholders($aPlaceholders, 'current_contact', UserRights::GetContactObject(), $aCurrentContact);
 			}
 		}
 
 		return $aPlaceholders;
 	}
 
+	/**
+	 * @since 3.1.1 N°6824
+	 * @param array $aPlaceholders
+	 * @param string $sPlaceHolderPrefix
+	 * @param ?\DBObject $oObject
+	 * @param array $aCurrentUser
+	 *
+	 * @return void
+	 *
+	 */
+	private static function FillObjectPlaceholders(array &$aPlaceholders, string $sPlaceHolderPrefix, ?\DBObject $oObject, array $aCurrentUser) : void {
+		$sPlaceHolderKey = $sPlaceHolderPrefix."->object()";
+		if (is_null($oObject)){
+			$aContext = [
+				"current_user_id" => UserRights::GetUserId(),
+				"null object type" => $sPlaceHolderPrefix,
+				"fields" => $aCurrentUser,
+			];
+			IssueLog::Warning("Unresolved placeholders due to null object in current context", null,
+				$aContext);
+			$aPlaceholders[$sPlaceHolderKey] = \Dict::Format("Core:Placeholder:CannotBeResolved", $sPlaceHolderKey);
+			foreach ($aCurrentUser as $sField) {
+				$sPlaceHolderKey = $sPlaceHolderPrefix . "->$sField";
+				$aPlaceholders[$sPlaceHolderKey] = \Dict::Format("Core:Placeholder:CannotBeResolved", $sPlaceHolderKey);
+			}
+		} else {
+			$aPlaceholders[$sPlaceHolderKey] = $oObject;
+			foreach ($aCurrentUser as $sField) {
+				$sPlaceHolderKey = $sPlaceHolderPrefix . "->$sField";
+				if (false === MetaModel::IsValidAttCode(get_class($oObject), $sField)){
+					$aContext = [
+						"current_user_id" => UserRights::GetUserId(),
+						"obj_class" => get_class($oObject),
+						"placeholder" => $sPlaceHolderKey,
+						"invalid_field" => $sField,
+					];
+					IssueLog::Warning("Unresolved placeholder due to invalid attribute", null,
+						$aContext);
+					$aPlaceholders[$sPlaceHolderKey] = \Dict::Format("Core:Placeholder:CannotBeResolved", $sPlaceHolderKey);
+					continue;
+				}
+
+				$aPlaceholders[$sPlaceHolderKey] = $oObject->Get($sField);
+			}
+		}
+	}
+
 	/**
 	 * @param \DBSearch $oFilter
 	 *
@@ -6479,7 +6516,7 @@ abstract class MetaModel
 				$aCache['m_aExtensionClassNames'] = self::$m_aExtensionClassNames;
 				$aCache['m_Category2Class'] = self::$m_Category2Class;
 				$aCache['m_aRootClasses'] = self::$m_aRootClasses; // array of "classname" => "rootclass"
-				$aCache['m_aParentClasses'] = self::$m_aParentClasses; // array of ("classname" => array of "parentclass") 
+				$aCache['m_aParentClasses'] = self::$m_aParentClasses; // array of ("classname" => array of "parentclass")
 				$aCache['m_aChildClasses'] = self::$m_aChildClasses; // array of ("classname" => array of "childclass")
 				$aCache['m_aClassParams'] = self::$m_aClassParams; // array of ("classname" => array of class information)
 				$aCache['m_aAttribDefs'] = self::$m_aAttribDefs; // array of ("classname" => array of attributes)

dictionaries/en.dictionary.itop.core.php

@@ -49,6 +49,7 @@ Dict::Add('EN US', 'English', 'English', array(
 	'Core:AttributeTagSet' => 'List of tags',
 	'Core:AttributeTagSet+' => '',
 	'Core:AttributeSet:placeholder' => 'click to add',
+	'Core:Placeholder:CannotBeResolved' => '(%1$s : cannot be resolved)',
 	'Core:AttributeClassAttCodeSet:ItemLabel:AttributeFromClass' => '%1$s (%2$s)',
 	'Core:AttributeClassAttCodeSet:ItemLabel:AttributeFromOneChildClass' => '%1$s (%2$s from %3$s)',
 	'Core:AttributeClassAttCodeSet:ItemLabel:AttributeFromSeveralChildClasses' => '%1$s (%2$s from child classes)',

dictionaries/fr.dictionary.itop.core.php

@@ -39,6 +39,7 @@ Dict::Add('FR FR', 'French', 'Français', array(
 	'Core:AttributeTagSet' => 'Liste d\'étiquettes',
 	'Core:AttributeTagSet+' => '',
 	'Core:AttributeSet:placeholder' => 'cliquer pour ajouter',
+	'Core:Placeholder:CannotBeResolved' => '(%1$s : non remplacé)',
 	'Core:AttributeClassAttCodeSet:ItemLabel:AttributeFromClass' => '%1$s (%2$s)',
 	'Core:AttributeClassAttCodeSet:ItemLabel:AttributeFromOneChildClass' => '%1$s (%2$s de la classe %3$s)',
 	'Core:AttributeClassAttCodeSet:ItemLabel:AttributeFromSeveralChildClasses' => '%1$s (%2$s d\'une sous-classe)',

tests/php-unit-tests/unitary-tests/application/LoginWebPageTest.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace Combodo\iTop\Test\UnitTest\Application;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+
+/**
+ * @covers LoginWebPage
+ */
+class LoginWebPageTest extends ItopDataTestCase
+{
+	public function testProvisionUserOnly(){
+		$iNum = uniqid();
+		$sEmail = "ProvisionUser".$iNum . "@gabuzomeu.shadok";
+		$oUser = \LoginWebPage::ProvisionUser($sEmail, null, [ 'Portal User']);
+
+		$this->assertNotNull($oUser);
+
+		$oUser = \MetaModel::GetObject(\UserExternal::class, $oUser->GetKey());
+		$this->assertNotNull($oUser);
+		$this->assertEquals($sEmail, $oUser->Get('login'));
+		$this->assertEquals(\MetaModel::GetConfig()->GetDefaultLanguage(), $oUser->Get('language'));
+		$this->assertEquals(0, $oUser->Get('contactid'));
+	}
+
+	public function testProvisionUserWithPerson(){
+		$iNum = uniqid();
+		$this->CreateTestOrganization();
+		$oPerson = $this->CreatePerson($iNum);
+
+		$sEmail = "ProvisionUser".$iNum . "@gabuzomeu.shadok";
+		$oUser = \LoginWebPage::ProvisionUser($sEmail, $oPerson, [ 'Portal User']);
+
+		$this->assertNotNull($oUser);
+
+		$oUser = \MetaModel::GetObject(\UserExternal::class, $oUser->GetKey());
+		$this->assertNotNull($oUser);
+		$this->assertEquals($sEmail, $oUser->Get('login'));
+		$this->assertEquals(\MetaModel::GetConfig()->GetDefaultLanguage(), $oUser->Get('language'));
+		$this->assertEquals($oPerson->GetKey(), $oUser->Get('contactid'));
+	}
+}

tests/php-unit-tests/unitary-tests/core/MetaModelMagicPlaceholderTest.php

@@ -0,0 +1,206 @@
+<?php
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use MetaModel;
+use PHPUnit\Framework\ExpectationFailedException;
+use UserRights;
+use VariableExpression;
+
+/**
+ * Class MetaModelMagicPlaceholderTest
+ * @since 3.1.1 N°6824
+ * @covers MetaModel::AddMagicPlaceholders()
+ * @package Combodo\iTop\Test\UnitTest\Core
+ */
+class MetaModelMagicPlaceholderTest extends ItopDataTestCase
+{
+	/**
+	 * Asserts that two array with DBObjects are equal (the important is to check the {class,id} couple
+	 *
+	 * @throws \SebastianBergmann\RecursionContext\InvalidArgumentException
+	 * @throws ExpectationFailedException
+	 */
+	public static function assertEqualsShallow($expected, $actual, string $message = ''): void
+	{
+		if (is_array($expected)) {
+			foreach ($expected as $key => $value) {
+				if ($value instanceof \DBObject) {
+					$expected[$key] = get_class($value).'::'.$value->GetKey();
+				}
+			}
+			foreach ($actual as $key => $value) {
+				if ($value instanceof \DBObject) {
+					$actual[$key] = get_class($value).'::'.$value->GetKey();
+				}
+			}
+		}
+		parent::assertEquals($expected, $actual, $message);
+	}
+
+	public function testAddMagicPlaceholdersWhenLoggedInUserHasAContact()
+	{
+		// Create data fixture => User + Person
+		$iNum = uniqid();
+		$sLogin = "AddMagicPlaceholders".$iNum;
+		$this->CreateTestOrganization();
+		$oPerson = $this->CreatePerson($iNum);
+		$sContactId = $oPerson->GetKey();
+		$oUser = $this->CreateUser($sLogin, 1, "Abcdef@12345678", $oPerson->GetKey());
+		UserRights::Login($sLogin);
+
+		// Test legacy behavior (no expected args)
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"]);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+				'current_contact_id' => $sContactId,
+				'current_user->object()' => $oUser,
+				'current_contact->object()' => $oPerson,
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders without second parameter (legacy) should add "curent_contact_id/current_user->object()/current_contact->object()"'
+		);
+
+		// With expected arguments explicitly given as "none"
+		$aExpectedArgs = [];
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"], $aExpectedArgs);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders should add only expected arguments'
+		);
+
+		// Test new behavior (with expected args)
+		$aExpectedArgs = [
+			new VariableExpression('current_user->login'),
+			new VariableExpression('current_user->not_existing_attribute'),
+			new VariableExpression('current_contact_id'),
+			new VariableExpression('current_contact->org_id'),
+			new VariableExpression('current_contact->not_existing_attribute'),
+		];
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"], $aExpectedArgs);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+				'current_contact_id' => $sContactId,
+				'current_user->object()' => $oUser,
+				'current_user->not_existing_attribute' => '(current_user->not_existing_attribute : cannot be resolved)',
+				'current_user->login' => $sLogin,
+				'current_contact->object()' => $oPerson,
+				'current_contact->org_id' => $oPerson->Get('org_id'),
+				'current_contact->not_existing_attribute' => '(current_contact->not_existing_attribute : cannot be resolved)',
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders should add expected arguments and render them with an explicit error when the information could not be known'
+		);
+	}
+
+	public function testAddMagicPlaceholdersWhenLoggedInUserHasNoContact()
+	{
+		// Create data fixture => User without contact
+		$iNum = uniqid();
+		$sLogin = "AddMagicPlaceholders".$iNum;
+		$oUser = $this->CreateContactlessUser($sLogin, 1, "Abcdef@12345678");
+		UserRights::Login($sLogin);
+
+		// Test legacy behavior (no expected args)
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"]);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+				'current_contact_id' => 0,
+				'current_user->object()' => $oUser,
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders without second parameter (legacy) should add "current_contact_id=0/current_user->object()"'
+		);
+
+		// Test with expected arguments explicitly given as "none"
+		$aExpectedArgs = [];
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"], $aExpectedArgs);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders should add only expected arguments'
+		);
+
+		// Test with a few expected arguments, some of which being invalid attributes
+		$aExpectedArgs = [
+			new VariableExpression('current_user->login'),
+			new VariableExpression('current_user->not_existing_attribute'),
+			new VariableExpression('current_contact_id'),
+			new VariableExpression('current_contact->org_id'),
+			new VariableExpression('current_contact->not_existing_attribute'),
+		];
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"], $aExpectedArgs);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+				'current_contact_id' => 0,
+				'current_user->object()' => $oUser,
+				'current_user->not_existing_attribute' => '(current_user->not_existing_attribute : cannot be resolved)',
+				'current_user->login' => $sLogin,
+				'current_contact->object()' => '(current_contact->object() : cannot be resolved)',
+				'current_contact->org_id' => '(current_contact->org_id : cannot be resolved)',
+				'current_contact->not_existing_attribute' => '(current_contact->not_existing_attribute : cannot be resolved)',
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders should add expected arguments and render them with an explicit error when the information could not be known'
+		);
+	}
+
+	public function testAddMagicPlaceholdersWhenThereIsNoLoggedInUser()
+	{
+		// Test legacy behavior (no expected args)
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"]);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+				'current_contact_id' => '',
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders without second parameter (legacy) should add "curent_contact_id"'
+		);
+
+		// Test with expected arguments explicitly given as "none"
+		$aExpectedArgs = [];
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"], $aExpectedArgs);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders should add only expected arguments'
+		);
+
+		// Test with a few expected arguments, some of which being invalid attributes
+		$aExpectedArgs = [
+			new VariableExpression('current_user->login'),
+			new VariableExpression('current_user->not_existing_attribute'),
+			new VariableExpression('current_contact_id'),
+			new VariableExpression('current_contact->org_id'),
+			new VariableExpression('current_contact->not_existing_attribute'),
+		];
+		$aPlaceholders = MetaModel::AddMagicPlaceholders(['gabu' => "zomeu"], $aExpectedArgs);
+		$this->assertEqualsShallow(
+			[
+				'gabu' => 'zomeu',
+			    'current_contact_id' => '',
+				'current_user->object()' => '(current_user->object() : cannot be resolved)',
+			    'current_user->not_existing_attribute' => '(current_user->not_existing_attribute : cannot be resolved)',
+				'current_user->login' => '(current_user->login : cannot be resolved)',
+			    'current_contact->object()' => '(current_contact->object() : cannot be resolved)',
+				'current_contact->org_id' => '(current_contact->org_id : cannot be resolved)',
+			    'current_contact->not_existing_attribute' => '(current_contact->not_existing_attribute : cannot be resolved)',
+			],
+			$aPlaceholders,
+			'AddMagicPlaceholders should add expected arguments and render them with an explicit error when the information could not be known'
+		);
+	}
+}