From c42cf2f6229d3395eb8c44fd8a8b0794a5bf25d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tautvidas=20Sipavi=C4=8Dius?= Date: Mon, 6 Nov 2017 18:09:38 +0200 Subject: [PATCH] Switch to using `current_user_can` function to check capabilities --- lib/Config/AccessControl.php | 26 +------------ tests/unit/API/JSON/APITest.php | 51 ++++++++++++++++++++----- tests/unit/Config/AccessControlTest.php | 20 ++++++++-- tests/unit/Router/RouterTest.php | 44 +++++++++++++++++---- 4 files changed, 96 insertions(+), 45 deletions(-) diff --git a/lib/Config/AccessControl.php b/lib/Config/AccessControl.php index e0f7412a09..c8172c9b3f 100644 --- a/lib/Config/AccessControl.php +++ b/lib/Config/AccessControl.php @@ -22,8 +22,6 @@ class AccessControl { function __construct() { $this->permissions = self::getDefaultPermissions(); - $this->user_roles = $this->getUserRoles(); - $this->user_capabilities = $this->getUserCapabilities(); } static function getDefaultPermissions() { @@ -80,30 +78,8 @@ class AccessControl { ); } - function getUserRoles() { - $user = wp_get_current_user(); - return $user->roles; - } - - function getUserCapabilities() { - $user = wp_get_current_user(); - return array_keys($user->allcaps); - } - - function getUserFirstCapability() { - return (!empty($this->user_capabilities)) ? - $this->user_capabilities[0] : - null; - } - function validatePermission($permission) { if($permission === self::NO_ACCESS_RESTRICTION) return true; - foreach($this->user_roles as $role) { - $role_object = get_role($role); - if($role_object && $role_object->has_cap($permission)) { - return true; - } - } - return false; + return current_user_can($permission); } } diff --git a/tests/unit/API/JSON/APITest.php b/tests/unit/API/JSON/APITest.php index 8af732cc9f..274de39601 100644 --- a/tests/unit/API/JSON/APITest.php +++ b/tests/unit/API/JSON/APITest.php @@ -143,7 +143,6 @@ class APITest extends \MailPoetTest { 'data' => array('test' => 'data') ); $access_control = new AccessControl(); - $access_control->user_roles = $access_control->permissions[AccessControl::PERMISSION_MANAGE_SETTINGS]; $api = Stub::make( new \MailPoet\API\JSON\API($access_control), array( @@ -179,8 +178,10 @@ class APITest extends \MailPoetTest { 'api_version' => 'v1', 'data' => array('test' => 'data') ); - $access_control = new AccessControl(); - $access_control->user_roles = array(); + $access_control = Stub::make( + new AccessControl(), + array('validatePermission' => false) + ); $api = new \MailPoet\API\JSON\API($access_control); $api->addEndpointNamespace($namespace['name'], $namespace['version']); $api->setRequestData($data); @@ -189,22 +190,36 @@ class APITest extends \MailPoetTest { } function testItValidatesGlobalPermission() { - $access_control = new AccessControl(); $permissions = array( 'global' => AccessControl::PERMISSION_MANAGE_SETTINGS, ); - $access_control->user_roles = array(); + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return false; + }) + ) + ); $api = new JSONAPI($access_control); expect($api->validatePermissions(null, $permissions))->false(); - $access_control->user_roles = $access_control->permissions[AccessControl::PERMISSION_MANAGE_SETTINGS]; + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return true; + }) + ) + ); $api = new JSONAPI($access_control); expect($api->validatePermissions(null, $permissions))->true(); } function testItValidatesEndpointMethodPermission() { - $access_control = new AccessControl(); $permissions = array( 'global' => null, 'methods' => array( @@ -212,11 +227,27 @@ class APITest extends \MailPoetTest { ) ); - $access_control->user_roles = array(); + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return false; + }) + ) + ); $api = new JSONAPI($access_control); expect($api->validatePermissions('test', $permissions))->false(); - $access_control->user_roles = $access_control->permissions[AccessControl::PERMISSION_MANAGE_SETTINGS]; + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return true; + }) + ) + ); $api = new JSONAPI($access_control); expect($api->validatePermissions('test', $permissions))->true(); } @@ -245,4 +276,4 @@ class APITest extends \MailPoetTest { WPHooksHelper::releaseAllHooks(); wp_delete_user($this->wp_user_id); } -} \ No newline at end of file +} diff --git a/tests/unit/Config/AccessControlTest.php b/tests/unit/Config/AccessControlTest.php index 3538090712..ede6db79ea 100644 --- a/tests/unit/Config/AccessControlTest.php +++ b/tests/unit/Config/AccessControlTest.php @@ -2,6 +2,9 @@ namespace MailPoet\Test\Config; +use AspectMock\Test as Mock; +use Codeception\Util\Stub; +use Helper\WordPress as WPHelper; use Helper\WordPressHooks as WPHooksHelper; use MailPoet\Config\AccessControl; use MailPoet\WP\Hooks; @@ -103,7 +106,18 @@ class AccessControlTest extends \MailPoetTest { expect(count($permissions))->equals(count($labels)); } - function _after() { - WPHooksHelper::releaseAllHooks(); + function testItValidatesIfUserHasCapability() { + $capability = 'some_capability'; + $access_control = new AccessControl(); + + $func = Mock::func('MailPoet\Config', 'current_user_can', true); + + expect($access_control->validatePermission($capability))->true(); + $func->verifyInvoked([$capability]); } -} \ No newline at end of file + + function _after() { + Mock::clean(); + WPHelper::releaseAllFunctions(); + } +} diff --git a/tests/unit/Router/RouterTest.php b/tests/unit/Router/RouterTest.php index 27d1d75fc1..c449cdcd36 100644 --- a/tests/unit/Router/RouterTest.php +++ b/tests/unit/Router/RouterTest.php @@ -95,23 +95,37 @@ class RouterTest extends \MailPoetTest { } function testItValidatesGlobalPermission() { - $access_control = new AccessControl(); $router = $this->router; $permissions = array( 'global' => AccessControl::PERMISSION_MANAGE_SETTINGS, ); - $access_control->user_roles = array(); + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return false; + }) + ) + ); $router->access_control = $access_control; expect($router->validatePermissions(null, $permissions))->false(); - $access_control->user_roles = $access_control->permissions[AccessControl::PERMISSION_MANAGE_SETTINGS]; + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return true; + }) + ) + ); $router->access_control = $access_control; expect($router->validatePermissions(null, $permissions))->true(); } function testItValidatesEndpointActionPermission() { - $access_control = new AccessControl(); $router = $this->router; $permissions = array( @@ -121,11 +135,27 @@ class RouterTest extends \MailPoetTest { ) ); - $access_control->user_roles = array(); + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return false; + }) + ) + ); $router->access_control = $access_control; expect($router->validatePermissions('test', $permissions))->false(); - $access_control->user_roles = $access_control->permissions[AccessControl::PERMISSION_MANAGE_SETTINGS]; + $access_control = Stub::make( + new AccessControl(), + array( + 'validatePermission' => Stub::once(function($cap) { + expect($cap)->equals(AccessControl::PERMISSION_MANAGE_SETTINGS); + return true; + }) + ) + ); $router->access_control = $access_control; expect($router->validatePermissions('test', $permissions))->true(); } @@ -222,4 +252,4 @@ class RouterTest extends \MailPoetTest { ); expect($result)->contains(Router::NAME . '&endpoint=router_test_mock_endpoint&action=test&data=' . $encoded_data); } -} \ No newline at end of file +}