diff --git a/application/utils.inc.php b/application/utils.inc.php index 92915bfc2..443bf3781 100644 --- a/application/utils.inc.php +++ b/application/utils.inc.php @@ -782,14 +782,29 @@ class utils } } + public static function IsProxyTrusted() + { + if (empty($_SERVER['REMOTE_ADDR'])) { + return false; + } + + $bTrustProxies = (bool) self::GetConfig()->Get('trust_proxies'); + + return $bTrustProxies; + } + /** * Returns the absolute URL to the application root path * + * @param bool $bForceTrustProxy + * * @return string The absolute URL to the application root, without the first slash * * @throws \Exception + * + * @since 2.7.4 $bForceTrustProxy param added */ - public static function GetAbsoluteUrlAppRoot() + public static function GetAbsoluteUrlAppRoot($bForceTrustProxy = false) { static $sUrl = null; if ($sUrl === null) @@ -797,7 +812,7 @@ class utils $sUrl = self::GetConfig()->Get('app_root_url'); if ($sUrl == '') { - $sUrl = self::GetDefaultUrlAppRoot(); + $sUrl = self::GetDefaultUrlAppRoot($bForceTrustProxy); } elseif (strpos($sUrl, SERVER_NAME_PLACEHOLDER) > -1) { @@ -821,40 +836,31 @@ class utils * For most usages, when an root url is needed, use utils::GetAbsoluteUrlAppRoot() instead as uses this only as a fallback when the * app_root_url conf parameter is not defined. * + * @param bool $bForceTrustProxy + * * @return string * * @throws \Exception - */ - public static function GetDefaultUrlAppRoot() + * + * @since 2.7.4 $bForceTrustProxy param added + */ + public static function GetDefaultUrlAppRoot($bForceTrustProxy = false) { // Build an absolute URL to this page on this server/port - $sServerName = isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : ''; - $sProtocol = self::IsConnectionSecure() ? 'https' : 'http'; - $iPort = isset($_SERVER['SERVER_PORT']) ? $_SERVER['SERVER_PORT'] : 80; - if ($sProtocol == 'http') - { - $sPort = ($iPort == 80) ? '' : ':'.$iPort; - } - else + $sServerName = self::GetServerName($bForceTrustProxy); + $bIsSecure = self::IsConnectionSecure($bForceTrustProxy) ; + $sProtocol = $bIsSecure ? 'https' : 'http'; + $iPort = self::GetServerPort($bForceTrustProxy); + if ($bIsSecure) { $sPort = ($iPort == 443) ? '' : ':'.$iPort; } - // $_SERVER['REQUEST_URI'] is empty when running on IIS - // Let's use Ivan Tcholakov's fix (found on www.dokeos.com) - if (!empty($_SERVER['REQUEST_URI'])) - { - $sPath = $_SERVER['REQUEST_URI']; - } else { - $sPath = $_SERVER['SCRIPT_NAME']; - if (!empty($_SERVER['QUERY_STRING'])) - { - $sPath .= '?'.$_SERVER['QUERY_STRING']; - } - $_SERVER['REQUEST_URI'] = $sPath; + $sPort = ($iPort == 80) ? '' : ':'.$iPort; } - $sPath = $_SERVER['REQUEST_URI']; + + $sPath = self::GetRequestUri($bForceTrustProxy); // remove all the parameters from the query string $iQuestionMarkPos = strpos($sPath, '?'); @@ -870,6 +876,61 @@ class utils return self::GetAppRootUrl($sCurrentScript, $sAppRoot, $sAbsoluteUrl); } + /** + * @param false $bForceTrustProxy + * @since 2.7.4 + */ + public static function GetServerName($bForceTrustProxy = false) + { + $bTrustProxy = $bForceTrustProxy || self::IsProxyTrusted(); + + $sServerName = isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : ''; + + if ($bTrustProxy) { + $sServerName = isset($_SERVER['X_FORWARDED_HOST']) ? $_SERVER['X_FORWARDED_HOST'] : $sServerName; + } + + return $sServerName; + } + + /** + * @param false $bForceTrustProxy + * @since 2.7.4 + */ + public static function GetServerPort($bForceTrustProxy = false) + { + $bTrustProxy = $bForceTrustProxy || self::IsProxyTrusted(); + + $sServerPort = isset($_SERVER['SERVER_PORT']) ? $_SERVER['SERVER_PORT'] : 80; + + if ($bTrustProxy) { + $sServerPort = isset($_SERVER['X_FORWARDED_PORT']) ? $_SERVER['X_FORWARDED_PORT'] : $sServerPort; + } + + return $sServerPort; + } + + /** + * @since 2.7.4 + */ + public static function GetRequestUri() + { + // $_SERVER['REQUEST_URI'] is empty when running on IIS + // Let's use Ivan Tcholakov's fix (found on www.dokeos.com) + if (empty($_SERVER['REQUEST_URI'])) + { + $sPath = $_SERVER['SCRIPT_NAME']; + if (!empty($_SERVER['QUERY_STRING'])) + { + $sPath .= '?'.$_SERVER['QUERY_STRING']; + } + $_SERVER['REQUEST_URI'] = $sPath; + } + $sPath = $_SERVER['REQUEST_URI']; + + return $sPath; + } + /** * @param $sCurrentScript * @param $sAppRoot @@ -914,16 +975,33 @@ class utils * * Though the official specs says 'a non empty string', some servers like IIS do set it to 'off' ! * nginx set it to an empty string - * Others might leave it unset (no array entry) + * Others might leave it unset (no array entry) + * + * @param bool $bForceTrustProxy + * + * @return bool + * + * @since 2.7.4 reverse proxies handling */ - public static function IsConnectionSecure() + public static function IsConnectionSecure($bForceTrustProxy = false) { $bSecured = false; - if (!empty($_SERVER['HTTPS']) && (strtolower($_SERVER['HTTPS']) != 'off')) + $bTrustProxy = $bForceTrustProxy || self::IsProxyTrusted(); + + if ($bTrustProxy && !empty($_SERVER['HTTP_X_FORWARDED_PROTO'])) + { + $bSecured = ($_SERVER['HTTP_X_FORWARDED_PROTO'] === 'https'); + } + elseif ($bTrustProxy && !empty($_SERVER['HTTP_X_FORWARDED_PROTOCOL'])) + { + $bSecured = ($_SERVER['HTTP_X_FORWARDED_PROTOCOL'] === 'https'); + } + elseif ((!empty($_SERVER['HTTPS'])) && (strtolower($_SERVER['HTTPS']) != 'off')) { $bSecured = true; } + return $bSecured; } diff --git a/core/config.class.inc.php b/core/config.class.inc.php index 5d6e63938..313bb1b94 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -1265,6 +1265,14 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ], + 'trust_proxies' => [ + '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)', + 'default' => false, + 'value' => false, + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ], ); diff --git a/setup/setuppage.class.inc.php b/setup/setuppage.class.inc.php index a25ad3d23..2c4bbef22 100644 --- a/setup/setuppage.class.inc.php +++ b/setup/setuppage.class.inc.php @@ -144,7 +144,7 @@ class SetupPage extends NiceWebPage public function output() { - $sLogo = utils::GetAbsoluteUrlAppRoot().'/images/itop-logo.png'; + $sLogo = utils::GetAbsoluteUrlAppRoot(true).'/images/itop-logo.png'; $this->s_content = "

\" ".htmlentities($this->s_title, ENT_QUOTES, self::PAGES_CHARSET)."

\n
{$this->s_content}\n
\n"; diff --git a/setup/wizardsteps.class.inc.php b/setup/wizardsteps.class.inc.php index 40d407055..7edbaeb40 100644 --- a/setup/wizardsteps.class.inc.php +++ b/setup/wizardsteps.class.inc.php @@ -978,7 +978,7 @@ class WizStepMiscParams extends WizardStep public function Display(WebPage $oPage) { $sDefaultLanguage = $this->oWizard->GetParameter('default_language', $this->oWizard->GetParameter('admin_language')); - $sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetDefaultUrlAppRoot()); + $sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetDefaultUrlAppRoot(true)); $sDefaultGraphvizPath = (strtolower(substr(PHP_OS, 0, 3)) === 'win') ? 'C:\\Program Files\\Graphviz\\bin\\dot.exe' : '/usr/bin/dot'; $sGraphvizPath = $this->oWizard->GetParameter('graphviz_path', $sDefaultGraphvizPath); $sSampleData = $this->oWizard->GetParameter('sample_data', 'yes'); @@ -996,14 +996,14 @@ class WizStepMiscParams extends WizardStep $oPage->add('
'); $oPage->add('Application URL'); $oPage->add(''); - $oPage->add(''); - $oPage->add(''); + $oPage->add(''); + $oPage->add(''); $oPage->add('
URL:
Change the value above if the end-users will be accessing the application by another path due to a specific configuration of the web server.
URL:
Change the value above if the end-users will be accessing the application by another path due to a specific configuration of the web server.
'); $oPage->add('
'); $oPage->add('
'); $oPage->add('Path to Graphviz\' dot application'); $oPage->add(''); - $oPage->add(''); + $oPage->add(''); $oPage->add(''); $oPage->add(''); $oPage->add('
Path:
Path:
Graphviz is required to display the impact analysis graph (i.e. impacts / depends on).
'); @@ -1125,21 +1125,21 @@ class WizStepUpgradeMiscParams extends WizardStep public function Display(WebPage $oPage) { - $sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetDefaultUrlAppRoot()); + $sApplicationURL = $this->oWizard->GetParameter('application_url', utils::GetAbsoluteUrlAppRoot(true)); //Preserve existing configuration. $sDefaultGraphvizPath = (strtolower(substr(PHP_OS, 0, 3)) === 'win') ? 'C:\\Program Files\\Graphviz\\bin\\dot.exe' : '/usr/bin/dot'; $sGraphvizPath = $this->oWizard->GetParameter('graphviz_path', $sDefaultGraphvizPath); $oPage->add('

Additional parameters

'); $oPage->add('
'); $oPage->add('Application URL'); $oPage->add(''); - $oPage->add(''); - $oPage->add(''); + $oPage->add(''); + $oPage->add(''); $oPage->add('
URL:
Change the value above if the end-users will be accessing the application by another path due to a specific configuration of the web server.
URL:
Change the value above if the end-users will be accessing the application by another path due to a specific configuration of the web server.
'); $oPage->add('
'); $oPage->add('
'); $oPage->add('Path to Graphviz\' dot application'); $oPage->add(''); - $oPage->add(''); + $oPage->add(''); $oPage->add(''); $oPage->add(''); $oPage->add('
Path:
Path:
Graphviz is required to display the impact analysis graph (i.e. impacts / depends on).
'); @@ -1376,14 +1376,14 @@ class WizStepModulesChoice extends WizardStep if (substr($sBannerPath, 0, 1) == '/') { // absolute path, means relative to APPROOT - $sBannerUrl = utils::GetDefaultUrlAppRoot().$sBannerPath; + $sBannerUrl = utils::GetDefaultUrlAppRoot(true).$sBannerPath; } else { // relative path: i.e. relative to the directory containing the XML file $sFullPath = dirname($this->GetSourceFilePath()).'/'.$sBannerPath; $sRealPath = realpath($sFullPath); - $sBannerUrl = utils::GetDefaultUrlAppRoot().str_replace(realpath(APPROOT), '', $sRealPath); + $sBannerUrl = utils::GetDefaultUrlAppRoot(true).str_replace(realpath(APPROOT), '', $sRealPath); } $oPage->add(''); } @@ -2515,7 +2515,7 @@ class WizStepDone extends WizardStep $aManualSteps = array(); $aAvailableModules = SetupUtils::AnalyzeInstallation($this->oWizard); - $sRootUrl = utils::GetAbsoluteUrlAppRoot(); + $sRootUrl = utils::GetAbsoluteUrlAppRoot(true); $aSelectedModules = json_decode($this->oWizard->GetParameter('selected_modules'), true); foreach($aSelectedModules as $sModuleId) { @@ -2547,7 +2547,7 @@ class WizStepDone extends WizardStep { // To mitigate security risks: pass only the filename without the extension, the download will add the extension itself $oPage->p('Your backup is ready'); - $oPage->p(' Download '.basename($sBackupDestination).''); + $oPage->p(' Download '.basename($sBackupDestination).''); } else { diff --git a/test/application/UtilsTest.php b/test/application/UtilsTest.php index ee3d02559..93e2ab241 100644 --- a/test/application/UtilsTest.php +++ b/test/application/UtilsTest.php @@ -168,4 +168,151 @@ class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopTestCase ); } + /** + * @dataProvider GetDefaultUrlAppRootProvider + */ + public function testGetDefaultUrlAppRoot($bForceTrustProxy, $bConfTrustProxy, $aServerVars, $sExpectedAppRootUrl) + { + $_SERVER = $aServerVars; + utils::GetConfig()->Set('trust_proxies', $bConfTrustProxy); + $sAppRootUrl = utils::GetDefaultUrlAppRoot($bForceTrustProxy); + $this->assertEquals($sExpectedAppRootUrl, $sAppRootUrl); + } + + public function GetDefaultUrlAppRootProvider() + { + $this->setUp(); + + $baseServerVar = [ + 'REMOTE_ADDR' => '127.0.0.1', //is not set, disable IsProxyTrusted + 'SERVER_NAME' => 'example.com', + 'X_FORWARDED_HOST' => null, + 'SERVER_PORT' => '80', + 'X_FORWARDED_PORT' => null, + 'REQUEST_URI' => '/index.php?baz=1', + 'SCRIPT_NAME' => '/index.php', + 'SCRIPT_FILENAME' => APPROOT.'index.php', + 'QUERY_STRING' => 'baz=1', + 'HTTP_X_FORWARDED_PROTO' => null, + 'HTTP_X_FORWARDED_PROTOCOL' => null, + 'HTTPS' => null, + ]; + + return [ + 'no proxy, http' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, []), + 'sExpectedAppRootUrl' => 'http://example.com/', + ], + 'no proxy, subPath, http' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, [ + 'REQUEST_URI' => '/foo/index.php?baz=1', + ]), + 'sExpectedAppRootUrl' => 'http://example.com/foo/', + ], + 'IIS lack REQUEST_URI' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, [ + 'REQUEST_URI' => null, + 'SCRIPT_NAME' => '/foo/index.php', + ]), + 'sExpectedAppRootUrl' => 'http://example.com/foo/', + ], + 'no proxy, https' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, [ + 'SERVER_PORT' => '443', + 'HTTPS' => 'on', + ]), + 'sExpectedAppRootUrl' => 'https://example.com/', + ], + 'no proxy, https on 4443' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, [ + 'SERVER_PORT' => '4443', + 'HTTPS' => 'on', + ]), + 'sExpectedAppRootUrl' => 'https://example.com:4443/', + ], + 'with proxy, not enabled' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, [ + 'X_FORWARDED_HOST' => 'proxy.com', + 'X_FORWARDED_PORT' => '4443', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ]), + 'sExpectedAppRootUrl' => 'http://example.com/', + ], + 'with proxy, enabled' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => true, + 'aServerVars' => array_merge($baseServerVar, [ + 'X_FORWARDED_HOST' => 'proxy.com', + 'X_FORWARDED_PORT' => '4443', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ]), + 'sExpectedAppRootUrl' => 'https://proxy.com:4443/', + ], + 'with proxy, enabled - alt' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => true, + 'aServerVars' => array_merge($baseServerVar, [ + 'X_FORWARDED_HOST' => 'proxy.com', + 'X_FORWARDED_PORT' => '4443', + 'HTTP_X_FORWARDED_PROTOCOL' => 'https', + ]), + 'sExpectedAppRootUrl' => 'https://proxy.com:4443/', + ], + 'with proxy, disabled, forced' => [ + 'bForceTrustProxy' => true, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, [ + 'X_FORWARDED_HOST' => 'proxy.com', + 'X_FORWARDED_PORT' => '4443', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ]), + 'sExpectedAppRootUrl' => 'https://proxy.com:4443/', + ], + 'with proxy, enabled, forced' => [ + 'bForceTrustProxy' => true, + 'bConfTrustProxy' => true, + 'aServerVars' => array_merge($baseServerVar, [ + 'X_FORWARDED_HOST' => 'proxy.com', + 'X_FORWARDED_PORT' => '4443', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ]), + 'sExpectedAppRootUrl' => 'https://proxy.com:4443/', + ], + + 'with proxy, disabled, forced, no remote addr' => [ + 'bForceTrustProxy' => true, + 'bConfTrustProxy' => false, + 'aServerVars' => array_merge($baseServerVar, [ + 'REMOTE_ADDR' => null, + 'X_FORWARDED_HOST' => 'proxy.com', + 'X_FORWARDED_PORT' => '4443', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ]), + 'sExpectedAppRootUrl' => 'https://proxy.com:4443/', + ], + 'with proxy, enabled, no remote addr' => [ + 'bForceTrustProxy' => false, + 'bConfTrustProxy' => true, + 'aServerVars' => array_merge($baseServerVar, [ + 'REMOTE_ADDR' => null, + 'X_FORWARDED_HOST' => 'proxy.com', + 'X_FORWARDED_PORT' => '4443', + 'HTTP_X_FORWARDED_PROTO' => 'https', + ]), + 'sExpectedAppRootUrl' => 'http://example.com/', + ], + ]; + } }