WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 6 weeks ago

#24447 new defect (bug)

Avoid losing data after nonces expire

Reported by: azaozz Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Administration Keywords:
Focuses: Cc:

Description

Happens when an admin page containing a form is left open for more than 24 hours and the user decides to submit the form. This is quite rare for most admin pages as the users typically spend short time there. However this can happen on the Edit Post screen too despite that we refresh the basic nonces every wp_nonce_tick (12 hours):

  • The user starts new post.
  • At some point the Internet connection is lost.
  • The user decides to finish later and puts the computer to sleep (closes the laptop, etc.).
  • The user decides to continue writing more than 24 hours after that.

At this point all nonces have expired and cannot be updated as we've missed the previous nonce_tick update.

Attachments (1)

24447.patch (649 bytes) - added by johnbillion 13 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 azaozz14 months ago

If a user hits the AYS (nonce expired) after trying to save from the Edit Post screen, the post data would be saved in session storage. Clicking the AYS link or the back button will load the screen and offer to restore the post. If another user is currently editing the same post, this won't be accessible until the current user takes over.

If we refresh the basic nonces (regardless that they have expired), another user may have edited and/or published the post and the current screen will not reflect that. Saving the post will overwrite it to its current state. Also refreshing the nonces will not refresh any nonces added by plugins.

For all other screens we can (at least) warn the user that the page has expired and needs to be reloaded before any changes can be saved.

comment:2 azaozz14 months ago

Thinking more about this especially for the Edit Post screen: refreshing the nonces fails every time the user's computer goes offline (or to "sleep") whitin 12 hours of loading the screen and stays offline for at least 12 hours (so the total time since loading the screen exceeds 24 hours).

In this case we don't refresh the nonces as we cannot check the old values. Possible solutions:

  • Add a "grace period" for some nonces.
    • May have security implications.
    • Would not cover nonces added by plugins.
    • Even if we extend certain nonces' life to lets say 48 hours, they would still expire and some users may still loose data or at least see the AYS screen.
  • When nonces have expired, ask the user to enter his/her password and override them.
    • Will work on form submission, would be harder to do for ajax requests.
    • May cover nonces added by plugins but not for ajax.
  • Show an error that the page has expired including a link to open the same screen in a new window so the user can copy/paste any unsaved content.

All three options are more or less lame and/or don't solve this completely.

There are other implications of keeping a page open for a long time: a post may have been edited by another user or a setting may have been changed and the current screen won't show this. So even if we make it possible to save changes after an extended period, the user may be overwriting or deleting data. In that terms the third option looks like the right one, perhaps in combination with one of the others.

Other suggestions welcome.

comment:3 follow-up: knutsp14 months ago

Show an error and present the user with two options:

  1. Log in and save the content as a new draft
  2. Log in and discard the content

If option 1 is selected, after login, redirect the user to this draft in the editor.
If option 2 is selected, after login, redirect the user to the current post in the editor.

comment:4 aaroncampbell14 months ago

  • Summary changed from Avoid loosing data after nonces expire to Avoid losing data after nonces expire

comment:5 in reply to: ↑ 3 azaozz14 months ago

Replying to knutsp:

Show an error and present the user with two options:

  1. Log in and save the content as a new draft
  2. Log in and discard the content

Yes, that will work.

Nonces expiration is different than login expiration. The user can still be logged in and the nonces may have expired. In this case 2 can proceed and reload the page.

If 1 is selected we ask for the user's password again and override nonce checking (see the second option in the previous comment). The password will be submitted with the form so we will be logging the user in (or verifying the password) at the same time as saving the submitted form data.

If we go this way we will have to "pre-verify" the entered password in case the user makes a mistake, doable with ajax.

comment:6 azaozz13 months ago

Currently the most "workable" solution for the Edit Post screen is to use session storage to save the title and content (this happens anyway), warn the user and reload the page to refresh the nonces, then offer to restore from session storage. This is pretty much how it works now except the user won't see the AYS screen.

For all other screens we should warn the user (with a modal dialog) and maybe disable any submit buttons.

comment:7 johnbillion13 months ago

A few of us were discussing this in #wordpress-dev after the meeting tonight.

We could fetch a new nonce via an AJAX call that authenticates the user and returns a nonce value for the required action(s). Currently this happens during autosave but you only get a new nonce if your current one is within the expiry period (12-24 hours).

The current autosave nonce needs to be checked here to verify intent to make an autosave, but the other nonces don't need to be verified. Authentication is enough.

We should generate new nonces for update_post, autosave, and whatever others there are (metaboxes etc). The autosave then fires again immediately with the new autosave nonce.

Thoughts? Does this inadvertently circumvent the autosave intent? I don't think it does. It's no different to requesting the edit screen and grabbing the nonce out of the HTML.

Thoughts?

johnbillion13 months ago

comment:8 johnbillion13 months ago

24447.patch removes the superfluous nonce check when refreshing nonces.

The nonce refresh has moved into the heartbeat call (it was previously in the autosave call), so I'm not sure how we could immediately re-fire the autosave if its nonce has expired.

Last edited 13 months ago by johnbillion (previous) (diff)

comment:9 azaozz12 months ago

  • Milestone changed from 3.6 to Future Release

Not a regression or new bug, lets revisit in 3.7.

comment:10 vloo6 weeks ago

I'm not quite sure but this seems like a duplicate of something 5 years old: #9604

Last edited 6 weeks ago by SergeyBiryukov (previous) (diff)

comment:11 SergeyBiryukov6 weeks ago

#9604 was marked as a duplicate.

Note: See TracTickets for help on using tickets.