diff --git a/.gitattributes b/.gitattributes index 28a7274..ac24691 100644 --- a/.gitattributes +++ b/.gitattributes @@ -14,4 +14,6 @@ /README.md export-ignore /composer.json export-ignore /composer.lock export-ignore -/.DS_Store export-ignore \ No newline at end of file +/.DS_Store export-ignore +# Tests are not part of the distributed plugin. +/tests export-ignore diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4ee246e..ab05024 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,13 +10,68 @@ env: VERSION_FILE_PATH: './pressable-basic-authentication.php' jobs: + # A separate job from the release below, rather than steps inside it, so the + # checks can fan out across the supported PHP range while the release itself + # still happens exactly once. Putting the matrix on the release job would run + # `git archive` and the publish step once per PHP version, racing to create the + # same tag. + # + # The same span as test.yml, deliberately: a version good enough to block a pull + # request is good enough to block a release, and gating the release on a + # narrower range would let a break on the untested version ship precisely + # because nothing stopped it. + check: + name: Lint and hook registration (PHP ${{ matrix.php }}) + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + # The floor declared in readme.txt, and a current release. The runner's + # ambient PHP is whatever the image ships and tracks neither. + php: [ '8.1', '8.4' ] + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + # This job lints and executes repository PHP, so the GITHUB_TOKEN must + # not be left in the local git config where that code could read it. + persist-credentials: false + + - name: Set up PHP ${{ matrix.php }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: none + + - name: Report PHP version + run: php -v + + - name: Lint PHP files + run: find . -path ./vendor -prune -o -name '*.php' -print0 | xargs -0 -n1 -- php -l + + - name: Hook registration regression test + run: php tests/hook-registration-test.php + build: name: Package Release Project runs-on: ubuntu-latest + # Every matrix leg must pass before anything is published. Two separate + # workflow files have no ordering between them, so test.yml alone could never + # stop a broken commit from shipping a Release -- within one workflow, this + # does. + needs: check + steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 + with: + # This job runs `git archive` (a local ref) and the release script, + # which uses its own explicit GITHUB_TOKEN -- so the checkout token is + # not needed after checkout and must not be left in the git config. + persist-credentials: false - name: Install Ruby uses: ruby/setup-ruby@v1 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..1442198 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,47 @@ +name: Tests + +# Pull requests only. A push to main runs the same checks as the `check` job in +# main.yml, where they additionally gate the release; running them here too would +# just duplicate that. +on: + pull_request: + +permissions: + contents: read + +jobs: + test: + name: Lint and hook registration (PHP ${{ matrix.php }}) + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + # The floor declared in readme.txt, and a current release. The runner's + # ambient PHP is whatever the image ships and tracks neither, so relying + # on it tested a version the plugin does not claim to support. + php: [ '8.1', '8.4' ] + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + # The job lints and executes PHP straight from the checked-out pull + # request, so the GITHUB_TOKEN must not be left in the local git + # config where that code could read it. + persist-credentials: false + + - name: Set up PHP ${{ matrix.php }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: none + + - name: Report PHP version + run: php -v + + - name: Lint PHP files + run: find . -path ./vendor -prune -o -name '*.php' -print0 | xargs -0 -n1 -- php -l + + - name: Hook registration regression test + run: php tests/hook-registration-test.php diff --git a/pressable-basic-authentication.php b/pressable-basic-authentication.php index 91e10c5..30c783e 100644 --- a/pressable-basic-authentication.php +++ b/pressable-basic-authentication.php @@ -8,7 +8,7 @@ /* Plugin Name: Hosting Basic Authentication Description: Forces all users to authenticate using Basic Authentication before accessing any page. -Version: 1.0.4 +Version: 1.0.5 License: GPL2 Text Domain: hosting-basic-authentication */ @@ -30,6 +30,30 @@ public function __construct() { // Hook into WordPress before anything is outputted. add_action( 'plugins_loaded', array( $this, 'init' ), 1 ); + // Logout is handled on `init`, deliberately later than the rest of `init()`. + // wp_logout() fires the `wp_logout` action, and its subscribers may rely on + // constants their own plugin defines in a `plugins_loaded` callback. Firing it + // from `plugins_loaded` priority 1 races that setup, and which plugin wins the + // race depends on load order -- `active_plugins` ordering, anything filtering + // it, and network-activated plugins, which load earlier still. User Switching + // defines its cookie constants that way, so wherever this plugin happens to run + // first, User Switching's `wp_logout` subscriber fatals on a constant it has + // not defined yet. Hooking to `init` drops the dependency on load order + // entirely: every `plugins_loaded` callback has completed by then. + // + // The cost of running this late is that output may already have been sent: + // anything echoed while plugins load -- a `_doing_it_wrong()` notice under + // WP_DEBUG display, a stray BOM -- makes the 401 header and the cookie + // clearing below fail, leaving a 200 with no challenge. wp_logout() itself + // still runs, so the session is destroyed server-side; what is lost is the + // browser-visible half. `plugins_loaded` at PHP_INT_MAX was measured as an + // alternative and degrades identically, because the output is emitted during + // that same hook. The two requirements are in tension: running after every + // `plugins_loaded` callback necessarily means running after any of them may + // have printed, and priority 1 -- the only position that avoids output -- is + // the position that causes the race above. + add_action( 'init', array( $this, 'handle_logout_request' ), 1 ); + // Add filter for logout URL. add_filter( 'logout_url', array( $this, 'modify_logout_url' ), 10, 2 ); @@ -41,36 +65,45 @@ public function __construct() { * Initialize the plugin */ public function init() { - // Skip if we're doing AJAX. - if ( $this->is_ajax_request() ) { + if ( $this->skip_request() ) { return; } - // Skip if we're doing CRON. - if ( $this->is_cron_request() ) { - return; - } + // Redirect from wp-login.php when already authenticated via Basic Auth + $this->maybe_redirect_from_login_page(); - // Skip if we're in CLI mode. - if ( $this->is_cli_request() ) { + // Force authentication. + $this->force_basic_authentication(); + } + + /** + * Handles the Basic Auth logout request. + * + * Hooked to `init` rather than running with the rest of init() on + * `plugins_loaded` -- see the hook registration in the constructor for why. + */ + public function handle_logout_request() { + if ( $this->skip_request() ) { return; } - // Skip requests to excluded endpoints - if ($this->should_skip_auth()) { - return; - } - - // Handle logout request. - if ( isset( $_GET['basic-auth-logout'] ) ) { - $this->handle_basic_auth_logout(); + if ( ! isset( $_GET['basic-auth-logout'] ) ) { + return; } - // Redirect from wp-login.php when already authenticated via Basic Auth - $this->maybe_redirect_from_login_page(); + $this->handle_basic_auth_logout(); + } - // Force authentication. - $this->force_basic_authentication(); + /** + * Whether this request is outside the scope of Basic Authentication. + * + * @return bool + */ + private function skip_request() { + return $this->is_ajax_request() + || $this->is_cron_request() + || $this->is_cli_request() + || $this->should_skip_auth(); } /** @@ -109,6 +142,27 @@ private function force_basic_authentication() { $this->send_auth_headers(); } + // A request asking to log out must still clear the authentication gate above -- + // that is what keeps an anonymous caller from reaching wp_logout() -- but it must + // not be given a session that handle_logout_request() discards moments later on + // `init`. Establishing one fires set_auth_cookie and set_logged_in_cookie on what + // is only ever a logout, which an audit or session-tracking plugin can reasonably + // record as a real login. + // + // Skipping wp_set_current_user() as well as the cookies is correct, not a + // shortcut: execution only reaches here when nobody is logged in -- a live session + // returns above -- so there is no established identity for the following + // wp_logout() to report. It passes whatever get_current_user_id() actually holds, + // which is 0, instead of one this method manufactured moments earlier purely to + // tear it down again. + // + // Placement is load-bearing in both directions. Above the credential handling this + // would skip the 401 as well, readmitting the unauthenticated caller it exists to + // exclude; below the cookie calls it would do nothing at all. + if ( isset( $_GET['basic-auth-logout'] ) ) { + return; + } + // Log the user in programmatically. wp_set_current_user( $user->ID ); wp_set_auth_cookie( $user->ID ); @@ -135,9 +189,10 @@ private function log_failed_auth( $message ) { * @return bool */ private function should_skip_auth() { - // List of endpoints to exclude from Basic Auth + // REST rewrite targets only. xmlrpc.php is deliberately NOT in this list -- + // it is matched on SCRIPT_NAME below, which is authoritative in a way a + // requested path is not. $excluded_endpoints = array( - 'xmlrpc.php', 'wp-json/jetpack', 'wp-json/wp/v2', 'wp-json/wp/v3' @@ -147,19 +202,65 @@ private function should_skip_auth() { $request_uri = $_SERVER['REQUEST_URI'] ?? ''; $script_name = $_SERVER['SCRIPT_NAME'] ?? ''; - // Check if this is a direct xmlrpc.php request + // SCRIPT_NAME is the script the server actually resolved, so this holds + // however the caller spelled the request. if (basename($script_name) === 'xmlrpc.php') { return true; } - // Check all excluded endpoints - foreach ($excluded_endpoints as $endpoint) { - if (strpos($request_uri, $endpoint) !== false) { - return true; + // Everything below matches the REQUESTED path, which is not necessarily + // what the server serves. Three spellings made a gated page look like an + // excluded endpoint, each waiving authentication entirely and allowing a + // full WordPress sign-in with no credentials: + // + // /?x=wp-json/wp/v2 query string read as part of the path + // /xmlrpc.php/../wp-login.php `..` resolved by the server afterwards + // /wp-login.php/wp-json/wp/v2/ trailing segments land in PATH_INFO + // + // The guards below are written against the general fault rather than those + // three shapes: the endpoints above are rewrite targets, so they only mean + // anything when the server routes the request to index.php. Decoded first, + // because the server decodes before it resolves. + // The query and fragment are cut by hand rather than with parse_url(), which + // reads a target beginning `//` as a protocol-relative URL and discards the + // first segment as an authority. `//wp-login.php/wp-json/wp/v2/` parsed to + // `/wp-json/wp/v2/` -- the script vanished, leaving nothing before the + // endpoint to object to -- while the server preserved the target, executed + // wp-login.php and passed the rest as PATH_INFO. That served the login form + // and allowed a full WordPress sign-in with no Basic Auth at all. Collapsing + // the leading slashes afterwards is what makes the resulting path comparable. + $cut = strcspn($request_uri, '?#'); + $request_path = rawurldecode('/' . ltrim(substr($request_uri, 0, $cut), '/')); + $haystack = rtrim($request_path, '/') . '/'; + + // A `.` or `..` segment means the path resolves to something other than what + // it reads as, so nothing in it can be trusted to name a destination. + $has_traversal = false; + + foreach (explode('/', $haystack) as $segment) { + if ('.' === $segment || '..' === $segment) { + $has_traversal = true; + break; + } + } + + // Check all excluded endpoints. Anchored on a slash at both ends so a needle + // matches whole path segments -- `/notwp-json/wp/v2` must not satisfy + // `wp-json/wp/v2` -- but not anchored at the start of the path, because a + // subdirectory or multisite subsite install serves these below a prefix. + if (!$has_traversal) { + foreach ($excluded_endpoints as $endpoint) { + $position = strpos($haystack, '/' . trim($endpoint, '/') . '/'); + + if (false !== $position && !$this->path_runs_another_script(substr($haystack, 0, $position))) { + return true; + } } } - // Check WordPress constants + // Check WordPress constants. xmlrpc.php and the REST bootstrap define these + // themselves, so they are evidence from the request's own execution rather + // than from how it was spelled. if (defined('XMLRPC_REQUEST') && XMLRPC_REQUEST) { return true; } @@ -171,6 +272,42 @@ private function should_skip_auth() { return false; } + /** + * Whether a path prefix names a script the server would execute. + * + * Only what sits BEFORE an excluded endpoint is asked about. A `.php` segment + * there means the server runs that script and hands the endpoint to it as + * PATH_INFO, so the endpoint is decoration on a gated page: + * `/wp-login.php/wp-json/wp/v2/` served the login form and allowed a full + * WordPress sign-in with no Basic Auth at all. + * + * A `.php` segment AFTER the endpoint is part of the REST route itself -- + * `/wp-json/wp/v2/custom-route.php` is routed to index.php and dispatched to + * the REST API -- so an earlier version of this check, which scanned the whole + * path, wrongly demanded authentication for a valid REST request. + * + * Known limitation, accepted deliberately: a WordPress install inside a + * DIRECTORY named `*.php` has a prefix that reads like a script but is not one, + * so REST under it is challenged rather than excluded. Separating the two needs + * either the absence of PATH_INFO as evidence -- trusting a variable's absence, + * which turns this fail-closed edge case into a fail-open one wherever the SAPI + * does not populate it -- or a filesystem lookup that a subdirectory install + * defeats anyway. Refusing a REST request under a pathologically named directory + * is the cheaper error of the two. + * + * @param string $prefix The portion of the request path preceding the endpoint. + * @return bool + */ + private function path_runs_another_script($prefix) { + foreach (explode('/', $prefix) as $segment) { + if ('.php' === strtolower(substr($segment, -4))) { + return true; + } + } + + return false; + } + /** * Sends authentication headers. */ @@ -270,6 +407,15 @@ public function modify_logout_url( $logout_url, $redirect ) { public function maybe_redirect_from_login_page() { global $pagenow; + // A request that asks to log out is never redirected away from the logout. This + // guard only matters on wp-login.php, and only for a logout URL that omits + // `action=logout` -- the URL modify_logout_url() builds always carries it. Since + // the logout moved to `init`, this method now runs first, and without the guard + // such a request would redirect to the home page still logged in, with no error. + if ( isset( $_GET['basic-auth-logout'] ) ) { + return; + } + // Check if we're on the login page and have Basic Auth credentials if ( 'wp-login.php' === $pagenow && ! empty( $_SERVER['PHP_AUTH_USER'] ) && @@ -311,11 +457,22 @@ private function prevent_caching() { /** * Check if the current request is an AJAX request * + * Matched only on the `DOING_AJAX` constant, which WordPress defines itself + * when `admin-ajax.php` runs -- evidence from the request's own execution that + * a caller cannot forge. The `X-Requested-With: XMLHttpRequest` request header + * was deliberately removed: it is set by the caller, so keying an auth waiver + * on it let any anonymous request turn Basic Auth off on any URL, + * `wp-login.php` included, by sending one header -- the same full bypass the + * `should_skip_auth()` rewrite closes for path spellings, reached with a + * header instead. It covered nothing the constant does not: real WordPress + * AJAX runs through admin-ajax.php with `DOING_AJAX` set, and the REST API is + * handled separately by `should_skip_auth()`. A custom endpoint that needs + * access sends Basic Auth like anything else. + * * @return bool */ private function is_ajax_request() { - return ( defined( 'DOING_AJAX' ) && DOING_AJAX ) || - ( ! empty( $_SERVER['HTTP_X_REQUESTED_WITH'] ) && 'xmlhttprequest' === strtolower( $_SERVER['HTTP_X_REQUESTED_WITH'] ) ); + return defined( 'DOING_AJAX' ) && DOING_AJAX; } /** diff --git a/readme.txt b/readme.txt index ebf5922..144252a 100644 --- a/readme.txt +++ b/readme.txt @@ -2,9 +2,9 @@ Contributors: pressable Tags: pressable, basic auth, authentication, security Requires at least: 6.7 -Tested up to: 6.8 +Tested up to: 7.1 Requires PHP: 8.1 -Stable tag: 1.0.4 +Stable tag: 1.0.5 License: GPLv2 or later License URI: http://www.gnu.org/licenses/gpl-2.0.html @@ -40,4 +40,36 @@ No manual installation is necessary.​ == Screenshots ==​ -* Initial release​ \ No newline at end of file +* Initial release​ + +== Changelog == + += 1.0.5 = +* Fixed: Basic Authentication could be bypassed entirely on any URL, with no + credentials, by making a request to a gated page resemble one of the endpoints + excluded from authentication -- by naming one in the query string + (`/?x=wp-json/wp/v2`), by reaching the page through one + (`/xmlrpc.php/../wp-login.php`), or by trailing one after it + (`/wp-login.php/wp-json/wp/v2/`). All three served the login form and allowed a + full WordPress sign-in, as did a request target beginning `//` + (`//wp-login.php/wp-json/wp/v2/`). xmlrpc.php is now matched on the script the + server actually resolved, and a REST endpoint only when nothing preceding it in + the request path names a script the server would execute instead. +* Fixed: sending the `X-Requested-With: XMLHttpRequest` request header waived + Basic Authentication on any URL, a full sign-in included. That header is + caller-supplied, so it no longer counts as AJAX -- only WordPress's own + `DOING_AJAX` constant (set by admin-ajax.php) does. +* Fixed: logging out on a site also running User Switching caused a fatal error. +* Fixed: a logout request no longer reaches wp_logout() without valid credentials. +* Fixed: a logout URL without action=logout is no longer redirected away from the + logout while still signed in. + += 1.0.4 = +* Reverted the 1.0.3 changes pending verification. Functionally identical to 1.0.2. + += 1.0.3 = +* Deferred logout handling to init to avoid the User Switching conflict. Withdrawn + in 1.0.4; re-issued, with the exclusion fix above, in 1.0.5. + += 1.0.2 = +* PHP 8.4 compatibility. diff --git a/tests/hook-registration-test.php b/tests/hook-registration-test.php new file mode 100644 index 0000000..12ab148 --- /dev/null +++ b/tests/hook-registration-test.php @@ -0,0 +1,363 @@ + $hook, 'callback' => $callback, 'priority' => $priority ); +} + +function add_filter( $hook, $callback, $priority = 10, $accepted_args = 1 ) { + $GLOBALS['hooks'][] = array( 'hook' => $hook, 'callback' => $callback, 'priority' => $priority ); +} + +require __DIR__ . '/../pressable-basic-authentication.php'; + +$failures = array(); + +/** + * Records a single assertion. + * + * @param bool $passed Whether the assertion held. + * @param string $description What was asserted. + */ +function check( $passed, $description ) { + global $failures; + + if ( $passed ) { + echo " PASS $description\n"; + return; + } + + $failures[] = $description; + echo " FAIL $description\n"; +} + +/** + * Finds the hook a given method of the plugin class was registered against. + * + * @param string $method Method name. + * @return array|null The recorded registration, or null when unregistered. + */ +function registration_for( $method ) { + foreach ( $GLOBALS['hooks'] as $registration ) { + if ( is_array( $registration['callback'] ) && $registration['callback'][1] === $method ) { + return $registration; + } + } + + return null; +} + +/** + * Returns the source of one method of the plugin class. + * + * @param string $method Method name. + * @return string + */ +function source_of( $method ) { + if ( ! method_exists( 'Pressable_Basic_Auth', $method ) ) { + return ''; + } + + $reflected = new ReflectionMethod( 'Pressable_Basic_Auth', $method ); + $lines = file( $reflected->getFileName() ); + + return implode( + '', + array_slice( $lines, $reflected->getStartLine() - 1, $reflected->getEndLine() - $reflected->getStartLine() + 1 ) + ); +} + +echo "Basic Auth hook registration\n"; + +$logout = registration_for( 'handle_logout_request' ); +check( null !== $logout, 'the logout handler is registered' ); +check( null !== $logout && 'init' === $logout['hook'], "the logout handler is hooked to 'init'" ); + +$boot = registration_for( 'init' ); +check( null !== $boot && 'plugins_loaded' === $boot['hook'], "init() is still hooked to 'plugins_loaded'" ); +check( null !== $boot && 1 === $boot['priority'], 'init() still runs at priority 1, so enforcement stays early' ); + +// Asserted from both sides on purpose. The negative check alone would pass +// vacuously if handle_basic_auth_logout() were renamed -- silently losing the +// coverage this test exists for -- so the positive check pins the name as live. +// Both match on the trailing "(" so a rename to a superstring (…_logout_renamed) +// does not satisfy either check. +check( + false !== strpos( source_of( 'handle_logout_request' ), 'handle_basic_auth_logout(' ), + 'the init callback reaches the logout handler' +); + +check( + false === strpos( source_of( 'init' ), 'handle_basic_auth_logout(' ), + 'init() does not invoke the logout handler, so wp_logout() cannot fire on plugins_loaded' +); + +check( + // Short-circuits so a missing method reports a failure rather than throwing a + // ReflectionException, which would abort the run and hide any later check. + method_exists( 'Pressable_Basic_Auth', 'handle_logout_request' ) + && ( new ReflectionMethod( 'Pressable_Basic_Auth', 'handle_logout_request' ) )->isPublic(), + 'the logout handler is public, as a hook callback must be' +); + +// The guard that skips session setup on a logout request is bracketed rather than +// merely ordered, because both neighbours are hazards. Above the credential +// handling it would suppress the spurious session -- so the symptom it was added +// for would look fixed -- while also skipping the 401, readmitting the +// unauthenticated caller the logout move excluded. Below the cookie calls it would +// be inert. Anchoring on the LAST send_auth_headers() rather than wp_authenticate() +// is deliberate: between the two, a guard still clears every ordering check yet lets +// INVALID credentials bypass the challenge. +$force = source_of( 'force_basic_authentication' ); +$guard_at = strpos( $force, "\$_GET['basic-auth-logout']" ); +$last_challenge = strrpos( $force, 'send_auth_headers(' ); +$cookie_at = strpos( $force, 'wp_set_auth_cookie(' ); + +check( + false !== $guard_at && false !== $last_challenge && $guard_at > $last_challenge, + 'the logout guard sits after every credential challenge, so a logout still requires valid credentials' +); + +check( + false !== $guard_at && false !== $cookie_at && $guard_at < $cookie_at, + 'the logout guard sits before wp_set_auth_cookie(), so a logout establishes no session' +); + +// Position alone would be satisfied by a guard whose body no longer returns. +check( + 1 === preg_match( '/if \(\s*isset\(\s*\$_GET\[.basic-auth-logout.\]\s*\)\s*\)\s*\{\s*return;\s*\}/', $force ), + 'the logout guard actually returns, rather than only appearing in the right place' +); + +/** + * Whether should_skip_auth() would waive Basic Authentication for a request URI. + * + * Invoked for real rather than inspected as source: the method touches only + * basename() and parse_url(), so it runs without WordPress, and a behavioural + * assertion cannot be satisfied by a rewrite that merely looks different. + * + * @param string $uri Value to place in REQUEST_URI. + * @param string $script_name Value to place in SCRIPT_NAME -- the script the server + * resolved, which is what xmlrpc.php is matched on and + * what a PATH_INFO request leaves pointing at the script + * rather than at the endpoint trailing it. + * @return bool + */ +function skips_auth_for( $uri, $script_name = '/index.php' ) { + // $_SERVER is restored and the plugin instance reused so these checks leave no + // state behind: every hook-wiring check above reads $GLOBALS['hooks'] and + // $_SERVER, and a later one appended below this point would otherwise read + // whatever the last URI here happened to set. + static $plugin = null; + + if ( null === $plugin ) { + $plugin = new Pressable_Basic_Auth(); + } + + $original = $_SERVER; + + $_SERVER['REQUEST_URI'] = $uri; + $_SERVER['SCRIPT_NAME'] = $script_name; + + try { + $method = new ReflectionMethod( 'Pressable_Basic_Auth', 'should_skip_auth' ); + $method->setAccessible( true ); + + return (bool) $method->invoke( $plugin ); + } finally { + $_SERVER = $original; + } +} + +// An excluded endpoint appearing in the QUERY STRING must never waive +// authentication. Matching those needles against the whole REQUEST_URI let any +// caller disable the plugin on any URL -- `/?x=wp-json/wp/v2` served the front +// page, and the same string on wp-login.php exposed the login form and allowed a +// full WordPress login with no Basic Auth at all. +foreach ( array( + '/?x=wp-json/wp/v2', + '/?foo=xmlrpc.php', + '/?x=wp-json/jetpack', + '/?x=wp-json/wp/v3', + '/wp-login.php?x=wp-json/wp/v2', + '/?p=1&x=wp-json/wp/v2', +) as $uri ) { + check( false === skips_auth_for( $uri ), "an excluded endpoint in the query string does not waive auth: $uri" ); +} + +// Trailing segments after a real script land in PATH_INFO: the server executes the +// script and hands the rest to it, so an endpoint spelled there is decoration on a +// gated page. `/wp-login.php/wp-json/wp/v2/` served the login form and allowed a +// full WordPress sign-in with no Basic Auth at all. SCRIPT_NAME is passed as the +// server would set it, and the guard holds on the path alone regardless. +foreach ( array( + array( '/wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '/wp-login.php/xmlrpc.php/', '/wp-login.php' ), + array( '/index.php/wp-json/wp/v2/', '/index.php' ), + array( '/wp-login.PHP/wp-json/wp/v2/', '/wp-login.PHP' ), + array( '/sub1/wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '/index.php/wp-json/wp/v2/', '/index.php' ), + array( '/index.PHP/wp-json/wp/v2/', '/index.php' ), + array( '/index.php/hello-world/wp-json/wp/v2/', '/index.php' ), +) as $case ) { + check( false === skips_auth_for( $case[0], $case[1] ), "a PATH_INFO endpoint after a script does not waive auth: {$case[0]}" ); +} + +// The endpoint must not be preceded by a script the server would execute -- but a +// `.php` segment AFTER it is part of the REST route and must still be excluded. +// Scanning the whole path for `.php` instead, as an earlier fix did, wrongly +// demanded authentication for a valid REST request (caught by the Codex pre-PR +// review, verified against a live install: WordPress dispatches +// /wp-json/wp/v2/custom-route.php to the REST API and returns rest_no_route). +foreach ( array( + '/wp-json/wp/v2/custom-route.php', + '/wp-json/wp/v2/media/thing.php', + '/wp-json/jetpack/v4/x.php', + '/wp-json/wp/v3/anything.php', +) as $uri ) { + check( true === skips_auth_for( $uri ), "a .php segment INSIDE a REST route still waives auth: $uri" ); +} + +// A target beginning `//` must not lose its first segment. parse_url() reads such a +// target as protocol-relative and discards that segment as an authority, so +// `//wp-login.php/wp-json/wp/v2/` parsed to `/wp-json/wp/v2/` with nothing left +// before the endpoint -- while the server preserved it, ran wp-login.php and served +// the login form with no Basic Auth at all. Caught by the Codex pre-PR review. +foreach ( array( + array( '//wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '///wp-login.php/wp-json/wp/v2/', '/wp-login.php' ), + array( '//index.php/wp-json/wp/v2/', '/index.php' ), + array( '//wp-login.php/wp-json%2Fwp%2Fv2', '/wp-login.php' ), +) as $case ) { + check( false === skips_auth_for( $case[0], $case[1] ), "a protocol-relative-looking target keeps its first segment: {$case[0]}" ); +} + +// Collapsing those leading slashes must not break the endpoint underneath them. +check( true === skips_auth_for( '//wp-json/wp/v2' ), 'an endpoint behind a doubled leading slash is still excluded' ); + +// A path carrying a traversal segment is not the path the server ends up serving, +// so it must never waive authentication: `/xmlrpc.php/../wp-login.php` resolves to +// wp-login.php while reading as the excluded xmlrpc endpoint, which served the login +// form and allowed a full WordPress login with no Basic Auth at all. +foreach ( array( + '/xmlrpc.php/../wp-login.php', + '/xmlrpc.php/%2e%2e/wp-login.php', + '/xmlrpc%2ephp/../wp-login.php', + '/wp-json/wp/v2/../wp-login.php', + '/wp-json/wp/v2/../../wp-login.php', + '/xmlrpc.php/./../wp-login.php', + '/xmlrpc.php/../', +) as $uri ) { + check( false === skips_auth_for( $uri ), "a traversal segment does not waive auth: $uri" ); +} + +// A needle must match whole path segments, not any substring of one. +check( false === skips_auth_for( '/notwp-json/wp/v2' ), 'a path segment merely ENDING in an excluded endpoint does not waive auth' ); + +// The genuine exclusions still have to work, including below a subdirectory or +// multisite subsite prefix -- which is why the path is not anchored at its start. +foreach ( array( + '/wp-json/wp/v2/posts', + '/wp-json/wp/v2/posts?per_page=1', + '/wp-json/jetpack/v4/whatever', + '/wp-json/wp/v3/anything', + '/sub1/wp-json/wp/v2/posts', +) as $uri ) { + check( true === skips_auth_for( $uri ), "a genuine excluded endpoint still waives auth: $uri" ); +} + +// xmlrpc.php is matched on SCRIPT_NAME, not on the requested path, so it holds +// however the request was spelled -- including below a multisite subsite prefix, +// which the network rewrite resolves back to the root script. +foreach ( array( + array( '/xmlrpc.php', '/xmlrpc.php' ), + array( '/sub1/xmlrpc.php', '/xmlrpc.php' ), + array( '/xmlrpc.php?for=jetpack', '/xmlrpc.php' ), +) as $case ) { + check( true === skips_auth_for( $case[0], $case[1] ), "xmlrpc.php still waives auth via SCRIPT_NAME: {$case[0]}" ); +} + +check( false === skips_auth_for( '/' ), 'an ordinary request is still gated' ); + +// is_ajax_request() is tested directly rather than through skip_request(): this file runs +// under the CLI SAPI, so is_cli_request() (a sibling arm of skip_request()) is always true +// here and would mask everything else. +function is_ajax_for( $x_requested_with ) { + static $plugin = null; + + if ( null === $plugin ) { + $plugin = new Pressable_Basic_Auth(); + } + + $original = isset( $_SERVER['HTTP_X_REQUESTED_WITH'] ) ? $_SERVER['HTTP_X_REQUESTED_WITH'] : null; + if ( null === $x_requested_with ) { + unset( $_SERVER['HTTP_X_REQUESTED_WITH'] ); + } else { + $_SERVER['HTTP_X_REQUESTED_WITH'] = $x_requested_with; + } + + try { + $method = new ReflectionMethod( 'Pressable_Basic_Auth', 'is_ajax_request' ); + $method->setAccessible( true ); + + return (bool) $method->invoke( $plugin ); + } finally { + if ( null === $original ) { + unset( $_SERVER['HTTP_X_REQUESTED_WITH'] ); + } else { + $_SERVER['HTTP_X_REQUESTED_WITH'] = $original; + } + } +} + +// The `X-Requested-With: XMLHttpRequest` request header must NOT count as AJAX. It is +// caller-controlled, so keying an auth waiver on it let any anonymous request turn Basic +// Auth off on any URL, wp-login.php included, by sending one header (caught in review by +// Mitch). is_ajax_request() is the first arm of skip_request(), so a true here is a waiver. +check( false === is_ajax_for( 'XMLHttpRequest' ), 'the X-Requested-With header does not count as AJAX' ); +check( false === is_ajax_for( 'xmlhttprequest' ), 'the X-Requested-With header (lowercased) does not count as AJAX' ); +check( false === is_ajax_for( null ), 'a plain request with no such header is not AJAX' ); + +// Real WordPress AJAX still bypasses: admin-ajax.php defines DOING_AJAX itself, which a +// caller cannot forge. Asserted last, because define() is process-global and irreversible. +define( 'DOING_AJAX', true ); +check( true === is_ajax_for( null ), 'a genuine DOING_AJAX request is still AJAX (bypasses)' ); + +echo "\n"; + +if ( $failures ) { + echo count( $failures ) . " failure(s)\n"; + exit( 1 ); +} + +echo "All checks passed\n"; +exit( 0 );