REST API: refresh expired nonces

[iseulde]

Is there a reason to not refresh an expired nonce?

At the moment the API only refreshes nonces when it's given an unexpired nonce, so that the client can refresh it for future requests. When the nonce expired though, it will just error. The only thing a client can do then is to refresh the page, or get a new nonce some other way. Since a simple authenticated request is enough to get a new nonce elsewhere, I don't see why the API itself can't provide the client with a new one, so that the client can refresh its nonce, and repeat the request with the new nonce.

[iseulde]

Okay, so the reason is CORS. In that case the client should be able to refresh the nonce some other way. Maybe a separate endpoint, maybe admin-ajax.php, and some docs?

[iseulde]

So sending the nonce with admin-ajax.php is exactly what I end up doing, as there is nothing that core provides. It would be great if core had some documentation on how to do this best and/or provided an endpoint for cookie based auth to renew an expired nonce.

[rmccue]

Refresh nonce via Heartbeat

[rmccue]

We can refresh the nonce via Heartbeat, similar to the existing code that does the same for the regular nonces in the admin. 37569.diff​ adds a new wp-api-nonce script that does just that. It only refreshes the nonce in the second tick (12-24 hours).

This is a separate script to avoid creating a dependency on heartbeat for wp-api, as you may not want Heartbeat loading constantly on the frontend of your site.

@iseulde Does this suit your use case?

[rmccue]

Note: I used heartbeat_received rather than wp_refresh_nonces, as the latter only fires if the heartbeat-nonce is in the second tick. These should be the same, but with the pluggability of nonces, I'd rather avoid that.

[iseulde]

At first glance this looks great. It's fine not having a separate script IMO, as long as there is an endpoint for it, it's the canonical way to refresh the nonce, and it's documented as such.

[rmccue]

Replying to iseulde:

At first glance this looks great. It's fine not having a separate script IMO, as long as there is an endpoint for it, it's the canonical way to refresh the nonce, and it's documented as such.

Do you mean we should roll it into the regular Heartbeat refreshes? We could potentially roll it into the base wp-api library as well, if @adamsilverstein thinks that's a good idea.

[adamsilverstein]

Do you mean we should roll it into the regular Heartbeat refreshes? We could potentially roll it into the base wp-api library as well, if @adamsilverstein thinks that's a good idea.

@rmccue I definitely think this is a good idea, that was my plan all along with #35662. Let's open a separate ticket to track this enhancement.

[adamsilverstein]

Created a new ticket to add nonce refreshing to the JS client in #40422

[iseulde]

Not sure about rolling into heartbeat, but yes to the base wp-api library.

[jbpaul17]

Punting to 4.8.1 per today's bug scrub.

[jbpaul17]

Punting to 4.9 per today's bug scrub.

[jnylen0]

Do we still want this change, given that #40422 is going to add this functionality to the wp-api.js client?

If so, let's commit #40919 first and then roll this change into the script that will be added there.

[iseulde]

@rmccue Stumbled upon this ticket again and saw that the patch has a check to see if the old nonce is in its second half life. So this patch doesn't really solve this issue? We need to send a new nonce regardless of the old nonce's validity.

Gutenberg issue: ​https://github.com/WordPress/gutenberg/pull/2790

[adamsilverstein]

@iseulde the way this should work is if your 24 hr nonce is more than 12 hrs old, you get a new 24 hr nonce. So every 12 hours you should get a new nonce that would last 24 hours. As long as you keep checking at least once every < 12 hrs, you should always have a nonce that will last you 12+ hrs. Can you test the latest patch on the PR, it should work as expected.

[adamsilverstein]

Probably makes the most sense to test this feature in Gutenberg first since its needed there and we don't really need it in core otherwise and it would be easy for clients to add if they need it. In 37569.2.diff​ I cleaned up a tiny bit and added wp-api-nonce as a dependency of wp-api, however we should probably only load it if heartbeat is active. Also, it would be great to add some unit tests demonstrating how this works.

[iseulde]

I have some concerns with the latest patch.

It still doesn't solve anything for an expired nonce, as there's a nonce check...

As mentioned in a previous comment, I'm not sure if it's a good idea to roll this into the Heartbeat API. The API client might be a better place for it. ​A possible approach could be to create a separate endpoint for cookie auth only, either on the REST API root, or admin-ajax.php. I feel that setting it on the root would make it more official for other clients to adopt. With this endpoint, the client could get a new nonce if a request fails because of an invalid nonce. No need for Heartbeat. With this new nonce, the client should resend the same request (as if nothing happened). To the user of this API, the resolution of the promise will just take a bit longer, but it doesn't need to do anything.

A long time ago, I had to do something ​similar for a plugin.

[adamsilverstein]

create a separate endpoint for cookie auth only

@iseulde I like this idea! What would be required to get a fresh nonce? Does offering up the nonce without requiring a previous valid nonce weaken their value? Previous thinking was you could always keep your nonce fresh by checking at least once every 12 hrs and returning a new nonce in the second 'half'.

[iseulde]

See ​https://github.com/WordPress/gutenberg/pull/3006#issuecomment-337241027. We wouldn't require anything. It's the same mechanism as a page refresh, which checks if the user is still logged in and has the right capabilities to otherwise get the nonce on first page load. It's like requesting the the same page in JS and then parsing the right nonce out of it*. :) May be a good idea to pass by other security team members, but as far as I see, this is no problem.

As of ​https://github.com/WordPress/gutenberg/pull/2790, we're already doing it without requiring anything.

*

fetch( http://wordpress.test/wp-admin/admin.php?page=gutenberg-demo, {credentials: "same-origin"})

would return:

<!DOCTYPE html>
/* ... */
<script type='text/javascript'>
var wpApiSettings = {"root":"http:\/\/wordpress.test\/wp-json\/","nonce":"...","versionString":"wp\/v2\/"};
</script>
/* ... */

[adamsilverstein]

@iseulde Thanks for the feedback and suggestions, I'll work on adding a new endpoint and a fallback mechanism to the client.

[adamsilverstein]

In 37569.3.diff​

• include a fresh nonce in rest responses when the user is logged in, even if the nonce check fails
• retry the sync when a nonce failure is detected and a new nonce is available

My only concern here is potentially infinite recursion, if the returned nonce continues to change, the request will be made repeatedly. This shouldn't happen, but might be worth explicitly preventing it.

[iseulde]

Would be grateful for a second opinion here.

My only concern here is potentially infinite recursion, if the returned nonce continues to change, the request will be made repeatedly. This shouldn't happen, but might be worth explicitly preventing it.

Yeah, I agree.

[adamsilverstein]

prevent recursion

[adamsilverstein]

In 37569.4.diff​

• explicitly prevent recursion, resyncing with a fresh nonce at most once each time a nonce failure occurs.

[peterwilsoncc]

Switching milestone due to the focus on the new editor (Gutenberg) for WordPress 5.0.

[TimothyBlynJacobs]

I don't think this is necessary anymore after #48076. If I'm incorrect, please reopen.