Skip to content

Commit 174f01a

Browse files
Dan0szDan0sz
authored
Merge pull request #287 from plausible/fix_invalid_api_token
Fixed: prevent data corruption when options are saved during an AJAX request.
2 parents b590b12 + 65a8e16 commit 174f01a

File tree

10 files changed

+239
-79
lines changed

10 files changed

+239
-79
lines changed

‎src/Admin/Module.php‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,12 @@ private function init() {
4343
*
4444
*/
4545
public function maybe_install_module( $old_settings, $settings ) {
46-
if ( $settings['proxy_enabled'] === 'on' && $old_settings['proxy_enabled'] !== 'on' ) {
46+
$settings_proxy = ( is_array( $settings ) && isset( $settings['proxy_enabled'] ) ) ? $settings['proxy_enabled'] : '';
47+
$old_proxy = ( is_array( $old_settings ) && isset( $old_settings['proxy_enabled'] ) ) ? $old_settings['proxy_enabled'] : '';
48+
49+
if ( $settings_proxy === 'on' && $old_proxy !== 'on' ) {
4750
$this->install();
48-
} elseif ( $settings['proxy_enabled'] === '' && $old_settings['proxy_enabled'] === 'on' ) {
51+
} elseif ( $settings_proxy === '' && $old_proxy === 'on' ) {
4952
$this->uninstall();
5053
}
5154
}
@@ -196,7 +199,10 @@ public function maybe_enable_proxy( $settings, $old_settings ) {
196199
/**
197200
* No need to run this on each update run, or when the proxy is disabled.
198201
*/
199-
if ( empty( $settings['proxy_enabled'] ) || ( $settings['proxy_enabled'] === 'on' && $old_settings['proxy_enabled'] === 'on' ) ) {
202+
$new_proxy_setting = ( is_array( $settings ) && isset( $settings['proxy_enabled'] ) ) ? $settings['proxy_enabled'] : '';
203+
$old_proxy_setting = ( is_array( $old_settings ) && isset( $old_settings['proxy_enabled'] ) ) ? $old_settings['proxy_enabled'] : '';
204+
205+
if ( empty( $new_proxy_setting ) || ( $new_proxy_setting === 'on' && $old_proxy_setting === 'on' ) ) {
200206
return $settings;
201207
}
202208

@@ -259,6 +265,11 @@ private function is_ssl() {
259265
* @since 1.3.0
260266
*/
261267
private function test_proxy( $run = true ) {
268+
// Always succeed if this is a CI environment.
269+
if ( defined( 'PLAUSIBLE_CI' ) ) {
270+
return true;
271+
}
272+
262273
// Should we run the test?
263274
if ( ! apply_filters( 'plausible_analytics_module_run_test_proxy', $run ) ) {
264275
return false; // @codeCoverageIgnore

‎src/Ajax.php‎

Lines changed: 69 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,15 @@ public function fetch_messages() {
7676
public function quit_wizard() {
7777
$request_data = $this->clean( $_REQUEST );
7878

79-
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $request_data[ '_nonce' ], 'plausible_analytics_quit_wizard' ) < 1 ) {
79+
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $request_data['_nonce'], 'plausible_analytics_quit_wizard' ) < 1 ) {
8080
Messages::set_error( __( 'Not allowed', 'plausible-analytics' ) );
8181

8282
wp_send_json_error( null, 403 );
8383
}
8484

8585
update_option( 'plausible_analytics_wizard_done', true );
8686

87-
$this->maybe_handle_redirect( $request_data[ 'redirect' ] );
87+
$this->maybe_handle_redirect( $request_data['redirect'] );
8888

8989
wp_send_json_success();
9090
}
@@ -93,26 +93,41 @@ public function quit_wizard() {
9393
* Clean variables using `sanitize_text_field`.
9494
* Arrays are cleaned recursively. Non-scalar values are ignored.
9595
*
96-
* @since 1.3.0
97-
* @access public
98-
*
9996
* @param string|array $var Sanitize the variable.
10097
*
10198
* @return string|array
99+
* @since 1.3.0
100+
* @access public
101+
*
102102
*/
103-
private function clean( $var ) {
103+
private function clean( $var, $key = '' ) {
104104
// If the variable is an array, recursively apply the function to each element of the array.
105105
if ( is_array( $var ) ) {
106-
return array_map( [ $this, 'clean' ], $var );
106+
$cleaned = [];
107+
108+
foreach ( $var as $k => $v ) {
109+
$cleaned[ $k ] = $this->clean( $v, $k );
110+
}
111+
112+
return $cleaned;
107113
}
108114

109115
// If the variable is a scalar value (string, integer, float, boolean).
110116
if ( is_scalar( $var ) ) {
117+
/**
118+
* If the variable is the options object, we only unslash it, but don't sanitize it yet.
119+
* Sanitization will happen after json_decode.
120+
*/
121+
if ( $key === 'options' ) {
122+
return wp_unslash( $var );
123+
}
124+
111125
// Parse the variable using the wp_parse_url function.
112126
$parsed = wp_parse_url( $var );
127+
113128
// If the variable has a scheme (e.g. http:// or https://), sanitize the variable using the esc_url_raw function.
114-
if ( isset( $parsed[ 'scheme' ] ) ) {
115-
return esc_url_raw( wp_unslash( $var ), [ $parsed[ 'scheme' ] ] );
129+
if ( isset( $parsed['scheme'] ) ) {
130+
return esc_url_raw( wp_unslash( $var ) );
116131
}
117132

118133
// If the variable does not have a scheme, sanitize the variable using the sanitize_text_field function.
@@ -157,54 +172,54 @@ private function maybe_handle_redirect( $direction ) {
157172
public function show_wizard() {
158173
$request_data = $this->clean( $_REQUEST );
159174

160-
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $request_data[ '_nonce' ], 'plausible_analytics_show_wizard' ) < 1 ) {
175+
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $request_data['_nonce'], 'plausible_analytics_show_wizard' ) < 1 ) {
161176
Messages::set_error( __( 'Not allowed.', 'plausible-analytics' ) );
162177

163178
wp_send_json_error( null, 403 );
164179
}
165180

166181
delete_option( 'plausible_analytics_wizard_done' );
167182

168-
$this->maybe_handle_redirect( $request_data[ 'redirect' ] );
183+
$this->maybe_handle_redirect( $request_data['redirect'] );
169184

170185
wp_send_json_success();
171186
}
172187

173188
/**
174189
* Save Admin Settings
175190
*
176-
* @since 1.0.0
177191
* @return void
192+
* @since 1.0.0
178193
*/
179194
public function toggle_option() {
180195
// Sanitize all the post data before using.
181196
$post_data = $this->clean( $_POST );
182197
$settings = Helpers::get_settings();
183198

184-
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $post_data[ '_nonce' ], 'plausible_analytics_toggle_option' ) < 1 ) {
199+
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $post_data['_nonce'], 'plausible_analytics_toggle_option' ) < 1 ) {
185200
wp_send_json_error( __( 'Not allowed.', 'plausible-analytics' ), 403 );
186201
}
187202

188-
if ( $post_data[ 'is_list' ] ) {
203+
if ( $post_data['is_list'] ) {
189204
/**
190205
* Toggle lists.
191206
*/
192-
if ( $post_data[ 'toggle_status' ] === 'on' ) {
207+
if ( $post_data['toggle_status'] === 'on' ) {
193208
// If toggle is on, store the value under a new key.
194-
if ( ! in_array( $post_data[ 'option_value' ], $settings[ $post_data[ 'option_name' ] ] ) ) {
195-
$settings[ $post_data[ 'option_name' ] ][] = $post_data[ 'option_value' ];
209+
if ( ! in_array( $post_data['option_value'], $settings[ $post_data['option_name'] ] ) ) {
210+
$settings[ $post_data['option_name'] ][] = $post_data['option_value'];
196211
}
197212
} else {
198213
// If toggle is off, find the key by its value and unset it.
199-
if ( ( $key = array_search( $post_data[ 'option_value' ], $settings[ $post_data[ 'option_name' ] ] ) ) !== false ) {
200-
unset( $settings[ $post_data[ 'option_name' ] ][ $key ] );
214+
if ( ( $key = array_search( $post_data['option_value'], $settings[ $post_data['option_name'] ] ) ) !== false ) {
215+
unset( $settings[ $post_data['option_name'] ][ $key ] );
201216
}
202217
}
203218
} else {
204219
/**
205220
* Single toggles.
206221
*/
207-
$settings[ $post_data[ 'option_name' ] ] = $post_data[ 'toggle_status' ];
222+
$settings[ $post_data['option_name'] ] = $post_data['toggle_status'];
208223
}
209224

210225
// Update all the options to plausible settings.
@@ -213,22 +228,22 @@ public function toggle_option() {
213228
/**
214229
* Allow devs to perform additional actions.
215230
*/
216-
do_action( 'plausible_analytics_settings_saved', $settings, $post_data[ 'option_name' ], $post_data[ 'toggle_status' ] );
231+
do_action( 'plausible_analytics_settings_saved', $settings, $post_data['option_name'], $post_data['toggle_status'] );
217232

218-
$option_label = $post_data[ 'option_label' ];
219-
$toggle_status = $post_data[ 'toggle_status' ] === 'on' ? __( 'enabled', 'plausible-analytics' ) : __( 'disabled', 'plausible-analytics' );
233+
$option_label = $post_data['option_label'];
234+
$toggle_status = $post_data['toggle_status'] === 'on' ? __( 'enabled', 'plausible-analytics' ) : __( 'disabled', 'plausible-analytics' );
220235
$message = apply_filters(
221236
'plausible_analytics_toggle_option_success_message',
222237
sprintf( '%s %s.', $option_label, $toggle_status ),
223-
$post_data[ 'option_name' ],
224-
$post_data[ 'toggle_status' ]
238+
$post_data['option_name'],
239+
$post_data['toggle_status']
225240
);
226241

227242
Messages::set_success( $message );
228243

229-
$additional = $this->maybe_render_additional_message( $post_data[ 'option_name' ], $post_data[ 'toggle_status' ] );
244+
$additional = $this->maybe_render_additional_message( $post_data['option_name'], $post_data['toggle_status'] );
230245

231-
Messages::set_additional( $additional, $post_data[ 'option_name' ] );
246+
Messages::set_additional( $additional, $post_data['option_name'] );
232247

233248
wp_send_json_success( null, 200 );
234249
}
@@ -271,17 +286,21 @@ public function save_options() {
271286
$post_data = $this->clean( $_POST );
272287
$settings = Helpers::get_settings();
273288

274-
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $post_data[ '_nonce' ], 'plausible_analytics_toggle_option' ) < 1 ) {
289+
if ( ! current_user_can( 'manage_options' ) || wp_verify_nonce( $post_data['_nonce'], 'plausible_analytics_toggle_option' ) < 1 ) {
275290
Messages::set_error( __( 'Not allowed.', 'plausible-analytics' ) );
276291

277292
wp_send_json_error( null, 403 );
278293
}
279294

280-
$options = json_decode( $post_data[ 'options' ] );
295+
$options = json_decode( $post_data['options'] );
281296

282297
if ( empty( $options ) ) {
283298
Messages::set_error( __( 'No options found to save.', 'plausible-analytics' ) );
284299

300+
if ( defined( 'PLAUSIBLE_CI' ) ) {
301+
return;
302+
}
303+
285304
wp_send_json_error( null, 400 );
286305
}
287306

@@ -298,46 +317,52 @@ function ( $option ) {
298317
);
299318

300319
if ( count( $input_array_elements ) > 0 ) {
301-
$options = [];
302-
$array_name = preg_replace( '/\[[0-9]+]/', '', $input_array_elements[ 0 ]->name );
303-
$options[ 0 ] = (object) [];
304-
$options[ 0 ]->name = $array_name;
320+
$options = [];
321+
$array_name = preg_replace( '/\[[0-9]+]/', '', $input_array_elements[0]->name );
322+
$options[0] = (object) [];
323+
$options[0]->name = $array_name;
324+
$options[0]->value = [];
305325

306326
foreach ( $input_array_elements as $input_array_element ) {
307327
if ( $input_array_element->value ) {
308-
$options[ 0 ]->value[] = $input_array_element->value;
328+
$options[0]->value[] = $input_array_element->value;
309329
}
310330
}
311331
}
312332

313333
foreach ( $options as $option ) {
334+
$name = sanitize_text_field( $option->name );
335+
$value = $this->clean( $option->value );
336+
314337
// Clean spaces
315-
if ( is_string( $option->value ) ) {
316-
$settings[ $option->name ] = trim( $option->value );
338+
if ( is_string( $value ) ) {
339+
$settings[ $name ] = trim( $value );
317340
} else {
318-
$settings[ $option->name ] = $option->value;
341+
$settings[ $name ] = $value;
319342
}
320343

321344
// Validate Plugin Token if this is the Plugin Token field.
322-
if ( $option->name === 'api_token' ) {
323-
$this->validate_api_token( $option->value );
345+
if ( $name === 'api_token' ) {
346+
$this->validate_api_token( $value );
324347

325-
$additional = $this->maybe_render_additional_message( $option->name, $option->value );
348+
$additional = $this->maybe_render_additional_message( $name, $value );
326349

327-
Messages::set_additional( $additional, $option->name );
350+
Messages::set_additional( $additional, $name );
328351
}
329352

330353
// Refresh Tracker ID if Domain Name has changed (e.g. after migration from staging to production)
331-
if ($option->name === 'domain_name') {
332-
delete_option('plausible_analytics_tracker_id');
354+
if ( $name === 'domain_name' ) {
355+
delete_option( 'plausible_analytics_tracker_id' );
333356
}
334357
}
335358

336359
update_option( 'plausible_analytics_settings', $settings );
337360

338361
Messages::set_success( __( 'Settings saved.', 'plausible-analytics' ) );
339362

340-
wp_send_json_success( null, 200 );
363+
if ( ! defined( 'PLAUSIBLE_CI' ) ) {
364+
wp_send_json_success( null, 200 );
365+
}
341366
}
342367

343368
/**

‎src/Assets.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function __construct() {
2121
* @return void
2222
*/
2323
private function init() {
24-
add_action( 'wp_enqueue_scripts', [ $this, 'maybe_enqueue_main_script' ] );
24+
add_action( 'wp_enqueue_scripts', [ $this, 'maybe_enqueue_main_script' ], 1 );
2525
add_action( 'wp_enqueue_scripts', [ $this, 'maybe_enqueue_cloaked_affiliate_links_assets' ], 11 );
2626
add_action( 'wp_enqueue_scripts', [ $this, 'maybe_enqueue_four_o_four_script' ], 11 );
2727
add_action( 'wp_enqueue_scripts', [ $this, 'maybe_enqueue_query_params_script' ], 11 );
@@ -41,17 +41,18 @@ public function maybe_enqueue_main_script() {
4141
$settings = Helpers::get_settings();
4242
$user_role = Helpers::get_user_role();
4343

44+
/**
45+
* This is a dummy script that will allow us to attach inline scripts further down the line.
46+
*/
47+
wp_register_script( 'plausible-analytics', '' );
48+
4449
/**
4550
* Bail if tracked_user_roles is empty (which means no roles should be tracked) or if the current role should not be tracked.
4651
*/
4752
if ( ( ! empty( $user_role ) && ! isset( $settings['tracked_user_roles'] ) ) || ( ! empty( $user_role ) && ! in_array( $user_role, $settings['tracked_user_roles'], true ) ) ) {
4853
return; // @codeCoverageIgnore
4954
}
5055

51-
/**
52-
* This is a dummy script that will allow us to attach inline scripts further down the line.
53-
*/
54-
wp_register_script( 'plausible-analytics', '' );
5556
wp_enqueue_script(
5657
'plausible-analytics',
5758
'',

‎src/Helpers.php‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,6 @@ public static function get_settings() {
116116

117117
$settings = get_option( 'plausible_analytics_settings', [] );
118118

119-
/**
120-
* If this is an AJAX request, make sure the latest settings are used.
121-
*/
122-
if ( isset( $_POST['action'] ) && $_POST['action'] === 'plausible_analytics_save_options' ) {
123-
$options = json_decode( str_replace( '\\', '', $_POST['options'] ) );
124-
125-
foreach ( $options as $option ) {
126-
$settings[ $option->name ] = $option->value;
127-
}
128-
}
129-
130119
return apply_filters( 'plausible_analytics_settings', wp_parse_args( $settings, $defaults ) );
131120
}
132121

‎tests/TestCase.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ public function __construct() {
2121
define( 'PLAUSIBLE_TESTS_ROOT', __DIR__ . '/' );
2222
}
2323

24+
if ( ! defined( 'PLAUSIBLE_CI' ) ) {
25+
define( 'PLAUSIBLE_CI', true );
26+
}
27+
2428
parent::__construct();
2529
}
2630

@@ -241,6 +245,12 @@ public function setQueryParams( $settings ) {
241245
return $settings;
242246
}
243247

248+
public function enableAdministratorTracking( $settings ) {
249+
$settings['tracked_user_roles'][] = 'administrator';
250+
251+
return $settings;
252+
}
253+
244254
/**
245255
* Checks an array for a (partial) match with $string.
246256
*

‎tests/TestableHelpers.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
namespace Plausible\Analytics\Tests;
77

8+
use Plausible\Analytics\WP\Client;
89
use Plausible\Analytics\WP\Helpers;
910

1011
/**
@@ -15,7 +16,7 @@ class TestableHelpers extends Helpers {
1516
* @return
1617
*/
1718
protected static function get_client() {
18-
return new class {
19+
return new class extends Client {
1920
public function get_tracker_id() {
2021
return 'pa-test-tracker-id';
2122
}

‎tests/integration/AdminBarTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ public function testAdminBarNode() {
2121
}
2222

2323
wp_set_current_user( 1 );
24+
$user = wp_get_current_user();
25+
$user->add_role( 'administrator' );
2426
$admin_bar = new WP_Admin_Bar();
2527
$class->admin_bar_node( $admin_bar );
2628
$this->assertNotEmpty( $admin_bar->get_node( 'plausible-analytics' ) );

0 commit comments

Comments
 (0)