Add check for forwarded headers in redirect URL generation

[adhil0]

I'm using GLPI on K8S, and was having trouble getting the RelayState to be passed using https rather than http. To fix this I changed this line in setup.php to:

$returnTo = (isset($_GET['redirect']) ? $_GET['redirect'] : (isset($_SERVER['HTTP_X_FORWARDED_PROTO']) ? $_SERVER['HTTP_X_FORWARDED_PROTO'] : (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] === 'on' ? "https" : "http")) . "://" . $realhost . $_SERVER['REQUEST_URI']);

Essentially this adds a check for the protocol in Forwarded Headers. So far it's working in my config(apache, K8S, phpsaml==1.2.1, php==7.4.19, GLPI==10.0.1), but I haven't been able to test in a different configuration. If others can confirm it's working for them, I can open a PR to merge this into main.

Feel free to close this if it isn't compatible with this project's goals.

[DonutsNL]

Hi Adhil0,

Thanks for this suggestion. Currently we have not considered proxied environments because of their complexity and setup flexibility.

I think as a first step we might want to introduce a configuration toggle to make phpSaml evaluate the various proxy headers. For my understanding could you provide us a basic diagram of your setup and the challenge with redirects in that setup? This will help me understand the logic we need to implement.

Rgrds,

[adhil0]

Maybe we could do something similar to the upstream repo:

When the PHP application is behind a proxy or a load balancer we can execute setProxyVars(true) and setSelfPort and isHTTPS will take care of the $_SERVER["HTTP_X_FORWARDED_PORT"] and $_SERVER['HTTP_X_FORWARDED_PROTO'] vars (otherwise they are ignored).

So as suggested in #120, you could create a UI option in the configuration to toggle $_proxyVars on and off, which should take care of most of the issues with proxies etc.

For things that remain, the functions in Utils.php could be used. For example, in this particular issue, we could use Utils::getSelfProtocol()

if (isset($_GET['redirect'])) {
	$returnTo = $_GET['redirect'];
} else {
	$returnTo = Utils::getSelfProtocol() . "://"  .  $realhost  .  $_SERVER['REQUEST_URI'];
}

[DonutsNL]

I did not dive into the other issues / repos (yet). So thanks for the research so far. Ill have a look at it and propose a fix in the JIT branch where i am currently working on both the config and JIT rules. Then I will prob need you guys to help me test it. Would that be a problem?

It will be somewhere in here: https://github.com/derricksmith/phpsaml/pull/118 when finished

[adhil0]

Sure, I would be willing to help test your changes regarding this issue