Skip to content

Commit 2c669a9

Browse files
colinmurphycolinmurphy
committed
Tidied up nonces.
1 parent 0591625 commit 2c669a9

File tree

5 files changed

+77
-71
lines changed

5 files changed

+77
-71
lines changed

‎plugins/wpgraphql-logging/assets/js/settings/wp-graphql-logging-view.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
jQuery(document).ready(function($) {
2+
$(".wpgraphql-logging-datepicker").datetimepicker({
3+
dateFormat: "yy-mm-dd",
4+
timeFormat: "HH:mm:ss"
5+
});
6+
});

‎plugins/wpgraphql-logging/src/Admin/View/List/ListTable.php‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace WPGraphQL\Logging\Admin\View\List;
66

77
use DateTime;
8+
use WPGraphQL\Logging\Admin\ViewLogsPage;
89
use WPGraphQL\Logging\Logger\Api\LogServiceInterface;
910
use WPGraphQL\Logging\Logger\Database\WordPressDatabaseEntity;
1011
use WP_List_Table;
@@ -58,10 +59,6 @@ public function __construct(
5859
* @phpcs:disable WordPress.Security.NonceVerification.Recommended
5960
*/
6061
public function prepare_items(): void {
61-
if ( array_key_exists( 'orderby', $_REQUEST ) || array_key_exists( 'order', $_REQUEST ) ) {
62-
check_admin_referer( 'wpgraphql-logging-sort' );
63-
}
64-
6562
$this->process_bulk_action();
6663
$this->_column_headers =
6764
apply_filters(
@@ -118,8 +115,6 @@ public function get_bulk_actions(): array {
118115
/**
119116
* Handle bulk actions.
120117
*
121-
* @phpcs:disable WordPress.Security.NonceVerification.Missing
122-
* @phpcs:disable WordPress.Security.NonceVerification.Recommended
123118
* @phpcs:disable Generic.Metrics.NestingLevel.TooHigh
124119
* @phpcs:disable Generic.Metrics.CyclomaticComplexity.TooHigh
125120
* @phpcs:disable Generic.Metrics.CyclomaticComplexity.MaxExceeded
@@ -299,14 +294,18 @@ public function column_cb( $item ): string {
299294
* @return string The rendered ID column or null.
300295
*/
301296
public function column_id( WordPressDatabaseEntity $item ): string {
302-
$url = \WPGraphQL\Logging\Admin\ViewLogsPage::ADMIN_PAGE_SLUG;
303-
$download_nonce = wp_create_nonce( 'wpgraphql-logging-download_' . $item->get_id() );
297+
$url = \WPGraphQL\Logging\Admin\ViewLogsPage::ADMIN_PAGE_SLUG;
298+
$log_id = $item->get_id();
299+
300+
$view_nonce = wp_create_nonce( ViewLogsPage::ADMIN_PAGE_VIEW_NONCE . '_' . $log_id );
301+
$download_nonce = wp_create_nonce( ViewLogsPage::ADMIN_PAGE_DOWNLOAD_NONCE . '_' . $log_id );
304302
$actions = [
305303
'view' => sprintf(
306-
'<a href="?page=%s&action=%s&log=%d">%s</a>',
304+
'<a href="?page=%s&action=%s&log=%d&_wpnonce=%s">%s</a>',
307305
esc_attr( $url ),
308306
'view',
309307
$item->get_id(),
308+
esc_attr( $view_nonce ),
310309
esc_html__( 'View', 'wpgraphql-logging' )
311310
),
312311
'download' => sprintf(

‎plugins/wpgraphql-logging/src/Admin/View/Templates/WPGraphQLLoggerList.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,7 @@
2222
<hr class="wp-header-end">
2323

2424
<form method="post">
25-
<?php wp_nonce_field( 'wpgraphql-logging-sort', 'wpgraphql-logging-sort-nonce' ); ?>
26-
<?php
27-
$list_table->prepare_items();
28-
$list_table->display();
29-
?>
25+
<?php $list_table->prepare_items(); ?>
26+
<?php $list_table->display(); ?>
3027
</form>
3128
</div>

‎plugins/wpgraphql-logging/src/Admin/ViewLogsPage.php‎

Lines changed: 59 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,20 @@ class ViewLogsPage {
2424
*/
2525
public const ADMIN_PAGE_SLUG = 'wpgraphql-logging-view';
2626

27+
/**
28+
* The nonce for the view page.
29+
*
30+
* @var string
31+
*/
32+
public const ADMIN_PAGE_VIEW_NONCE = 'wp_graphql_logging_admin';
33+
34+
/**
35+
* The nonce for the download page.
36+
*
37+
* @var string
38+
*/
39+
public const ADMIN_PAGE_DOWNLOAD_NONCE = 'wp_graphql_logging_download';
40+
2741
/**
2842
* The hook suffix for the admin page.
2943
*
@@ -124,36 +138,33 @@ public function enqueue_admin_scripts( string $hook_suffix ): void {
124138

125139
wp_enqueue_style( 'jquery-ui-style', 'https://ajax.googleapis.com/ajax/libs/jqueryui/1.12.1/themes/smoothness/jquery-ui.css', [], '1.12.1' );
126140

127-
// Add inline script to initialize the datetimepicker.
128-
wp_add_inline_script(
129-
'jquery-ui-timepicker-addon',
130-
'jQuery(document).ready(function($){ $(".wpgraphql-logging-datepicker").datetimepicker({ dateFormat: "yy-mm-dd", timeFormat: "HH:mm:ss" }); });'
131-
);
132-
133-
// Add nonce to sorting links.
134-
wp_add_inline_script(
135-
'jquery',
136-
'jQuery(document).ready(function($){
137-
var nonce = $("#wpgraphql-logging-sort-nonce").val();
138-
if ( nonce ) {
139-
$("th.sortable a").each(function(){
140-
this.href = this.href + "&_wpnonce=" + nonce;
141-
});
142-
}
143-
});'
144-
);
141+
// Enqueue admin scripts if they exist.
142+
$script_path = trailingslashit( WPGRAPHQL_LOGGING_PLUGIN_URL ) . 'assets/js/settings/wp-graphql-logging-view.js';
143+
if ( file_exists( trailingslashit( WPGRAPHQL_LOGGING_PLUGIN_DIR ) . 'assets/js/settings/wp-graphql-logging-view.js' ) ) {
144+
wp_enqueue_script(
145+
'wpgraphql-logging-view-js',
146+
$script_path,
147+
[ 'jquery' ],
148+
WPGRAPHQL_LOGGING_VERSION,
149+
true
150+
);
151+
}
145152

146153
// Allow other plugins to enqueue their own scripts/styles.
147154
do_action( 'wpgraphql_logging_view_logs_admin_enqueue_scripts', $hook_suffix );
148155
}
149156

150157
/**
151158
* Renders the admin page for the logs.
159+
*
160+
* @phpcs:disable WordPress.Security.NonceVerification.Recommended
152161
*/
153162
public function render_admin_page(): void {
163+
164+
154165
$action = isset( $_REQUEST['action'] ) && is_string( $_REQUEST['action'] )
155166
? sanitize_text_field( $_REQUEST['action'] )
156-
: 'link'; // @phpcs:ignore WordPress.Security.NonceVerification.Recommended
167+
: 'list';
157168

158169
switch ( $action ) {
159170
case 'view':
@@ -173,8 +184,10 @@ public function render_admin_page(): void {
173184
* This runs before any HTML is output.
174185
*/
175186
public function process_page_actions_before_rendering(): void {
176-
// Check for a download request.
177-
if ( isset( $_GET['action'] ) && 'download' === $_GET['action'] ) { // @phpcs:ignore WordPress.Security.NonceVerification.Recommended
187+
188+
// Nonce handled in process_log_download and process_filters_redirect.
189+
// phpcs:ignore WordPress.Security.NonceVerification.Recommended
190+
if ( isset( $_GET['action'] ) && 'download' === $_GET['action'] ) {
178191
$this->process_log_download();
179192
}
180193

@@ -238,14 +251,16 @@ public function get_redirect_url(): string {
238251
*
239252
* @param string $key The key to retrieve from $_POST.
240253
*
254+
* @phpcs:disable WordPress.Security.NonceVerification.Missing
255+
*
241256
* @return string|null The sanitized value or null if not set or invalid.
242257
*/
243258
protected function get_post_value(string $key): ?string {
244-
$value = $_POST[ $key ] ?? null; // @phpcs:ignore WordPress.Security.NonceVerification.Missing, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
245-
if ( ! is_string( $value ) || '' === $value ) {
259+
if ( ! array_key_exists( $key, $_POST ) || ! is_string( $_POST[ $key ] ) ) {
246260
return null;
247261
}
248-
return sanitize_text_field( wp_unslash( $value ) );
262+
$post_value = sanitize_text_field( wp_unslash( $_POST[ $key ] ) );
263+
return '' !== $post_value ? $post_value : null;
249264
}
250265

251266
/**
@@ -257,17 +272,14 @@ protected function render_list_page(): void {
257272
require_once $list_template; // @phpcs:ignore WordPressVIPMinimum.Files.IncludingFile.UsingVariable
258273
}
259274

260-
/**
261-
* Renders the list page for log entries.
262-
*/
275+
/**
276+
* Renders the list page for log entries.
277+
*/
263278
protected function process_log_download(): void {
264-
if ( ! current_user_can( 'manage_options' ) || ! is_admin() ) {
265-
wp_die( esc_html__( 'You do not have sufficient permissions to access this page.', 'wpgraphql-logging' ) );
266-
}
267-
268279
$log_id = isset( $_GET['log'] ) ? absint( $_GET['log'] ) : 0;
269-
if ( $log_id > 0 ) {
270-
check_admin_referer( 'wpgraphql-logging-download_' . $log_id );
280+
$this->verify_admin_page_nonce( self::ADMIN_PAGE_DOWNLOAD_NONCE . '_' . $log_id );
281+
if ( 0 === (int) $log_id ) {
282+
wp_die( esc_html__( 'Invalid log ID.', 'wpgraphql-logging' ) );
271283
}
272284
$downloader = new DownloadLogService( $this->get_log_service() );
273285
$downloader->generate_csv( $log_id );
@@ -277,7 +289,8 @@ protected function process_log_download(): void {
277289
* Renders the view page for a single log entry.
278290
*/
279291
protected function render_view_page(): void {
280-
$log_id = isset( $_GET['log'] ) ? absint( $_GET['log'] ) : 0; // @phpcs:ignore WordPress.Security.NonceVerification.Recommended
292+
$log_id = isset( $_GET['log'] ) ? absint( $_GET['log'] ) : 0;
293+
$this->verify_admin_page_nonce( self::ADMIN_PAGE_DOWNLOAD_NONCE . '_' . $log_id );
281294

282295
if ( 0 === (int) $log_id ) {
283296
echo '<div class="notice notice-error"><p>' . esc_html__( 'Invalid log ID.', 'wpgraphql-logging' ) . '</p></div>';
@@ -297,6 +310,18 @@ protected function render_view_page(): void {
297310
require_once $log_template; // @phpcs:ignore WordPressVIPMinimum.Files.IncludingFile.UsingVariable
298311
}
299312

313+
/**
314+
* Verifies the admin page nonce.
315+
*
316+
* @param string $nonce The nonce to verify.
317+
*/
318+
protected function verify_admin_page_nonce(string $nonce): void {
319+
if ( ! current_user_can( 'manage_options' ) || ! is_admin() ) {
320+
wp_die( esc_html__( 'You do not have sufficient permissions to access this page.', 'wpgraphql-logging' ) );
321+
}
322+
check_admin_referer( $nonce );
323+
}
324+
300325
/**
301326
* Retrieves the log service instance.
302327
*

‎plugins/wpgraphql-logging/tests/wpunit/Admin/View/ViewLogsPageTest.php‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -116,27 +116,6 @@ public function test_register_admin_page() : void {
116116
$instance = ViewLogsPage::init();
117117
$instance->register_settings_page();
118118

119-
// View
120-
$_REQUEST['action'] = 'view';
121-
$_GET['log'] = '123';
122-
123-
ob_start();
124-
$instance->render_admin_page();
125-
$output = ob_get_clean();
126-
$this->assertNotFalse($output);
127-
128-
// Download
129-
$_REQUEST['action'] = 'download';
130-
ob_start();
131-
$instance->render_admin_page();
132-
$output = ob_get_clean();
133-
134-
$this->assertEquals('', $output);
135-
136-
// Clean up
137-
unset($_REQUEST['action'], $_GET['log']);
138-
139-
140119
// Default
141120
ob_start();
142121
$instance->render_admin_page();
@@ -156,7 +135,7 @@ public function test_process_page_actions_before_rendering_as_download_action()
156135

157136
ob_start();
158137
$this->expectException(\WPDieException::class);
159-
$this->expectExceptionMessage('Invalid log ID.');
138+
$this->expectExceptionMessage('The link you followed has expired.');
160139
$instance->process_page_actions_before_rendering();
161140
$output = ob_get_clean();
162141

@@ -222,7 +201,7 @@ public function test_process_log_download_dies_without_nonce(): void {
222201
$_GET['log'] = 'nonexistent-log-id';
223202
ob_start();
224203
$this->expectException(\WPDieException::class);
225-
$this->expectExceptionMessage('Invalid log ID.');
204+
$this->expectExceptionMessage('The link you followed has expired.');
226205

227206
// Use reflection to call the protected method
228207
$reflection = new \ReflectionClass($instance);

0 commit comments

Comments
 (0)