jumbojett / OpenID-Connect-PHP

Minimalist OpenID Connect client
https://github.com/jumbojett/OpenID-Connect-PHP
Apache License 2.0
606 stars 363 forks source link

SERVER_PORT always causes port in redirect URL #403

Closed timsmid closed 5 months ago

timsmid commented 9 months ago

When $_SERVER['SERVER_PORT'] is set, the getRedirectUrl method always includes a port number, even when the port is 80 or 443. This is caused by comparing a string ($_SERVER['SERVER_PORT']) with an integer (80 and 443).

I will submit a PR to resolve this issue.

jasongill commented 9 months ago

@timsmid thank you for this, after upgrading to 1.0.0 we couldn't log in with Okta due to redirect URI mismatch so adding :443 to the allowed URL's in Okta allowed us to work around this bug until your PR is merged.

NicoHaase commented 7 months ago

Any news on this topic? We're facing this bug as well

azurams commented 5 months ago

thank you @timsmid !

DeepDiver1975 commented 5 months ago

PR merged -> close

q2apro commented 1 month ago

⚠️ Note that now having always :443 inside the redirect_uri breaks a lot of SSO sites, that have whitelisted in their firewall e.g. mydomain.com/* but not mydomain.com:443/*.

Is there any option to prevent that :443 gets added?

timsmid commented 1 month ago

That is exactly the problem that this issue (and PR) solved. The port will be dropped from the redirect URL if it is 443 or 80. See https://github.com/jumbojett/OpenID-Connect-PHP/blob/470895e/src/OpenIDConnectClient.php#L716

$port = (443 === $port) || (80 === $port) ? '' : ':' . $port;

Previously, $port contained 443 as a string, which caused the port to be incorrectly included in the redirect URL. With the latest version of this library, the redirect URL should not contain :443.

q2apro commented 1 month ago

Strange, in my case the :443 is always added, even with this exact code.

I have error_log'ed the $port variable, it is 443. Still it is added to the URL.

error_log( gettype($port) );

gives string not int.

That's why (443 === $port) is false, not true.


Ah, now I see that in my version it is: $port = $_SERVER['SERVER_PORT']; and not $port = (int)$_SERVER['SERVER_PORT'];

Case closed.