Changeset 40111 for branches/4.7

Timestamp:

02/24/2017 08:45:52 PM (8 years ago)

Author:

SergeyBiryukov

Message:

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.
Merges [40106] to the 4.7 branch.
Fixes #39701.

Location:

branches/4.7

Files:

• . (modified) (1 prop)
• src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php (modified) (2 diffs)
• tests/phpunit/tests/rest-api/rest-users-controller.php (modified) (3 diffs)

branches/4.7/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php

@@ -351 +351 @@
         }
 
+        if ( is_multisite() && ! is_user_member_of_blog( $user->ID ) ) {
+            return $error;
+        }
+
         return $user;
     }
@@ -638 +642 @@
         /* This action is documented in lib/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'] ) ) {

branches/4.7/tests/phpunit/tests/rest-api/rest-users-controller.php

@@ -686 +686 @@
         $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() );
+        }
     }
 
@@ -997 +1002 @@
             }
         }
-    }
-
-    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 );
     }
 
@@ -2152 +2119 @@
     }
 
+    /**
+     * @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 );