site-kit-wp: Enhance wording of "Invalid nonce" messaging

Feature Description

There are a few cases in the plugin where we currently wp_die with “Invalid nonce”. This is not the most useful message for a case that is likely uncommon but still fairly possible for a user to encounter. As such, we should re-evaluate the messaging used in each case and potentially add a retry mechanism for better UX such as that discussed in https://github.com/google/site-kit-wp/issues/2935#issuecomment-804290498.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • A new private method Authentication::invalid_nonce_error( $action ) should be introduced:
    • It should be inspired by the wp_nonce_ays function of WordPress core, in fact its else case can be used as is for the “default” case, so the new method should probably actually call wp_nonce_ays in certain situations.
    • Instead of the log-out condition that wp_nonce_ays has, we should have special behavior for proxy-related invalid nonces: If the nonce $action starts with googlesitekit_proxy_, instead of using the referer like WordPress core does by default, our “link back” URL should be the googlesitekit-splash screen. The messaging should remain the same as in WordPress core.
  • Everywhere Site Kit currently dies with an “Invalid nonce.” error message, Site Kit should instead rely on the new method introduced above.

Implementation Brief

Add new invalid nonce error method

Add a new private method, Authentication::invalid_nonce_error( $action ).

  • The $action parameter here will be a string describing the nonce action.
  • The idea is that the function will test $action in order to deal with certain edge cases, but will fall through to the wp_nonce_ays function in WP core as the default behavior if none apply.

Here we need to add a check to determine whether the invalid nonce is proxy-related.

  • Test if the $action parameter starts with the substring googlesitekit_proxy_.
  • If so, return the same message and linked text as wp_nonce_ays, i.e. 'The link you followed has expired. Please try again' (the linked text being ‘Please try again’).
  • However, instead of using the referrer for the redirect link, redirect to the googlesitekit-splash screen instead.
  • If the $action is not proxy-related, fall through to calling wp_nonce_ays as the default behavior.

Add test coverage for this new method.

Refactor instances of wp_die to use the new method

Refactor all instances where Site Kit currently dies with an “Invalid nonce.” error message to instead rely on the new Authentication::invalid_nonce_error method, for example:

includes/Core/Authentication/Authentication.php:

654:	wp_die( esc_html__( 'Invalid nonce.', 'google-site-kit' ), 400 );
683:	wp_die( esc_html__( 'Invalid nonce.', 'google-site-kit' ), 400 );
1066:	wp_die( esc_html__( 'Invalid nonce.', 'google-site-kit' ), 400 );
1253:	wp_die( esc_html__( 'Invalid nonce.', 'google-site-kit' ) );

includes/Core/Util/Reset.php:

259:	wp_die( esc_html__( 'Invalid nonce.', 'google-site-kit' ), 400 );

You will likely need to refactor the test coverage for these files as well to reflect the changes.

Test Coverage

  • Test coverage should be added for the new method, and some existing tests might need to be refactored as mentioned above.

Visual Regression Changes

  • None anticipated, unless there are images that handle invalid nonce cases.

QA Brief

  • Add a call to invalid nonce error and ensure the hyperlink on the resulting error screen is pointing to wp-admin/admin.php?page=googlesitekit-splash
<?php
add_action( 'admin_init', function() {
    Google\Site_Kit\Core\Authentication\Authentication::invalid_nonce_error('googlesitekit_proxy_foo');
});

Changelog entry

  • Improve wording of “Invalid nonce” errors.

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Comments: 15 (8 by maintainers)

Most upvoted comments

Good point @eugene-manuilov, I’ve updated the ACs to cater differently for invalid nonces for actions related to the proxy.

@johnPhillips would it be possible to add an estimate here now the IB is completed please?

@felixarntz @aaemnnosttv unfortunately, I think wp_nonce_ays won’t work in our case because it uses a referer URL for the retry link in the generated markup. It means that users will be redirected to the wrong link on the proxy server instead of starting authentication from the beginning when they finish the previous authentication too late and click on the retry link.

I think we need to create our own version of the wp_nonce_ays function which will let us provide a redirect URL.

Yeah, let’s go with wp_nonce_ays for now (unless you’ve additional input Eugene).