Skip to content

Conversation

@ioigoume
Copy link
Contributor

@ioigoume ioigoume commented Jan 29, 2026

Improve HTTP::getSelfURL() path detection and URL reconstruction

HTTP::getSelfURL() mis-reconstructs the current URL when SimpleSAMLphp is accessed through a rewritten path such as RewriteRule ^/cas/login(.*) /${SSP_APACHE_ALIAS}module.php/casserver/login.php$1 [PT]. In this scenario the public URL is /cas/login?service=..., while Apache internally rewrites it to /simplesaml/module.php/casserver/login.php?service=.... The old implementation assumes that the script-relative filesystem path (for example module.php/casserver/login.php) appears verbatim in $_SERVER['REQUEST_URI'], searches for that substring in the entire REQUEST_URI, and rebuilds the URL by stitching together getBaseURL(), that matched path, and the remainder of REQUEST_URI. With mod_rewrite, REQUEST_URI contains only the external path (/cas/login?...), so the expected script path is not present, or may only appear by accident inside query parameters. This causes getSelfURL() to either fall back to an incorrect URL or to construct one that points to module.php/... instead of /cas/login, or that has a malformed path/query combination, breaking redirects and return URLs that rely on getSelfURL() to reflect the actual public URL.

@ioigoume ioigoume force-pushed the http-getselfurl-robust-path-and-query branch from b043a77 to 9b816fd Compare January 29, 2026 09:48
@ioigoume
Copy link
Contributor Author

Relates to simplesamlphp-module-casserver Pull Request 65

@ioigoume ioigoume changed the title http-getselfurl-robust-path-and-query BUGFIX: http-getselfurl-robust-path-and-query Jan 29, 2026
$hostname = $this->getServerHost();
$port = $this->getServerPort();
}
return $protocol . '://' . $hostname . $port . $_SERVER['REQUEST_URI'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this line need to change? The REQUEST_URI is not being parsed or interpreted.

Copy link
Contributor Author

@ioigoume ioigoume Jan 30, 2026

Choose a reason for hiding this comment

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

@pradtke I changed it for consistency. I wanted all $_SERVER accesses to be grouped at the top of the method and then reconstruct the needed parts from those parsed values. I will revert this part.

@pradtke
Copy link
Contributor

pradtke commented Jan 29, 2026

I ran a test in our environment where we rewrite the path /cas/login to /module.php/casserver/login.php. This bug previously triggered if any of the query parameters contained the text module.php (such as when we test the SSP CAS client module against the CAS server module). With this patch I no longer have that issue.

@ioigoume I think we would need the test case expanded. Below are some of the SERVER variables from when I tested.

  "SCRIPT_URI": "https://tr-monitor-okta2.qa.athena-institute.net/cas/login",
  "HTTPS": "on",
  "HTTP_HOST": "tr-monitor-okta2.qa.athena-institute.net",
  "SCRIPT_NAME": "/module.php",
  "PATH_TRANSLATED": "/var/simplesamlphp/public/casserver/login.php",
  "PHP_SELF": "/module.php/casserver/login.php",
  "QUERY_STRING": "service=https%3A%2F%2Fcas-test-bridge.bridge.qa.cirrusidentity.com%2Fmodule.php%2Fcas%2Flinkback.php%3FstateId%3D_somestate",
  "REQUEST_URI": "/cas/login?service=https%3A%2F%2Fcas-test-bridge.bridge.qa.cirrusidentity.com%2Fmodule.php%2Fcas%2Flinkback.php%3FstateId%3D_somestate"

…Add specific test for the modrewrite scenario.
@ioigoume ioigoume requested a review from pradtke January 30, 2026 11:28
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 this pull request may close these issues.

2 participants