From 539b85406d440f6ff7274aaf24bdbb29206ec8d1 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Mon, 12 Dec 2016 21:41:44 +0000 Subject: [PATCH] Multisite: Handle capability check for removing oneself via `map_meta_cap()`. Site administrators should not be able to remove themselves from a site. This moves the enforcement of this rule from `wp-admin/users.php` to `remove_user_from_blog()` via the `remove_user` capability, which furthermore allows us to get rid of two additional clauses and their `is_super_admin()` checks in `wp-admin/users.php`. A unit test for the new behavior has been added. Fixes #39063. See #37616. git-svn-id: https://develop.svn.wordpress.org/trunk@39588 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-admin/users.php | 9 +-------- src/wp-includes/capabilities.php | 7 ++++++- tests/phpunit/tests/user/capabilities.php | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/wp-admin/users.php b/src/wp-admin/users.php index 214c06d8d3..95d9f768bb 100644 --- a/src/wp-admin/users.php +++ b/src/wp-admin/users.php @@ -321,10 +321,6 @@ case 'doremove': $update = 'remove'; foreach ( $userids as $id ) { $id = (int) $id; - if ( $id == $current_user->ID && !is_super_admin() ) { - $update = 'err_admin_remove'; - continue; - } if ( !current_user_can('remove_user', $id) ) { $update = 'err_admin_remove'; continue; @@ -377,10 +373,7 @@ case 'remove': foreach ( $userids as $id ) { $id = (int) $id; $user = get_userdata( $id ); - if ( $id == $current_user->ID && !is_super_admin() ) { - /* translators: 1: user id, 2: user login */ - echo "
  • " . sprintf(__('ID #%1$s: %2$s The current user will not be removed.'), $id, $user->user_login) . "
  • \n"; - } elseif ( !current_user_can('remove_user', $id) ) { + if ( ! current_user_can( 'remove_user', $id ) ) { /* translators: 1: user id, 2: user login */ echo "
  • " . sprintf(__('ID #%1$s: %2$s Sorry, you are not allowed to remove this user.'), $id, $user->user_login) . "
  • \n"; } else { diff --git a/src/wp-includes/capabilities.php b/src/wp-includes/capabilities.php index 308c7063c8..78cde1c2ee 100644 --- a/src/wp-includes/capabilities.php +++ b/src/wp-includes/capabilities.php @@ -32,7 +32,12 @@ function map_meta_cap( $cap, $user_id ) { switch ( $cap ) { case 'remove_user': - $caps[] = 'remove_users'; + // In multisite the user must be a super admin to remove themselves. + if ( isset( $args[0] ) && $user_id == $args[0] && ! is_super_admin( $user_id ) ) { + $caps[] = 'do_not_allow'; + } else { + $caps[] = 'remove_users'; + } break; case 'promote_user': case 'add_users': diff --git a/tests/phpunit/tests/user/capabilities.php b/tests/phpunit/tests/user/capabilities.php index da1c313b88..d818ded9bf 100644 --- a/tests/phpunit/tests/user/capabilities.php +++ b/tests/phpunit/tests/user/capabilities.php @@ -1757,4 +1757,21 @@ class Tests_User_Capabilities extends WP_UnitTestCase { wp_set_current_user( self::$users['editor']->ID ); $this->assertFalse( current_user_can( 'add_user_meta', self::$users['subscriber']->ID, 'foo' ) ); } + + /** + * @ticket 39063 + */ + public function test_only_super_admins_can_remove_themselves_on_multisite() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Test only runs in multisite.' ); + } + + $this->assertTrue( user_can( self::$super_admin->ID, 'remove_user', self::$super_admin->ID ) ); + + $this->assertFalse( user_can( self::$users['administrator']->ID, 'remove_user', self::$users['administrator']->ID ) ); + $this->assertFalse( user_can( self::$users['editor']->ID, 'remove_user', self::$users['editor']->ID ) ); + $this->assertFalse( user_can( self::$users['author']->ID, 'remove_user', self::$users['author']->ID ) ); + $this->assertFalse( user_can( self::$users['contributor']->ID, 'remove_user', self::$users['contributor']->ID ) ); + $this->assertFalse( user_can( self::$users['subscriber']->ID, 'remove_user', self::$users['subscriber']->ID ) ); + } }