From 0dc1e0633bc48b788fbf2e26fffa71bf2ac2175b Mon Sep 17 00:00:00 2001 From: flixos90 Date: Thu, 3 Aug 2017 21:40:02 +0000 Subject: [PATCH] Multisite: Introduce a `can_add_user_to_blog` filter to prevent adding a user to a site. Under certain circumstances, it can be necessary that a user should not be added to a site, beyond the restrictions that WordPress core applies. With the new `can_add_user_to_blog` filter, plugin developers can run custom checks and return an error in case of a failure, that will prevent the user from being added. The user-facing parts and the REST API route that interact with `add_user_to_blog()` have been adjusted accordingly to provide appropriate error feedback when a user could not be added to a site. Furthermore, two existing error feedback messages in the site admin's "New User" screen have been adjusted to properly show inside an error notice instead of a success notice. Props jmdodd. Fixes #41101. git-svn-id: https://develop.svn.wordpress.org/trunk@41225 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-admin/network/site-users.php | 39 +++++++---- src/wp-admin/user-new.php | 21 ++++-- src/wp-includes/ms-functions.php | 64 ++++++++++++++----- .../class-wp-rest-users-controller.php | 5 +- .../tests/rest-api/rest-users-controller.php | 24 +++++++ tests/phpunit/tests/user/multisite.php | 26 ++++++++ 6 files changed, 146 insertions(+), 33 deletions(-) diff --git a/src/wp-admin/network/site-users.php b/src/wp-admin/network/site-users.php index 5165996d44..68cc29c723 100644 --- a/src/wp-admin/network/site-users.php +++ b/src/wp-admin/network/site-users.php @@ -66,16 +66,21 @@ if ( $action ) { if ( false === $user_id ) { $update = 'err_new_dup'; } else { - add_user_to_blog( $id, $user_id, $_POST['new_role'] ); - $update = 'newuser'; - /** - * Fires after a user has been created via the network site-users.php page. - * - * @since 4.4.0 - * - * @param int $user_id ID of the newly created user. - */ - do_action( 'network_site_users_created_user', $user_id ); + $result = add_user_to_blog( $id, $user_id, $_POST['new_role'] ); + + if ( is_wp_error( $result ) ) { + $update = 'err_add_fail'; + } else { + $update = 'newuser'; + /** + * Fires after a user has been created via the network site-users.php page. + * + * @since 4.4.0 + * + * @param int $user_id ID of the newly created user. + */ + do_action( 'network_site_users_created_user', $user_id ); + } } } break; @@ -87,10 +92,15 @@ if ( $action ) { $newuser = $_POST['newuser']; $user = get_user_by( 'login', $newuser ); if ( $user && $user->exists() ) { - if ( ! is_user_member_of_blog( $user->ID, $id ) ) - add_user_to_blog( $id, $user->ID, $_POST['new_role'] ); - else + if ( ! is_user_member_of_blog( $user->ID, $id ) ) { + $result = add_user_to_blog( $id, $user->ID, $_POST['new_role'] ); + + if ( is_wp_error( $result ) ) { + $update = 'err_add_fail'; + } + } else { $update = 'err_add_member'; + } } else { $update = 'err_add_notfound'; } @@ -223,6 +233,9 @@ if ( isset($_GET['update']) ) : case 'err_add_member': echo '

' . __( 'User is already a member of this site.' ) . '

'; break; + case 'err_add_fail': + echo '

' . __( 'User could not be added to this site.' ) . '

'; + break; case 'err_add_notfound': echo '

' . __( 'Enter the username of an existing user.' ) . '

'; break; diff --git a/src/wp-admin/user-new.php b/src/wp-admin/user-new.php index b6b4c5f08f..254e5f0f77 100644 --- a/src/wp-admin/user-new.php +++ b/src/wp-admin/user-new.php @@ -67,8 +67,13 @@ if ( isset($_REQUEST['action']) && 'adduser' == $_REQUEST['action'] ) { $redirect = add_query_arg( array('update' => 'addexisting'), 'user-new.php' ); } else { if ( isset( $_POST[ 'noconfirmation' ] ) && current_user_can( 'manage_network_users' ) ) { - add_existing_user_to_blog( array( 'user_id' => $user_id, 'role' => $_REQUEST[ 'role' ] ) ); - $redirect = add_query_arg( array( 'update' => 'addnoconfirmation' , 'user_id' => $user_id ), 'user-new.php' ); + $result = add_existing_user_to_blog( array( 'user_id' => $user_id, 'role' => $_REQUEST[ 'role' ] ) ); + + if ( ! is_wp_error( $result ) ) { + $redirect = add_query_arg( array( 'update' => 'addnoconfirmation', 'user_id' => $user_id ), 'user-new.php' ); + } else { + $redirect = add_query_arg( array( 'update' => 'could_not_add' ), 'user-new.php' ); + } } else { $newuser_key = substr( md5( $user_id ), 0, 5 ); add_option( 'new_user_' . $newuser_key, array( 'user_id' => $user_id, 'email' => $user_details->user_email, 'role' => $_REQUEST[ 'role' ] ) ); @@ -157,6 +162,8 @@ Please click the following link to confirm the invite: $new_user = wpmu_activate_signup( $key ); if ( is_wp_error( $new_user ) ) { $redirect = add_query_arg( array( 'update' => 'addnoconfirmation' ), 'user-new.php' ); + } elseif ( ! is_user_member_of_blog( $new_user['user_id'] ) ) { + $redirect = add_query_arg( array( 'update' => 'created_could_not_add' ), 'user-new.php' ); } else { $redirect = add_query_arg( array( 'update' => 'addnoconfirmation', 'user_id' => $new_user['user_id'] ), 'user-new.php' ); } @@ -261,11 +268,17 @@ if ( isset($_GET['update']) ) { case "addexisting": $messages[] = __('That user is already a member of this site.'); break; + case "could_not_add": + $add_user_errors = new WP_Error( 'could_not_add', __( 'That user could not be added to this site.' ) ); + break; + case "created_could_not_add": + $add_user_errors = new WP_Error( 'created_could_not_add', __( 'User has been created, but could not be added to this site.' ) ); + break; case "does_not_exist": - $messages[] = __('The requested user does not exist.'); + $add_user_errors = new WP_Error( 'does_not_exist', __( 'The requested user does not exist.' ) ); break; case "enter_email": - $messages[] = __('Please enter a valid email address.'); + $add_user_errors = new WP_Error( 'enter_email', __( 'Please enter a valid email address.' ) ); break; } } else { diff --git a/src/wp-includes/ms-functions.php b/src/wp-includes/ms-functions.php index e47c543c59..03b1c37220 100644 --- a/src/wp-includes/ms-functions.php +++ b/src/wp-includes/ms-functions.php @@ -59,9 +59,12 @@ function get_active_blog_for_user( $user_id ) { } } else { //TODO Review this call to add_user_to_blog too - to get here the user must have a role on this blog? - add_user_to_blog( $first_blog->userblog_id, $user_id, 'subscriber' ); - update_user_meta( $user_id, 'primary_blog', $first_blog->userblog_id ); - $primary = $first_blog; + $result = add_user_to_blog( $first_blog->userblog_id, $user_id, 'subscriber' ); + + if ( ! is_wp_error( $result ) ) { + update_user_meta( $user_id, 'primary_blog', $first_blog->userblog_id ); + $primary = $first_blog; + } } if ( ( ! is_object( $primary ) ) || ( $primary->archived == 1 || $primary->spam == 1 || $primary->deleted == 1 ) ) { @@ -160,6 +163,29 @@ function add_user_to_blog( $blog_id, $user_id, $role ) { return new WP_Error( 'user_does_not_exist', __( 'The requested user does not exist.' ) ); } + /** + * Filters whether a user should be added to a site. + * + * @since 4.9.0 + * + * @param bool|WP_Error $retval True if the user should be added to the site, false + * or error object otherwise. + * @param int $user_id User ID. + * @param string $role User role. + * @param int $blog_id Site ID. + */ + $can_add_user = apply_filters( 'can_add_user_to_blog', true, $user_id, $role, $blog_id ); + + if ( true !== $can_add_user ) { + restore_current_blog(); + + if ( is_wp_error( $can_add_user ) ) { + return $can_add_user; + } + + return new WP_Error( 'user_cannot_be_added', __( 'User cannot be added to this site.' ) ); + } + if ( !get_user_meta($user_id, 'primary_blog', true) ) { update_user_meta($user_id, 'primary_blog', $blog_id); $site = get_site( $blog_id ); @@ -2081,15 +2107,19 @@ function add_existing_user_to_blog( $details = false ) { if ( is_array( $details ) ) { $blog_id = get_current_blog_id(); $result = add_user_to_blog( $blog_id, $details[ 'user_id' ], $details[ 'role' ] ); - /** - * Fires immediately after an existing user is added to a site. - * - * @since MU (3.0.0) - * - * @param int $user_id User ID. - * @param mixed $result True on success or a WP_Error object if the user doesn't exist. - */ - do_action( 'added_existing_user', $details['user_id'], $result ); + + if ( ! is_wp_error( $result ) ) { + /** + * Fires immediately after an existing user is added to a site. + * + * @since MU (3.0.0) + * + * @param int $user_id User ID. + * @param mixed $result True on success or a WP_Error object if the user doesn't exist. + */ + do_action( 'added_existing_user', $details['user_id'], $result ); + } + return $result; } } @@ -2111,9 +2141,13 @@ function add_new_user_to_blog( $user_id, $password, $meta ) { if ( !empty( $meta[ 'add_to_blog' ] ) ) { $blog_id = $meta[ 'add_to_blog' ]; $role = $meta[ 'new_role' ]; - remove_user_from_blog($user_id, get_network()->site_id); // remove user from main blog. - add_user_to_blog( $blog_id, $user_id, $role ); - update_user_meta( $user_id, 'primary_blog', $blog_id ); + remove_user_from_blog( $user_id, get_network()->site_id ); // remove user from main blog. + + $result = add_user_to_blog( $blog_id, $user_id, $role ); + + if ( ! is_wp_error( $result ) ) { + update_user_meta( $user_id, 'primary_blog', $blog_id ); + } } } 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 5afe3a2264..fa688a2614 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 @@ -492,7 +492,10 @@ class WP_REST_Users_Controller extends WP_REST_Controller { return $user_id; } - add_user_to_blog( get_site()->id, $user_id, '' ); + $result= add_user_to_blog( get_site()->id, $user_id, '' ); + if ( is_wp_error( $result ) ) { + return $result; + } } else { $user_id = wp_insert_user( wp_slash( (array) $user ) ); diff --git a/tests/phpunit/tests/rest-api/rest-users-controller.php b/tests/phpunit/tests/rest-api/rest-users-controller.php index 30246932bf..7228fbf7b2 100644 --- a/tests/phpunit/tests/rest-api/rest-users-controller.php +++ b/tests/phpunit/tests/rest-api/rest-users-controller.php @@ -1020,6 +1020,30 @@ class WP_Test_REST_Users_Controller extends WP_Test_REST_Controller_Testcase { $this->assertFalse( $user_is_member ); } + /** + * @ticket 41101 + * @group ms-required + */ + public function test_create_new_network_user_with_add_user_to_blog_failure() { + $this->allow_user_to_manage_multisite(); + + $params = array( + 'username' => 'testuser123', + 'password' => 'testpassword', + 'email' => 'test@example.com', + 'name' => 'Test User 123', + 'roles' => array( 'editor' ), + ); + + add_filter( 'can_add_user_to_blog', '__return_false' ); + + $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 ); + $this->assertErrorResponse( 'user_cannot_be_added', $response ); + } + /** * @group ms-required */ diff --git a/tests/phpunit/tests/user/multisite.php b/tests/phpunit/tests/user/multisite.php index e8d707997a..af34244c79 100644 --- a/tests/phpunit/tests/user/multisite.php +++ b/tests/phpunit/tests/user/multisite.php @@ -397,6 +397,32 @@ class Tests_Multisite_User extends WP_UnitTestCase { $this->assertWPError( $result ); } + /** + * @ticket 41101 + */ + public function test_should_fail_can_add_user_to_blog_filter() { + $site_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create(); + + add_filter( 'can_add_user_to_blog', '__return_false' ); + $result = add_user_to_blog( $site_id, $user_id, 'subscriber' ); + + $this->assertWPError( $result ); + } + + /** + * @ticket 41101 + */ + public function test_should_succeed_can_add_user_to_blog_filter() { + $site_id = self::factory()->blog->create(); + $user_id = self::factory()->user->create(); + + add_filter( 'can_add_user_to_blog', '__return_true' ); + $result = add_user_to_blog( $site_id, $user_id, 'subscriber' ); + + $this->assertTrue( $result ); + } + /** * @ticket 23016 */