Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SERVER_PORT always causes port in redirect URL #403

Closed
timsmid opened this issue Dec 15, 2023 · 7 comments · Fixed by #437
Closed

SERVER_PORT always causes port in redirect URL #403

timsmid opened this issue Dec 15, 2023 · 7 comments · Fixed by #437

Comments

@timsmid
Copy link
Contributor

timsmid commented Dec 15, 2023

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
Copy link
Contributor

@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
Copy link

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

@azurams
Copy link

azurams commented Apr 26, 2024

thank you @timsmid !

@DeepDiver1975
Copy link
Collaborator

PR merged -> close

@q2apro
Copy link

q2apro commented Aug 23, 2024

⚠️ 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
Copy link
Contributor Author

timsmid commented Aug 23, 2024

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
Copy link

q2apro commented Aug 23, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants