diff --git a/core/config.class.inc.php b/core/config.class.inc.php index 3f4d5b22c..fe95fd521 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -1756,11 +1756,11 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ], - 'security.force_login_when_no_delegated_authentication_endpoints_list' => [ + 'security.disable_exec_forced_login_for_all_enpoints' => [ '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, + 'description' => 'If true, when no delegated authentication module is defined, no login will be forced on modules exec endpoints', + 'default' => true, + 'value' => true, 'source_of_value' => '', 'show_in_conf_sample' => false, ], diff --git a/pages/exec.php b/pages/exec.php index c478602d7..d0ef7cea1 100644 --- a/pages/exec.php +++ b/pages/exec.php @@ -105,7 +105,7 @@ if ($sTargetPage === false || $sModule === 'core' || $sModule === 'dictionaries' $aModuleDelegatedAuthenticationEndpointsList = GetModuleDelegatedAuthenticationEndpoints($sModule); // If module doesn't have the delegated authentication endpoints list defined, we rely on the conf. param. to decide if we force login or not. if (is_null($aModuleDelegatedAuthenticationEndpointsList)) { - $bForceLoginWhenNoDelegatedAuthenticationEndpoints = utils::GetConfig()->Get('security.force_login_when_no_delegated_authentication_endpoints_list'); + $bForceLoginWhenNoDelegatedAuthenticationEndpoints = !utils::GetConfig()->Get('security.disable_exec_forced_login_for_all_enpoints'); if ($bForceLoginWhenNoDelegatedAuthenticationEndpoints) { require_once(APPROOT.'/application/startup.inc.php'); LoginWebPage::DoLoginEx(); diff --git a/tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php b/tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php index 1afa57ab1..45ad48d6b 100644 --- a/tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php +++ b/tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php @@ -26,14 +26,14 @@ class LoginWebPageTest extends ItopDataTestCase $this->BackupConfiguration(); $sFolderPath = APPROOT.'env-production/extension-with-delegated-authentication-endpoints-list'; if (file_exists($sFolderPath)) { - throw new Exception("Folder $sFolderPath already exists, please remove it before running the test"); + $this->RecurseRmdir($sFolderPath); } mkdir($sFolderPath); $this->RecurseCopy(__DIR__.'/extension-with-delegated-authentication-endpoints-list', $sFolderPath); $sFolderPath = APPROOT.'env-production/extension-without-delegated-authentication-endpoints-list'; if (file_exists($sFolderPath)) { - throw new Exception("Folder $sFolderPath already exists, please remove it before running the test"); + $this->RecurseRmdir($sFolderPath); } mkdir($sFolderPath); $this->RecurseCopy(__DIR__.'/extension-without-delegated-authentication-endpoints-list', $sFolderPath); @@ -81,8 +81,7 @@ class LoginWebPageTest extends ItopDataTestCase public function testUserCanAccessAnyFile() { - // generate random login - $sUserLogin = 'user-'.date('YmdHis'); + $sUserLogin = 'user-'.uniqid(); $this->CreateUser($sUserLogin, self::$aURP_Profiles['Service Desk Agent'], self::PASSWORD); $this->GivenConfigFileAllowedLoginTypes(explode('|', 'form')); @@ -102,7 +101,7 @@ class LoginWebPageTest extends ItopDataTestCase public function testWithoutDelegatedAuthenticationEndpointsListWithForceLoginConf() { @chmod($this->oConfig->GetLoadedFile(), 0770); - $this->oConfig->Set('security.force_login_when_no_delegated_authentication_endpoints_list', true, 'AnythingButEmptyOrUnknownValue'); // 3rd param to write file even if show_in_conf_sample is false + $this->oConfig->Set('security.disable_exec_forced_login_for_all_enpoints', false, 'AnythingButEmptyOrUnknownValue'); // 3rd param to write file even if show_in_conf_sample is false $this->oConfig->WriteToFile(); @chmod($this->oConfig->GetLoadedFile(), 0444); $sPageContent = $this->CallItopUri( diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php index 4bfcccdfc..508fb3d2b 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php @@ -1553,4 +1553,19 @@ abstract class ItopDataTestCase extends ItopTestCase @chmod($sConfigPath, 0440); @unlink($this->sConfigTmpBackupFile); } + protected function AddLoginModeAndSaveConfiguration(string $sLoginMode): void + { + $aAllowedLoginTypes = $this->oiTopConfig->GetAllowedLoginTypes(); + if (!in_array($sLoginMode, $aAllowedLoginTypes)) { + $aAllowedLoginTypes[] = $sLoginMode; + $this->oiTopConfig->SetAllowedLoginTypes($aAllowedLoginTypes); + $this->SaveItopConfFile(); + } + } + protected function SaveItopConfFile(): void + { + @chmod($this->oiTopConfig->GetLoadedFile(), 0770); + $this->oiTopConfig->WriteToFile(); + @chmod($this->oiTopConfig->GetLoadedFile(), 0440); + } } diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index 2ff905b19..3aa2bdf71 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -687,7 +687,7 @@ abstract class ItopTestCase extends KernelTestCase } curl_setopt($ch, CURLOPT_URL, $sUrl); - curl_setopt($ch, CURLOPT_POST, 1);// set post data to true + curl_setopt($ch, CURLOPT_POST, $aCurlOptions[CURLOPT_POST] ?? 1);// set post data to true curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); // Force disable of certificate check as most of dev / test env have a self-signed certificate curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false); diff --git a/tests/php-unit-tests/unitary-tests/pages/AjaxRenderTest.php b/tests/php-unit-tests/unitary-tests/pages/AjaxRenderTest.php new file mode 100644 index 000000000..3177261d3 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/pages/AjaxRenderTest.php @@ -0,0 +1,180 @@ +BackupConfiguration(); + $this->oiTopConfig->Set('log_level_min', 'Error'); + $this->oiTopConfig->Set('login_debug', true); + + $this->CreateTestOrganization(); + + // Add URL authentication mode + $this->AddLoginModeAndSaveConfiguration('url'); + + // Create ticket + $description = date('dmY H:i:s'); + $oTicket = $this->createObject('UserRequest', [ + 'org_id' => $this->getTestOrgId(), + "title" => "Houston, got a problem", + "description" => $description, + ]); + self::$iTicketId = $oTicket->GetKey(); + } + + // Test that if a user with the right permissions tries to acquire the lock on a ticket, it succeeds and returns the correct success message + public function testAcquireLockSuccess(): void + { + $sOutput = $this->CreateSupportAgentUserAndAcquireLock(); + $this->assertStringContainsString('"success":true', $sOutput); + } + + // Test that if a user tries to acquire the lock on an object that does not exist, it fails and logs the correct error message + public function testAcquireLockFailsIfObjectDoesNotExist(): void + { + // Create a user with Support Agent Profile + $this->CreateUserWithProfile(self::$aURP_Profiles['Support Agent']); + + // Try to acquire the lock on a non-existent object + $sOutput = $this->AcquireLockAsUser(self::$sLogin, 99999999); + + // The output should indicate a fatal error because we hide the existence of the object when it does not exist or is not accessible by the user + $this->assertEquals(Dict::S('UI:PageTitle:FatalError'), $sOutput); + + // Check that the error log contains the expected error message about the object not existing + $sLastErrorLogLines = $this->GetErrorLogLastLines(APPROOT.'log/error.log', 10); + $this->assertStringContainsString(Dict::S('UI:ObjectDoesNotExist'), $sLastErrorLogLines); + } + + // Test that if a user tries to acquire the lock on an object for which they don't have modification rights, it fails and logs the correct error message + public function testAcquireLockFailsIfUserHasNoModifyRights(): void + { + // Create a user with a profile without modification rights on UserRequest + $this->CreateUserWithProfile(self::$aURP_Profiles['Configuration Manager']); + + // Try to acquire the lock on the ticket + $sOutput = $this->AcquireLockAsUser(self::$sLogin, self::$iTicketId); + + // The output should indicate a fatal error because we hide the existence of the object when it does not exist or is not accessible by the user + $this->assertEquals(Dict::S('UI:PageTitle:FatalError'), $sOutput); + + // The user should not have the rights to acquire the lock, and an error should be logged + $sLastErrorLogLines = $this->GetErrorLogLastLines(APPROOT.'log/error.log', 10); + $this->assertStringContainsString(Dict::S('UI:ObjectDoesNotExist'), $sLastErrorLogLines); + } + + // Test that if a user tries to acquire the lock on an object that belongs to another organization, it fails and logs the correct error message + public function testAcquireLockFailsIfObjectInOtherOrg(): void + { + // Create an organization and a ticket in this organization + $iOtherOrgId = $this->createObject('Organization', ['name' => 'OtherOrg'])->GetKey(); + $oTicket = $this->createObject('UserRequest', [ + 'org_id' => $iOtherOrgId, + 'title' => 'Ticket autre org', + 'description' => 'Test', + ]); + + // Create a user who only has access to the main test organization + $oUser = $this->CreateUserWithProfile(self::$aURP_Profiles['Support Agent']); + $oAllowedOrgList = $oUser->Get('allowed_org_list'); + $oUserOrg = \MetaModel::NewObject('URP_UserOrg', ['allowed_org_id' => $this->getTestOrgId()]); + $oAllowedOrgList->AddItem($oUserOrg); + $oUser->Set('allowed_org_list', $oAllowedOrgList); + $oUser->DBWrite(); + + // Try to acquire the lock on the ticket of the other organization + $sOutput = $this->AcquireLockAsUser(self::$sLogin, $oTicket->GetKey()); + + // The output should indicate a fatal error because we hide the existence of the object when it does not exist or is not accessible by the user + $this->assertEquals(Dict::S('UI:PageTitle:FatalError'), $sOutput); + + // The user should not have access to the ticket of the other organization, so an error should be logged + $sLastErrorLogLines = $this->GetErrorLogLastLines(APPROOT.'log/error.log', 10); + $this->assertStringContainsString(Dict::S('UI:ObjectDoesNotExist'), $sLastErrorLogLines); + } + + // Test that if a user has already acquired the lock on an object, another user cannot acquire it and gets the correct error message + public function testAcquireLockFailsIfAlreadyLockedByAnotherUser(): void + { + // First, acquire the lock with a user (User A) + $this->CreateSupportAgentUserAndAcquireLock(); + $sUserALogin = self::$sLogin; + + // Create a second user (User B) who tries to acquire the lock + $sOutput = $this->CreateSupportAgentUserAndAcquireLock(); + + // The second user should not be able to acquire the lock, and the output should contain the correct error message indicating that the object is already locked by User A + $this->assertStringContainsString('"success":false', $sOutput); + $this->assertStringContainsString('"message":"'.Dict::Format('UI:CurrentObjectIsSoftLockedBy_User', $sUserALogin).'"', $sOutput); + } + + // Helper method to create a user with Support Agent profile and acquire the lock on the ticket + private function CreateSupportAgentUserAndAcquireLock(): string + { + // Create a user with Support Agent Profile + $this->CreateUserWithProfile(self::$aURP_Profiles['Support Agent']); + + return $this->AcquireLockAsUser(self::$sLogin, self::$iTicketId); + } + + // Helper method to create a user with a specific profile + private function CreateUserWithProfile(int $iProfileId): UserLocal + { + self::$sLogin = uniqid('AjaxRenderTest'); + return $this->CreateContactlessUser(self::$sLogin, $iProfileId, self::AUTHENTICATION_PASSWORD); + } + + // Helper method to acquire the lock on a ticket as a specific user + private function AcquireLockAsUser(string $sLogin, int $iTicketId): string + { + $aGetFields = [ + 'operation' => 'acquire_lock', + 'auth_user' => $sLogin, + 'auth_pwd' => self::AUTHENTICATION_PASSWORD, + 'obj_class' => UserRequest::class, + 'obj_key' => $iTicketId, + ]; + + return $this->CallItopUri( + "pages/ajax.render.php?".http_build_query($aGetFields), + [], + [ + CURLOPT_HTTPHEADER => ['X-Combodo-Ajax:1'], + CURLOPT_POST => 0, + ] + ); + } + + // Returns the last lines of the error log containing only errors (Error level) + private function GetErrorLogLastLines(string $sErrorLogPath, int $iLineNumbers = 1): string + { + if (!file_exists($sErrorLogPath)) { + return ''; + } + + $aLines = file($sErrorLogPath, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); + + // Keep only lines containing '| Error |' + $aErrorLines = array_filter($aLines, function ($line) { + return preg_match('/\|\s*Error\s*\|/', $line); + }); + + // Return the last requested lines + return implode("\n", array_slice($aErrorLines, -$iLineNumbers)); + } +}