WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#31294 closed defect (bug) (fixed)

Customizer no longer gracefully handles session expiration

Reported by: westonruter Owned by: ocean90
Milestone: 4.2 Priority: normal
Severity: major Version: 4.0
Component: Customize Keywords: has-patch commit
Focuses: javascript Cc:

Description (last modified by westonruter)

Reported by ocean90:

In r31370 you've changed doing_ajax() to return only true for defined( 'DOING_AJAX' ) && DOING_AJAX, and not isset( $_POST['customized'] ) anymore. This breaks the preview if you're logged out because you get a regular wp_die() with HTML instead of the 0.

https://wordpress.slack.com/archives/core-customize/p1423601418000599

Issue introduced (at least partially) by changes introduced in #30936.

Attachments (5)

31294.diff (3.1 KB) - added by westonruter 5 years ago.
https://github.com/xwp/wordpress-develop/pull/69
31294.2.diff (4.3 KB) - added by westonruter 4 years ago.
https://github.com/xwp/wordpress-develop/pull/79
31294.3.diff (5.1 KB) - added by ocean90 4 years ago.
31294.4.diff (5.2 KB) - added by ocean90 4 years ago.
31294.5.diff (6.0 KB) - added by ocean90 4 years ago.

Download all attachments as: .zip

Change History (17)

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


5 years ago

#2 @westonruter
5 years ago

  • Keywords has-patch added; needs-patch removed

31294.diff restores the auth behavior from 4.1, but even in 4.1 it is still broken. Steps to reproduce:

  1. Log-in to Customizer
  2. Open WP Admin in another tab and log-out
  3. Go back to Customizer and try navigating around the preview.
  4. See preview window update with a login prompt.
  5. After logging-in, the whole Customizer will refresh with a Cheatin' message instead of allowing the user to continue using the Customizer as expected.
  6. Reloading the whole page then allows the Customizer to be used as expected, but after having lost all of their settings.

#3 @westonruter
5 years ago

  • Description modified (diff)

#4 @DrewAPicture
5 years ago

  • Severity changed from normal to major

#5 @ocean90
5 years ago

In 31421:

Customizer: Restore showing a login form inside the previewer if an user is logged out.

Broken since [31370].

props westonruter.
see #31294.

This ticket was mentioned in Slack in #core-customize by ocean90. View the logs.


5 years ago

#7 @westonruter
5 years ago

  • Keywords needs-patch added; has-patch removed
  • Version changed from trunk to 4.0

That comment from @ocean90 on Slack:

"Cheatin’ uh?" exists in 4.0 too, so 3.9 is the latest version where it worked.

So I'm moving version back.

#8 @westonruter
4 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to ocean90
  • Status changed from new to reviewing

In 31294.2.diff, the Customizer login now updates nonces upon successful login.

Prevent cheatin' message after re-authenticating in Customizer. If the user's session expired while in the Customizer, and they were prompted to re-authenticate inside the Preview, before this the Customizer would throw up a cheatin message because the nonce used to get request the preview or to save the settings was tied to the user's previous session which is no longer valid.

As noted by @ocean90, the regression started in 4.0. I see that the regression is due to the introduction of the user session tokens since the nonces are now tied to session tokens as opposed to user IDs, and thus they change with each re-login.

This is a nasty bug because it can result in a user losing their changes, and getting an unhelpful cheatin' message to boot.

This ticket was mentioned in Slack in #core-customize by ocean90. View the logs.


4 years ago

@ocean90
4 years ago

@ocean90
4 years ago

#10 follow-up: @ocean90
4 years ago

@westonruter Instead of hacking wp-login.php I would like to propose to do an AJAX request which fetches refreshed nonces, see 31294.4.diff. This makes it more generic and future-proof (in terms of the heartbeat API). Thoughts?

#11 in reply to: ↑ 10 @westonruter
4 years ago

  • Focuses javascript added
  • Keywords commit added

Replying to ocean90:

@westonruter Instead of hacking wp-login.php I would like to propose to do an AJAX request which fetches refreshed nonces, see 31294.4.diff. This makes it more generic and future-proof (in terms of the heartbeat API). Thoughts?

Nice work! I agree this is a better approach, especially considering the current state of wp_signon in how it doesn't set the expected $_COOKIE variables in the current request. It's also unfortunate that an additional Ajax request is then required, but like you said it will be used in a future heartbeat “keep-alive” of the Customizer, so that preview refreshing isn't the only mechanism for keeping nonces up to date (nevermind it doesn't also refresh the update-widget nonce). I just opened #31897 to implement heartbeat-updating for the nonces for 4.3.

@ocean90
4 years ago

#12 @ocean90
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 32054:

Customizer: Refresh nonces when a session expires and the user logs in again.

This was broken since 4.0 and the introduction of user session tokens. The nonces are now tied to session tokens as opposed to user IDs, and thus they change with each re-login.
Custom nonces can be added through the customize_refresh_nonces filter. On a successful refresh request the JavaScript API will trigger a nonce-refresh event. See widget's update nonce as an example.

props westonruter for initial patch.
fixes #31294.

Note: See TracTickets for help on using tickets.