diff --git a/core/config.class.inc.php b/core/config.class.inc.php index cdc08ddf9c..7cf960a3a9 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -1738,6 +1738,14 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ], + 'security.force_login_when_no_delegated_authentication_endpoints_list' => [ + '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' => false, + ], '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)', diff --git a/pages/exec.php b/pages/exec.php index 83e82b548d..7cf9de6554 100644 --- a/pages/exec.php +++ b/pages/exec.php @@ -87,7 +87,7 @@ function CheckPageExists(string $sPagePath, array $aPossibleBasePaths) } $sTargetPage = CheckPageExists($sPageEnvFullPath, $aPossibleBasePaths); -if ($sTargetPage === false) { +if ($sTargetPage === false || $sModule === 'core' || $sModule === 'dictionaries') { // Do not recall the page parameters (security takes precedence) echo "Wrong module, page name or environment..."; exit; @@ -97,4 +97,35 @@ function CheckPageExists(string $sPagePath, array $aPossibleBasePaths) // // GO! // +// check module white list +// check conf param +// force login if needed + +$aModuleDelegatedAuthenticationEndpointsList = GetModuleDelegatedAuthenticationEndpoints($sModule); +if (is_null($aModuleDelegatedAuthenticationEndpointsList)) { + $bForceLoginWhenNoDelegatedAuthenticationEndpoints = utils::GetConfig()->Get('security.force_login_when_no_delegated_authentication_endpoints_list'); + if ($bForceLoginWhenNoDelegatedAuthenticationEndpoints) { + require_once(APPROOT.'/application/startup.inc.php'); + LoginWebPage::DoLoginEx(); + } +} +if (is_array($aModuleDelegatedAuthenticationEndpointsList) && !in_array($sPage, $aModuleDelegatedAuthenticationEndpointsList)) { + // if module defined a delegated authentication endpoints but not for the current page, we consider that the page is not allowed to be executed without login + require_once(APPROOT.'/application/startup.inc.php'); + LoginWebPage::DoLoginEx(); +} +if (is_null($aModuleDelegatedAuthenticationEndpointsList) && !UserRights::IsLoggedIn()) { + // check if user is not logged in, if not log a warning in the log file as the page is executed without login, which is not recommended for security reason + IssueLog::Debug("The '$sPage' page is executed without logging in. This call will be blocked in the future and will likely cause unwanted behaviour in the '$sModule' module. Please define a delegated authentication endpoint for the module, as described at https://www.itophub.io/wiki/page?id=latest:customization:new_extension#security."); +} + require_once($sTargetPage); + +function GetModuleDelegatedAuthenticationEndpoints(string $sModuleName): ?array +{ + $sModuleFile = utils::GetAbsoluteModulePath($sModuleName).'/module.'.$sModuleName.'.php'; + require_once APPROOT.'setup/extensionsmap.class.inc.php'; + $oExtensionMap = new iTopExtensionsMap(); + $aModuleParam = $oExtensionMap->GetModuleInfo($sModuleFile)[2]; + return $aModuleParam['delegated_authentication_endpoints'] ?? null; +} diff --git a/setup/extensionsmap.class.inc.php b/setup/extensionsmap.class.inc.php index 6c113a231f..6a685f7707 100644 --- a/setup/extensionsmap.class.inc.php +++ b/setup/extensionsmap.class.inc.php @@ -390,7 +390,7 @@ protected function CheckDependencies($sFromEnvironment) * @param string $sModuleFile * @return array */ - protected function GetModuleInfo($sModuleFile) + public function GetModuleInfo($sModuleFile) { static $iDummyClassIndex = 0; diff --git a/tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php b/tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php new file mode 100644 index 0000000000..0d63293a11 --- /dev/null +++ b/tests/php-unit-tests/integration-tests/login-tests/LoginWebPageTest.php @@ -0,0 +1,177 @@ +GetLoadedFile(); + $this->oConfig = new Config($sConfigPath); + + $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"); + } + 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"); + } + mkdir($sFolderPath); + $this->RecurseCopy(__DIR__.'/extension-without-delegated-authentication-endpoints-list', $sFolderPath); + } + public function tearDown(): void + { + parent::tearDown(); + $sFolderPath = APPROOT.'env-production/extension-with-delegated-authentication-endpoints-list'; + 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-delegated-authentication-endpoints-list'; + 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($this->oConfig->GetLoadedFile(), 0770); + $this->oConfig->SetAllowedLoginTypes($aAllowedLoginTypes); + $this->oConfig->WriteToFile($this->oConfig->GetLoadedFile()); + @chmod($this->oConfig->GetLoadedFile(), 0444); + } + + /** + * + * @throws \Exception + */ + public function testInDelegatedAuthenticationEndpoints() + { + $sPageContent = $this->CallItopUri( + "pages/exec.php?exec_module=extension-with-delegated-authentication-endpoints-list&exec_page=src/Controller/FileInDelegatedAuthenticationEndpointsList.php", + [], + [], + true + ); + + $this->assertStringNotContainsString('iTop login', $sPageContent, 'File listed in delegated authentication endpoints list (in the module), login should not be requested by exec.'); + } + + public function testUserCanAccessAnyFile() + { + // generate random login + $sUserLogin = 'user-'.date('YmdHis'); + $this->CreateUser($sUserLogin, self::$aURP_Profiles['Service Desk Agent'], self::PASSWORD); + $this->GivenConfigFileAllowedLoginTypes(explode('|', 'form')); + + $sPageContent = $this->CallItopUri( + "pages/exec.php?exec_module=extension-with-delegated-authentication-endpoints-list&exec_page=src/Controller/FileNotInDelegatedAuthenticationEndpointsList.php", + [ + 'auth_user' => $sUserLogin, + 'auth_pwd' => self::PASSWORD, + ], + [], + true + ); + + $this->assertStringContainsString('Yo', $sPageContent, 'Logged in user should access any file via exec.php even if the page isn\'t listed in delegated authentication endpoints list'); + } + + public function testWithoutDelegatedAuthenticationEndpointsListWithForceLoginConf() + { + @chmod($this->oConfig->GetLoadedFile(), 0770); + $this->oConfig->Set('security.force_login_when_no_delegated_authentication_endpoints_list', true); + $this->oConfig->WriteToFile(); + @chmod($this->oConfig->GetLoadedFile(), 0444); + $sPageContent = $this->CallItopUri( + "pages/exec.php?exec_module=extension-without-delegated-authentication-endpoints-list&exec_page=src/Controller/File.php", + ); + + $this->assertStringContainsString('iTop login', $sPageContent, 'if itop is configured to force login when no there is no delegated authentication endpoints list, then login should be required.'); + } + + public function testWithoutDelegatedAuthenticationEndpointsListWithDefaultConfiguration() + { + $sPageContent = $this->CallItopUri( + "pages/exec.php?exec_module=extension-without-delegated-authentication-endpoints-list&exec_page=src/Controller/File.php", + [], + [], + true + ); + + $this->assertStringContainsString('Yo', $sPageContent, 'by default (until N°9343) if no delegated authentication endpoints list is defined, not logged in persons should access pages'); + } + + public function testNotInDelegatedAuthenticationEndpointsList() + { + $sPageContent = $this->CallItopUri( + "pages/exec.php?exec_module=extension-with-delegated-authentication-endpoints-list&exec_page=src/Controller/FileNotInDelegatedAuthenticationEndpointsList.php", + [], + [], + true + ); + + $this->assertStringContainsString('iTop login', $sPageContent, 'Since an delegated authentication endpoints list is defined and file isn\'t listed in it, login should be required'); + } + + /** + * @dataProvider InDelegatedAuthenticationEndpointsWithAdminRequiredProvider + * + * @throws \Exception + */ + public function testInDelegatedAuthenticationEndpointsWithAdminRequired($iProfileId, $bShouldSeeForbiddenAdminPage) + { + // generate random login + $sUserLogin = 'user-'.date('YmdHis'); + $this->CreateUser($sUserLogin, $iProfileId, self::PASSWORD); + $this->GivenConfigFileAllowedLoginTypes(explode('|', 'form')); + + $sPageContent = $this->CallItopUri( + "pages/exec.php?exec_module=extension-with-delegated-authentication-endpoints-list&exec_page=src/Controller/FileInDelegatedAuthenticationEndpointsListAndAdminRequired.php", + [ + 'auth_user' => $sUserLogin, + 'auth_pwd' => self::PASSWORD, + ], + [], + true + ); + $bShouldSeeForbiddenAdminPage ? + $this->assertStringContainsString('Access restricted to people having administrator privileges', $sPageContent, 'Should prevent non admin user to access this page') : // in delegated authentication endpoints list (in the module), login should not be required + $this->assertStringContainsString('Yo !', $sPageContent, 'Should execute the file and see its content since user has admin profile'); + + } + + public function InDelegatedAuthenticationEndpointsWithAdminRequiredProvider() + { + return [ + 'Administrator profile' => [ + self::$aURP_Profiles['Administrator'], + 'Should see forbidden admin page' => false, + ], + 'ReadOnly profile' => [ + self::$aURP_Profiles['Service Desk Agent'], + 'Should see forbidden admin page' => true, + ], + ]; + } +} diff --git a/tests/php-unit-tests/integration-tests/login-tests/extension-with-delegated-authentication-endpoints-list/module.extension-with-delegated-authentication-endpoints-list.php b/tests/php-unit-tests/integration-tests/login-tests/extension-with-delegated-authentication-endpoints-list/module.extension-with-delegated-authentication-endpoints-list.php new file mode 100644 index 0000000000..34493f366d --- /dev/null +++ b/tests/php-unit-tests/integration-tests/login-tests/extension-with-delegated-authentication-endpoints-list/module.extension-with-delegated-authentication-endpoints-list.php @@ -0,0 +1,51 @@ + 'Templates foundation', + 'category' => 'business', + + // Setup + // + 'dependencies' => [], + 'mandatory' => true, + 'visible' => false, + 'installer' => 'TemplatesBaseInstaller', + + // Security + 'delegated_authentication_endpoints' => [ + 'src/Controller/FileInDelegatedAuthenticationEndpointsList.php', + 'src/Controller/FileInDelegatedAuthenticationEndpointsListAndAdminRequired.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', + ], + ] +); diff --git a/tests/php-unit-tests/integration-tests/login-tests/extension-with-delegated-authentication-endpoints-list/src/Controller/FileInDelegatedAuthenticationEndpointsList.php b/tests/php-unit-tests/integration-tests/login-tests/extension-with-delegated-authentication-endpoints-list/src/Controller/FileInDelegatedAuthenticationEndpointsList.php new file mode 100644 index 0000000000..825c9d411b --- /dev/null +++ b/tests/php-unit-tests/integration-tests/login-tests/extension-with-delegated-authentication-endpoints-list/src/Controller/FileInDelegatedAuthenticationEndpointsList.php @@ -0,0 +1,3 @@ + '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', + ], + ] +); diff --git a/tests/php-unit-tests/integration-tests/login-tests/extension-without-delegated-authentication-endpoints-list/src/Controller/File.php b/tests/php-unit-tests/integration-tests/login-tests/extension-without-delegated-authentication-endpoints-list/src/Controller/File.php new file mode 100644 index 0000000000..825c9d411b --- /dev/null +++ b/tests/php-unit-tests/integration-tests/login-tests/extension-without-delegated-authentication-endpoints-list/src/Controller/File.php @@ -0,0 +1,3 @@ +