Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

Cleanup #119

Merged
merged 11 commits into from
May 9, 2020
Merged

Cleanup #119

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6169591
Cleanup: make caps checks more explicit/easier to see
azaozz May 7, 2020
0fb247a
Pass window.wp_autoupdates as a parameter to the JS closure. Change …
pbiron May 7, 2020
9694a66
Merge branch 'master' into fix/pre-merge-cleanup
azaozz May 7, 2020
c4ece2d
The nonce check should be first
azaozz May 7, 2020
a7abdcd
Various coding standards and consistency changes.
pbiron May 8, 2020
0855266
Always be explicit with the enable/disable action.
pbiron May 8, 2020
a58a879
Sanitize $_GET['paged'] with absint().
pbiron May 8, 2020
f4f593e
Correct boolean logic error.
pbiron May 8, 2020
18847cd
Fix typo in plugin bulk action keys.
pbiron May 8, 2020
5e0359b
Remove unneeded array_unique() in plugin disable bulk actions handler.
pbiron May 8, 2020
42639f1
Fix typo in selector for the manual theme update success handler.
pbiron May 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 34 additions & 36 deletions functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function wp_autoupdates_prepare_themes_for_js( $prepared_themes ) {

return $prepared_themes;
}
add_action( 'wp_prepare_themes_for_js', 'wp_autoupdates_prepare_themes_for_js' );
add_filter( 'wp_prepare_themes_for_js', 'wp_autoupdates_prepare_themes_for_js' );


/**
Expand Down Expand Up @@ -309,8 +309,8 @@ function wp_autoupdates_plugins_bulk_actions( $actions ) {
$actions['disable-autoupdate-selected'] = __( 'Disable auto-updates', 'wp-autoupdates' );
return $actions;
}
add_action( 'bulk_actions-plugins', 'wp_autoupdates_plugins_bulk_actions' );
add_action( 'bulk_actions-plugins-network', 'wp_autoupdates_plugins_bulk_actions' );
add_filter( 'bulk_actions-plugins', 'wp_autoupdates_plugins_bulk_actions' );
add_filter( 'bulk_actions-plugins-network', 'wp_autoupdates_plugins_bulk_actions' );


/**
Expand Down Expand Up @@ -488,9 +488,10 @@ function wp_autoupdates_plugins_bulk_actions_handle( $redirect_to, $doaction, $i
return $redirect_to;
}

return $redirect_to;
}
add_action( 'handle_bulk_actions-plugins', 'wp_autoupdates_plugins_bulk_actions_handle', 10, 3 );
add_action( 'handle_bulk_actions-plugins-network', 'wp_autoupdates_plugins_bulk_actions_handle', 10, 3 );
add_filter( 'handle_bulk_actions-plugins', 'wp_autoupdates_plugins_bulk_actions_handle', 10, 3 );
add_filter( 'handle_bulk_actions-plugins-network', 'wp_autoupdates_plugins_bulk_actions_handle', 10, 3 );


/**
Expand Down Expand Up @@ -1211,7 +1212,7 @@ function wp_autoupdates_themes_bulk_actions( $actions ) {
$actions['disable-autoupdate-selected'] = __( 'Disable auto-updates', 'wp-autoupdates' );
return $actions;
}
add_action( 'bulk_actions-themes-network', 'wp_autoupdates_themes_bulk_actions' );
add_filter( 'bulk_actions-themes-network', 'wp_autoupdates_themes_bulk_actions' );


/**
Expand Down Expand Up @@ -1255,9 +1256,7 @@ function wp_autoupdates_themes_bulk_actions_handle( $redirect_to, $doaction, $it

$redirect_to = self_admin_url( "themes.php?enable-autoupdate=true&theme_status=$status&paged=$page&s=$s" );
return $redirect_to;
}

if ( 'disable-autoupdate-selected' === $doaction ) {
} elseif ( 'disable-autoupdate-selected' === $doaction ) {
if ( ! current_user_can( 'update_themes' ) || ! wp_autoupdates_is_themes_auto_update_enabled() ) {
wp_die( __( 'Sorry, you are not allowed to enable themes automatic updates.', 'wp-autoupdates' ) );
}
Expand Down Expand Up @@ -1288,8 +1287,10 @@ function wp_autoupdates_themes_bulk_actions_handle( $redirect_to, $doaction, $it
$redirect_to = self_admin_url( "themes.php?disable-autoupdate=true&theme_status=$status&paged=$page&s=$s" );
return $redirect_to;
}

return $redirect_to;
}
add_action( 'handle_network_bulk_actions-themes-network', 'wp_autoupdates_themes_bulk_actions_handle', 10, 3 );
add_filter( 'handle_network_bulk_actions-themes-network', 'wp_autoupdates_themes_bulk_actions_handle', 10, 3 );

/**
* Toggle auto updates via Ajax.
Expand All @@ -1299,63 +1300,60 @@ function wp_autoupdates_toggle_auto_updates() {
wp_send_json_error( array( 'error' => __( 'Invalid data. No selected item.', 'wp-autoupdates' ) ) );
}

$type = sanitize_text_field( $_POST['type'] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the sanitizing was at the request of @whyisjake . If he's OK with these no longer being sanitized, then rest looks good to me.

@azaozz I just pushed an additional commit with 2 other minor changes, so be sure to check those.

Copy link
Contributor Author

@azaozz azaozz May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. All strings coming from the user, ($_GET, $_POST, $_REQUEST, many from $_SERVER, etc.) must be sanitized. However strings that must match a particular, limited example can be matched directly. In this case $_POST['state'] can only be enable or disable, all other strings there are ignored. Then a direct match makes more sense as it's faster.

Similarly when a number is expected, it would be enough to cast to int or do intval() (with some limitations), instead of first sanitizing the string.

$state = sanitize_text_field( $_POST['state'] );
$asset = sanitize_text_field( urldecode( $_POST['asset'] ) );

if ( ! in_array( $state, array( 'enable', 'disable', true ) ) ) {
if ( 'enable' !== $_POST['state'] && 'disable' !== $_POST['state'] ) {
wp_send_json_error( array( 'error' => __( 'Invalid data. Unknown state.', 'wp-autoupdates' ) ) );
}
$state = $_POST['state'];

if ( 'plugin' !== $_POST['type'] && 'theme' !== $_POST['type'] ) {
wp_send_json_error( array( 'error' => __( 'Invalid data. Unknown type.', 'wp-autoupdates' ) ) );
}
$type = $_POST['type'];

check_ajax_referer( 'updates' );

switch ( $type ) {
case 'plugin':
$cap = 'update_plugins';
if ( ! current_user_can( 'update_plugins' ) ) {
$error_message = __( 'You do not have permission to modify plugins.', 'wp-autoupdates' );
wp_send_json_error( array( 'error' => $error_message ) );
}

$option = 'wp_auto_update_plugins';
/** This filter is documented in wp-admin/includes/class-wp-plugins-list-table.php */
$all_items = apply_filters( 'all_plugins', get_plugins() );
break;
case 'theme':
$cap = 'update_themes';
if ( ! current_user_can( 'update_themes' ) ) {
$error_message = __( 'You do not have permission to modify themes.', 'wp-autoupdates' );
wp_send_json_error( array( 'error' => $error_message ) );
}

$option = 'wp_auto_update_themes';
$all_items = wp_get_themes();
break;
default:
wp_send_json_error( array( 'error' => __( 'Invalid data. Unknown type.', 'wp-autoupdates' ) ) );
}

if ( ! $all_items[ $asset ] ) {
wp_send_json_error(
array(
'error' => sprintf(
__( 'Invalid data. %s does not exist.', 'wp-autoupdates' ),
'plugin' === $type ? __( 'Plugin', 'wp-autoupdates' ) : __( 'Theme', 'wp-autoupdates' )
),
)
);
}

if ( ! current_user_can( $cap ) ) {
wp_send_json_error(
array(
'error' => sprintf(
__( 'You do not have permission to modify %s.', 'wp-autoupdates' ),
'plugin' === $type ? __( 'plugins', 'wp-autoupdates' ) : __( 'themes', 'wp-autoupdates' )
),
)
);
if ( ! array_key_exists( $asset, $all_items ) ) {
$error_message = __( 'Invalid data. The item does not exist.', 'wp-autoupdates' );
wp_send_json_error( array( 'error' => $error_message ) );
}

$wp_autoupdates = (array) get_site_option( $option, array() );

if ( 'disable' === $state ) {
$wp_autoupdates = array_diff( $wp_autoupdates, array( $asset ) );
} else {
$wp_autoupdates[] = $asset;
$wp_autoupdates = array_unique( $wp_autoupdates );
}

update_site_option( $option, $wp_autoupdates );

wp_send_json_success();
}
add_action( 'wp_ajax_toggle_auto_updates', 'wp_autoupdates_toggle_auto_updates' );
add_action( 'wp_ajax_toggle-auto-updates', 'wp_autoupdates_toggle_auto_updates' );
25 changes: 14 additions & 11 deletions js/wp-autoupdates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/* global wp_autoupdates */
( function( $, settings, pagenow ) {
// For merging with wp-admin/js/updates.js
// in this plugin, translatable strings are in l10n. settings is used only for the
// ajax_nonce. Once merged into core, all the strings will be in settings.l10n
// and the l10n param will not be passed.
( function( $, l10n, settings, pagenow ) {
'use strict';

$( document ).ready(
Expand All @@ -21,18 +24,18 @@
$parent.find( '.auto-updates-error' ).removeClass( 'notice error' ).addClass( 'hidden' );

// Show loading status.
$label.text( 'enable' === action ? wp_autoupdates.enabling : wp_autoupdates.disabling );
$label.text( 'enable' === action ? l10n.enabling : l10n.disabling );
$anchor.find( '.dashicons-update' ).removeClass( 'hidden' );

data = {
action: 'toggle_auto_updates',
action: 'toggle-auto-updates',
_ajax_nonce: settings.ajax_nonce,
state: action,
type: type,
asset: $anchor.attr( 'data-wp-asset' ),
};

$.post( ajaxurl, data )
$.post( window.ajaxurl, data )
.done(
function( response ) {
var $enabled, $disabled, enabledNumber, disabledNumber;
Expand Down Expand Up @@ -69,15 +72,15 @@

if ( 'enable' === action ) {
$anchor.attr( 'data-wp-action', 'disable' );
$label.text( wp_autoupdates.disable );
$label.text( l10n.disable );
$parent.find( '.auto-update-time' ).removeClass( 'hidden' );
} else {
$anchor.attr( 'data-wp-action', 'enable' );
$label.text( wp_autoupdates.enable );
$label.text( l10n.enable );
$parent.find( '.auto-update-time' ).addClass( 'hidden' );
}

wp.a11y.speak( 'enable' === action ? wp_autoupdates.enabled : wp_autoupdates.disabled, 'polite' );
wp.a11y.speak( 'enable' === action ? l10n.enabled : l10n.disabled, 'polite' );
} else {
$parent.find( '.auto-updates-error' ).removeClass( 'hidden' ).addClass( 'notice error' ).find( 'p' ).text( response.data.error );
wp.a11y.speak( response.data.error, 'polite' );
Expand All @@ -86,8 +89,8 @@
)
.fail(
function( response ) {
$parent.find( '.auto-updates-error' ).removeClass( 'hidden' ).addClass( 'notice error' ).find( 'p' ).text( wp_autoupdates.auto_update_error );
wp.a11y.speak( wp_autoupdates.auto_update_error, 'polite' );
$parent.find( '.auto-updates-error' ).removeClass( 'hidden' ).addClass( 'notice error' ).find( 'p' ).text( l10n.auto_update_error );
wp.a11y.speak( l10n.auto_update_error, 'polite' );
}
)
.always(
Expand All @@ -112,4 +115,4 @@
);
}
);
} )( jQuery, window._wpUpdatesSettings, window.pagenow );
} )( window.jQuery, window.wp_autoupdates, window._wpUpdatesSettings, window.pagenow );