WIP

* N°8632 - Check existence of parameter file within iTop

* N°8632 - block parameter file from request

* log on error

* PHP CS fixer

* N°8632 - param files must be outside iTop

* PHP CS fixer

* Update webservices/export.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update tests/php-unit-tests/unitary-tests/application/utilsTest.php

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* ✅ Fix CI

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

.gitignore

@@ -58,6 +58,9 @@ tests/*/vendor/*
 /tests/php-unit-tests/phpunit.xml
 /tests/php-unit-tests/postbuild_integration.xml
 
+# PHP CS Fixer: Cache file
+/.php-cs-fixer.cache
+
 
 # Jetbrains
 /.idea/**

application/utils.inc.php

@@ -181,6 +181,9 @@ class utils
 
 	protected static function LoadParamFile($sParamFile)
 	{
+		if (utils::RealPath($sParamFile, APPROOT) !== false) {
+			throw new Exception("File '".utils::HtmlEntities($sParamFile)."' should be outside iTop");
+		}
 		if (!file_exists($sParamFile)) {
 			throw new Exception("Could not find the parameter file: '".utils::HtmlEntities($sParamFile)."'");
 		}

core/config.class.inc.php

@@ -1738,6 +1738,14 @@ class Config
 			'source_of_value'     => '',
 			'show_in_conf_sample' => false,
 		],
+		'security.force_login_when_no_execution_policy' => [
+			'type'                => 'bool',
+			'description'         => 'If true, when no execution policy is defined, the user will be forced to log in (instead of being automatically logged in with the default profile)',
+			'default'             => false,
+			'value'               => false,
+			'source_of_value'     => '',
+			'show_in_conf_sample' => true,
+		],
 		'behind_reverse_proxy' => [
 			'type' => 'bool',
 			'description' => 'If true, then proxies custom header (X-Forwarded-*) are taken into account. Use only if the webserver is not publicly accessible (reachable only by the reverse proxy)',

datamodels/2.x/itop-backup/check-backup.php

@@ -199,15 +199,15 @@ function RaiseAlarm($sMessage)
 //////////
 // Main
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	echo "Error: ".$e->GetMessage()."\n";
-	exit;
-}
-
 if (utils::IsModeCLI()) {
-	SetupUtils::CheckPhpAndExtensionsForCli(new CLIPage('Check backup utility'));
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		echo 'Error: '.$e->GetMessage()."\n";
+		exit;
+	}
+	$oP = new CLIPage('Check backup utility');
+	SetupUtils::CheckPhpAndExtensionsForCli($oP);
 
 	echo date('Y-m-d H:i:s')." - running check-backup utility\n";
 	try {

datamodels/2.x/itop-backup/common.cli-execution.php

@@ -88,16 +88,15 @@ if (utils::IsModeCLI()) {
 	$oP = new CLIPage(GetOperationName());
 
 	SetupUtils::CheckPhpAndExtensionsForCli($oP);
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		ExitError($oP, $e->GetMessage());
+	}
 } else {
 	$oP = new WebPage(GetOperationName());
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	ExitError($oP, $e->GetMessage());
-}
-
 ExecuteMainOperation($oP);
 
 $oP->output();

pages/exec.php

@@ -97,4 +97,29 @@ if ($sTargetPage === false) {
 //
 // GO!
 //
+// check module white list
+// check conf param
+// force login if needed
+require_once(APPROOT.'/application/startup.inc.php');
+
+$aModuleDelegatedExecutionPolicy = GetModuleDelegatedExecutionPolicy($sModule);
+if (is_null($aModuleDelegatedExecutionPolicy) || !in_array($sPage, $aModuleDelegatedExecutionPolicy)) {
+	$bForceLoginWhenNoExecutionPolicy = MetaModel::GetConfig()->Get('security.force_login_when_no_execution_policy');
+	// TODO in N°9343 : remove the conf and this 'if' condition to perform login by default when no execution policy is defined
+	LoginWebPage::DoLoginEx();
+}
+if (is_array($aModuleDelegatedExecutionPolicy) && !in_array($sPage, $aModuleDelegatedExecutionPolicy)) {
+	// if module defined a delegated execution policy but not for the current page, we consider that the page is not allowed to be executed without login
+	LoginWebPage::DoLoginEx();
+}
+
 require_once($sTargetPage);
+
+function GetModuleDelegatedExecutionPolicy(string $sModuleName): ?array
+{
+	$sModuleFile =  APPROOT.'/env-'.utils::GetCurrentEnvironment().'/'.$sModuleName.'/module.'.$sModuleName.'.php';
+
+	$oExtensionMap = new iTopExtensionsMap();
+	$aModuleParam = $oExtensionMap->GetModuleInfo($sModuleFile)[2];
+	return $aModuleParam['execution_policy'] ?? null;
+}

setup/extensionsmap.class.inc.php

@@ -390,7 +390,7 @@ class iTopExtensionsMap
 	 * @param string $sModuleFile
 	 * @return array
 	 */
-	protected function GetModuleInfo($sModuleFile)
+	public function GetModuleInfo($sModuleFile)
 	{
 		static $iDummyClassIndex = 0;
 

synchro/synchro_exec.php

@@ -67,18 +67,18 @@ function ReadMandatoryParam($oP, $sParam, $sSanitizationFilter = 'parameter')
 if (utils::IsModeCLI()) {
 	$oP = new CLIPage(Dict::S("TitleSynchroExecution"));
 	SetupUtils::CheckPhpAndExtensionsForCli($oP, -2);
+
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		$oP->p('Error: '.$e->GetMessage());
+		$oP->output();
+		exit - 2;
+	}
 } else {
 	$oP = new WebPage(Dict::S("TitleSynchroExecution"));
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	$oP->p("Error: ".$e->GetMessage());
-	$oP->output();
-	exit -2;
-}
-
 if (utils::IsModeCLI()) {
 	$sAuthUser = ReadMandatoryParam($oP, 'auth_user', 'raw_data');
 	$sAuthPwd = ReadMandatoryParam($oP, 'auth_pwd', 'raw_data');

synchro/synchro_import.php

@@ -224,18 +224,18 @@ function ChangeDateFormat($sProposedDate, $sFormat, $bDateOnly)
 if (utils::IsModeCLI()) {
 	$oP = new CLIPage(Dict::S('TitleSynchroExecution'));
 	SetupUtils::CheckPhpAndExtensionsForCli($oP, -2);
+
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		$oP->p('Error: '.$e->GetMessage());
+		$oP->output();
+		exit - 2;
+	}
 } else {
 	$oP = new CLILikeWebPage(Dict::S('TitleSynchroExecution'));
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	$oP->p("Error: ".$e->GetMessage());
-	$oP->output();
-	exit -2;
-}
-
 if (utils::IsModeCLI()) {
 	// Next steps:
 	//   specific arguments: 'csvfile'

tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php

@@ -0,0 +1,104 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Combodo\iTop\Test\UnitTest\Application;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use Exception;
+use MetaModel;
+
+class LoginWebPageTest extends ItopDataTestCase
+{
+	public const USE_TRANSACTION = false;
+
+	public const PASSWORD = 'a209320P!ù;ralùqpi,pàcqi"nr';
+
+	public function setUp(): void
+	{
+		parent::setUp();
+		$this->BackupConfiguration();
+		$sFolderPath = APPROOT.'env-production/extension-with-execution-policy';
+		if (file_exists($sFolderPath)) {
+			throw new Exception("Folder $sFolderPath already exists, please remove it before running the test");
+		}
+		mkdir($sFolderPath);
+		$this->RecurseCopy(__DIR__.'/extension-with-execution-policy', $sFolderPath);
+
+		$sFolderPath = APPROOT.'env-production/extension-without-execution-policy';
+		if (file_exists($sFolderPath)) {
+			throw new Exception("Folder $sFolderPath already exists, please remove it before running the test");
+		}
+		mkdir($sFolderPath);
+		$this->RecurseCopy(__DIR__.'/extension-without-execution-policy', $sFolderPath);
+	}
+	public function tearDown(): void
+	{
+		parent::tearDown();
+		$sFolderPath = APPROOT.'env-production/extension-with-execution-policy';
+		if (file_exists($sFolderPath)) {
+			$this->RecurseRmdir($sFolderPath);
+		} else {
+			throw new Exception("Folder $sFolderPath does not exist, it should have been created in setUp");
+		}
+		$sFolderPath = APPROOT.'env-production/extension-without-execution-policy';
+		if (file_exists($sFolderPath)) {
+			$this->RecurseRmdir($sFolderPath);
+		} else {
+			throw new Exception("Folder $sFolderPath does not exist, it should have been created in setUp");
+		}
+	}
+
+	protected function GivenConfigFileAllowedLoginTypes($aAllowedLoginTypes): void
+	{
+		@chmod(MetaModel::GetConfig()->GetLoadedFile(), 0770);
+		MetaModel::GetConfig()->SetAllowedLoginTypes($aAllowedLoginTypes);
+		MetaModel::GetConfig()->WriteToFile();
+		@chmod(MetaModel::GetConfig()->GetLoadedFile(), 0444);
+	}
+
+	/**
+	 *
+	 * @throws \Exception
+	 */
+	public function testInExecutionPolicyFile()
+	{
+		// generate random login
+		$sUserLogin = 'user-'.date('YmdHis');
+		$this->CreateUser($sUserLogin, self::$aURP_Profiles['Administrator'], self::PASSWORD);
+		$this->GivenConfigFileAllowedLoginTypes(explode('|', 'form'));
+
+		$sPageContent = $this->CallItopUri(
+			"pages/exec.php?exec_module=extension-with-execution-policy&exec_page=src/Controller/CheckAnything.php",
+			[
+				'auth_user' => $sUserLogin,
+				'auth_pwd' => self::PASSWORD,
+			],
+			[],
+			true
+		);
+
+		$this->assertStringNotContainsString('<title>iTop login</title>', $sPageContent); // in execution policy file (in the module), login should not be proposed, file handle its own policy
+	}
+
+	public function testNotInExecutionPolicyFileWithForceLoginConf()
+	{
+		MetaModel::GetConfig()->Set('security.force_login_when_no_execution_policy', true);
+
+		$sPageContent = $this->CallItopUri(
+			"pages/exec.php?exec_module=extension-with-execution-policy&exec_page=src/Controller/AnotherFile.php",
+		);
+
+		$this->assertStringContainsString('<title>iTop login</title>', $sPageContent); // if itop is configured to force login when no execution policy, then login should be proposed since file is not in execution policy file
+	}
+
+	public function testNotInExecutionPolicyFileWithoutForceLoginConf()
+	{
+		$sPageContent = $this->CallItopUri(
+			"pages/exec.php?exec_module=extension-with-execution-policy&exec_page=src/Controller/AnotherFile.php",
+		);
+
+		$this->assertStringNotContainsString('<title>iTop login</title>', $sPageContent); // by default (until N°9343) if no execution policy is defined, login is not forced
+	}
+
+}

tests/php-unit-tests/integration-tests/login-tests/extension-with-execution-policy/module.extension-with-execution-policy.php

@@ -0,0 +1,50 @@
+<?php
+
+SetupWebPage::AddModule(
+	__FILE__, // Path to the current file, all other file names are relative to the directory containing this file
+	'extension-with-execution-policy/0.0.1',
+	[
+		// Identification
+		//
+		'label'                => 'Templates foundation',
+		'category'             => 'business',
+
+		// Setup
+		//
+		'dependencies'         => [],
+		'mandatory'            => true,
+		'visible'              => false,
+		'installer'            => 'TemplatesBaseInstaller',
+
+		// Security
+		'execution_policy' => [
+			'src/Controller/CheckAnything.php',
+		],
+
+		// Components
+		//
+		'datamodel'            => [
+			'model.templates-base.php',
+		],
+		'webservice'           => [],
+		'data.struct'          => [// add your 'structure' definition XML files here,
+		],
+		'data.sample'          => [// add your sample data XML files here,
+		],
+
+		// Documentation
+		//
+		'doc.manual_setup'     => '', // hyperlink to manual setup documentation, if any
+		'doc.more_information' => '', // hyperlink to more information, if any
+
+		// Default settings
+		//
+		'settings'             => [
+			// Select where, in the main UI, the extra data should be displayed:
+			//     tab (dedicated tab)
+			//     properties (right after the properties, but before the log if any)
+			//     none (extra data accessed only by programs)
+			'view_extra_data' => 'relations',
+		],
+	]
+);

tests/php-unit-tests/integration-tests/login-tests/extension-with-execution-policy/src/Controller/AnotherFile.php

@@ -0,0 +1,3 @@
+<?php
+
+echo 'Yo !';

tests/php-unit-tests/integration-tests/login-tests/extension-with-execution-policy/src/Controller/CheckAnything.php

@@ -0,0 +1,3 @@
+<?php
+
+echo 'Yo !';

tests/php-unit-tests/integration-tests/login-tests/extension-without-execution-policy/module.extension-without-execution-policy.php

@@ -0,0 +1,45 @@
+<?php
+
+SetupWebPage::AddModule(
+	__FILE__, // Path to the current file, all other file names are relative to the directory containing this file
+	'extension-without-execution-policy/0.0.1',
+	[
+		// Identification
+		//
+		'label'                => 'Templates foundation',
+		'category'             => 'business',
+
+		// Setup
+		//
+		'dependencies'         => [],
+		'mandatory'            => true,
+		'visible'              => false,
+		'installer'            => 'TemplatesBaseInstaller',
+
+		// Components
+		//
+		'datamodel'            => [
+			'model.templates-base.php',
+		],
+		'webservice'           => [],
+		'data.struct'          => [// add your 'structure' definition XML files here,
+		],
+		'data.sample'          => [// add your sample data XML files here,
+		],
+
+		// Documentation
+		//
+		'doc.manual_setup'     => '', // hyperlink to manual setup documentation, if any
+		'doc.more_information' => '', // hyperlink to more information, if any
+
+		// Default settings
+		//
+		'settings'             => [
+			// Select where, in the main UI, the extra data should be displayed:
+			//     tab (dedicated tab)
+			//     properties (right after the properties, but before the log if any)
+			//     none (extra data accessed only by programs)
+			'view_extra_data' => 'relations',
+		],
+	]
+);

tests/php-unit-tests/integration-tests/login-tests/extension-without-execution-policy/src/Controller/AnotherFile.php

@@ -0,0 +1,3 @@
+<?php
+
+echo 'Yo !';

tests/php-unit-tests/integration-tests/login-tests/extension-without-execution-policy/src/Controller/CheckAnything.php

@@ -0,0 +1,3 @@
+<?php
+
+echo 'Yo !';

tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php

@@ -123,7 +123,7 @@ abstract class ItopDataTestCase extends ItopTestCase
 	{
 		parent::setUp();
 
-		\IssueLog::Error($this->getName());
+		\IssueLog::Info("Running phpunit test: ".$this->getName());
 
 		$this->PrepareEnvironment();
 
@@ -1446,9 +1446,6 @@ abstract class ItopDataTestCase extends ItopTestCase
 	{
 		$sConfigPath = MetaModel::GetConfig()->GetLoadedFile();
 		clearstatcache();
-		echo sprintf("rights via ls on %s:\n %s \n", $sConfigPath, exec("ls -al $sConfigPath"));
-		$sFilePermOutput = substr(sprintf('%o', fileperms('/etc/passwd')), -4);
-		echo sprintf("rights via fileperms on %s:\n %s \n", $sConfigPath, $sFilePermOutput);
 
 		$this->sConfigTmpBackupFile = tempnam(sys_get_temp_dir(), "config_");
 		MetaModel::GetConfig()->WriteToFile($this->sConfigTmpBackupFile);

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

@@ -996,4 +996,46 @@ HTML,
 			],
 		];
 	}
+
+	public function testLoadParamFile()
+	{
+		$sTmpFileInsideItop = APPROOT.'data/test/testLoadParamFile.params';
+		$sDir = dirname($sTmpFileInsideItop);
+		if (!is_dir($sDir)) {
+			mkdir($sDir, 0777, true);
+		}
+		$sParamName = 'IP1';
+		$sParamValue = 'IV1';
+		$sParams = <<<INI
+# comment
+$sParamName = $sParamValue
+INI;
+		file_put_contents($sTmpFileInsideItop, $sParams);
+
+		try {
+			$this->expectException(\Exception::class);
+			$this->expectExceptionMessage("File '$sTmpFileInsideItop' should be outside iTop");
+			self::InvokeNonPublicStaticMethod(utils::class, 'LoadParamFile', [$sTmpFileInsideItop]);
+			self::assertNotEquals($sParamValue, utils::ReadParam($sParamName, null), "utils::LoadParamFile() should NOT have loaded the file: $sTmpFileInsideItop");
+		} finally {
+			if (file_exists($sTmpFileInsideItop)) {
+				unlink($sTmpFileInsideItop);
+			}
+		}
+
+		$sParamName = 'OP2';
+		$sParamValue = 'OV2';
+
+		$sTmpFileOutsideItop = tempnam(sys_get_temp_dir(), 'utils-test');
+		$sParams = <<<INI
+# comment
+$sParamName = $sParamValue
+INI;
+
+		file_put_contents($sTmpFileOutsideItop, $sParams);
+		self::InvokeNonPublicStaticMethod(utils::class, 'LoadParamFile', [$sTmpFileOutsideItop]);
+		self::assertEquals($sParamValue, utils::ReadParam($sParamName, null), "utils::LoadParamFile() should have loaded the file: $sTmpFileOutsideItop");
+
+		unlink($sTmpFileOutsideItop);
+	}
 }

webservices/cron.php

@@ -446,18 +446,16 @@ function ReSyncProcesses($oP, $bVerbose, $bDebug)
 //
 
 set_time_limit(0); // Some background actions may really take long to finish (like backup)
-
-$bIsModeCLI = utils::IsModeCLI();
-if ($bIsModeCLI) {
-	$oP = new CLIPage("iTop - cron");
-
-	SetupUtils::CheckPhpAndExtensionsForCli($oP, EXIT_CODE_FATAL);
-} else {
-	$oP = new WebPage("iTop - cron");
-}
-
 try {
-	utils::UseParamFile();
+	$bIsModeCLI = utils::IsModeCLI();
+	if ($bIsModeCLI) {
+		$oP = new CLIPage("iTop - cron");
+
+		SetupUtils::CheckPhpAndExtensionsForCli($oP, EXIT_CODE_FATAL);
+		utils::UseParamFile();
+	} else {
+		$oP = new WebPage("iTop - cron");
+	}
 
 	$bVerbose = utils::ReadParam('verbose', false, true /* Allow CLI */);
 	$bDebug = utils::ReadParam('debug', false, true /* Allow CLI */);

webservices/export.php

@@ -43,10 +43,12 @@ const EXIT_CODE_ERROR = -1;
 const EXIT_CODE_FATAL = -2;
 
 try {
-	// Do this before loging, in order to allow setting user credentials from within the file
-	utils::UseParamFile();
+	if (utils::IsModeCLI()) {
+		// Do this before logging, in order to allow setting user credentials from within the file
+		utils::UseParamFile();
+	}
 } catch (Exception $e) {
-	echo "Error: ".$e->GetMessage()."<br/>\n";
+	echo "Error: ".$e->GetMessage()."\n";
 	exit(EXIT_CODE_FATAL);
 }
 

webservices/import.php

@@ -208,18 +208,18 @@ $oCtx = new ContextTag(ContextTag::TAG_IMPORT);
 if (utils::IsModeCLI()) {
 	$oP = new CLIPage("iTop - Bulk import");
 	SetupUtils::CheckPhpAndExtensionsForCli($oP, -2);
+
+	try {
+		utils::UseParamFile();
+	} catch (Exception $e) {
+		$oP->p('Error: '.$e->GetMessage());
+		$oP->output();
+		exit(-2);
+	}
 } else {
 	$oP = new CSVPage("iTop - Bulk import");
 }
 
-try {
-	utils::UseParamFile();
-} catch (Exception $e) {
-	$oP->p("Error: ".$e->GetMessage());
-	$oP->output();
-	exit(-2);
-}
-
 if (utils::IsModeCLI()) {
 	// Next steps:
 	//   specific arguments: 'csvfile'

webservices/rest.php

@@ -91,10 +91,8 @@ if (empty($sJsonString)) {
 
 $sProvider = '';
 
-$oKPI = new ExecutionKPI();
 try {
-	utils::UseParamFile();
-
+	$oKPI = new ExecutionKPI();
 	$oKPI->ComputeAndReport('Data model loaded');
 
 	// N°6358 - force credentials for REST calls