2

I've created an admin page where I'm displaying a list of a MySQL Table using WP_List_Table. In this table I need to be able to delete a record when I want to. And that's where I have a problem.

To delete a record I've set up the following code.

class Genres_List extends WP_List_Table {

    [...]

    public static function delete_genre( $id ) {
        global $wpdb;
        $wpdb->delete(
            "{$wpdb->prefix}genres",
            [ 'id' => $id ],
            [ '%d' ]
            );
    }

    function column_cb( $item ) {
        return sprintf('<input type="checkbox" name="id[]" value="%s" />', $item['id']);
    }

    function column_name( $item ) {
        $delete_nonce = wp_create_nonce( 'sp_delete_genre' );
        $title = '<strong>' . stripslashes($item['name']) . '</strong>';
        $actions = [
            'edit' => sprintf( '<a href="?page=%s&action=%s&id=%s">Bewerken</a>', esc_attr( $_REQUEST['page'] ), 'edit', absint( $item['id'] ) ),
            'delete' => sprintf( '<a href="?page=%s&action=delete&id=%s&_wpnonce=%s">Verwijderen</a>', esc_attr( $_REQUEST['page'] ), absint( $item['id'] ), $delete_nonce )
            ];
        return '<a href="?page=genres&action=edit&id=' . $item['id'] . '"><strong>' . stripslashes($item['name']) . '</strong>' . $this->row_actions( $actions );
    }

    function get_columns() {
        $columns = [
            'cb' => '<input type="checkbox" />',
            'name' => 'Naam'
            ];

        return $columns;
    }

    public function get_bulk_actions() {
        $actions = [
            'delete' => 'Verwijderen'
            ];

        return $actions;
    }

    public function process_bulk_action() {
        if ( $this->current_action() === 'delete' ) {
            if ( ! wp_verify_nonce( $_REQUEST['_wpnonce'], 'sp_delete_genre' ) ) {
                die('This is a secure website. Your nonce did not verify. Go get a coffee.');
            } else {
                self::delete_genre( absint( $_GET['id'] ) );
                wp_redirect( esc_url( add_query_arg() ) );
                exit;
            }

            if ( $_POST['action'] === 'delete' || $_POST['action2'] === 'delete') {
                $delete_ids = esc_sql( $_POST['id'] );

                foreach ( $delete_ids as $id ) { self::delete_genre( $id ); }

                wp_redirect( esc_url( add_query_arg() ) );
                exit;
            }
        }
    }
}

The problem is that wp_verify_nonce( $_REQUEST['_wpnonce'], 'sp_delete_genre' ) returns false when trying to delete a record. I can't seem to find what I'm doing wrong, since I'm exactly following the Wordpress Codex:

  1. Creating a nonce using wp_create_nonce( 'sp_delete_genre' ).
  2. Using the nonce in a _wpnonce= parameter.
  3. Verifying the nonce using wp_verify_nonce( $_REQUEST['_wpnonce'], 'sp_delete_genre' )
Improve this question
5
  • 1
    What was the reasoning for using a custom table for genres rather than a custom post type or taxonomy? Also keep in mind that $_REQUEST can also contain cookies and session variables, it's always better to be specific an use either GET or POST for security reasons. I also notice you use add_query_arg with no parameters, this is highly unusual, that function has required arguments, and needs both a key and value, as well as a URL. Your use of esc_sql is also odd, it would be better to use wpdb prepare inside the delete_genre method, and instead use intval on the POST id
    – Tom J Nowell ♦
    Commented Jul 21, 2019 at 14:44
  • Because i'm not creating posts about genres and because I'm not creating posts that should have a genre as a taxonomy. I have a table with programs, a table with episodes, a table with genres etc. so that I can display these programs through a page template on my WordPress site.
    – Stef
    Commented Jul 21, 2019 at 14:51
  • When I var_dump $_REQUEST['_wpnonce'], it returns a nonce, so that's okay. The add_query_arg() returns a legit url, even without parameters. But I'm not even getting that far because of the problem I described. I didn't write the code myself, so I wasn't aware of the problems with it.
    – Stef
    Commented Jul 21, 2019 at 14:53
  • 1
    All of those could be represented as taxonomies or custom posts. Custom post types and taxonomies don't have to be about blog posts. For example I have a CPT named talks on my website, with a taxonomy for locations. They have their own menu in the admin, their own URLs archives and templates, independent REST APIs, hooks, and access controls, etc. Have you confirmed that the nonce is making it through to the other side, even if it's not being accepted?
    – Tom J Nowell ♦
    Commented Jul 21, 2019 at 14:54
  • I wouldn't really know how to do that, since I have MySQL table with lots of rows. I couldn't simply change that to a CPT and abandon all of the work I did. But I'll keep it in mind for the future. I would't know how to do what your suggesting. I var_dumped $_REQUEST['_wpnonce'] and that returns the nonce, but how could I confirm that the nonce is making it through to the other side, even if it's not being accepted ?
    – Stef
    Commented Jul 21, 2019 at 14:59

1 Answer 1

Reset to default
1

This question still unanswered for long time. I answer it if anyone reaches here, just in case.

When you are using WP_List_Table (perhaps by extending the class), the generated code for single delete and bulk delete differs. The single delete is sent over a GET request by setting query params, but the bulk delete is sent over a POST request by setting form-data. (You can investigate it by checking the network tab of developer tools.)

So, generated nonce also differs in these situations, and IMO it is a bad design by Wordpress that both requests call process_bulk_action method. BTW you must check the conditions, like:

if (is_array($_REQUEST['id'])) {
    $true = wp_verify_nonce($_REQUEST['_wpnonce'], 'bulk-' . $this->_args['plural']);
} else if (is_string($_REQUEST['id'])) {
    $true = wp_verify_nonce($_REQUEST['_wpnonce'], 'sp_delete_genre');
}

You could also check the request method, but I guess $_REQUEST['id'] is more reliable.

P.S. Wordpress official website notices that extending WP_List_Table is not safe enough!

Note: This class’s access is marked as private. That means it is not intended for use by plugin and theme developers as it is subject to change without warning in any future WordPress release. If you would still like to make use of the class, you should make a copy to use and distribute with your own project, or else use it at your own risk.

P.S. Two resources that I used and I'd like to mention.
https://gist.github.com/petenelson/8981536
https://stackoverflow.com/questions/25000403/verifying-nonce-from-wp-list-table

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.