From c9b579fbc7e0960c7517f7f31b8323df1dd32bed Mon Sep 17 00:00:00 2001 From: latenighthackathon Date: Mon, 20 Apr 2026 22:29:08 -0500 Subject: [PATCH 1/5] fix[faustwp]: (#1872) use home_url() in handle_generate_endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Bedrock-style installs where WordPress core lives under /wp/, site_url('/generate') returns https://example.com/wp/generate but $_SERVER['REQUEST_URI'] is still /generate, so the preg_match in handle_generate_endpoint() never fires and the auth-code redirect silently breaks. home_url() returns the public-facing URL (what REQUEST_URI actually contains), which matches on both standard and Bedrock installs. Per @josephfusco's guidance on #2315, which was closed in favor of this simpler fix — a new filter is not needed, just the correct URL helper. --- .changeset/handle-generate-home-url.md | 5 +++++ plugins/faustwp/includes/auth/callbacks.php | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/handle-generate-home-url.md diff --git a/.changeset/handle-generate-home-url.md b/.changeset/handle-generate-home-url.md new file mode 100644 index 000000000..c53e9b08e --- /dev/null +++ b/.changeset/handle-generate-home-url.md @@ -0,0 +1,5 @@ +--- +"@faustwp/wordpress-plugin": patch +--- + +fix[faustwp]: use home_url() in handle_generate_endpoint so Bedrock-style installs (where WordPress core lives under /wp/) match against the public REQUEST_URI diff --git a/plugins/faustwp/includes/auth/callbacks.php b/plugins/faustwp/includes/auth/callbacks.php index da82180d0..5d33f7867 100644 --- a/plugins/faustwp/includes/auth/callbacks.php +++ b/plugins/faustwp/includes/auth/callbacks.php @@ -22,7 +22,7 @@ * @return void */ function handle_generate_endpoint() { - $search_pattern = ':^' . site_url( '/generate', 'relative' ) . ':'; + $search_pattern = ':^' . home_url( '/generate', 'relative' ) . ':'; if ( ! preg_match( $search_pattern, $_SERVER['REQUEST_URI'] ) ) { // phpcs:ignore WordPress.Security return; From 155f2fa0dce4d074bfd27be8e7331ed401e4f55d Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 13 May 2026 10:08:05 -0400 Subject: [PATCH 2/5] test[faustwp]: regression test for handle_generate_endpoint() Bedrock case (#1872) Adds AuthCallbacksTests.php covering handle_generate_endpoint() with four scenarios: - Standard install reaches wp_safe_redirect (login redirect path). - Bedrock layout (site_url=/wp/x, home_url=/x) still matches REQUEST_URI=/x. This is the regression guard against future reverts to site_url(): verified to fail when callbacks.php is reverted to site_url() and pass with home_url(). - Unrelated REQUEST_URI early-returns without redirecting. - /generate without redirect_uri early-returns. Uses Patchwork to redefine wp_safe_redirect so the bare 'exit;' is bypassed and the captured redirect target can be asserted. Patchwork is already loaded via phpunit.xml.dist's LOAD_PATCHWORK=1. Also adds tests/mu-plugins/mu-bedrock-simulator.php -- a tiny mu-plugin that makes site_url() return /wp/ while home_url() stays at /, for manual browser reproduction inside the docker-compose stack without requiring a full roots/bedrock install. --- .../tests/integration/AuthCallbacksTests.php | 150 ++++++++++++++++++ .../tests/mu-plugins/mu-bedrock-simulator.php | 44 +++++ 2 files changed, 194 insertions(+) create mode 100644 plugins/faustwp/tests/integration/AuthCallbacksTests.php create mode 100644 plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php diff --git a/plugins/faustwp/tests/integration/AuthCallbacksTests.php b/plugins/faustwp/tests/integration/AuthCallbacksTests.php new file mode 100644 index 000000000..c86c65109 --- /dev/null +++ b/plugins/faustwp/tests/integration/AuthCallbacksTests.php @@ -0,0 +1,150 @@ +, home_url() + * stays at /. Mirrors the divergence reported in #1872. + * + * Real Bedrock installs return /wp/ from site_url($path, 'relative') because + * the siteurl option is configured as /wp. We replicate that here by handling + * both relative (just /) and absolute (:///) URL shapes. + */ + private function set_bedrock_layout(): void { + add_filter( + 'site_url', + static function ( $url ) { + // Relative URL (scheme='relative'): / -> /wp/ + if ( '' !== $url && '/' === $url[0] && ( ! isset( $url[1] ) || '/' !== $url[1] ) ) { + return '/wp' . $url; + } + // Absolute URL: :/// -> :///wp/ + return preg_replace( '#(https?://[^/]+)(/.*)?#', '$1/wp$2', $url ); + }, + 10, + 1 + ); + // home_url left at the default ('/'). + } + + /** + * Invoke handle_generate_endpoint() and return the captured wp_safe_redirect + * target, or null if the function returned before reaching the redirect. + * + * @return string|null + */ + private function invoke_handler() { + try { + Auth\handle_generate_endpoint(); + } catch ( \RuntimeException $e ) { + if ( 'test:wp_safe_redirect' === $e->getMessage() ) { + return $GLOBALS['__test_captured_redirect']; + } + throw $e; + } + return null; // function returned before reaching wp_safe_redirect. + } + + /** + * Standard install: REQUEST_URI matches the default search pattern; the function + * proceeds to wp_safe_redirect (login redirect when not authenticated). + */ + public function test_standard_install_matches_and_redirects(): void { + $_SERVER['REQUEST_URI'] = '/generate?redirect_uri=https://frontend.example/'; + $_GET['redirect_uri'] = 'https://frontend.example/'; + + $redirect = $this->invoke_handler(); + + $this->assertNotNull( + $redirect, + 'Standard install must reach wp_safe_redirect.' + ); + $this->assertStringContainsString( 'wp-login.php', $redirect ); + } + + /** + * Bedrock layout: home_url() returns /generate while site_url() returns /wp/generate. + * With the home_url() fix in place, REQUEST_URI=/generate still matches. + * + * This is the regression guard for #1872: if a future change reverts callbacks.php + * to site_url(), this test goes red because the regex would no longer match. + */ + public function test_bedrock_divergence_matches_with_home_url(): void { + $this->set_bedrock_layout(); + $_SERVER['REQUEST_URI'] = '/generate?redirect_uri=https://frontend.example/'; + $_GET['redirect_uri'] = 'https://frontend.example/'; + + $redirect = $this->invoke_handler(); + + $this->assertNotNull( + $redirect, + 'Bedrock layout (site_url=/wp/x, home_url=/x) must still match REQUEST_URI=/x once home_url() is used.' + ); + } + + /** + * Unrelated REQUEST_URI must early-return without touching wp_safe_redirect. + */ + public function test_unrelated_request_uri_is_a_noop(): void { + $_SERVER['REQUEST_URI'] = '/some-other-path'; + $_GET = array(); + + $this->assertNull( $this->invoke_handler() ); + } + + /** + * /generate without a redirect_uri query arg must early-return. + */ + public function test_generate_without_redirect_uri_is_a_noop(): void { + $_SERVER['REQUEST_URI'] = '/generate'; + $_GET = array(); + + $this->assertNull( $this->invoke_handler() ); + } +} diff --git a/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php b/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php new file mode 100644 index 000000000..ee72c424d --- /dev/null +++ b/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php @@ -0,0 +1,44 @@ + while home_url() stays at /. This + * mirrors a Bedrock-style WordPress layout without requiring a full roots/bedrock + * install. + * + * Activate inside the Docker container: + * + * docker compose -f plugins/faustwp/docker-compose.yml exec wordpress sh -c \ + * 'mkdir -p /var/www/html/wp-content/mu-plugins \ + * && cp /var/www/html/wp-content/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php /var/www/html/wp-content/mu-plugins/' + * + * Then visit: + * + * http://localhost:8080/generate?redirect_uri=https://example.test/ + * + * - canary (pre-fix): 404 / silent no-op + * - this branch (post-fix): redirects to wp-login.php + * + * Not loaded by PHPUnit; this file exists for manual browser reproduction only. + * + * @package FaustWP\Tests + */ + +if ( ! defined( 'ABSPATH' ) ) { + exit; +} + +add_filter( + 'site_url', + static function ( $url ) { + // Relative URL (scheme='relative'): / -> /wp/ + if ( '' !== $url && '/' === $url[0] && ( ! isset( $url[1] ) || '/' !== $url[1] ) ) { + return '/wp' . $url; + } + // Absolute URL: :/// -> :///wp/ + return preg_replace( '#(https?://[^/]+)(/.*)?#', '$1/wp$2', $url ); + }, + 10, + 1 +); From 4a595d4a6bcc10dae38f1f48af646951af032219 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 13 May 2026 10:19:59 -0400 Subject: [PATCH 3/5] test[faustwp]: tighten AuthCallbacks regression test fidelity Peer-review polish on the earlier test commit: - Use update_option('siteurl', ...) instead of an add_filter('site_url') to drive Bedrock-style URL divergence. This matches what real Bedrock configures via WP_SITEURL in wp-config.php, so every consumer of get_option('siteurl') and site_url() sees the divergent value, not just callers that go through the site_url filter. Removes the remove_all_filters() tearDown that nuked unrelated core filters. - Replace the RuntimeException + magic-string catch trick with a dedicated RedirectAttempted exception class. Eliminates the risk of accidentally swallowing unrelated runtime errors during the redirect-capture flow. - Tighten assertions on the Bedrock-divergence test: now also asserts the captured redirect contains 'wp-login.php' (matching the standard-install test) and adds two sanity-check assertions that confirm the simulated divergence is actually in place before the handler is invoked. Total assertions in the suite increase from 5 to 8. - Document the simulator mu-plugin's narrower scope vs the test's full fidelity: the mu-plugin filters site_url() output (sufficient for browser reproduction), while the PHPUnit suite updates the option directly. Verified via temporary revert of callbacks.php to site_url(): the Bedrock divergence test now fails with the polished message; restoring home_url() greens the suite on both single-site and multisite. --- .../tests/integration/AuthCallbacksTests.php | 130 ++++++++++-------- .../tests/mu-plugins/mu-bedrock-simulator.php | 26 +++- 2 files changed, 95 insertions(+), 61 deletions(-) diff --git a/plugins/faustwp/tests/integration/AuthCallbacksTests.php b/plugins/faustwp/tests/integration/AuthCallbacksTests.php index c86c65109..cf8458b3d 100644 --- a/plugins/faustwp/tests/integration/AuthCallbacksTests.php +++ b/plugins/faustwp/tests/integration/AuthCallbacksTests.php @@ -12,65 +12,73 @@ /** * Regression tests for handle_generate_endpoint(). * - * Guards the home_url() vs site_url() behavior so the fix for #1872 -- Bedrock-style - * installs where WordPress core lives under /wp/ -- cannot be silently reverted by a - * future change. + * Guards the home_url() vs site_url() behavior in handle_generate_endpoint() so + * the fix for #1872 -- Bedrock-style installs where WordPress core lives under + * /wp/ -- cannot be silently reverted. + * + * The Bedrock case is reproduced by overriding the `siteurl` option directly + * (matching what Bedrock configures via WP_SITEURL in wp-config.php) rather + * than filtering `site_url`. That way every consumer of `get_option('siteurl')` + * and `site_url()` sees the divergent value, which is faithful to real Bedrock. * * @group auth */ class AuthCallbacksTests extends \WP_UnitTestCase { + /** + * Redirect URL captured by the Patchwork-redefined wp_safe_redirect. + * + * Static so the redefine closure can write to it without binding $this. + * + * @var string|null + */ + public static $captured_redirect = null; + + /** + * Original siteurl option value, restored in tearDown so tests stay isolated + * even if WP_UnitTestCase's transaction rollback misses something. + * + * @var string + */ + private $original_siteurl = ''; + public function setUp(): void { parent::setUp(); - $GLOBALS['__test_captured_redirect'] = null; + + self::$captured_redirect = null; + $this->original_siteurl = get_option( 'siteurl' ); // handle_generate_endpoint() calls wp_safe_redirect() and then a bare exit;. - // Redefine wp_safe_redirect via Patchwork to throw, which bypasses the exit - // and lets us assert which redirect target was reached. + // Redefine wp_safe_redirect via Patchwork to throw a dedicated exception, + // which bypasses the exit and lets us assert which redirect target was + // reached. A dedicated exception class avoids the string-matching trap of + // a generic RuntimeException (which could collide with unrelated errors). \Patchwork\redefine( 'wp_safe_redirect', - function ( $location ) { - $GLOBALS['__test_captured_redirect'] = $location; - throw new \RuntimeException( 'test:wp_safe_redirect' ); + static function ( $location ) { + AuthCallbacksTests::$captured_redirect = $location; + throw new RedirectAttempted( (string) $location ); } ); } public function tearDown(): void { \Patchwork\restoreAll(); - unset( - $_SERVER['REQUEST_URI'], - $_GET['redirect_uri'], - $GLOBALS['__test_captured_redirect'] - ); - remove_all_filters( 'site_url' ); - remove_all_filters( 'home_url' ); + update_option( 'siteurl', $this->original_siteurl ); + unset( $_SERVER['REQUEST_URI'], $_GET['redirect_uri'] ); + self::$captured_redirect = null; parent::tearDown(); } /** - * Simulate a Bedrock-style URL layout: site_url() returns /wp/, home_url() - * stays at /. Mirrors the divergence reported in #1872. - * - * Real Bedrock installs return /wp/ from site_url($path, 'relative') because - * the siteurl option is configured as /wp. We replicate that here by handling - * both relative (just /) and absolute (:///) URL shapes. + * Reconfigure the WordPress siteurl option to mirror a Bedrock-style install + * (WP core under /wp/, public site at root). Matches what + * `composer create-project roots/bedrock` configures via WP_SITEURL in + * wp-config.php. */ - private function set_bedrock_layout(): void { - add_filter( - 'site_url', - static function ( $url ) { - // Relative URL (scheme='relative'): / -> /wp/ - if ( '' !== $url && '/' === $url[0] && ( ! isset( $url[1] ) || '/' !== $url[1] ) ) { - return '/wp' . $url; - } - // Absolute URL: :/// -> :///wp/ - return preg_replace( '#(https?://[^/]+)(/.*)?#', '$1/wp$2', $url ); - }, - 10, - 1 - ); - // home_url left at the default ('/'). + private function set_bedrock_siteurl(): void { + $home = (string) get_option( 'home' ); + update_option( 'siteurl', rtrim( $home, '/' ) . '/wp' ); } /** @@ -82,18 +90,15 @@ static function ( $url ) { private function invoke_handler() { try { Auth\handle_generate_endpoint(); - } catch ( \RuntimeException $e ) { - if ( 'test:wp_safe_redirect' === $e->getMessage() ) { - return $GLOBALS['__test_captured_redirect']; - } - throw $e; + } catch ( RedirectAttempted $e ) { + return self::$captured_redirect; } - return null; // function returned before reaching wp_safe_redirect. + return null; } /** - * Standard install: REQUEST_URI matches the default search pattern; the function - * proceeds to wp_safe_redirect (login redirect when not authenticated). + * Standard install: REQUEST_URI matches the default search pattern; the + * function proceeds to wp_safe_redirect (login-redirect branch). */ public function test_standard_install_matches_and_redirects(): void { $_SERVER['REQUEST_URI'] = '/generate?redirect_uri=https://frontend.example/'; @@ -101,22 +106,27 @@ public function test_standard_install_matches_and_redirects(): void { $redirect = $this->invoke_handler(); - $this->assertNotNull( - $redirect, - 'Standard install must reach wp_safe_redirect.' - ); + $this->assertNotNull( $redirect, 'Standard install must reach wp_safe_redirect.' ); $this->assertStringContainsString( 'wp-login.php', $redirect ); } /** - * Bedrock layout: home_url() returns /generate while site_url() returns /wp/generate. - * With the home_url() fix in place, REQUEST_URI=/generate still matches. + * Bedrock-shaped install: the siteurl option includes /wp while home does not. + * With home_url() in callbacks.php, REQUEST_URI=/generate still matches. * - * This is the regression guard for #1872: if a future change reverts callbacks.php - * to site_url(), this test goes red because the regex would no longer match. + * Regression guard for #1872: this test fails if callbacks.php is reverted + * to site_url() because the regex becomes /wp/generate and stops matching + * the public REQUEST_URI. */ public function test_bedrock_divergence_matches_with_home_url(): void { - $this->set_bedrock_layout(); + $this->set_bedrock_siteurl(); + + // Sanity-check the divergence we just configured: site_url carries /wp, + // home_url does not. If these fail, the test environment itself is broken + // before we even exercise the handler. + $this->assertStringEndsWith( '/wp/generate', site_url( '/generate', 'relative' ) ); + $this->assertStringEndsWith( '/generate', home_url( '/generate', 'relative' ) ); + $_SERVER['REQUEST_URI'] = '/generate?redirect_uri=https://frontend.example/'; $_GET['redirect_uri'] = 'https://frontend.example/'; @@ -124,8 +134,9 @@ public function test_bedrock_divergence_matches_with_home_url(): void { $this->assertNotNull( $redirect, - 'Bedrock layout (site_url=/wp/x, home_url=/x) must still match REQUEST_URI=/x once home_url() is used.' + 'Bedrock layout (siteurl includes /wp, home does not) must still match REQUEST_URI=/generate when home_url() is used.' ); + $this->assertStringContainsString( 'wp-login.php', $redirect ); } /** @@ -148,3 +159,12 @@ public function test_generate_without_redirect_uri_is_a_noop(): void { $this->assertNull( $this->invoke_handler() ); } } + +/** + * Thrown by the Patchwork redefine of wp_safe_redirect inside AuthCallbacksTests + * to bypass the bare `exit;` that immediately follows wp_safe_redirect() in + * handle_generate_endpoint(). Keeping this as a dedicated subclass (rather than + * a generic RuntimeException with a magic string) means the catch in + * invoke_handler() can't accidentally swallow unrelated runtime errors. + */ +class RedirectAttempted extends \Exception {} diff --git a/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php b/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php index ee72c424d..4d6043722 100644 --- a/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php +++ b/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php @@ -1,11 +1,23 @@ while home_url() stays at /. This - * mirrors a Bedrock-style WordPress layout without requiring a full roots/bedrock - * install. + * mirrors the URL divergence of a Bedrock-style WordPress layout without + * requiring a full `composer create-project roots/bedrock` install. + * + * Scope and faithfulness: + * + * This mu-plugin filters the OUTPUT of site_url(). It does NOT change the + * underlying `siteurl` option value. That is sufficient for end-to-end browser + * verification of #1872, where the only thing that matters is the regex match + * inside handle_generate_endpoint(). + * + * For test-time fidelity that more closely matches a real Bedrock configuration + * (where every consumer of `get_option('siteurl')` and `site_url()` sees the + * divergent value), the PHPUnit suite updates the siteurl option directly -- + * see tests/integration/AuthCallbacksTests.php::set_bedrock_siteurl(). * * Activate inside the Docker container: * @@ -17,10 +29,12 @@ * * http://localhost:8080/generate?redirect_uri=https://example.test/ * - * - canary (pre-fix): 404 / silent no-op - * - this branch (post-fix): redirects to wp-login.php + * - canary (pre-fix): handler early-returns; WordPress's normal routing kicks in + * - this branch (post-fix): handler matches, redirects to wp-login.php * - * Not loaded by PHPUnit; this file exists for manual browser reproduction only. + * Not loaded by PHPUnit (the testsuite config in phpunit.xml.dist only scans + * ./tests/integration/ and ./tests/unit/). This file exists for manual browser + * reproduction only. * * @package FaustWP\Tests */ From 47c9f7ed6b5028b36c855c4df8cae7ac6e895f39 Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 13 May 2026 10:27:55 -0400 Subject: [PATCH 4/5] test[faustwp]: widen AuthCallbacks coverage and document home_url() filter caveat Three small additions on top of the regression-test commits: 1. Assert that the redirect_uri query arg is preserved through to the wp-login redirect_to param, in both the standard and Bedrock cases. A future change that successfully matched the request but mangled or dropped redirect_uri would previously slip through. 2. Add test_wp_in_subdirectory_install_matches_with_home_url for the Codex-documented 'Giving WordPress its own directory' pattern -- WP core in /wordpress, public site at root. Different prefix from Bedrock, same shape of divergence. Proves the fix is not a /wp-special case. Renames set_bedrock_siteurl() to set_split_install_siteurl($suffix) so both tests share the helper. 3. Document the home_url() filter dependency in handle_generate_endpoint()'s docblock. Multilingual plugins (WPML, Polylang, TranslatePress) that filter home_url() to prepend locale paths can still cause this match to miss for non-locale-prefixed frontend callers. Notes the likely follow-up direction (REST route migration) so the next person to touch this knows what they're inheriting. Suite is now 5 tests / 14 assertions. Verified red-on-revert: both the Bedrock and WP-in-subdirectory tests fail cleanly when callbacks.php is reverted to site_url(); restoring home_url() greens both single-site and multisite suites. --- plugins/faustwp/includes/auth/callbacks.php | 7 +++ .../tests/integration/AuthCallbacksTests.php | 56 ++++++++++++++++--- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/plugins/faustwp/includes/auth/callbacks.php b/plugins/faustwp/includes/auth/callbacks.php index 5d33f7867..1e5b29a80 100644 --- a/plugins/faustwp/includes/auth/callbacks.php +++ b/plugins/faustwp/includes/auth/callbacks.php @@ -19,6 +19,13 @@ * * Generate an authorization code and redirect to the requested url. * + * Note: matches REQUEST_URI against the filtered output of home_url(). Plugins + * that filter home_url() to prepend locale paths (WPML, Polylang, TranslatePress) + * may cause this match to fail when the frontend calls '/generate' directly + * without a locale prefix. If that combination surfaces in support, the + * follow-up will likely migrate '/generate' to a proper REST route so locale + * filters cannot affect the match. + * * @return void */ function handle_generate_endpoint() { diff --git a/plugins/faustwp/tests/integration/AuthCallbacksTests.php b/plugins/faustwp/tests/integration/AuthCallbacksTests.php index cf8458b3d..ed9b77aef 100644 --- a/plugins/faustwp/tests/integration/AuthCallbacksTests.php +++ b/plugins/faustwp/tests/integration/AuthCallbacksTests.php @@ -71,14 +71,19 @@ public function tearDown(): void { } /** - * Reconfigure the WordPress siteurl option to mirror a Bedrock-style install - * (WP core under /wp/, public site at root). Matches what - * `composer create-project roots/bedrock` configures via WP_SITEURL in - * wp-config.php. + * Reconfigure the WordPress siteurl option to a split-install layout where WP + * core lives in a subdirectory of the public site. + * + * Default suffix '/wp' mirrors Bedrock (what `composer create-project roots/bedrock` + * sets via WP_SITEURL in wp-config.php). Pass '/wordpress' for the Codex-documented + * "Giving WordPress its own directory" pattern, or any other prefix to exercise + * other split-install configurations. + * + * @param string $suffix Path appended to the home option to form siteurl. Defaults to '/wp'. */ - private function set_bedrock_siteurl(): void { + private function set_split_install_siteurl( string $suffix = '/wp' ): void { $home = (string) get_option( 'home' ); - update_option( 'siteurl', rtrim( $home, '/' ) . '/wp' ); + update_option( 'siteurl', rtrim( $home, '/' ) . $suffix ); } /** @@ -98,7 +103,10 @@ private function invoke_handler() { /** * Standard install: REQUEST_URI matches the default search pattern; the - * function proceeds to wp_safe_redirect (login-redirect branch). + * function proceeds to wp_safe_redirect (login-redirect branch). The + * redirect_uri query arg from the original request must be carried through + * into the wp-login redirect_to param so the user lands back at the right + * frontend after authenticating. */ public function test_standard_install_matches_and_redirects(): void { $_SERVER['REQUEST_URI'] = '/generate?redirect_uri=https://frontend.example/'; @@ -108,6 +116,11 @@ public function test_standard_install_matches_and_redirects(): void { $this->assertNotNull( $redirect, 'Standard install must reach wp_safe_redirect.' ); $this->assertStringContainsString( 'wp-login.php', $redirect ); + $this->assertStringContainsString( + 'frontend.example', + $redirect, + 'redirect_uri value must be preserved in the wp-login redirect_to param.' + ); } /** @@ -119,7 +132,7 @@ public function test_standard_install_matches_and_redirects(): void { * the public REQUEST_URI. */ public function test_bedrock_divergence_matches_with_home_url(): void { - $this->set_bedrock_siteurl(); + $this->set_split_install_siteurl( '/wp' ); // Sanity-check the divergence we just configured: site_url carries /wp, // home_url does not. If these fail, the test environment itself is broken @@ -137,6 +150,33 @@ public function test_bedrock_divergence_matches_with_home_url(): void { 'Bedrock layout (siteurl includes /wp, home does not) must still match REQUEST_URI=/generate when home_url() is used.' ); $this->assertStringContainsString( 'wp-login.php', $redirect ); + $this->assertStringContainsString( 'frontend.example', $redirect ); + } + + /** + * "Giving WordPress its own directory" install: the Codex-documented pattern + * where WP core is installed in a subdirectory (e.g. /wordpress) but the + * site is served from the public root. Different prefix from Bedrock, same + * shape of divergence -- proves the fix generalises beyond '/wp' specifically. + * + * Ref: https://wordpress.org/documentation/article/giving-wordpress-its-own-directory/ + */ + public function test_wp_in_subdirectory_install_matches_with_home_url(): void { + $this->set_split_install_siteurl( '/wordpress' ); + + $this->assertStringEndsWith( '/wordpress/generate', site_url( '/generate', 'relative' ) ); + $this->assertStringEndsWith( '/generate', home_url( '/generate', 'relative' ) ); + + $_SERVER['REQUEST_URI'] = '/generate?redirect_uri=https://frontend.example/'; + $_GET['redirect_uri'] = 'https://frontend.example/'; + + $redirect = $this->invoke_handler(); + + $this->assertNotNull( + $redirect, + 'WP-in-subdirectory layout (siteurl includes /wordpress, home does not) must still match REQUEST_URI=/generate.' + ); + $this->assertStringContainsString( 'wp-login.php', $redirect ); } /** From 4d19bbd0ccea2acc31a069d00a7069446a95da0d Mon Sep 17 00:00:00 2001 From: Joe Fusco Date: Wed, 13 May 2026 10:40:05 -0400 Subject: [PATCH 5/5] test[faustwp]: stop AuthCallbacksTests leaking REQUEST_URI to other test classes phpunit.xml.dist sets backupGlobals="false", so anything mutated on $_SERVER and $_GET inside one test class persists to the next. Our tearDown was doing 'unset($_SERVER[REQUEST_URI])' as cleanup, which poisoned the cron path that runs during TelemetryCallbacksTests::setUp() (do_action('muplugins_loaded') -> wp-cron -> reads REQUEST_URI). With convertNoticesToExceptions=true, the missing key turned into 9 hard errors on the next test class. Local --filter=AuthCallbacks didn't surface this because TelemetryCallbacks was excluded. CI runs the full suite and caught it on WP 6.5 and Nightly. Fix: snapshot $_SERVER['REQUEST_URI'] and $_GET in setUp; restore them in tearDown. The world is left as we found it, regardless of what the WP test bootstrap or earlier test classes had configured. Verified locally: - Full suite (composer test, single-site + multisite): 126 tests, 291 assertions, all pass. - Regression guard preserved: reverting callbacks.php to site_url() still fails the Bedrock + WP-in-subdirectory tests cleanly. - Without this fix, the same full suite errors 9 times in TelemetryCallbacksTests with 'Undefined array key REQUEST_URI' -- matching the CI failure exactly. --- .../tests/integration/AuthCallbacksTests.php | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/plugins/faustwp/tests/integration/AuthCallbacksTests.php b/plugins/faustwp/tests/integration/AuthCallbacksTests.php index ed9b77aef..428cd8dde 100644 --- a/plugins/faustwp/tests/integration/AuthCallbacksTests.php +++ b/plugins/faustwp/tests/integration/AuthCallbacksTests.php @@ -42,11 +42,31 @@ class AuthCallbacksTests extends \WP_UnitTestCase { */ private $original_siteurl = ''; + /** + * Snapshots of superglobal values taken in setUp and restored in tearDown. + * + * `phpunit.xml.dist` sets backupGlobals="false" so anything we mutate on + * $_SERVER / $_GET persists across test classes. The WP test bootstrap and + * other test classes downstream of this one (e.g. TelemetryCallbacksTests, + * which triggers wp-cron) depend on REQUEST_URI being defined. Snapshot it + * here so our tearDown leaves the world exactly as we found it. + * + * @var array{request_uri: string|null, get: array} + */ + private $original_globals = array( + 'request_uri' => null, + 'get' => array(), + ); + public function setUp(): void { parent::setUp(); self::$captured_redirect = null; $this->original_siteurl = get_option( 'siteurl' ); + $this->original_globals = array( + 'request_uri' => isset( $_SERVER['REQUEST_URI'] ) ? (string) $_SERVER['REQUEST_URI'] : null, + 'get' => $_GET, + ); // handle_generate_endpoint() calls wp_safe_redirect() and then a bare exit;. // Redefine wp_safe_redirect via Patchwork to throw a dedicated exception, @@ -65,7 +85,17 @@ static function ( $location ) { public function tearDown(): void { \Patchwork\restoreAll(); update_option( 'siteurl', $this->original_siteurl ); - unset( $_SERVER['REQUEST_URI'], $_GET['redirect_uri'] ); + + // Restore superglobals to whatever the WP test bootstrap (or a prior test + // class) had them at. Unsetting REQUEST_URI here would break the cron path + // in subsequent test classes -- it leaked into TelemetryCallbacksTests on CI. + if ( null === $this->original_globals['request_uri'] ) { + unset( $_SERVER['REQUEST_URI'] ); + } else { + $_SERVER['REQUEST_URI'] = $this->original_globals['request_uri']; + } + $_GET = $this->original_globals['get']; + self::$captured_redirect = null; parent::tearDown(); }