3

I'm rewording this because, as it's correctly been pointed out, the original post was "woefully inadequate." I made a plugin which has tabbed views for the various settings. It's based on this Wordpress Plugin Template. The template uses the WP Settings API to construct the settings page and display the tabs. The form uses the default _wpnonce for the submit/save settings button.

The tabs are link elements that alter the page URL query string, e.g., from /wp-admin/options-general.php?page=my_plugin_settings&tab=tab_1 to /wp-admin/options-general.php?page=my_plugin_settings&tab=tab_2.

The question is now: how to create a nonce for the tabs and check it when the page loads or the user selects one of the tabs.

class-my-plugin.php. Code example is simplified for clarity.

class My_Plugin {

    private static $_instance = null;

    public $admin = null;

    public $settings = null;

    public $_token;

    public function __construct( $file = '', $version = '1.0.0' ) {
        $this->_version = $version;
        $this->_token   = 'my_plugin';
    }
}

class-my-plugin-settings.php. Code example is simplified for clarity.

class My_Plugin_Settings {

    private static $_instance = null;

    public $parent = null;

    public $base = '';

    public $settings = array();

    public function __construct( $parent ) {
        $this->parent = $parent;

        $this->base = 'wpt_';

        add_action( 'init', array( $this, 'init_settings' ), 11 );

        add_action( 'admin_init', array( $this, 'register_settings' ) );

        add_action( 'admin_menu', array( $this, 'add_menu_item' ) );

        add_filter( $this->base . 'menu_settings', array( $this, 'configure_settings' ) );
    }

    /**
     * Initialise settings
     */
    public function init_settings() {
        $this->settings = $this->settings_fields();
    }

    /**
     * Add settings page to admin menu
     */
    public function add_menu_item() {
        // Code omitted for brevity.
    }

    /**
     * Prepare default settings page arguments
     */
    private function menu_settings() {
        // Code omitted for brevity.
    }

    /**
     * Container for settings page arguments
     */
    public function configure_settings( $settings = array() ) {
        return $settings;
    }

    /**
     * Build settings fields
     */
    private function settings_fields() {

        $settings['tab_1'] = array(
            'title'       => __( 'Tab 1', 'my_plugin' ),
            'description' => __( 'The first settings screen.', 'my_plugin' ),
            'fields'      => array(
                // Form fields etc. here
            ),
        );

        $settings['tab_2'] = array(
            'title'       => __( 'Tab 2', 'my_plugin' ),
            'description' => __( 'The second settings screen.', 'my_plugin' ),
            'fields'      => array(
                // Form fields etc. here
            ),
        );

        $settings = apply_filters( $this->parent->_token . '_settings_fields', $settings );

        return $settings;
    }

    /**
     * Register plugin settings
     */
    public function register_settings() {
        if ( is_array( $this->settings ) ) {

            // Check posted/selected tab.
            $current_section = '';
            if ( isset( $_POST['tab'] ) && $_POST['tab'] ) { // NONCE warning
                $current_section = $_POST['tab']; // NONCE warning
            } else {
                if ( isset( $_GET['tab'] ) && $_GET['tab'] ) { // NONCE warning
                    $current_section = $_GET['tab']; // Nonce warning
                }
            }

            foreach ( $this->settings as $section => $data ) {

                if ( $current_section && $current_section !== $section ) {
                    continue;
                }

                // Add section to page.
                add_settings_section( $section, $data['title'], array( $this, 'settings_section' ), $this->parent->_token . '_settings' );

                foreach ( $data['fields'] as $field ) {

                    // Validation callback for field.
                    $validation = '';
                    if ( isset( $field['callback'] ) ) {
                        $validation = $field['callback'];
                    }

                    // Register field.
                    $option_name = $this->base . $field['id'];
                    register_setting( $this->parent->_token . '_settings', $option_name, $validation );

                    // Add field to page.
                    add_settings_field(
                        $field['id'],
                        $field['label'],
                        array( $this->parent->admin, 'display_field' ),
                        $this->parent->_token . '_settings',
                        $section,
                        array(
                            'field'  => $field,
                            'prefix' => $this->base,
                        )
                    );
                }

                if ( ! $current_section ) {
                    break;
                }
            }
        }
    }

    /**
     * Settings section.
     *
     * @param array $section Array of section ids.
     * @return void
     */
    public function settings_section( $section ) {
        $html = '<p> ' . $this->settings[ $section['id'] ]['description'] . '</p>' . "\n";
        echo $html;
    }

    /**
     * Load settings page content.
     *
     * @return void
     */
    public function settings_page() {

        // Build page HTML.
        $html      = '<div class="wrap" id="' . $this->parent->_token . '_settings">' . "\n";
            $html .= '<h2>' . __( 'Plugin Settings', 'my_plugin' ) . '</h2>' . "\n";

            $tab = '';
        //phpcs:disable
        if ( isset( $_GET['tab'] ) && $_GET['tab'] ) {
            $tab .= $_GET['tab'];
        }
        //phpcs:enable

        // Show page tabs.
        if ( is_array( $this->settings ) && 1 < count( $this->settings ) ) {

            $html .= '<h2 class="nav-tab-wrapper">' . "\n";

            $c = 0;
            foreach ( $this->settings as $section => $data ) {

                // Set tab class.
                $class = 'nav-tab';
                if ( ! isset( $_GET['tab'] ) ) { // NONCE warning
                    if ( 0 === $c ) {
                        $class .= ' nav-tab-active'; 
                    }
                } else {
                    if ( isset( $_GET['tab'] ) && $section == $_GET['tab'] ) { // Nonce warning
                        $class .= ' nav-tab-active';
                    }
                }

                // Set tab link.
                $tab_link = add_query_arg( array( 'tab' => $section ) );
                if ( isset( $_GET['settings-updated'] ) ) { // NONCE warning
                    $tab_link = remove_query_arg( 'settings-updated', $tab_link );
                }

                // Output tab.
                $html .= '<a href="' . $tab_link . '" class="' . esc_attr( $class ) . '">' . esc_html( $data['title'] ) . '</a>' . "\n";

                ++$c;
            }

            $html .= '</h2>' . "\n";
        }

            $html .= '<form method="post" action="options.php" enctype="multipart/form-data">' . "\n";

                // Get settings fields.
                ob_start();
                settings_fields( $this->parent->_token . '_settings' );
                do_settings_sections( $this->parent->_token . '_settings' );
                $html .= ob_get_clean();

                $html .= '<p class="submit">' . "\n";
                $html .= '<input type="hidden" name="tab" value="' . esc_attr( $tab ) . '" />' . "\n";
                $html .= '<input name="Submit" type="submit" class="button-primary" value="' . esc_attr( __( 'Save Settings', 'my_plugin' ) ) . '" />' . "\n";
                $html .= '</p>' . "\n";
                $html .= '</form>' . "\n";
                $html .= '</div>' . "\n";

        echo $html;
    }

    /**
     * Main My_Plugin_Settings Instance
     *
     * Ensures only one instance of My_Plugin_Settings is loaded or can be loaded.
     *
     * @since 1.0.0
     * @static
     * @see My_Plugin()
     * @param object $parent Object instance.
     * @return object My_Plugin_Settings instance
     */
    public static function instance( $parent ) {
        if ( is_null( self::$_instance ) ) {
            self::$_instance = new self( $parent );
        }
        return self::$_instance;
    } // End instance()

}

Form output. Changes depending on which tab is selected.

<div class="wrap">
    <h2>Heading</h2>
    <p>Plugin description.</p>
    <h2 class="nav-tab-wrapper">
        <a href="/wp-admin/options-general.php?page=my_plugin_settings&amp;tab=tab_1" class="nav-tab nav-tab-active">Tab 1</a>
        <a href="/wp-admin/options-general.php?page=my_plugin_settings&amp;tab=tab_2" class="nav-tab">Tab 2</a>
    </h2>
    <form method="post" action="options.php" enctype="multipart/form-data">
        <input type="hidden" name="option_page" value="my_plugin_settings"><input type="hidden" name="action" value="update"><input type="hidden" id="_wpnonce" name="_wpnonce" value="$integer"><input type="hidden" name="_wp_http_referer" value="/wp-admin/options-general.php?page=my_plugin_settings&amp;tab=tab_1">
        <h2>Tab 1</h2>
        <p>
            Description for Tab 1 screen.</p>
        <table class="form-table">
            <!-- Form table contents -->
        </table>
        <p class="submit">
            <input type="hidden" name="tab" value="upload">
            <input name="Submit" type="submit" class="button-primary" value="Save Settings">
        </p>
    </form>
</div>

ORIGINAL POST: I'm using PHPCS with Wordpress-Extra coding standards to check my plugin code. I'm getting this warning:

WARNING | Processing form data without nonce verification.

The code in question displays tabbed navigation in the settings page:

class Example_Class {
    public function settings_page() {
        $tab = '';

        if ( isset( $_GET['tab'] ) && $_GET['tab'] ) { // WARNING
            $tab .= $_GET['tab']; // WARNING
        }

        if ( isset( $_GET['tab'] ) && $_GET['tab'] ) { // WARNING
            $tab .= $_GET['tab']; // WARNING
        }

        // Show page tabs
        if ( is_array( $this->settings ) && 1 < count( $this->settings ) ) {

            $html .= '<h2 class="nav-tab-wrapper">' . chr( 0x0D ) . chr( 0x0A );

            $c = 0;
            foreach ( $this->settings as $section => $data ) {

                // Set tab class
                $class = 'nav-tab';
                if ( ! isset( $_GET['tab'] ) ) { // WARNING
                    if ( 0 === $c ) {
                        $class .= ' nav-tab-active';
                    }
                } else {
                    if ( isset( $_GET['tab'] ) && $section === $_GET['tab'] ) { // WARNING
                        $class .= ' nav-tab-active';
                    }
                }

                // Set tab link
                $tab_link = add_query_arg( array( 'tab' => $section ) );
                if ( isset( $_GET['settings-updated'] ) ) { // WARNING
                    $tab_link = remove_query_arg( 'settings-updated', $tab_link );
                }

                // Output tab
                $html .= '<a href="' . $tab_link . '" class="' . esc_attr( $class ) . '">' . esc_html( $data['title'] ) . '</a>' . chr( 0x0D ) . chr( 0x0A );

                ++$c;
            }

            $html .= '</h2>' . chr( 0x0D ) . chr( 0x0A );
        }
    }

    public function register_settings() {
        if ( is_array( $this->settings ) ) {

            // Check posted/selected tab
            $current_section = '';
            if ( isset( $_POST['tab'] ) && $_POST['tab'] ) { // WARNING
                $current_section = $_POST['tab'];
            } else {
                if ( isset( $_GET['tab'] ) && $_GET['tab'] ) { // WARNING
                    $current_section = $_GET['tab']; // WARNING
                }
            }

            // Unrelated code omitted
        }
    }
}

I thought the API handled nonces automagically? Should I be concerned? Or is the code OK as it is? If not, how should I fix this?

EDIT: In light of the answers, the API provides a default nonce <input type="hidden" id="_wpnonce" name="_wpnonce" value="$int"> & the associated hidden field <input type="hidden" name="_wp_http_referer" value="/wp-admin/options-general.php?page=_settings">. How do I verify? I've tried, unsuccessfully:

if ( isset( $_POST['tab'] ) && $_POST['tab'] ) {
    if ( ! wp_verify_nonce( '_wpnonce' ) ) {
        wp_die( 'Go away!' );
    } else {
        $current_section = sanitize_text_field( wp_unslash( $_POST['tab'] ) );
    }
} else {
    if ( isset( $_GET['tab'] ) && $_GET['tab'] ) {
        if ( ! wp_verify_nonce( '_wpnonce' ) ) {
            wp_die( 'Go away!' );
        } else {
            $current_section = sanitize_text_field( wp_unslash( $_GET['tab'] ) );
        }
    }
}
  • In response to your edit, you need to provide more information. What form are you referring to? Complete details are needed to understand what the form looks like, and what the nonce is in and submitted for. – Otto Jul 2 at 0:28
  • See latest edit. I've stripped a lot of unrelated code out but if something vital appears to be missing or if you need me to add any comments let me know, please. – Chris Jul 3 at 22:37
1

The API handles the nonce for the form part because you're using the settings_fields call, which outputs the *-options nonce, and you're passing the data to the options.php file for saving, which checks that nonce for you before saving the settings. This part the Settings API does indeed do for you.

However, your tab code is not in that form. It's just links. The links have no nonces on them, and the code you have to use the data from the GET parts don't do any nonce checking.

Now, technically this is okay as long as you're not saving any data here. The purpose of a nonce is to verify intent when submitting data, and if you're not submitting any data that gets saved or used in any real way, then you don't need to verify that intent. The only thing your tab selection does here is to change what fields are displayed on the page.

You might want to consider eliminating the tabs entirely and displaying the whole form, with all settings, on the same page. If you want organization in tabbed form, you'd be better off with javascript or CSS to decorate the page. Also consider accessibility, in that having the form as-a-whole might be better for users who want to make all the changes at once rather than have to configure things on multiple pages or navigate to them with links at the top of the page.

  • Thank you. So it seems I don't actually need to worry about it in this case. Will look into pure CSS or CSS + JS solution to the tabs — I was already annoyed that you have to save your settings before moving onto the next view each time anyway. – Chris Jul 4 at 16:29
1

This is proper warning. No API takes care about nonce.

You have to use verify_nonce() or check_admin_referer() before reading from $_GET or $_POST.

And it is better to use the full set of coding standards, named simply WordPress, which includes Core, Docs, and Extra.

  • Thanks, using WordPress CS now. See edited question for additional question. – Chris Jun 30 at 17:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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