Skip to content

Commit

Permalink
Prevent modifications of the VIP machine user (#1206)
Browse files Browse the repository at this point in the history
It should never be editable/removable.
  • Loading branch information
mjangda committed Apr 16, 2019
1 parent dbc7e39 commit efa962d
Show file tree
Hide file tree
Showing 4 changed files with 216 additions and 1 deletion.
9 changes: 8 additions & 1 deletion bin/phpunit-docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@
# Stop on failures: ./bin/phpunit-docker.sh --stop-on-failure
# etc.

WP_VERSION=${1-latest}
WP_VERSION=${2-latest}
WP_MULTISITE=${3-0}

echo "--------------"
echo "Will test with WP_VERSION=$WP_VERSION and WP_MULTISITE=$WP_MULTISITE"
echo "--------------"
echo

MYSQL_ROOT_PASSWORD='wordpress'
docker network create tests
Expand All @@ -27,6 +33,7 @@ done

docker run \
--network tests \
-e WP_MULTISITE="$WP_MULTISITE" \
-v $(pwd):/app \
-v /tmp/wordpress-tests-lib-$WP_VERSION:/tmp/wordpress-tests-lib \
-v /tmp/wordpress-$WP_VERSION:/tmp/wordpress \
Expand Down
1 change: 1 addition & 0 deletions security.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/

require_once( __DIR__ . '/security/class-lockout.php' );
require_once( __DIR__ . '/security/machine-user.php' );

define( 'CACHE_GROUP_LOGIN_LIMIT', 'login_limit' );
define( 'CACHE_GROUP_LOST_PASSWORD_LIMIT', 'lost_password_limit' );
Expand Down
28 changes: 28 additions & 0 deletions security/machine-user.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

namespace Automattic\VIP\Security;

const USER_MODIFICATION_CAPS = [
'edit_user',
'delete_user',
'remove_user',
'promote_user',
];

// Don't allow the VIP machine user to be edited or removed.
function prevent_machine_user_mods( $caps, $requested_cap, $user_id, $args ) {
if ( in_array( $requested_cap, USER_MODIFICATION_CAPS, true ) ) {
// args[0] is who we're performing the action on.
$user_to_edit_id = $args[ 0 ];
$user_to_edit = get_userdata( $user_to_edit_id );

if ( WPCOM_VIP_MACHINE_USER_LOGIN === $user_to_edit->user_login ) {
return [ 'do_not_allow' ];
}
}

return $caps;
}
// Use map_meta_cap instead of user_has_cap so we can restrict superadmins as well.
// WP_User::has_cap runs the user_has_cap filter after the superadmin check, which makes it impossible to use for restricting them.
add_filter( 'map_meta_cap', __NAMESPACE__ . '\prevent_machine_user_mods', PHP_INT_MAX, 4 );
179 changes: 179 additions & 0 deletions tests/security/test-machine-user.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
<?php

namespace Automattic\VIP\Security;

class Machine_User_Test extends \WP_UnitTestCase {

public static function setUpBeforeClass() {
parent::setUpBeforeClass();

require_once __DIR__ . '/../../security/machine-user.php';
}

public function setUp() {
parent::setUp();

$this->machine_user = $this->factory->user->create_and_get( [
'user_login' => WPCOM_VIP_MACHINE_USER_LOGIN,
'user_email' => WPCOM_VIP_MACHINE_USER_EMAIL,
'role' => WPCOM_VIP_MACHINE_USER_ROLE,
] );

}

public function get_test_data__user_modification_caps() {
return [
'edit_user' => [ 'edit_user' ],
'remove_user' => [ 'remove_user' ],
'delete_user' => [ 'delete_user' ],
'promote_user' => [ 'promote_user' ],
];
}

// For testing non-superadmin Administrator users
public function get_test_data__selective_user_modification_caps() {
return [
'remove_user' => [ 'remove_user' ],
'promote_user' => [ 'promote_user' ],
];
}

/**
* @dataProvider get_test_data__user_modification_caps
*/
public function test__machine_user_cannot_modify_self( $test_cap ) {
$actual_has_cap = $this->machine_user->has_cap( $test_cap, $this->machine_user->ID );

$this->assertFalse( $actual_has_cap );
}

/**
* @dataProvider get_test_data__user_modification_caps
*/
public function test__non_admin_users_cannot_modify_machine_user( $test_cap ) {
$test_user = $this->factory->user->create_and_get( [ 'role' => 'editor' ] );

$actual_has_cap = $test_user->has_cap( $test_cap, $this->machine_user->ID );

$this->assertFalse( $actual_has_cap );
}

/**
* @dataProvider get_test_data__user_modification_caps
*/
public function test__admin_users_cannot_modify_machine_user( $test_cap ) {
$test_user = $this->factory->user->create_and_get( [ 'role' => 'administrator' ] );

$actual_has_cap = $test_user->has_cap( $test_cap, $this->machine_user->ID );

$this->assertFalse( $actual_has_cap );
}

/**
* @dataProvider get_test_data__user_modification_caps
*/
public function test__superadmin_users_cannot_modify_machine_user( $test_cap ) {
if ( ! is_multisite() ) {
$this->markTestSkipped( 'No superadmins on single site installs.' );
}

$test_user = $this->factory->user->create_and_get( [ 'role' => 'administrator' ] );
grant_super_admin( $test_user->ID );

$actual_has_cap = $test_user->has_cap( $test_cap, $this->machine_user->ID );

$this->assertFalse( $actual_has_cap );
}

public function test__non_admin_users_can_still_modify_self() {
$test_user = $this->factory->user->create_and_get( [ 'role' => 'editor' ] );

$actual_has_cap = $test_user->has_cap( 'edit_user', $test_user->ID );

$this->assertTrue( $actual_has_cap );
}

public function test__admin_users_can_still_modify_self() {
$test_user = $this->factory->user->create_and_get( [ 'role' => 'administrator' ] );

$actual_has_cap = $test_user->has_cap( 'edit_user', $test_user->ID );

$this->assertTrue( $actual_has_cap );
}

public function test__superadmin_users_can_still_modify_self() {
if ( ! is_multisite() ) {
$this->markTestSkipped( 'No superadmins on single site installs.' );
}

$test_user = $this->factory->user->create_and_get( [ 'role' => 'administrator' ] );
grant_super_admin( $test_user->ID );

$actual_has_cap = $test_user->has_cap( 'edit_user', $test_user->ID );

$this->assertTrue( $actual_has_cap );
}

/**
* @dataProvider get_test_data__user_modification_caps
*/
public function test__non_admin_users_cannot_modify_others( $test_cap ) {
$test_user = $this->factory->user->create_and_get( [ 'role' => 'editor' ] );
$user_to_edit = $this->factory->user->create_and_get( [ 'role' => 'editor' ] );

$actual_has_cap = $test_user->has_cap( $test_cap, $user_to_edit->ID );

$this->assertFalse( $actual_has_cap );
}

/**
* @dataProvider get_test_data__user_modification_caps
*/
public function test__admin_users_can_still_modify_others_on_single_site( $test_cap ) {
if ( is_multisite() ) {
$this->markTestSkipped( 'Single site test for administrator user; multisite tested separately.' );
}

$test_user = $this->factory->user->create_and_get( [ 'role' => 'administrator' ] );
$user_to_edit = $this->factory->user->create_and_get( [ 'role' => 'editor' ] );

$actual_has_cap = $test_user->has_cap( $test_cap, $user_to_edit->ID );

$this->assertTrue( $actual_has_cap );
}

/**
* Administrators without super admin have a more restricted set of caps on multisite (no edit or delete).
*
* @dataProvider get_test_data__selective_user_modification_caps
*/
public function test__admin_users_can_still_selectively_modify_others_on_multsite( $test_cap ) {
if ( ! is_multisite() ) {
$this->markTestSkipped( 'Multisite test for administrator user; single site tested separately.' );
}

$test_user = $this->factory->user->create_and_get( [ 'role' => 'administrator' ] );
$user_to_edit = $this->factory->user->create_and_get( [ 'role' => 'editor' ] );

$actual_has_cap = $test_user->has_cap( $test_cap, $user_to_edit->ID );

$this->assertTrue( $actual_has_cap );
}

/**
* @dataProvider get_test_data__user_modification_caps
*/
public function test__superadmin_users_can_still_modify_others( $test_cap ) {
if ( ! is_multisite() ) {
$this->markTestSkipped( 'No superadmins on single site installs.' );
}

$test_user = $this->factory->user->create_and_get( [ 'role' => 'administrator' ] );
grant_super_admin( $test_user->ID );
$user_to_edit = $this->factory->user->create_and_get( [ 'role' => 'editor' ] );

$actual_has_cap = $test_user->has_cap( $test_cap, $user_to_edit->ID );

$this->assertTrue( $actual_has_cap );
}
}

0 comments on commit efa962d

Please sign in to comment.