Skip to content

Commit 6758cbd

Browse files
namithjnamithj
authored
Merge pull request #278 from costdev/add_capability_checks
2 parents 5378799 + 7bc81ec commit 6758cbd

7 files changed

+150
-21
lines changed

‎includes/class-admin-settings.php

+13-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ private function get_default_settings() {
8484
* @return void
8585
*/
8686
public function reset_settings() {
87+
$capability = is_multisite() ? 'manage_network_options' : 'manage_options';
8788
if (
89+
current_user_can( $capability ) &&
8890
isset( $_GET['reset'] ) &&
8991
( 'reset' === $_GET['reset'] ) &&
9092
isset( $_GET['reset-nonce'] ) &&
@@ -123,6 +125,11 @@ public function delete_all_settings() {
123125
* @return void
124126
*/
125127
public function admin_notices() {
128+
$capability = is_multisite() ? 'manage_network_options' : 'manage_options';
129+
if ( ! current_user_can( $capability ) ) {
130+
return;
131+
}
132+
126133
/**
127134
* The Admin Notice to convey a Reset Operation has happened.
128135
*/
@@ -259,7 +266,12 @@ private function get_settings_from_config_file() {
259266
*/
260267
public function update_settings() {
261268
// Exit if improper privileges.
262-
if ( ! isset( $_POST['_wpnonce'] ) || ! wp_verify_nonce( sanitize_key( wp_unslash( $_POST['_wpnonce'] ) ), 'aspireupdate-settings' ) ) {
269+
$capability = is_multisite() ? 'manage_network_options' : 'manage_options';
270+
if (
271+
! current_user_can( $capability ) ||
272+
! isset( $_POST['_wpnonce'] ) ||
273+
! wp_verify_nonce( sanitize_key( wp_unslash( $_POST['_wpnonce'] ) ), 'aspireupdate-settings' )
274+
) {
263275
return;
264276
}
265277

‎includes/class-controller.php

+12-2
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,12 @@ private function api_rewrite() {
6060
* @return void
6161
*/
6262
public function clear_log() {
63-
if ( ! isset( $_POST['nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['nonce'] ), 'aspireupdate-ajax' ) ) {
63+
$capability = is_multisite() ? 'manage_network_options' : 'manage_options';
64+
if (
65+
! current_user_can( $capability ) ||
66+
! isset( $_POST['nonce'] ) ||
67+
! wp_verify_nonce( sanitize_key( $_POST['nonce'] ), 'aspireupdate-ajax' )
68+
) {
6469
wp_send_json_error(
6570
[
6671
'message' => __( 'Error: You are not authorized to access this resource.', 'aspireupdate' ),
@@ -92,7 +97,12 @@ public function clear_log() {
9297
* @return void
9398
*/
9499
public function read_log() {
95-
if ( ! isset( $_POST['nonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['nonce'] ), 'aspireupdate-ajax' ) ) {
100+
$capability = is_multisite() ? 'manage_network_options' : 'manage_options';
101+
if (
102+
! current_user_can( $capability ) ||
103+
! isset( $_POST['nonce'] ) ||
104+
! wp_verify_nonce( sanitize_key( $_POST['nonce'] ), 'aspireupdate-ajax' )
105+
) {
96106
wp_send_json_error(
97107
[
98108
'message' => __( 'Error: You are not authorized to access this resource.', 'aspireupdate' ),

‎tests/phpunit/includes/AdminSettings_UnitTestCase.php

+28-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,20 @@
55
* All \AspireUpdate\Admin_Settings unit tests should inherit from this class.
66
*/
77
abstract class AdminSettings_UnitTestCase extends WP_UnitTestCase {
8+
/**
9+
* The user ID of an administrator.
10+
*
11+
* @var int
12+
*/
13+
protected static $admin_id;
14+
15+
/**
16+
* The user ID of an editor.
17+
*
18+
* @var int
19+
*/
20+
protected static $editor_id;
21+
822
/**
923
* The Name of the Option.
1024
*
@@ -20,13 +34,26 @@ abstract class AdminSettings_UnitTestCase extends WP_UnitTestCase {
2034
protected static $options_page = 'aspireupdate-settings';
2135

2236
/**
23-
* Deletes settings before each test runs.
37+
* Creates administrator and editor users before any tests run.
38+
*
39+
* @return void
40+
*/
41+
public static function set_up_before_class() {
42+
parent::set_up_before_class();
43+
44+
self::$admin_id = self::factory()->user->create( [ 'role' => 'administrator' ] );
45+
self::$editor_id = self::factory()->user->create( [ 'role' => 'editor' ] );
46+
}
47+
48+
/**
49+
* Deletes settings and sets the current user before each test runs.
2450
*
2551
* @return void
2652
*/
2753
public function set_up() {
2854
parent::set_up();
2955

3056
delete_site_option( self::$option_name );
57+
wp_set_current_user( self::$admin_id );
3158
}
3259
}

‎tests/phpunit/tests/AdminSettings/AdminSettings_AdminNoticesTest.php

+35
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,29 @@
1111
* @covers \AspireUpdate\Admin_Settings::admin_notices
1212
*/
1313
class AdminSettings_AdminNoticesTest extends AdminSettings_UnitTestCase {
14+
/**
15+
* Test that the reset notice is not output when the user does not have the required capability.
16+
*/
17+
public function test_should_not_output_reset_notice_when_the_user_does_not_have_the_required_capability() {
18+
wp_set_current_user( self::$editor_id );
19+
20+
$this->assertFalse(
21+
current_user_can( is_multisite() ? 'manage_network_options' : 'manage_options' ),
22+
'The user has the required capability.'
23+
);
24+
25+
update_site_option( 'aspireupdate-reset', 'true' );
26+
$_GET['reset-success'] = 'success';
27+
$_GET['reset-success-nonce'] = wp_create_nonce( 'aspireupdate-reset-success-nonce' );
28+
29+
$admin_settings = new \AspireUpdate\Admin_Settings();
30+
$actual = get_echo( [ $admin_settings, 'admin_notices' ] );
31+
32+
unset( $_GET['reset-success'], $_GET['reset-success-nonce'] );
33+
34+
$this->assertEmpty( $actual );
35+
}
36+
1437
/**
1538
* Test that the reset notice is not output when the 'aspireupdate-reset' option is not set to (string) "true".
1639
*/
@@ -104,6 +127,10 @@ public function test_should_output_reset_notice() {
104127
$_GET['reset-success'] = 'success';
105128
$_GET['reset-success-nonce'] = wp_create_nonce( 'aspireupdate-reset-success-nonce' );
106129

130+
if ( is_multisite() ) {
131+
grant_super_admin( wp_get_current_user()->ID );
132+
}
133+
107134
$admin_settings = new \AspireUpdate\Admin_Settings();
108135
$actual = get_echo( [ $admin_settings, 'admin_notices' ] );
109136

@@ -123,6 +150,10 @@ public function test_should_delete_aspireupdatereset_option_after_output() {
123150
$_GET['reset-success'] = 'success';
124151
$_GET['reset-success-nonce'] = wp_create_nonce( 'aspireupdate-reset-success-nonce' );
125152

153+
if ( is_multisite() ) {
154+
grant_super_admin( wp_get_current_user()->ID );
155+
}
156+
126157
$admin_settings = new \AspireUpdate\Admin_Settings();
127158
get_echo( [ $admin_settings, 'admin_notices' ] );
128159

@@ -163,6 +194,10 @@ public function test_should_not_output_saved_notice_when_nonce_verification_fail
163194
public function test_should_output_saved_notice() {
164195
$_GET['settings-updated-wpnonce'] = wp_create_nonce( 'aspireupdate-settings-updated-nonce' );
165196

197+
if ( is_multisite() ) {
198+
grant_super_admin( wp_get_current_user()->ID );
199+
}
200+
166201
$admin_settings = new \AspireUpdate\Admin_Settings();
167202
$actual = get_echo( [ $admin_settings, 'admin_notices' ] );
168203

‎tests/phpunit/tests/AdminSettings/AdminSettings_RegisterAdminMenuTest.php

+1-17
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,6 @@
1717
* @covers \AspireUpdate\Admin_Settings::register_admin_menu
1818
*/
1919
class AdminSettings_RegisterAdminMenuTest extends AdminSettings_UnitTestCase {
20-
/**
21-
* The user ID of an administrator.
22-
*
23-
* @var int
24-
*/
25-
private static $admin_id;
26-
27-
/**
28-
* Create an administrator before any tests run.
29-
*
30-
* @return void
31-
*/
32-
public static function set_up_before_class() {
33-
self::$admin_id = self::factory()->user->create( [ 'role' => 'administrator' ] );
34-
}
3520

3621
/**
3722
* Test that menu is not registered when AP_REMOVE_UI is enabled.
@@ -41,7 +26,6 @@ public function test_should_not_register_menu_when_ap_remove_ui_is_enabled() {
4126
$original_submenu = $submenu;
4227

4328
define( 'AP_REMOVE_UI', true );
44-
wp_set_current_user( self::$admin_id );
4529

4630
$admin_settings = new \AspireUpdate\Admin_Settings();
4731
$admin_settings->register_admin_menu();
@@ -57,6 +41,7 @@ public function test_should_not_register_menu_when_user_lacks_appropriate_permis
5741
$original_submenu = $submenu;
5842

5943
define( 'AP_REMOVE_UI', false );
44+
wp_set_current_user( self::$editor_id );
6045

6146
$admin_settings = new \AspireUpdate\Admin_Settings();
6247
$admin_settings->register_admin_menu();
@@ -71,7 +56,6 @@ public function test_should_register_menu_when_ap_remove_ui_is_disabled() {
7156
global $submenu;
7257

7358
define( 'AP_REMOVE_UI', false );
74-
wp_set_current_user( self::$admin_id );
7559
grant_super_admin( self::$admin_id );
7660

7761
$admin_settings = new \AspireUpdate\Admin_Settings();

‎tests/phpunit/tests/AdminSettings/AdminSettings_ResetSettingsTest.php

+30
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,28 @@
1111
* @covers \AspireUpdate\Admin_Settings::reset_settings
1212
*/
1313
class AdminSettings_ResetSettingsTest extends AdminSettings_UnitTestCase {
14+
/**
15+
* Test that settings are not reset when the user does not have the required capability.
16+
*
17+
* @runInSeparateProcess
18+
* @preserveGlobalState disabled
19+
*/
20+
public function test_should_not_reset_settings_when_the_user_does_not_have_the_required_capability() {
21+
wp_set_current_user( self::$editor_id );
22+
$_GET['reset'] = 'reset';
23+
$_GET['reset-nonce'] = wp_create_nonce( 'aspireupdate-reset-nonce' );
24+
25+
$settings = [ 'api_host' => 'the.option.value' ];
26+
update_site_option( self::$option_name, $settings );
27+
28+
$admin_settings = new \AspireUpdate\Admin_Settings();
29+
$admin_settings->reset_settings();
30+
31+
unset( $_GET['reset'], $_GET['reset-nonce'] );
32+
33+
$this->assertSame( $settings, get_site_option( self::$option_name ) );
34+
}
35+
1436
/**
1537
* Test that settings are not reset when $_GET['reset'] is not set.
1638
*/
@@ -146,6 +168,10 @@ public function test_should_reset_settings_when_reset_requirements_are_met() {
146168
$_GET['reset'] = 'reset';
147169
$_GET['reset-nonce'] = wp_create_nonce( 'aspireupdate-reset-nonce' );
148170

171+
if ( is_multisite() ) {
172+
grant_super_admin( wp_get_current_user()->ID );
173+
}
174+
149175
$settings = [ 'api_host' => 'the.option.value' ];
150176
update_site_option( self::$option_name, $settings );
151177

@@ -167,6 +193,10 @@ public function test_should_redirect_when_reset_requirements_are_met() {
167193
$_GET['reset'] = 'reset';
168194
$_GET['reset-nonce'] = wp_create_nonce( 'aspireupdate-reset-nonce' );
169195

196+
if ( is_multisite() ) {
197+
grant_super_admin( wp_get_current_user()->ID );
198+
}
199+
170200
$settings = [ 'api_host' => 'the.option.value' ];
171201
update_site_option( self::$option_name, $settings );
172202

‎tests/phpunit/tests/AdminSettings/AdminSettings_UpdateSettingsTest.php

+31
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,29 @@
1111
* @covers \AspireUpdate\Admin_Settings::update_settings
1212
*/
1313
class AdminSettings_UpdateSettingsTest extends AdminSettings_UnitTestCase {
14+
/**
15+
* Test that settings are not updated when the user does not have the required capability.
16+
*
17+
* @runInSeparateProcess
18+
* @preserveGlobalState disabled
19+
*/
20+
public function test_should_not_update_settings_when_the_user_does_not_have_the_required_capability() {
21+
wp_set_current_user( self::$editor_id );
22+
$_POST['_wpnonce'] = wp_create_nonce( self::$options_page );
23+
$_POST['option_page'] = self::$option_name;
24+
$_POST['aspireupdate_settings'] = [ 'api_host' => 'the.option.value' ];
25+
26+
$settings = get_site_option( self::$option_name, false );
27+
28+
$admin_settings = new AspireUpdate\Admin_Settings();
29+
$admin_settings->update_settings();
30+
31+
$this->assertSame(
32+
$settings,
33+
get_site_option( self::$option_name, false )
34+
);
35+
}
36+
1437
/**
1538
* Test that settings are not updated when $_POST['_wpnonce'] is not set.
1639
*/
@@ -202,6 +225,10 @@ public function test_should_update_settings_when_update_requirements_are_met() {
202225
$_POST['option_page'] = self::$option_name;
203226
$_POST['aspireupdate_settings'] = [ 'api_host' => 'the.option.value' ];
204227

228+
if ( is_multisite() ) {
229+
grant_super_admin( wp_get_current_user()->ID );
230+
}
231+
205232
delete_site_option( self::$option_name );
206233

207234
$admin_settings = new AspireUpdate\Admin_Settings();
@@ -238,6 +265,10 @@ public function test_should_redirect_when_update_requirements_are_met() {
238265
$_POST['option_page'] = self::$option_name;
239266
$_POST['aspireupdate_settings'] = [ 'api_host' => 'the.option.value' ];
240267

268+
if ( is_multisite() ) {
269+
grant_super_admin( wp_get_current_user()->ID );
270+
}
271+
241272
$redirect = new MockAction();
242273
add_filter( 'wp_redirect', [ $redirect, 'filter' ] );
243274

0 commit comments

Comments
 (0)