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_aysfunction of WordPress core, in fact itselsecase can be used as is for the “default” case, so the new method should probably actually callwp_nonce_aysin certain situations. - Instead of the
log-outcondition thatwp_nonce_ayshas, we should have special behavior for proxy-related invalid nonces: If the nonce$actionstarts withgooglesitekit_proxy_, instead of using the referer like WordPress core does by default, our “link back” URL should be thegooglesitekit-splashscreen. The messaging should remain the same as in WordPress core.
- It should be inspired by the
- 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
$actionparameter here will be a string describing the nonce action. - The idea is that the function will test
$actionin order to deal with certain edge cases, but will fall through to thewp_nonce_aysfunction 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
$actionparameter starts with the substringgooglesitekit_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-splashscreen instead. - If the
$actionis not proxy-related, fall through to callingwp_nonce_aysas 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)
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_ayswon’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_aysfunction which will let us provide a redirect URL.Yeah, let’s go with
wp_nonce_aysfor now (unless you’ve additional input Eugene).