WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.46k stars 4.18k forks source link

Renew nonce to prevent expiration and `Updating failed` message #9260

Closed iandunn closed 5 years ago

iandunn commented 6 years ago

Describe the bug

I often will keep a tab open in the editor for hours, days, or weeks, while working on a post. I'll make some edits, then mull things over while doing something else, then come back to jot some some ideas or refine something.

While doing that with Gutenberg, I've often noticed that, when I come back and start typing, I'll see an Updating failed error. If I had to guess, I'd say that that the nonce used in the save draft API request has expired, so the request fails.

If that's the case, then I think that Gutenberg should automatically renew the nonce to prevent this from happening. IIRC, Core already does this with the Heartbeat API or something similar.

Related Issues

9146, #8241, and #4590 are related, but seem like a different underlying cause, because in those cases the error seems to happens on every save attempt (often caused by CloudFlare blocking the API request), whereas here the API requests work normally at first, and then start to fail only after a period of time has past.

IIRC, I've experienced this both in production environments behind CloudFlare, and also in my local development environment.

To Reproduce

  1. Draft a post
  2. Leave it sitting for 12+ hours. It may be possible to artificially reproduce the conditions by changing NONCE_KEY or something similar.
  3. Come back to the editor and start typing. You should get an Updating failed error.

Screenshots

screen shot 2018-08-22 at 8 00 20 pm

Desktop (please complete the following information):

designsimply commented 6 years ago

Thank you for writing this up! Do you think the underlying cause might be the same as in #8892?

Note: I consider this a bug. I'm also adding the Needs Testing label and would like to try to catch a copy of response for the failing API calls for this case in case it's helpful.

iandunn commented 6 years ago

Good catch, I missed that one. It does seem different to me from #8892, since I'm still logged in when I experience the error. The period of time needed to trigger this error seems much lower than the period of time that my login session expires; e.g., ~12-24 hours rather than ~2-14 days (depending on the Remember Me setting).

designsimply commented 6 years ago

Cool. Thanks for checking that! Let the testing commence. 😁

iandunn commented 6 years ago

Saw this happen again, it is the nonce expiring.

Request

POST /wp-json/wp/v2/posts/6900/autosaves HTTP/1.1
Host: wp.test
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:63.0) Gecko/20100101 Firefox/63.0
Accept: */*
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Referer: http://wp.test/wordpress/wp-admin/post.php?post=6900&action=edit
x-wp-nonce: [redacted]
content-type: application/json
origin: http://wp.test
Content-Length: 831
Connection: keep-alive
Cookie: wp-settings-1=editor%3Dtinymce%26libraryContent%3Dbrowse%26hidetb%3D1%26posts_list_mode%3Dlist%26mfold%3Do; wp-settings-time-1=1518459995; XDEBUG_SESSION=XDEBUG_ECLIPSE; wordpress_test_cookie=WP+Cookie+check; PHPSESSID=[redacted]; wordpress_logged_in_[redacted]=iandunn%7C[redacted]%7C[redacted]
Pragma: no-cache
Cache-Control: no-cache

Response

HTTP/1.1 403 Forbidden
Server: nginx/1.15.1
Date: Thu, 13 Sep 2018 18:20:50 GMT
Content-Type: application/json; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive
X-Powered-By: PHP/7.2.7
X-Robots-Tag: noindex
Link: <http://wp.test/wp-json/>; rel="https://api.w.org/"
X-Content-Type-Options: nosniff
Access-Control-Expose-Headers: X-WP-Total, X-WP-TotalPages
Access-Control-Allow-Headers: Authorization, Content-Type
Expires: Wed, 11 Jan 1984 05:00:00 GMT
Cache-Control: no-cache, must-revalidate, max-age=0
Access-Control-Allow-Origin: http://wp.test
Access-Control-Allow-Methods: OPTIONS, GET, POST, PUT, PATCH, DELETE
Access-Control-Allow-Credentials: true
Vary: Origin

{"code":"rest_cookie_invalid_nonce","message":"Cookie nonce is invalid","data":{"status":403}}
aduth commented 6 years ago

This is meant to be improved by adding a fresh nonce into WordPress' existing heartbeat response. Possible you have this disabled?

cc @adamsilverstein @youknowriad

iandunn commented 6 years ago

Hmm, I don't think so. I'm sure I've never intentionally done that, and can't think of any plugins that would. IIRC, I've seen this happen on production sites w/ plugins and also local dev sites without any other plugins.

adamsilverstein commented 6 years ago

The nonce should be refreshed automatically if heartbeat is enabled, I'll do some testing to see if the code that handles the nonce refresh is still working correctly - quite a bit has changed since this was introduced.

designsimply commented 6 years ago

Tested and confirmed using WordPress 4.9.8 and Gutenberg 4.0 that leaving an editor window open for a long time results in an "Updating failed" error on screen and the following error in the console:

Failed to load resource: the server responded with a status of 403 () /test/wp-json/wp/v2/posts/4957/autosaves:1

And the following API response for autosaves:

{"code":"rest_cookie_invalid_nonce","message":"Cookie nonce is invalid","data":{"status":403}}

screen shot 2018-10-18 at 10 16 47 am Seen at https://make.wordpress.org/test/wp-admin/post.php?post=4957&action=edit running WordPress 4.9.8 and Gutenberg 4.0 using Firefox 62.0.3 on macOS 10.13.6.

ellatrix commented 6 years ago

Might be related: https://core.trac.wordpress.org/ticket/37569 and https://core.trac.wordpress.org/ticket/24447

aduth commented 6 years ago

If that's the case, then I think that Gutenberg should automatically renew the nonce to prevent this from happening.

Would this even be technically possible upon returning from idle if the computer had idled longer than the valid nonce duration?

danielbachhuber commented 6 years ago

From Slack chat today:

timothybjacobs [10:31 AM] Shouldn’t the login modal pop up? kadamwhite [10:31 AM] don't we do something similar with the backbone api client?

Needs additional technical exploration.

adamsilverstein commented 6 years ago

my guess is this broke in https://github.com/WordPress/gutenberg/commit/b2ec40697bd7f2e13930e6fb3fffed12c7809baa

the nonce refresh was originally added in https://github.com/WordPress/gutenberg/pull/2790

i'm going to do some debugging by reducing my nonce life and checking to see if the update handler is being called.

earnjam commented 5 years ago

I looked into this a bit. Looks like all requests sent through api-fetch are using the same value for the X-WP-Nonce header that was initialized on load or the value returned in a response header from a REST API request.

The heartbeat ticks are returning a rest-nonce as expected, and the heartbeat.tick hook is firing, but that value is not getting used later by api-fetch, so once that initial value expires, you get the 403 response.

I need to dig into it a bit more to see why the api-fetch middleware isn't using the new value.

dd32 commented 5 years ago

@earnjam That's basically the same thing that I ended up at.

It looks to me, like the middleware function which contains the usedNonce field is being regenerated on every API request, which means that the contained nonce is discarded each time as it gets re-initialised from the original value.. or at least that's the best I can come up with.

I also noticed that the addAction() is running on every API request, so that function ends up adding another filter for every request, but that doesn't seem to affect the outcomes.

I also looked into having the middleware hook in after the API request is made to grab the latest nonce from the API response, but I can't find a way to actually hook in and get the headers data (Only the final response seems to be available) from middleware without also altering api-fetch itself to throw an action

dd32 commented 5 years ago

It looks to me, like the middleware function which contains the usedNonce field is being regenerated on every API request, which means that the contained nonce is discarded each time as it gets re-initialised from the original value.. or at least that's the best I can come up with.

That looks to be the case, Putting together #11347 after that comment seems to be working as expected. As a bonus, the addAction() issue is now also solved.

earnjam commented 5 years ago

@iandunn This is fixed now in my testing after #11347, but if you are still encountering the issue, please update and we can revisit.

Thanks for the help on this one @dd32!

iandunn commented 5 years ago

Thanks!

Suraj151 commented 5 years ago

i have fresh installed wordpress in ubuntu 18.04 local machine. and when i try to add page it says same message Updating failed. in console i can see POST http://localhost/wp-json/wp/v2/pages/7?_locale=user 403 (Forbidden) api-fetch.min.js?ver=2.2.7:1 error on specific interval. and after few seconds i get popup to login again. it seems it loosing sessions ( or something else. i dont know, i just installed it to getting start with it, and now searching for solution ) as well. is this issue related with subjected one ?

earnjam commented 5 years ago

@Suraj151 On the request that is returning 403, can you see what the response is?

screen shot 2019-01-31 at 8 37 33 am

Suraj151 commented 5 years ago

@earnjam when i first hit update its response is blank and response headers shows following

Access-Control-Allow-Headers: Authorization, Content-Type Access-Control-Expose-Headers: X-WP-Total, X-WP-TotalPages Cache-Control: no-cache, must-revalidate, max-age=0 Connection: Keep-Alive Content-Length: 0 Content-Type: application/json; charset=UTF-8 Date: Fri, 01 Feb 2019 05:57:58 GMT Expires: Wed, 11 Jan 1984 05:00:00 GMT Keep-Alive: timeout=5, max=100 Link: <http://localhost/wp-json/>; rel="https://api.w.org/" Location: / Referrer-Policy: no-referrer-when-downgrade Server: Apache/2.4.29 (Ubuntu) Set-Cookie: wordpress_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/wp-admin Set-Cookie: wordpress_sec_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/wp-admin Set-Cookie: wordpress_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/wp-content/plugins Set-Cookie: wordpress_sec_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/wp-content/plugins Set-Cookie: wordpress_logged_in_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpress_logged_in_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wp-settings-1=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wp-settings-time-1=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpress_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpress_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpress_sec_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpress_sec_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpressuser_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpresspass_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpressuser_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wordpresspass_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ Set-Cookie: wp-postpass_86a9106ae65537651a8e456835b316ab=+; expires=Thu, 01-Feb-2018 05:57:58 GMT; Max-Age=0; path=/ X-Content-Type-Options: nosniff X-Robots-Tag: noindex X-WP-Nonce: 940366ecdf

when i hit second time update then it shows response as { "code":"rest_cookie_invalid_nonce", "message":"Cookie nonce is invalid", "data":{"status":403} }

then autosave called on specific interval which got same above nonce invalid response.

one interesting thing that i have 3-4 pages and out of that pages only one page has this error while update. other pages gets update successfully. i have make one function like

if ( ! is_admin() ){ add_filter('widget_text', 'do_shortcode'); add_shortcode( 'wp-tttest-short-code-fnctn', 'wp_tttest_custom_front_html' ); }

function wp_tttest_custom_front_html(){ wp_logout(); wp_safe_redirect( '/' ); exit(); }

now whenever i comment lines inside above if condition then that page gets updated successfully and when i remove comment then it failed to update that page.

here i know that i am updating page as admin then it should not go inside that if condition then why above scenario happens. this is all i am playing around. as beginner i am interested to know about it.

i know its dirty to make such customs but my concern is about how that only page gets error and why commenting those lines make it successful.

Suraj151 commented 5 years ago

sorry its my mistake. i did not read functions description clearly

as per code reference doc

is_admin() Determines whether the current request is for an administrative interface page. it does not check if the user is an administrator ( which i was supposing )

then on some search i found is_super_admin() Determine if user is a site admin. ( that is what i was looking for )

so after replacing is_admin() with is_super_admin() it working fine.

earnjam commented 5 years ago

@Suraj151 Thanks for the follow-up. I would suggest using current_user_can() here with a specific capability check instead of is_super_admin(). That function was originally used for multisite, but since 4.8, it's usage has been discouraged in favor of capability checks. See: https://make.wordpress.org/core/2017/05/22/multisite-focused-changes-in-4-8/

Rolf-Herbert commented 5 years ago

Im getting this message all the time and it is basically rendering Wordpress unusable as you cant save your work. {"code":"rest_cookie_invalid_nonce","message":"Cookie nonce is invalid","data":{"status":403}}

Just updated Wordpress (5.1.1) Tried disabling Jetpack and all other plugins

the only thing that works is to log off and on again and then after a few auto saves its back failing again!

Massively annoying

designsimply commented 5 years ago

@TeamKinetic to confirm, are you seeing similar messages referencing the autosaves API call as shown in the examples above? And you still see the problem after clearing cache and testing using an unmodified WordPress 5.1.1 install with no plugins active?