From 0f9dd5f88f317751be0d659b75d62d817e4d7139 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 23 Feb 2017 22:36:54 +0000 Subject: [PATCH] REST API: Do not allow access to users from a different site in multisite. It has been unintendedly possible to both view and edit users from a different site than the current site in multisite environments. Moreover, when passing roles to a user in an update request, that user would implicitly be added to the current site. This changeset removes the incorrect behavior for now in order to be able to provide a proper REST API workflow for managing multisite users in the near future. Related unit tests have been adjusted as well. Props jnylen0, jeremyfelt, johnjamesjacoby. Fixes #39701. git-svn-id: https://develop.svn.wordpress.org/trunk@40106 602fd350-edb4-49c9-b593-d223f7449a82 --- .../class-wp-rest-users-controller.php | 8 +- .../tests/rest-api/rest-users-controller.php | 183 ++++++++++++++---- 2 files changed, 146 insertions(+), 45 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php index 802eb649c3..c762ed41d7 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php @@ -351,6 +351,10 @@ class WP_REST_Users_Controller extends WP_REST_Controller { return $error; } + if ( is_multisite() && ! is_user_member_of_blog( $user->ID ) ) { + return $error; + } + return $user; } @@ -639,10 +643,6 @@ class WP_REST_Users_Controller extends WP_REST_Controller { /** This action is documented in wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php */ do_action( 'rest_insert_user', $user, $request, false ); - if ( is_multisite() && ! is_user_member_of_blog( $id ) ) { - add_user_to_blog( get_current_blog_id(), $id, '' ); - } - if ( ! empty( $request['roles'] ) ) { array_map( array( $user, 'add_role' ), $request['roles'] ); } diff --git a/tests/phpunit/tests/rest-api/rest-users-controller.php b/tests/phpunit/tests/rest-api/rest-users-controller.php index 0da29ee27d..0a85336155 100644 --- a/tests/phpunit/tests/rest-api/rest-users-controller.php +++ b/tests/phpunit/tests/rest-api/rest-users-controller.php @@ -675,10 +675,15 @@ class WP_Test_REST_Users_Controller extends WP_Test_REST_Controller_Testcase { $request = new WP_REST_Request( 'GET', '/wp/v2/users/' . $lolz ); $request->set_param( 'context', 'edit' ); $response = $this->server->dispatch( $request ); - $data = $response->get_data(); - $this->assertEquals( $data['capabilities'], new stdClass() ); - $this->assertEquals( $data['extra_capabilities'], new stdClass() ); + if ( is_multisite() ) { + $this->assertErrorResponse( 'rest_user_invalid_id', $response, 404 ); + } else { + $data = $response->get_data(); + + $this->assertEquals( $data['capabilities'], new stdClass() ); + $this->assertEquals( $data['extra_capabilities'], new stdClass() ); + } } public function test_cannot_get_item_without_permission() { @@ -1036,44 +1041,6 @@ class WP_Test_REST_Users_Controller extends WP_Test_REST_Controller_Testcase { } } - public function test_update_existing_network_user_on_sub_site_adds_user_to_site() { - if ( ! is_multisite() ) { - $this->markTestSkipped( 'Test requires multisite.' ); - } - - $this->allow_user_to_manage_multisite(); - - $params = array( - 'username' => 'testuser123', - 'password' => 'testpassword', - 'email' => 'test@example.com', - 'name' => 'Test User 123', - 'roles' => array( 'editor' ), - ); - - $request = new WP_REST_Request( 'POST', '/wp/v2/users' ); - $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); - $request->set_body_params( $params ); - $response = $this->server->dispatch( $request ); - $data = $response->get_data(); - $user_id = $data['id']; - - switch_to_blog( self::$site ); - - $request = new WP_REST_Request( 'PUT', '/wp/v2/users/' . $user_id ); - $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); - $request->set_body_params( $params ); - $this->server->dispatch( $request ); - - restore_current_blog(); - - $user_is_member = is_user_member_of_blog( $user_id, self::$site ); - - wpmu_delete_user( $user_id ); - - $this->assertTrue( $user_is_member ); - } - public function test_json_create_user() { $this->allow_user_to_manage_multisite(); wp_set_current_user( self::$user ); @@ -2189,6 +2156,140 @@ class WP_Test_REST_Users_Controller extends WP_Test_REST_Controller_Testcase { $wp_rest_additional_fields = array(); } + /** + * @ticket 39701 + */ + public function test_get_item_from_different_site_as_site_administrator() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Test only runs in multisite' ); + } + + switch_to_blog( self::$site ); + $user_id = $this->factory->user->create( array( + 'role' => 'author', + ) ); + restore_current_blog(); + + wp_set_current_user( self::$user ); + $request = new WP_REST_Request( 'GET', sprintf( '/wp/v2/users/%d', $user_id ) ); + + $response = $this->server->dispatch( $request ); + $this->assertErrorResponse( 'rest_user_invalid_id', $response, 404 ); + } + + /** + * @ticket 39701 + */ + public function test_get_item_from_different_site_as_network_administrator() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Test only runs in multisite' ); + } + + switch_to_blog( self::$site ); + $user_id = $this->factory->user->create( array( + 'role' => 'author', + ) ); + restore_current_blog(); + + wp_set_current_user( self::$superadmin ); + $request = new WP_REST_Request( 'GET', sprintf( '/wp/v2/users/%d', $user_id ) ); + + $response = $this->server->dispatch( $request ); + $this->assertErrorResponse( 'rest_user_invalid_id', $response, 404 ); + } + + /** + * @ticket 39701 + */ + public function test_update_item_from_different_site_as_site_administrator() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Test only runs in multisite' ); + } + + switch_to_blog( self::$site ); + $user_id = $this->factory->user->create( array( + 'role' => 'author', + ) ); + restore_current_blog(); + + wp_set_current_user( self::$user ); + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/users/%d', $user_id ) ); + $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); + $request->set_body_params( array( 'first_name' => 'New Name' ) ); + + $response = $this->server->dispatch( $request ); + $this->assertErrorResponse( 'rest_user_invalid_id', $response, 404 ); + } + + /** + * @ticket 39701 + */ + public function test_update_item_from_different_site_as_network_administrator() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Test only runs in multisite' ); + } + + switch_to_blog( self::$site ); + $user_id = $this->factory->user->create( array( + 'role' => 'author', + ) ); + restore_current_blog(); + + wp_set_current_user( self::$superadmin ); + $request = new WP_REST_Request( 'PUT', sprintf( '/wp/v2/users/%d', $user_id ) ); + $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); + $request->set_body_params( array( 'first_name' => 'New Name' ) ); + + $response = $this->server->dispatch( $request ); + $this->assertErrorResponse( 'rest_user_invalid_id', $response, 404 ); + } + + /** + * @ticket 39701 + */ + public function test_delete_item_from_different_site_as_site_administrator() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Test only runs in multisite' ); + } + + switch_to_blog( self::$site ); + $user_id = $this->factory->user->create( array( + 'role' => 'author', + ) ); + restore_current_blog(); + + wp_set_current_user( self::$user ); + $request = new WP_REST_Request( 'DELETE', sprintf( '/wp/v2/users/%d', $user_id ) ); + $request->set_param( 'force', true ); + $request->set_param( 'reassign', false ); + + $response = $this->server->dispatch( $request ); + $this->assertErrorResponse( 'rest_user_invalid_id', $response, 404 ); + } + + /** + * @ticket 39701 + */ + public function test_delete_item_from_different_site_as_network_administrator() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Test only runs in multisite' ); + } + + switch_to_blog( self::$site ); + $user_id = $this->factory->user->create( array( + 'role' => 'author', + ) ); + restore_current_blog(); + + wp_set_current_user( self::$superadmin ); + $request = new WP_REST_Request( 'DELETE', sprintf( '/wp/v2/users/%d', $user_id ) ); + $request->set_param( 'force', true ); + $request->set_param( 'reassign', false ); + + $response = $this->server->dispatch( $request ); + $this->assertErrorResponse( 'rest_user_invalid_id', $response, 404 ); + } + public function additional_field_get_callback( $object ) { return get_user_meta( $object['id'], 'my_custom_int', true ); }