From c413f6cc9ee38fca5ef99c277a9bcf4ce1ef2e7f Mon Sep 17 00:00:00 2001 From: BE-Webdesign Date: Tue, 9 Aug 2016 19:48:00 -0400 Subject: [PATCH 1/3] #17-fix-permissions. Fixes #17. Currently the permissions do not match what core uses to restric plugin access. This does not account for multisite, which will be added in a separate commit on a separate PR. Tests are updated to reflect unauthenticated and authenticated users without permissions. --- lib/class-wp-rest-plugins-controller.php | 8 ++---- tests/test-rest-plugins-controller.php | 31 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/lib/class-wp-rest-plugins-controller.php b/lib/class-wp-rest-plugins-controller.php index 8e6ac19..50a9528 100644 --- a/lib/class-wp-rest-plugins-controller.php +++ b/lib/class-wp-rest-plugins-controller.php @@ -43,13 +43,11 @@ public function register_routes() { * @return WP_Error|boolean */ public function get_items_permissions_check( $request ) { - - if ( ! current_user_can( 'manage_options' ) ) { // TODO: Something related to plugins. activate_plugin capability seems to not be available for multi-site superadmin (?) + if ( ! current_user_can( 'activate_plugins' ) ) { return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); } return true; - } public function get_items( $request ) { @@ -76,13 +74,11 @@ public function get_items( $request ) { * @return WP_Error|boolean */ public function get_item_permissions_check( $request ) { - - if ( ! current_user_can( 'manage_options' ) ) { // TODO: Something related to plugins. activate_plugin capability seems to not be available for multi-site superadmin (?) + if ( ! current_user_can( 'activate_plugins' ) ) { return new WP_Error( 'rest_forbidden', __( 'Sorry, you do not have access to this resource' ), array( 'status' => rest_authorization_required_code() ) ); } return true; - } public function get_item( $request ) { diff --git a/tests/test-rest-plugins-controller.php b/tests/test-rest-plugins-controller.php index 350260b..fd7a407 100644 --- a/tests/test-rest-plugins-controller.php +++ b/tests/test-rest-plugins-controller.php @@ -9,6 +9,10 @@ public function setUp() { $this->admin_id = $this->factory->user->create( array( 'role' => 'administrator', ) ); + + $this->subscriber = $this->factory->user->create( array( + 'role' => 'subscriber', + ) ); } public function test_register_routes() { @@ -27,6 +31,11 @@ public function test_delete_item_without_permission() { $this->assertErrorResponse( 'rest_forbidden', $response, 401 ); + wp_set_current_user( $this->subscriber ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); } public function test_context_param() { @@ -54,6 +63,22 @@ public function test_get_item() { $this->check_get_plugins_response( $response, 'view' ); } + public function test_get_item_without_permissions() { + wp_set_current_user( 0 ); + + $request = new WP_REST_Request( 'GET', '/wp/v2/plugins/hello-dolly' ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 401 ); + + wp_set_current_user( $this->subscriber ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); + } + public function test_create_item() { } @@ -97,6 +122,12 @@ public function test_get_items_without_permissions() { $response = $this->server->dispatch( $request ); $this->assertErrorResponse( 'rest_forbidden', $response, 401 ); + + wp_set_current_user( $this->subscriber ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); } protected function check_get_plugins_response( $response, $context = 'view' ) { From 63d77ac74d570b9daf8fd1bcd85b3990f619eb23 Mon Sep 17 00:00:00 2001 From: BE-Webdesign Date: Tue, 9 Aug 2016 20:28:00 -0400 Subject: [PATCH 2/3] Fix permissions to support multisite. Fixing the permissions to support multisite. I was going to do this in a separate PR but I forgot that the CI runs both tests at once. This handles priviledges for site admins as well when plugin management is activated in the settings panel. --- lib/class-wp-rest-plugins-controller.php | 30 ++++++- tests/test-rest-plugins-controller.php | 102 ++++++++++++++++++++++- 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/lib/class-wp-rest-plugins-controller.php b/lib/class-wp-rest-plugins-controller.php index 50a9528..d4b1cf4 100644 --- a/lib/class-wp-rest-plugins-controller.php +++ b/lib/class-wp-rest-plugins-controller.php @@ -43,8 +43,19 @@ public function register_routes() { * @return WP_Error|boolean */ public function get_items_permissions_check( $request ) { - if ( ! current_user_can( 'activate_plugins' ) ) { - return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); + if ( is_multisite() ) { + $menu_permissions = get_site_option( 'menu_items' ); + // Check if the user cannot manage network plugins but site admins have access to + if ( ! current_user_can( 'manage_network_plugins' ) && ! isset( $menu_permissions['plugins'] ) && '1' !== $menu_permissions['plugins'] ) { + return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view plugins.', 'be-rest-endpoints' ), array( 'status' => rest_authorization_required_code() ) ); + } + if ( isset( $menu_permissions['plugins'] ) && '1' === $menu_permissions['plugins'] && ! current_user_can( 'activate_plugins' ) ) { + return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); + } + } else { + if ( ! current_user_can( 'activate_plugins' ) ) { + return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); + } } return true; @@ -74,8 +85,19 @@ public function get_items( $request ) { * @return WP_Error|boolean */ public function get_item_permissions_check( $request ) { - if ( ! current_user_can( 'activate_plugins' ) ) { - return new WP_Error( 'rest_forbidden', __( 'Sorry, you do not have access to this resource' ), array( 'status' => rest_authorization_required_code() ) ); + if ( is_multisite() ) { + $menu_permissions = get_site_option( 'menu_items' ); + // Check if the user cannot manage network plugins but site admins have access to + if ( ! current_user_can( 'manage_network_plugins' ) && ! isset( $menu_permissions['plugins'] ) && '1' !== $menu_permissions['plugins'] ) { + return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view plugins.', 'be-rest-endpoints' ), array( 'status' => rest_authorization_required_code() ) ); + } + if ( isset( $menu_permissions['plugins'] ) && '1' === $menu_permissions['plugins'] && ! current_user_can( 'activate_plugins' ) ) { + return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); + } + } else { + if ( ! current_user_can( 'activate_plugins' ) ) { + return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); + } } return true; diff --git a/tests/test-rest-plugins-controller.php b/tests/test-rest-plugins-controller.php index fd7a407..1c6f4fd 100644 --- a/tests/test-rest-plugins-controller.php +++ b/tests/test-rest-plugins-controller.php @@ -10,6 +10,10 @@ public function setUp() { 'role' => 'administrator', ) ); + $this->super_admin = $this->factory->user->create( array( + 'role' => 'administrator', + ) ); + $this->subscriber = $this->factory->user->create( array( 'role' => 'subscriber', ) ); @@ -43,9 +47,11 @@ public function test_context_param() { } public function test_get_items() { - wp_set_current_user( $this->admin_id ); + $this->set_user_with_priviledges(); + $request = new WP_REST_Request( 'GET', '/wp/v2/plugins' ); $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); $data = $response->get_data(); $this->assertEquals( 2, count( $data ) ); @@ -53,9 +59,42 @@ public function test_get_items() { $this->assertEquals( 'Akismet', $data[0]['name'] ); } - public function test_get_item() { + public function test_get_items_multisite_permissions() { + if ( is_multisite() ) { + wp_set_current_user( $this->admin_id ); - wp_set_current_user( $this->admin_id ); + // By default an admin cannot check plugins on a multisite install. + $request = new WP_REST_Request( 'GET', '/wp/v2/plugins' ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); + + // Now check when it is enabled for admins. + $menu_permissions = get_site_option( 'menu_items' ); + $menu_permissions['plugins'] = '1'; + update_site_option( 'menu_items', $menu_permissions ); + + // This should return a positive result now. + $response = $this->server->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + + // Check to make sure lower users can still not access plugins. + wp_set_current_user( $this->subscriber ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); + + // Clean up. + $menu_permissions['plugins'] = '0'; + update_site_option( 'menu_items', $menu_permissions ); + } + } + + public function test_get_item() { + $this->set_user_with_priviledges(); $request = new WP_REST_Request( 'GET', '/wp/v2/plugins/hello-dolly' ); $response = $this->server->dispatch( $request ); @@ -79,6 +118,40 @@ public function test_get_item_without_permissions() { $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); } + public function test_get_item_multisite_permissions() { + if ( is_multisite() ) { + wp_set_current_user( $this->admin_id ); + + // By default an admin cannot check plugins on a multisite install. + $request = new WP_REST_Request( 'GET', '/wp/v2/plugins/hello-dolly' ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); + + // Now check when it is enabled for admins. + $menu_permissions = get_site_option( 'menu_items' ); + $menu_permissions['plugins'] = '1'; + update_site_option( 'menu_items', $menu_permissions ); + + // This should return a positive result now. + $response = $this->server->dispatch( $request ); + + $this->assertEquals( 200, $response->get_status() ); + + // Check to make sure lower users can still not access plugins. + wp_set_current_user( $this->subscriber ); + + $response = $this->server->dispatch( $request ); + + $this->assertErrorResponse( 'rest_forbidden', $response, 403 ); + + // Clean up. + $menu_permissions['plugins'] = '0'; + update_site_option( 'menu_items', $menu_permissions ); + } + } + public function test_create_item() { } @@ -143,4 +216,27 @@ protected function check_get_plugins_response( $response, $context = 'view' ) { protected function check_plugin_data( $plugin ) { // todo: add plugin assertions } + + protected function allow_user_to_manage_multisite() { + wp_set_current_user( $this->super_admin ); + $user = wp_get_current_user(); + + if ( is_multisite() ) { + update_site_option( 'site_admins', array( $user->user_login ) ); + } + + return; + } + + /** + * Handles the difference between multisite and single site users. + * + * Special cases need to be handled in the tests still. + */ + protected function set_user_with_priviledges() { + wp_set_current_user( $this->admin_id ); + if ( is_multisite() ) { + $this->allow_user_to_manage_multisite(); + } + } } From add4865cd224c2097d1a55138e60dd18a121fb28 Mon Sep 17 00:00:00 2001 From: BE-Webdesign Date: Mon, 24 Oct 2016 12:38:30 -0400 Subject: [PATCH 3/3] Fix permissions check for plugins This is the proper permissions check for multisite and single site installs for WordPress. --- lib/class-wp-rest-plugins-controller.php | 26 ++++++++++++++++-------- tests/test-rest-plugins-controller.php | 8 ++++---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/class-wp-rest-plugins-controller.php b/lib/class-wp-rest-plugins-controller.php index 89b6484..0137829 100644 --- a/lib/class-wp-rest-plugins-controller.php +++ b/lib/class-wp-rest-plugins-controller.php @@ -44,21 +44,29 @@ public function register_routes() { */ public function get_items_permissions_check( $request ) { if ( is_multisite() ) { + /** + * The menu items option determines whether a site admin who is not + * a super admin can work with plugins for their site. This is a + * core feature. wp-admin\network\settings.php + */ $menu_permissions = get_site_option( 'menu_items' ); - // Check if the user cannot manage network plugins but site admins have access to - if ( ! current_user_can( 'manage_network_plugins' ) && ! isset( $menu_permissions['plugins'] ) && '1' !== $menu_permissions['plugins'] ) { - return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view plugins.', 'be-rest-endpoints' ), array( 'status' => rest_authorization_required_code() ) ); - } - if ( isset( $menu_permissions['plugins'] ) && '1' === $menu_permissions['plugins'] && ! current_user_can( 'activate_plugins' ) ) { - return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); + // Check if the user cannot manage network plugins. + if ( current_user_can( 'manage_network_plugins' ) ) { + return true; + } else { + // Check if the plugins menu is active for site administators. + if ( isset( $menu_permissions['plugins'] ) && '1' === $menu_permissions['plugins'] && current_user_can( 'activate_plugins' ) ) { + return true; + } } } else { - if ( ! current_user_can( 'activate_plugins' ) ) { - return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view the list of plugins' ), array( 'status' => rest_authorization_required_code() ) ); + if ( current_user_can( 'activate_plugins' ) ) { + return true; } } - return true; + // White list approach. This does not follow rest endpoint conventions. + return new WP_Error( 'rest_forbidden', __( 'Sorry, you cannot view plugins.', 'be-rest-endpoints' ), array( 'status' => rest_authorization_required_code() ) ); } public function get_items( $request ) { diff --git a/tests/test-rest-plugins-controller.php b/tests/test-rest-plugins-controller.php index d4e5179..b5dea68 100644 --- a/tests/test-rest-plugins-controller.php +++ b/tests/test-rest-plugins-controller.php @@ -64,8 +64,8 @@ public function test_get_items() { */ public function test_get_items_pagination_headers() { // Skipped until more plugins are added into wordpress-develop repo. + $this->set_user_with_priviledges(); - wp_set_current_user( $this->admin_id ); // One plugin installed by default. $request = new WP_REST_Request( 'GET', '/wp/v2/plugins' ); $request->set_param( 'per_page', 1 ); @@ -129,7 +129,7 @@ public function test_get_items_pagination_headers() { } public function test_get_items_per_page() { - wp_set_current_user( $this->admin_id ); + $this->set_user_with_priviledges(); $request = new WP_REST_Request( 'GET', '/wp/v2/plugins' ); $response = $this->server->dispatch( $request ); @@ -142,7 +142,7 @@ public function test_get_items_per_page() { } public function test_get_items_page() { - wp_set_current_user( $this->admin_id ); + $this->set_user_with_priviledges(); $request = new WP_REST_Request( 'GET', '/wp/v2/plugins' ); $request->set_param( 'per_page', 1 ); @@ -159,7 +159,7 @@ public function test_get_items_page() { } public function test_get_items_offset() { - wp_set_current_user( $this->admin_id ); + $this->set_user_with_priviledges(); // 2 Plugins installed by default. $request = new WP_REST_Request( 'GET', '/wp/v2/plugins' );