From 4aa194739e5ff086225e56ca433a065c4be88f7c Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Mon, 28 Sep 2026 22:30:58 +0100 Subject: [PATCH 1/5] Multisite: Require delete_sites capability to delete a site via REST, refine sites permission checks. `WP_REST_Sites_Controller::delete_item_permissions_check()` only checked the `delete_site` meta capability, which core maps unconditionally to `manage_options` and is not scoped to the target site. This let any per-site Administrator delete an arbitrary site network-wide, unlike core's own network admin Sites screen and this controller's own create/update checks, which correctly require the super-admin-only `create_sites`/`manage_sites` capabilities. - Require `delete_sites` (in addition to the existing `delete_site` meta cap check) before a site can be deleted, matching the layering `wp-admin/network/sites.php` already uses. - Return a specific `rest_cannot_create` error from `create_item_permissions_check()` instead of a bare boolean, matching `WP_REST_Posts_Controller` and this class's own `rest_cannot_edit`/ `rest_cannot_delete` errors. - Extract `check_multisite_support()`, `check_delete_permission()`, and reuse `check_edit_permission()` to de-duplicate capability checks that were repeated across every permission-check method. - `get_item_permissions_check()`: a site's default/view-context data is now readable by members of that site (or a super admin), not the general public; other contexts still require `manage_sites`. - Add regression tests for the capability gap, the multisite-check ordering, and the view/edit context permission boundaries. Props spacedmonkey. Fixes #40365. Co-Authored-By: Claude Sonnet 5 --- .../class-wp-rest-sites-controller.php | 107 ++++--- .../tests/rest-api/wpRestSitesController.php | 288 ++++++++++++++++++ 2 files changed, 347 insertions(+), 48 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php index 7c604035e34e4..9f330f8457030 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php @@ -118,16 +118,12 @@ public function register_routes() { * @return WP_Error|bool True if the request has read access, error object otherwise. */ public function get_items_permissions_check( $request ) { - - if ( 0 === get_current_user_id() ) { - return false; - } - - if ( ! is_multisite() ) { - return new WP_Error( 'rest_multisite_not_installed', __( 'Multisite is not installed' ), array( 'status' => 400 ) ); + $multisite_support = $this->check_multisite_support(); + if ( is_wp_error( $multisite_support ) ) { + return $multisite_support; } - if ( current_user_can( 'manage_sites' ) ) { + if ( $this->check_edit_permission() ) { return true; } @@ -395,20 +391,23 @@ public function get_user_site_ids( $user_id ) { * @return WP_Error|bool True if the request has read access for the item, error object otherwise. */ public function get_item_permissions_check( $request ) { + $multisite_support = $this->check_multisite_support(); + if ( is_wp_error( $multisite_support ) ) { + return $multisite_support; + } + $site = $this->get_site( $request['id'] ); if ( is_wp_error( $site ) ) { return $site; } - if ( 0 === get_current_user_id() ) { - return false; - } + $context = ! empty( $request['context'] ) ? $request['context'] : 'view'; - if ( ! is_multisite() ) { - return new WP_Error( 'rest_multisite_not_installed', __( 'Multisite is not installed' ), array( 'status' => 400 ) ); + if ( 'view' === $context && is_user_member_of_blog( get_current_user_id(), (int) $site->blog_id ) ) { + return true; } - if ( ! empty( $request['context'] ) && 'edit' === $request['context'] && ! current_user_can( 'manage_sites' ) ) { + if ( ! $this->check_edit_permission() ) { return new WP_Error( 'rest_forbidden_context', __( 'Sorry, you are not allowed to view sites.' ), array( 'status' => rest_authorization_required_code() ) ); } @@ -444,15 +443,16 @@ public function get_item( $request ) { * @return WP_Error|bool True if the request has access to create items, error object otherwise. */ public function create_item_permissions_check( $request ) { - if ( 0 === get_current_user_id() ) { - return false; + $multisite_support = $this->check_multisite_support(); + if ( is_wp_error( $multisite_support ) ) { + return $multisite_support; } - if ( ! is_multisite() ) { - return new WP_Error( 'rest_multisite_not_installed', __( 'Multisite is not installed' ), array( 'status' => 400 ) ); + if ( ! current_user_can( 'create_sites' ) ) { + return new WP_Error( 'rest_cannot_create', __( 'Sorry, you are not allowed to create sites.' ), array( 'status' => rest_authorization_required_code() ) ); } - return current_user_can( 'create_sites' ); + return true; } /** @@ -542,8 +542,7 @@ public function create_item( $request ) { return $fields_update; } - $context = current_user_can( 'manage_sites' ) ? 'edit' : 'view'; - + $context = ! empty( $request['context'] ) ? $request['context'] : 'view'; $request->set_param( 'context', $context ); $response = $this->prepare_item_for_response( $site, $request ); @@ -564,20 +563,17 @@ public function create_item( $request ) { * @return WP_Error|bool True if the request has access to update the item, error object otherwise. */ public function update_item_permissions_check( $request ) { + $multisite_support = $this->check_multisite_support(); + if ( is_wp_error( $multisite_support ) ) { + return $multisite_support; + } + $site = $this->get_site( $request['id'] ); if ( is_wp_error( $site ) ) { return $site; } - if ( 0 === get_current_user_id() ) { - return false; - } - - if ( ! is_multisite() ) { - return new WP_Error( 'rest_multisite_not_installed', __( 'Multisite is not installed' ), array( 'status' => 400 ) ); - } - - if ( ! $this->check_edit_permission( $site ) ) { + if ( ! $this->check_edit_permission() ) { return new WP_Error( 'rest_cannot_edit', __( 'Sorry, you are not allowed to edit this site.' ), array( 'status' => rest_authorization_required_code() ) ); } @@ -661,20 +657,17 @@ public function update_item( $request ) { * @return WP_Error|bool True if the request has access to delete the item, error object otherwise. */ public function delete_item_permissions_check( $request ) { + $multisite_support = $this->check_multisite_support(); + if ( is_wp_error( $multisite_support ) ) { + return $multisite_support; + } + $site = $this->get_site( $request['id'] ); if ( is_wp_error( $site ) ) { return $site; } - if ( 0 === (int) get_current_user_id() ) { - return false; - } - - if ( ! is_multisite() ) { - return new WP_Error( 'rest_multisite_not_installed', __( 'Multisite is not installed' ), array( 'status' => 400 ) ); - } - - if ( ! current_user_can( 'delete_site', $request['id'] ) ) { + if ( ! $this->check_delete_permission( $site ) ) { return new WP_Error( 'rest_cannot_delete', __( 'Sorry, you are not allowed to delete this site.' ), array( 'status' => rest_authorization_required_code() ) ); } @@ -730,7 +723,7 @@ public function delete_item( $request ) { } } - update_blog_status( $blog_id, 'deleted', 1 ); + update_blog_status( $blog_id, 'deleted', '1' ); /** This action is documented in wp-includes/ms-site.php */ do_action_deprecated( 'deleted_blog', array( $blog_id, false ), '5.1.0' ); @@ -1546,22 +1539,40 @@ public function get_collection_params() { } /** - * Checks if a site can be edited or deleted. + * Checks if the current user can edit sites. + * + * @since 7.2.0 + * + * @return bool Whether the current user can edit sites. + */ + protected function check_edit_permission() { + return current_user_can( 'manage_sites' ); + } + + /** + * Checks if a site can be deleted. * * @since 7.2.0 * * @param object $site Site object. - * @return bool Whether the site can be edited or deleted. + * @return bool Whether the site can be deleted. */ - protected function check_edit_permission( $site ) { - if ( 0 === (int) get_current_user_id() ) { - return false; - } + protected function check_delete_permission( $site ) { + return current_user_can( 'delete_sites' ) && current_user_can( 'delete_site', $site->blog_id ); + } + /** + * Checks that multisite is enabled, as this controller requires it. + * + * @since 7.2.0 + * + * @return true|WP_Error True if multisite is enabled, WP_Error otherwise. + */ + protected function check_multisite_support() { if ( ! is_multisite() ) { - return false; + return new WP_Error( 'rest_multisite_not_installed', __( 'Multisite is not installed' ), array( 'status' => 400 ) ); } - return current_user_can( 'manage_sites' ); + return true; } } diff --git a/tests/phpunit/tests/rest-api/wpRestSitesController.php b/tests/phpunit/tests/rest-api/wpRestSitesController.php index 621e534a9f86f..a274a4b3a0e93 100644 --- a/tests/phpunit/tests/rest-api/wpRestSitesController.php +++ b/tests/phpunit/tests/rest-api/wpRestSitesController.php @@ -126,6 +126,52 @@ public function test_get_items_no_ms() { $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); } + /** + * The multisite check runs before the logged-in check, so a logged-out + * request against a single-site install reports the environment problem, + * not a generic authentication error. + * + * @ticket 40365 + * @covers ::get_items_permissions_check + * @group ms-excluded + */ + public function test_get_items_no_ms_when_logged_out() { + $request = new WP_REST_Request( 'GET', '/wp/v2/sites' ); + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); + } + + /** + * A logged-out request with no `user` filter has neither `manage_sites` + * nor a matching own-user filter, so it's still forbidden. + * + * @ticket 40365 + * @covers ::get_items_permissions_check + * @group ms-required + */ + public function test_get_items_forbidden_when_logged_out() { + $request = new WP_REST_Request( 'GET', '/wp/v2/sites' ); + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_forbidden_context', $response, 401 ); + } + + /** + * The collection is intentionally readable while logged out, as long + * as the request is scoped to "my sites" via the `user` filter. + * + * @ticket 40365 + * @covers ::get_items_permissions_check + * @group ms-required + */ + public function test_get_items_me_filter_is_readable_when_logged_out() { + $request = new WP_REST_Request( 'GET', '/wp/v2/sites' ); + $request->set_param( 'user', 'me' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + } + /** * @ticket 40365 * @covers ::get_item @@ -140,6 +186,133 @@ public function test_get_item_no_ms() { $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); } + /** + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-excluded + */ + public function test_get_item_no_ms_when_logged_out() { + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/1' ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); + } + + /** + * A logged-out request can't be a site member and lacks `manage_sites`, + * so it's forbidden even in the default view context. + * + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-required + */ + public function test_get_item_forbidden_when_logged_out() { + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/1' ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden_context', $response, 401 ); + } + + /** + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-required + */ + public function test_get_item_edit_context_forbidden_when_logged_out() { + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/1' ); + $request->set_param( 'context', 'edit' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden_context', $response, 401 ); + } + + /** + * A member of the site (even without `manage_sites`) can view it in + * the default view context. + * + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-required + */ + public function test_get_item_view_context_allowed_for_site_member() { + $blog_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create( array( 'role' => 'subscriber' ) ); + add_user_to_blog( $blog_id, $user_id, 'subscriber' ); + + wp_set_current_user( $user_id ); + + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/' . $blog_id ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + } + + /** + * A logged-in user who is neither a super admin nor a member of the + * site is forbidden from viewing it, even in the default view context. + * + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-required + */ + public function test_get_item_view_context_forbidden_for_non_member() { + $blog_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create( array( 'role' => 'subscriber' ) ); + + wp_set_current_user( $user_id ); + + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/' . $blog_id ); + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden_context', $response, 403 ); + } + + /** + * Site membership only grants the default view context; edit context + * still requires `manage_sites`. + * + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-required + */ + public function test_get_item_edit_context_forbidden_for_site_member() { + $blog_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create( array( 'role' => 'subscriber' ) ); + add_user_to_blog( $blog_id, $user_id, 'subscriber' ); + + wp_set_current_user( $user_id ); + + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/' . $blog_id ); + $request->set_param( 'context', 'edit' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden_context', $response, 403 ); + } + + /** + * A logged-in user who is neither a super admin nor a member of the + * site is still forbidden from viewing it in edit context. + * + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-required + */ + public function test_get_item_edit_context_forbidden_for_non_member() { + $blog_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create( array( 'role' => 'subscriber' ) ); + + wp_set_current_user( $user_id ); + + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/' . $blog_id ); + $request->set_param( 'context', 'edit' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden_context', $response, 403 ); + } + /** * An unknown ID is a 404, not an empty site. * @@ -347,6 +520,36 @@ public function test_create_item_no_ms() { $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); } + /** + * @ticket 40365 + * @covers ::create_item_permissions_check + * @group ms-excluded + */ + public function test_create_item_no_ms_when_logged_out() { + $request = new WP_REST_Request( 'POST', '/wp/v2/sites' ); + $request->set_param( 'domain', WP_TESTS_DOMAIN ); + $request->set_param( 'path', '/tempor/' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); + } + + /** + * @ticket 40365 + * @covers ::create_item_permissions_check + * @group ms-required + */ + public function test_create_item_requires_being_logged_in() { + $request = new WP_REST_Request( 'POST', '/wp/v2/sites' ); + $request->set_param( 'domain', WP_TESTS_DOMAIN ); + $request->set_param( 'path', '/tempor/' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_cannot_create', $response, 401 ); + } + /** * @ticket 40365 * @covers ::update_item @@ -364,6 +567,37 @@ public function test_update_item_no_ms() { $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); } + /** + * @ticket 40365 + * @covers ::update_item_permissions_check + * @group ms-excluded + */ + public function test_update_item_no_ms_when_logged_out() { + $request = new WP_REST_Request( 'PUT', '/wp/v2/sites/1' ); + $request->set_param( 'path', '/incididunt/' ); + $request->set_param( 'mature', 1 ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); + } + + /** + * @ticket 40365 + * @covers ::update_item_permissions_check + * @group ms-required + */ + public function test_update_item_requires_being_logged_in() { + $blog_id = self::factory()->blog->create(); + + $request = new WP_REST_Request( 'PUT', '/wp/v2/sites/' . $blog_id ); + $request->set_param( 'path', '/incididunt/' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_cannot_edit', $response, 401 ); + } + /** * @ticket 40365 * @covers ::delete_item @@ -404,6 +638,37 @@ public function test_delete_item_no_ms() { $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); } + /** + * @ticket 40365 + * @covers ::delete_item_permissions_check + * @group ms-excluded + */ + public function test_delete_item_no_ms_when_logged_out() { + $request = new WP_REST_Request( 'DELETE', '/wp/v2/sites/1' ); + $request->set_param( 'force', true ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_multisite_not_installed', $response, 400 ); + } + + /** + * @ticket 40365 + * @covers ::delete_item_permissions_check + * @group ms-required + */ + public function test_delete_item_requires_being_logged_in() { + $blog_id = self::factory()->blog->create(); + + $request = new WP_REST_Request( 'DELETE', '/wp/v2/sites/' . $blog_id ); + $request->set_param( 'force', true ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_cannot_delete', $response, 401 ); + $this->assertNotNull( get_site( $blog_id ) ); + } + /** * Deleting a site drops its tables. * @@ -482,6 +747,29 @@ public function test_delete_main_site_is_not_allowed() { $this->assertNotNull( get_site( $main_site_id ) ); } + /** + * A regular site administrator does not have the network-level + * capability required to delete a site. + * + * @ticket 40365 + * @covers ::delete_item_permissions_check + * @group ms-required + */ + public function test_delete_item_requires_delete_sites_cap() { + $blog_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); + + wp_set_current_user( $user_id ); + + $request = new WP_REST_Request( 'DELETE', '/wp/v2/sites/' . $blog_id ); + $request->set_param( 'force', true ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_cannot_delete', $response, rest_authorization_required_code() ); + $this->assertNotNull( get_site( $blog_id ) ); + } + /** * @ticket 40365 * @covers ::prepare_item_for_response From 1a1c422304ba0d2f1b2755b11dcf6bf2bbf35737 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Mon, 28 Sep 2026 22:39:20 +0100 Subject: [PATCH 2/5] Tests: Fix WordPress coding standards violations in wpRestSitesController.php. phpcs flagged 34 WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned violations across the new test methods, auto-fixed via phpcbf. No behavioral change. Co-Authored-By: Claude Sonnet 5 --- .../tests/rest-api/wpRestSitesController.php | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/tests/phpunit/tests/rest-api/wpRestSitesController.php b/tests/phpunit/tests/rest-api/wpRestSitesController.php index a274a4b3a0e93..f4c67c056802f 100644 --- a/tests/phpunit/tests/rest-api/wpRestSitesController.php +++ b/tests/phpunit/tests/rest-api/wpRestSitesController.php @@ -1460,35 +1460,35 @@ private function normalize_domain_for_storage( $domain ) { */ public function data_domain_validation() { return array( - 'plain hostname' => array( 'example.org', true ), - 'subdomain' => array( 'sub.example.org', true ), - 'hostname with port' => array( 'example.org:8080', true ), + 'plain hostname' => array( 'example.org', true ), + 'subdomain' => array( 'sub.example.org', true ), + 'hostname with port' => array( 'example.org:8080', true ), // The shape produced by wp-admin/network/site-new.php for a subdomain install // when the network domain itself carries a port, e.g. "blogname.localhost:8889". 'subdomain of a network domain with a port' => array( 'blogname.localhost:8889', true ), - 'ipv4' => array( '198.51.100.10', true ), - 'ipv4 with port' => array( '198.51.100.10:8080', true ), - 'bare ipv6 loopback, no port' => array( '::1', true ), - 'bare ipv6, no port' => array( '2001:db8::1', true ), - 'bare full-length ipv6, no port' => array( '2001:0db8:0000:0000:0000:0000:0000:0001', true ), - 'bare ipv4-mapped ipv6, no port' => array( '::ffff:192.0.2.1', true ), + 'ipv4' => array( '198.51.100.10', true ), + 'ipv4 with port' => array( '198.51.100.10:8080', true ), + 'bare ipv6 loopback, no port' => array( '::1', true ), + 'bare ipv6, no port' => array( '2001:db8::1', true ), + 'bare full-length ipv6, no port' => array( '2001:0db8:0000:0000:0000:0000:0000:0001', true ), + 'bare ipv4-mapped ipv6, no port' => array( '::ffff:192.0.2.1', true ), // Ambiguous: no brackets to separate a port, so the whole string is // parsed as a literal (8-group, after :: expansion) IPv6 address. 'ambiguous unbracketed ipv6 with trailing digits treated as address' => array( '2001:db8::1:8080', true ), // Bracketed IPv6 is not supported at all: brackets aren't valid // hostname or bare-IP characters, so any leading `[` is rejected outright. - 'bracketed ipv6 is rejected' => array( '[2001:db8::1]', false ), - 'bracketed ipv6 with port is rejected' => array( '[2001:db8::1]:8080', false ), - 'bracketed ipv6 loopback is rejected' => array( '[::1]', false ), - 'empty brackets are rejected' => array( '[]', false ), + 'bracketed ipv6 is rejected' => array( '[2001:db8::1]', false ), + 'bracketed ipv6 with port is rejected' => array( '[2001:db8::1]:8080', false ), + 'bracketed ipv6 loopback is rejected' => array( '[::1]', false ), + 'empty brackets are rejected' => array( '[]', false ), 'unbracketed ipv6-shaped string with a port-like trailing group is rejected' => array( '1:2:3:4:5:6:7:8:9', false ), - 'empty domain' => array( '', false ), - 'domain with a space' => array( 'example org', false ), - 'domain with a scheme' => array( 'http://example.org', false ), - 'leading hyphen label' => array( '-example.org', false ), - 'empty label' => array( 'example..org', false ), - 'port out of range' => array( 'example.org:99999', false ), - 'non numeric port' => array( 'example.org:abc', false ), + 'empty domain' => array( '', false ), + 'domain with a space' => array( 'example org', false ), + 'domain with a scheme' => array( 'http://example.org', false ), + 'leading hyphen label' => array( '-example.org', false ), + 'empty label' => array( 'example..org', false ), + 'port out of range' => array( 'example.org:99999', false ), + 'non numeric port' => array( 'example.org:abc', false ), ); } @@ -1564,20 +1564,20 @@ public function test_update_item_path_validation( $path, $is_valid ) { */ public function data_path_validation() { return array( - 'root path' => array( '/', true ), - 'single segment' => array( '/tempor/', true ), - 'nested segments' => array( '/parent/child/', true ), - 'segment with apostrophe' => array( "/o'brien/", true ), - 'segment with punctuation' => array( '/with-hyphen_and.dot~tilde/', true ), - 'percent encoded segment' => array( '/percent%20encoded/', true ), - 'empty path' => array( '', false ), - 'no leading slash' => array( 'no-leading-slash/', false ), - 'no trailing slash' => array( '/no-trailing-slash', false ), - 'double slash' => array( '//double-slash//', false ), - 'path with a space' => array( '/with space/', false ), - 'path with a query string' => array( '/with?query/', false ), - 'path with a fragment' => array( '/with#fragment/', false ), - 'path with a double quote' => array( '/with"quote/', false ), + 'root path' => array( '/', true ), + 'single segment' => array( '/tempor/', true ), + 'nested segments' => array( '/parent/child/', true ), + 'segment with apostrophe' => array( "/o'brien/", true ), + 'segment with punctuation' => array( '/with-hyphen_and.dot~tilde/', true ), + 'percent encoded segment' => array( '/percent%20encoded/', true ), + 'empty path' => array( '', false ), + 'no leading slash' => array( 'no-leading-slash/', false ), + 'no trailing slash' => array( '/no-trailing-slash', false ), + 'double slash' => array( '//double-slash//', false ), + 'path with a space' => array( '/with space/', false ), + 'path with a query string' => array( '/with?query/', false ), + 'path with a fragment' => array( '/with#fragment/', false ), + 'path with a double quote' => array( '/with"quote/', false ), ); } From 8f76f4f3ef754886db9707bcf453822d60c351a0 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Mon, 28 Sep 2026 22:43:56 +0100 Subject: [PATCH 3/5] Tests: Regenerate wp-api-generated.js fixture to include network arg default. The REST API index schema fixture was missing "default": 1 on the sites endpoint's `network` arg (both the create and update route args), which current_user_can()/get_current_network_id() output but the committed fixture predates. Pre-existing drift on this branch, unrelated to the permission-check changes in this PR, caught by CI's generated-file diff check. Co-Authored-By: Claude Sonnet 5 --- tests/qunit/fixtures/wp-api-generated.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qunit/fixtures/wp-api-generated.js b/tests/qunit/fixtures/wp-api-generated.js index 99580f63b4e27..f54dfc053279d 100644 --- a/tests/qunit/fixtures/wp-api-generated.js +++ b/tests/qunit/fixtures/wp-api-generated.js @@ -11089,6 +11089,7 @@ mockedApiResponse.Schema = { ], "args": { "network": { + "default": 1, "description": "The site's network ID. Default is the current network ID.", "type": "integer", "required": false @@ -11208,6 +11209,7 @@ mockedApiResponse.Schema = { "required": false }, "network": { + "default": 1, "description": "The site's network ID. Default is the current network ID.", "type": "integer", "required": false From 0edb70ee8c593dfb41e7e79dab28ec18a3b5a6e1 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Mon, 28 Sep 2026 23:28:18 +0100 Subject: [PATCH 4/5] Multisite: Fix follow-up permission-check inconsistencies in the sites REST controller. A wrapido-based manual QA pass against 4aa194739e (which fixed the network-wide-delete and view-context info-disclosure bugs) surfaced four smaller inconsistencies introduced or left by that refactor: - `get_item_permissions_check()`'s membership bypass only matched the literal `view` context, so a site member got 200 on the default view context but 403 on `context=embed` - stricter than `view` even though embed is normally a strict subset of it. Now also allows `embed`. - `get_items_permissions_check()`'s fallback error said "not allowed to edit sites" for what is a read (GET) endpoint - copy-pasted from the manage_sites-gated edit check it now reuses. Reworded to match the single-item route's "not allowed to view sites". - `is_own_user_filter()` returned `true` for `user=me` unconditionally, so a logged-out `GET /sites?user=me` returned 200 (empty) instead of 401 - an unintentional anonymous-access bypass. Now requires being logged in. - `update_item()` unconditionally forced the response context to `edit`, while `create_item()` (changed by the same commit) now respects an explicit `context` request param, defaulting to `view`. Made `update_item()` consistent with `create_item()`. Updates test_get_items_me_filter_is_readable_when_logged_out to assert the corrected (403) behavior, and adds a regression test for the new embed-context allowance. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UGF7E1FnyB4fpDGdwoDMyc --- .../class-wp-rest-sites-controller.php | 9 +++--- .../tests/rest-api/wpRestSitesController.php | 31 ++++++++++++++++--- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php index 9f330f8457030..049409ef42797 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php @@ -132,7 +132,7 @@ public function get_items_permissions_check( $request ) { return true; } - return new WP_Error( 'rest_forbidden_context', __( 'Sorry, you are not allowed to edit sites.' ), array( 'status' => rest_authorization_required_code() ) ); + return new WP_Error( 'rest_forbidden_context', __( 'Sorry, you are not allowed to view sites.' ), array( 'status' => rest_authorization_required_code() ) ); } /** @@ -151,7 +151,7 @@ protected function is_own_user_filter( $request ) { } if ( 'me' === $user ) { - return true; + return is_user_logged_in(); } return get_current_user_id() === (int) $user; @@ -403,7 +403,7 @@ public function get_item_permissions_check( $request ) { $context = ! empty( $request['context'] ) ? $request['context'] : 'view'; - if ( 'view' === $context && is_user_member_of_blog( get_current_user_id(), (int) $site->blog_id ) ) { + if ( in_array( $context, array( 'view', 'embed' ), true ) && is_user_member_of_blog( get_current_user_id(), (int) $site->blog_id ) ) { return true; } @@ -641,7 +641,8 @@ public function update_item( $request ) { return $fields_update; } - $request->set_param( 'context', 'edit' ); + $context = ! empty( $request['context'] ) ? $request['context'] : 'view'; + $request->set_param( 'context', $context ); $response = $this->prepare_item_for_response( $site, $request ); diff --git a/tests/phpunit/tests/rest-api/wpRestSitesController.php b/tests/phpunit/tests/rest-api/wpRestSitesController.php index f4c67c056802f..7f2f079b9480e 100644 --- a/tests/phpunit/tests/rest-api/wpRestSitesController.php +++ b/tests/phpunit/tests/rest-api/wpRestSitesController.php @@ -156,20 +156,20 @@ public function test_get_items_forbidden_when_logged_out() { } /** - * The collection is intentionally readable while logged out, as long - * as the request is scoped to "my sites" via the `user` filter. + * The `user=me` own-sites filter requires being logged in, same as any + * other collection request - it is not an anonymous-access bypass. * * @ticket 40365 * @covers ::get_items_permissions_check * @group ms-required */ - public function test_get_items_me_filter_is_readable_when_logged_out() { + public function test_get_items_me_filter_forbidden_when_logged_out() { $request = new WP_REST_Request( 'GET', '/wp/v2/sites' ); $request->set_param( 'user', 'me' ); $response = rest_get_server()->dispatch( $request ); - $this->assertEquals( 200, $response->get_status() ); + $this->assertErrorResponse( 'rest_forbidden_context', $response, 401 ); } /** @@ -248,6 +248,29 @@ public function test_get_item_view_context_allowed_for_site_member() { $this->assertEquals( 200, $response->get_status() ); } + /** + * A member of the site (even without `manage_sites`) can also view it + * in the `embed` context, same as the default `view` context. + * + * @ticket 40365 + * @covers ::get_item_permissions_check + * @group ms-required + */ + public function test_get_item_embed_context_allowed_for_site_member() { + $blog_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create( array( 'role' => 'subscriber' ) ); + add_user_to_blog( $blog_id, $user_id, 'subscriber' ); + + wp_set_current_user( $user_id ); + + $request = new WP_REST_Request( 'GET', '/wp/v2/sites/' . $blog_id ); + $request->set_param( 'context', 'embed' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + } + /** * A logged-in user who is neither a super admin nor a member of the * site is forbidden from viewing it, even in the default view context. From 37343020fcaba1d6f5ea6ce91a0695c60812a871 Mon Sep 17 00:00:00 2001 From: Jonny Harrus Date: Tue, 29 Sep 2026 08:09:34 +0100 Subject: [PATCH 5/5] Multisite: Close a related anonymous-access gap in the sites `user` filter. Code review of the previous commit found the same class of bug it had just fixed for `user=me` also affects any non-numeric `user` value: `is_own_user_filter()` cast `$user` to `(int)` and compared it against `get_current_user_id()` with no logged-in guard, so a logged-out request (current user id 0) with e.g. `?user=xyz` satisfied `0 === (int) 'xyz'` and was treated as an authorized own-sites request. Require `is_user_logged_in()` for the general (non-"me") comparison too, matching the guard already added for the literal `me` case. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UGF7E1FnyB4fpDGdwoDMyc --- .../class-wp-rest-sites-controller.php | 2 +- .../tests/rest-api/wpRestSitesController.php | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php index 049409ef42797..cf09539e79853 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-sites-controller.php @@ -154,7 +154,7 @@ protected function is_own_user_filter( $request ) { return is_user_logged_in(); } - return get_current_user_id() === (int) $user; + return is_user_logged_in() && get_current_user_id() === (int) $user; } /** diff --git a/tests/phpunit/tests/rest-api/wpRestSitesController.php b/tests/phpunit/tests/rest-api/wpRestSitesController.php index 7f2f079b9480e..148c20919e2d4 100644 --- a/tests/phpunit/tests/rest-api/wpRestSitesController.php +++ b/tests/phpunit/tests/rest-api/wpRestSitesController.php @@ -172,6 +172,24 @@ public function test_get_items_me_filter_forbidden_when_logged_out() { $this->assertErrorResponse( 'rest_forbidden_context', $response, 401 ); } + /** + * A non-numeric `user` value casts to `0` via `(int)`, which must not be + * allowed to match a logged-out request's `get_current_user_id()` (also + * `0`) and grant the own-sites-filter bypass. + * + * @ticket 40365 + * @covers ::get_items_permissions_check + * @group ms-required + */ + public function test_get_items_non_numeric_user_filter_forbidden_when_logged_out() { + $request = new WP_REST_Request( 'GET', '/wp/v2/sites' ); + $request->set_param( 'user', 'xyz' ); + + $response = rest_get_server()->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden_context', $response, 401 ); + } + /** * @ticket 40365 * @covers ::get_item