diff --git a/application/loginexternal.class.inc.php b/application/loginexternal.class.inc.php index d07869595..23e0c7850 100644 --- a/application/loginexternal.class.inc.php +++ b/application/loginexternal.class.inc.php @@ -75,13 +75,10 @@ class LoginExternal extends AbstractLoginFSMExtension } /** - * @return bool + * @return bool|mixed */ private function GetAuthUser() { - $sExtAuthVar = MetaModel::GetConfig()->GetExternalAuthenticationVariable(); // In which variable is the info passed ? - eval('$sAuthUser = isset('.$sExtAuthVar.') ? '.$sExtAuthVar.' : false;'); // Retrieve the value - /** @var string $sAuthUser */ - return $sAuthUser; // Retrieve the value + return MetaModel::GetConfig()->GetExternalAuthenticationVariable(); } } diff --git a/core/config.class.inc.php b/core/config.class.inc.php index dc29ccd1e..0ee78f8a7 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -78,6 +78,7 @@ define('DEFAULT_EXT_AUTH_VARIABLE', '$_SERVER[\'REMOTE_USER\']'); define('DEFAULT_ENCRYPTION_KEY', '@iT0pEncr1pti0n!'); // We'll use a random generated key later (if possible) define('DEFAULT_ENCRYPTION_LIB', 'Mcrypt'); // We'll define the best encryption available later define('DEFAULT_HASH_ALGO', PASSWORD_DEFAULT); + /** * Config * configuration data (this class cannot not be localized, because it is responsible for loading the dictionaries) @@ -871,6 +872,14 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ], + 'ext_auth_variable' => [ + 'type' => 'string', + 'description' => 'External authentication expression (allowed: $_SERVER[\'key\'], $_COOKIE[\'key\'], $_REQUEST[\'key\'], getallheaders()[\'Header-Name\'])', + 'default' => '$_SERVER[\'REMOTE_USER\']', + 'value' => '$_SERVER[\'REMOTE_USER\']', + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ], 'login_debug' => [ 'type' => 'bool', 'description' => 'Activate the login FSM debug', @@ -1970,11 +1979,6 @@ class Config */ protected $m_sDefaultLanguage; - /** - * @var string Name of the PHP variable in which external authentication information is passed by the web server - */ - protected $m_sExtAuthVariable; - /** * @var string Encryption key used for all attributes of type "encrypted string". Can be set to a random value * unless you want to import a database from another iTop instance, in which case you must use @@ -2043,7 +2047,6 @@ class Config $this->m_iFastReloadInterval = DEFAULT_FAST_RELOAD_INTERVAL; $this->m_bSecureConnectionRequired = DEFAULT_SECURE_CONNECTION_REQUIRED; $this->m_sDefaultLanguage = 'EN US'; - $this->m_sExtAuthVariable = DEFAULT_EXT_AUTH_VARIABLE; $this->m_aCharsets = []; $this->m_bQueryCacheEnabled = DEFAULT_QUERY_CACHE_ENABLED; $this->m_iPasswordHashAlgo = DEFAULT_HASH_ALGO; @@ -2189,7 +2192,6 @@ class Config $this->m_aModuleSettings = isset($MyModuleSettings) ? $MyModuleSettings : []; $this->m_sDefaultLanguage = isset($MySettings['default_language']) ? trim($MySettings['default_language']) : 'EN US'; - $this->m_sExtAuthVariable = isset($MySettings['ext_auth_variable']) ? trim($MySettings['ext_auth_variable']) : DEFAULT_EXT_AUTH_VARIABLE; $this->m_sEncryptionKey = isset($MySettings['encryption_key']) ? trim($MySettings['encryption_key']) : $this->m_sEncryptionKey; $this->m_sEncryptionLibrary = isset($MySettings['encryption_library']) ? trim($MySettings['encryption_library']) : $this->m_sEncryptionLibrary; $this->m_aCharsets = isset($MySettings['csv_import_charsets']) ? $MySettings['csv_import_charsets'] : []; @@ -2351,9 +2353,73 @@ class Config return explode('|', $this->m_aSettings['allowed_login_types']['value']); } + /** + * @return bool|mixed + * @since 3.2.3 return the parsed value instead of an unsecured variable name + */ public function GetExternalAuthenticationVariable() { - return $this->m_sExtAuthVariable; + $sExpression = $this->Get('ext_auth_variable'); + $aParsed = $this->ParseExternalAuthVariableExpression($sExpression); + if ($aParsed === null) { + return false; + } + + $sKey = $aParsed['key']; + switch ($aParsed['type']) { + case 'server': + return $_SERVER[$sKey] ?? false; + case 'cookie': + return $_COOKIE[$sKey] ?? false; + case 'request': + return $_REQUEST[$sKey] ?? false; + case 'header': + if (!function_exists('getallheaders')) { + return false; + } + $aHeaders = getallheaders(); + if (!is_array($aHeaders)) { + return false; + } + return $aHeaders[$sKey] ?? false; + } + return false; + } + + /** + * @param $sExpression + * @return array|null + */ + private function ParseExternalAuthVariableExpression($sExpression) + { + // If it's a configuration parameter it's probably already trimmed, but just in case + $sExpression = trim((string) $sExpression); + if ($sExpression === '') { + return null; + } + + // Match $_SERVER/$_COOKIE/$_REQUEST['key'] with optional whitespace and single/double quotes. + if (preg_match('/^\$_(SERVER|COOKIE|REQUEST)\s*\[\s*(["\'])\s*([^"\']+)\2\s*\]\s*$/', $sExpression, $aMatches) === 1) { + $sContext = strtoupper($aMatches[1]); + $sKey = $aMatches[3]; + return [ + 'type' => strtolower($sContext), + 'key' => $sKey, + 'normalized' => '$_'.$sContext.'[\''.$sKey.'\']', + ]; + } + + // Match getallheaders()['Header-Name'] in a case-insensitive way. + if (preg_match('/^getallheaders\(\)\s*\[\s*(["\'])\s*([^"\']+)\1\s*\]\s*$/i', $sExpression, $aMatches) === 1) { + $sKey = $aMatches[2]; + return [ + 'type' => 'header', + 'key' => $sKey, + 'normalized' => 'getallheaders()[\''.$sKey.'\']', + ]; + } + + return null; } public function GetCSVImportCharsets() @@ -2449,7 +2515,7 @@ class Config public function SetExternalAuthenticationVariable($sExtAuthVariable) { - $this->m_sExtAuthVariable = $sExtAuthVariable; + $this->Set('ext_auth_variable', $sExtAuthVariable); } public function SetEncryptionKey($sKey) @@ -2503,7 +2569,6 @@ class Config $aSettings['fast_reload_interval'] = $this->m_iFastReloadInterval; $aSettings['secure_connection_required'] = $this->m_bSecureConnectionRequired; $aSettings['default_language'] = $this->m_sDefaultLanguage; - $aSettings['ext_auth_variable'] = $this->m_sExtAuthVariable; $aSettings['encryption_key'] = $this->m_sEncryptionKey; $aSettings['encryption_library'] = $this->m_sEncryptionLibrary; $aSettings['csv_import_charsets'] = $this->m_aCharsets; @@ -2606,7 +2671,6 @@ class Config // Old fashioned remaining values $aOtherValues = [ 'default_language' => $this->m_sDefaultLanguage, - 'ext_auth_variable' => $this->m_sExtAuthVariable, 'encryption_key' => $this->m_sEncryptionKey, 'encryption_library' => $this->m_sEncryptionLibrary, 'csv_import_charsets' => $this->m_aCharsets, diff --git a/js/forms-json-utils.js b/js/forms-json-utils.js index ee45b79bd..e9ffd69c8 100644 --- a/js/forms-json-utils.js +++ b/js/forms-json-utils.js @@ -301,88 +301,112 @@ function ValidateField(sFieldId, sPattern, bMandatory, sFormId, nullValue, origi return true; // Do not stop propagation ?? } -function ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue) +function EvaluateCKEditorValidation(oOptions) { - let oField = $('#'+sFieldId); + let oField = $('#'+oOptions.sFieldId); if (oField.length === 0) { return false; } - let oCKEditor = CombodoCKEditorHandler.GetInstanceSynchronous('#'+sFieldId); + let oCKEditor = CombodoCKEditorHandler.GetInstanceSynchronous('#'+oOptions.sFieldId); + let bValid = true; + let sExplain = ''; + let sTextContent; + let sTextOriginalContents; - var bValid; - var sExplain = ''; if (oField.prop('disabled')) { bValid = true; // disabled fields are not checked } else { // If the CKEditor is not yet loaded, we need to wait for it to be ready // but as we need this function to be synchronous, we need to call it again when the CKEditor is ready if (oCKEditor === undefined){ - CombodoCKEditorHandler.GetInstance('#'+sFieldId).then((oCKEditor) => { - ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue); + CombodoCKEditorHandler.GetInstance('#'+oOptions.sFieldId).then((oCKEditor) => { + oOptions.onRetry(); }); - return; + return false; } - let sTextContent; - let sFormattedContent = oCKEditor.getData(); - // Get the contents without the tags - // Check if we have a formatted content that is HTML, otherwise we just have plain text, and we can use it directly + let sFormattedContent = oCKEditor.getData(); sTextContent = $(sFormattedContent).length > 0 ? $(sFormattedContent).text() : sFormattedContent; - if (sTextContent === '') { + if (sTextContent === '') + { // No plain text, maybe there is just an image - let oImg = $(sFormattedContent).find("img"); - if (oImg.length !== 0) { + let oImg = $(sFormattedContent).find('img'); + if (oImg.length !== 0) + { sTextContent = 'image'; } } - // Get the original value without the tags - let oFormattedOriginalContents = (originalValue !== undefined) ? $('
').html(originalValue) : undefined; - let sTextOriginalContents = (oFormattedOriginalContents !== undefined) ? oFormattedOriginalContents.text() : undefined; + let oFormattedOriginalContents = (oOptions.sOriginalValue !== undefined) ? $('
').html(oOptions.sOriginalValue) : undefined; + sTextOriginalContents = (oFormattedOriginalContents !== undefined) ? oFormattedOriginalContents.text() : undefined; - if (bMandatory && (sTextContent === nullValue)) { - bValid = false; - sExplain = Dict.S('UI:ValueMustBeSet'); - } else if ((sTextOriginalContents !== undefined) && (sTextContent === sTextOriginalContents)) { - bValid = false; - if (sTextOriginalContents === nullValue) { - sExplain = Dict.S('UI:ValueMustBeSet'); - } else { - // Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value. - sExplain = Dict.S('UI:ValueMustBeChanged'); - } - } else { - bValid = true; + if (oOptions.validate !== undefined) { + let oValidation = oOptions.validate(sTextContent, sTextOriginalContents); + bValid = oValidation.bValid; + sExplain = oValidation.sExplain; } - - // Put and event to check the field when the content changes, remove the event right after as we'll call this same function again, and we don't want to call the event more than once (especially not ^2 times on each call) + + // Put an event to check the field when the content changes, remove the event right after as we'll call this same function again, and we don't want to call the event more than once (especially not ^2 times on each call) oCKEditor.model.document.once('change:data', (event) => { - ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue); + oOptions.onChange(); }); } - - ReportFieldValidationStatus(sFieldId, sFormId, bValid, sExplain); + + ReportFieldValidationStatus(oOptions.sFieldId, oOptions.sFormId, bValid, sExplain); return bValid; } +function ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue) +{ + return EvaluateCKEditorValidation({ + sFieldId: sFieldId, + sFormId: sFormId, + sOriginalValue: originalValue, + onRetry: function() { + ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue); + }, + onChange: function() { + ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue); + }, + validate: function(sTextContent, sTextOriginalContents) { + var bValid; + var sExplain = ''; + if (bMandatory && (sTextContent === nullValue)) { + bValid = false; + sExplain = Dict.S('UI:ValueMustBeSet'); + } else if ((sTextOriginalContents !== undefined) && (sTextContent === sTextOriginalContents)) { + bValid = false; + if (sTextOriginalContents === nullValue) { + sExplain = Dict.S('UI:ValueMustBeSet'); + } else { + // Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value. + sExplain = Dict.S('UI:ValueMustBeChanged'); + } + } else { + bValid = true; + } + return {bValid: bValid, sExplain: sExplain}; + } + }); +} function ResetPwd(id) { - // Reset the values of the password fields - $('#'+id).val('*****'); - $('#'+id+'_confirm').val('*****'); - // And reset the flag, to tell it that the password remains unchanged - $('#'+id+'_changed').val(0); - // Visual feedback, None when it's Ok - $('#v_'+id).html(''); + // Reset the values of the password fields + $('#'+id).val('*****'); + $('#'+id+'_confirm').val('*****'); + // And reset the flag, to tell it that the password remains unchanged + $('#'+id+'_changed').val(0); + // Visual feedback, None when it's Ok + $('#v_'+id).html(''); } // Called whenever the content of a one way encrypted password changes function PasswordFieldChanged(id) { - // Set the flag, to tell that the password changed - $('#'+id+'_changed').val(1); + // Set the flag, to tell that the password changed + $('#'+id+'_changed').val(1); } // Special validation function for one way encrypted password fields @@ -415,37 +439,48 @@ function ValidatePasswordField(id, sFormId) // to determine if the field is empty or not function ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue) { - var bValid = true; - var sExplain = ''; - var sTextContent; - - if ($('#'+sFieldId).prop('disabled')) - { - bValid = true; // disabled fields are not checked - } - else - { - // Get the contents (with tags) - // Note: For CaseLog we can't retrieve the formatted contents from CKEditor (unlike in ValidateCKEditorField() method) because of the place holder. - sTextContent = $('#' + sFieldId).val(); - var count = $('#'+sFieldId+'_count').val(); + return EvaluateCKEditorValidation({ + sFieldId: sFieldId, + sFormId: sFormId, + sOriginalValue: originalValue, + onRetry: function() { + ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue); + }, + onChange: function() { + ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue); + }, + validate: function(sTextContent, sTextOriginalContents) { + var bValid; + var sExplain = ''; + // CaseLog is special: history count matters when deciding if the field is empty + var count = $('#'+sFieldId+'_count').val(); - if (bMandatory && (count == 0) && (sTextContent == nullValue)) - { - // No previous entry and no content typed - bValid = false; - sExplain = Dict.S('UI:ValueMustBeSet'); + if (bMandatory && (count == 0) && (sTextContent === nullValue)) + { + // No previous entry and no content typed + bValid = false; + sExplain = Dict.S('UI:ValueMustBeSet'); + } + else if ((sTextOriginalContents !== undefined) && (sTextContent === sTextOriginalContents)) + { + bValid = false; + if (sTextOriginalContents === nullValue) + { + sExplain = Dict.S('UI:ValueMustBeSet'); + } + else + { + // Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value. + sExplain = Dict.S('UI:ValueMustBeChanged'); + } + } + else + { + bValid = true; + } + return {bValid: bValid, sExplain: sExplain}; } - else if ((originalValue != undefined) && (sTextContent == originalValue)) - { - bValid = false; - sExplain = Dict.S('UI:ValueMustBeChanged'); - } - } - ReportFieldValidationStatus(sFieldId, sFormId, bValid, '' /* sExplain */); - - // We need to check periodically as CKEditor doesn't trigger our events. More details in UIHTMLEditorWidget::Display() @ line 92 - setTimeout(function(){ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue);}, 500); + }); } // Validate the inputs depending on the current setting diff --git a/tests/php-unit-tests/unitary-tests/application/LoginExternalTest.php b/tests/php-unit-tests/unitary-tests/application/LoginExternalTest.php new file mode 100644 index 000000000..93f16d2b3 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/application/LoginExternalTest.php @@ -0,0 +1,95 @@ + + */ + +namespace Combodo\iTop\Test\UnitTest\Application; + +use Combodo\iTop\Test\UnitTest\ItopDataTestCase; +use utils; + +class LoginExternalTest extends ItopDataTestCase +{ + private $oConfig; + private $sOriginalExtAuthVariable; + + protected function setUp(): void + { + parent::setUp(); + require_once APPROOT.'application/loginexternal.class.inc.php'; + $this->oConfig = utils::GetConfig(); + $this->sOriginalExtAuthVariable = $this->oConfig->Get('ext_auth_variable'); + } + + protected function tearDown(): void + { + $this->oConfig->SetExternalAuthenticationVariable($this->sOriginalExtAuthVariable); + parent::tearDown(); + } + + private function CallGetAuthUser() + { + $oLoginExternal = new \LoginExternal(); + $oMethod = new \ReflectionMethod(\LoginExternal::class, 'GetAuthUser'); + $oMethod->setAccessible(true); + return $oMethod->invoke($oLoginExternal); + } + + public function testGetAuthUserFromServerVariable() + { + $_SERVER['REMOTE_USER'] = 'alice'; + $this->oConfig->SetExternalAuthenticationVariable('$_SERVER[\'REMOTE_USER\']'); + + $this->assertSame('alice', $this->CallGetAuthUser()); + } + + public function testGetAuthUserFromCookie() + { + $_COOKIE['auth_user'] = 'bob'; + $this->oConfig->SetExternalAuthenticationVariable('$_COOKIE[\'auth_user\']'); + + $this->assertSame('bob', $this->CallGetAuthUser()); + } + + public function testGetAuthUserFromRequest() + { + $_REQUEST['auth_user'] = 'carol'; + $this->oConfig->SetExternalAuthenticationVariable('$_REQUEST[\'auth_user\']'); + + $this->assertSame('carol', $this->CallGetAuthUser()); + } + + public function testInvalidExpressionReturnsFalse() + { + $this->oConfig->SetExternalAuthenticationVariable('$_SERVER[\'HTTP_X_CMD\']) ? print(\'x\') : false; //'); + + $this->assertFalse($this->CallGetAuthUser()); + } + + public function testGetAuthUserFromHeaderWithoutAllowlist() + { + if (!function_exists('getallheaders')) { + $this->markTestSkipped('getallheaders() not available'); + } + $_SERVER['HTTP_X_REMOTE_USER'] = 'CN=header-test'; + $this->oConfig->SetExternalAuthenticationVariable('getallheaders()[\'X-Remote-User\']'); + + $this->assertSame('CN=header-test', $this->CallGetAuthUser()); + } +}