Skip to content

fix: Update Redirect URI checks for different runtimes #23

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.

Sign up for GitHub

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

Open
LeoAdamek wants to merge 1 commit into
base: master
Choose a base branch
from
+2 −2
Open

fix: Update Redirect URI checks for different runtimes #23

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/LogtoClient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,9 @@ public function handleSignInCallback(): void
throw new LogtoException('Sign-in session not found.');
}

// Some loose checks
// Some loose checks: ensure the host and path of the current request URL matches what was set when initiating the sign-in.
if (
parse_url($signInSession->redirectUri, PHP_URL_HOST) !== ($_SERVER['SERVER_NAME'] ?? null) ||
parse_url($signInSession->redirectUri, PHP_URL_HOST) !== parse_url($_SERVER['REQUEST_URI'], PHP_URL_HOST) ||

@darcyYe darcyYe Mar 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_url($_SERVER['REQUEST_URI'], PHP_URL_HOST) looks problematic here. In a typical PHP request, $_SERVER['REQUEST_URI'] only contains the path and query string, such as /callback?code=..., so PHP_URL_HOST usually resolves to null. That means this change can turn a valid callback into a host mismatch and make handleSignInCallback() throw unexpectedly.

So while this PR is trying to address the RoadRunner case where $_SERVER['SERVER_NAME'] may be missing, the current fix risks breaking the common PHP/FPM flow. In other words, it may solve the original issue in one runtime by introducing a regression in more standard environments.

I think a safer approach would be to fall back to HTTP_HOST / SERVER_NAME, or skip the host check when the runtime does not expose host information, while still keeping the path validation.

Also, could you add tests for this? At minimum, it would be good to cover:

  • the original scenario this PR is trying to fix: SERVER_NAME missing but callback still succeeds when the request matches
  • the normal callback case in a typical PHP environment, to make sure this change does not introduce a regression

@LeoAdamek LeoAdamek May 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll give this a look. 👍🏽

parse_url($signInSession->redirectUri, PHP_URL_PATH) !== parse_url($_SERVER['REQUEST_URI'], PHP_URL_PATH)
) {
throw new LogtoException('The redirect URI in the sign-in session does not match the current request.');
Expand Down