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..1e5b29a80 100644 --- a/plugins/faustwp/includes/auth/callbacks.php +++ b/plugins/faustwp/includes/auth/callbacks.php @@ -19,10 +19,17 @@ * * 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() { - $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; diff --git a/plugins/faustwp/tests/integration/AuthCallbacksTests.php b/plugins/faustwp/tests/integration/AuthCallbacksTests.php new file mode 100644 index 000000000..428cd8dde --- /dev/null +++ b/plugins/faustwp/tests/integration/AuthCallbacksTests.php @@ -0,0 +1,240 @@ +} + */ + 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, + // 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', + static function ( $location ) { + AuthCallbacksTests::$captured_redirect = $location; + throw new RedirectAttempted( (string) $location ); + } + ); + } + + public function tearDown(): void { + \Patchwork\restoreAll(); + update_option( 'siteurl', $this->original_siteurl ); + + // 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(); + } + + /** + * 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_split_install_siteurl( string $suffix = '/wp' ): void { + $home = (string) get_option( 'home' ); + update_option( 'siteurl', rtrim( $home, '/' ) . $suffix ); + } + + /** + * 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 ( RedirectAttempted $e ) { + return self::$captured_redirect; + } + return null; + } + + /** + * Standard install: REQUEST_URI matches the default search pattern; the + * 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/'; + $_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 ); + $this->assertStringContainsString( + 'frontend.example', + $redirect, + 'redirect_uri value must be preserved in the wp-login redirect_to param.' + ); + } + + /** + * Bedrock-shaped install: the siteurl option includes /wp while home does not. + * With home_url() in callbacks.php, REQUEST_URI=/generate still matches. + * + * 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_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 + // 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/'; + + $redirect = $this->invoke_handler(); + + $this->assertNotNull( + $redirect, + '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 ); + } + + /** + * 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() ); + } +} + +/** + * 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 new file mode 100644 index 000000000..4d6043722 --- /dev/null +++ b/plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php @@ -0,0 +1,58 @@ + while home_url() stays at /. This + * 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: + * + * 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): handler early-returns; WordPress's normal routing kicks in + * - this branch (post-fix): handler matches, redirects to wp-login.php + * + * 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 + */ + +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 +);