Try: Return error when session is missing for REST nonce refresh

[Mamaduka]

What?

Fixes #36118.
Fixes #42400.

This PR updates the apiFetch nonce endpoint to return an error when the session token is missing. It also avoids infinite loops when apiFetch tries to refresh the nonces.

Why?

The endpoint always refreshes to nonce even when the session has expired. The wp_create_nonce doesn't do any validation and always returns a token value.

Testing Instructions

1. Open a post or page.
2. Add an Image block.
3. Select an image.
4. Temporarily renames wordpress_logged_in_* cookie via DevTools.
5. Refresh the editor and observer Networks tab.
6. Confirm that the browser doesn't make infinite requests.

Testing Instructions for Keyboard

Same.

Screenshots or screencast

[azaozz]

I'm a bit wary about making these changes. All the logged-in/authentication functions are "pluggable" (replaceable). There may be other authentication methods implemented on some sites that may be using other/different cookies or not using cookies at all.

Seems that directly using wp_get_session_token() which uses wp_parse_auth_cookie() under the hood may not work properly in these cases. Seems ideally this should be using is_user_logged_in()? If it doesn't work as expected it should be fixed, see: #13509 (comment).

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @jonnynews.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: jonnynews.

Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: azaozz <[email protected]>
Co-authored-by: skorasaurus <[email protected]>
Co-authored-by: ryanboswell <[email protected]>
Co-authored-by: mboynes <[email protected]>
Co-authored-by: desrosj <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[jonnynews]

Suggested change

	wp_send_json_error( null, rest_authorization_required_code() );
	$error = new WP_Error(
	'rest_unable_get_access_token',
	__( 'Sorry, unable to get session token.' ),
	array( 'status' => rest_authorization_required_code() )
	);
	wp_die( $error );

I love to see a wp_die and wp_error combo here. wp_die is clever enough to use _ajax_wp_die_handler if it is admin ajax request.

[azaozz]

I love to see a...

Any particular reason why this piece of code should be different/inconsistent with most other AJAX responses from admin-ajax? Don't think introducing inconsistencies is a good idea, sorry :)

Also, as mentioned above I think this code may introduce edge cases as it is only checking the default cookies and not using the proper functionality to determine if a user is logged in?

[Mamaduka]

The wp_send_json_error uses wp_die internally.

See: https://developer.wordpress.org/reference/functions/wp_send_json/.

[azaozz]

Sure, let me reformulate the question: why not use wp_send_json_error() as most other WP code but use wp_die() directly with a new WP_Error instance? What advantages does that bring? Are the advantages worth it the change/inconsistency? (And other in the same line of thought) :)

BTW if a new WP_Error instance is needed for some reason it seems it can be passed to wp_send_json_error().