Opened 8 years ago
Closed 4 years ago
#37569 closed enhancement (worksforme)
REST API: refresh expired nonces
Reported by: | iseulde | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 4.4 |
Component: | REST API | Keywords: | dev-feedback has-patch |
Focuses: | Cc: |
Description
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.
Attachments (5)
Change History (39)
#3
@
8 years ago
- Keywords needs-patch needs-docs dev-feedback added; has-patch removed
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.
#4
@
8 years ago
- Keywords has-patch added; needs-patch removed
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?
#5
@
8 years ago
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.
#6
@
8 years ago
- Milestone changed from Awaiting Review to 4.8
- Type changed from defect (bug) to enhancement
#7
follow-up:
↓ 8
@
8 years ago
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.
#8
in reply to:
↑ 7
;
follow-up:
↓ 9
@
8 years ago
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.
#9
in reply to:
↑ 8
@
8 years ago
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.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#19
@
7 years ago
@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
#20
@
7 years ago
@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.
This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.
7 years ago
#22
@
7 years ago
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.
#24
@
7 years ago
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.
#25
@
7 years ago
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'.
#26
@
7 years ago
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> /* ... */
#27
@
7 years ago
@iseulde Thanks for the feedback and suggestions, I'll work on adding a new endpoint and a fallback mechanism to the client.
#28
@
7 years ago
- Keywords needs-docs removed
- Milestone changed from Future Release to 5.0
- 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.
#30
@
7 years ago
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.
#31
@
7 years ago
- explicitly prevent recursion, resyncing with a fresh nonce at most once each time a nonce failure occurs.
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?
See https://wordpress.slack.com/archives/core-restapi/p1470310813000622