Update Customizer nonces via Heartbeat API

[westonruter]

Currently the Customizer's nonces get updated when the preview gets refreshed (only the save and preview nonces, not the update-widget nonce, however). If the user leaves the window open in the background for a long time, they will get stale nonces. We should be using the Heartbeat API and the wp_ajax_customize_refresh_nonces filter introduced in #31294 to keep the nonces up date. (This is no longer true as of #35617.)

See also #31436 where Heartbeat integration will also be required to handle Customizer concurrency issues.

[jdgrimes]

Related: #29312

[iseulde]

Related: #24447.

[westonruter]

I'll tackle this as part of #31436.

[ocean90]

No movement in 4.3.

[joeyblake]

Enqueues heartbeat and refreshes nonces on the standard heartbeat timing.

[kirasong]

Hi @joeyblake! Thanks for the patch.

Took a look with @azaozz -- this works, but it looks like it's only using Heartbeat as a timer, rather than using the Heartbeat API to transfer the nonce data. I'd suggest using that transport instead, since it will avoid extra requests.

As a second note, it looks like this is running every 60 seconds. Since a user shouldn't need nonces this often, you could change this so that it doesn't check as often.

[joeyblake]

Yep, I see. It also doesn't lend itself to using the heartbeat for other things like #31436 . I'll make those changes.

[westonruter]

See also #35617.

[westonruter]

Will be punted unless solid patch comes in the next several hours.

[joeyblake]

Sorry for the radio silence. Here is the issue that I've had with this ticket. I'm definitely missing something, and maybe someone else can see where.

My inclination when adding the filter to 'heartbeat_received' to refresh the nonces would be to have the logic and filter inside class-wp-customize-manager.php. However, when adding the filter there, it never gets executed.

If I were to move the filter out of there, to admin-filters.php it will execute, but admin-filters.php has no knowledge of WP_Customize_Manager, or the instantiated global at all. So I calling a method inside of there to retrieve the updated nonces is not possible.

I'm currently running adding data to the heartbeat-send event from the api.Previewer object in customize-controls.js

Is there anything obvious that I'm missing here?

Edit:
Also, if timeframe is an issue here, hopefully this info helps someone else, I most likely won't be able to touch it today.

[voldemortensen]

Punting because I wasn't able to get this done last night and beta is today.

[westonruter]

Replying to joeyblake:

My inclination when adding the filter to 'heartbeat_received' to refresh the nonces would be to have the logic and filter inside class-wp-customize-manager.php. However, when adding the filter there, it never gets executed.

Good point. This is due to the heartbeat requests being made without the wp_customize=on POST param, and so the WP_Customize_Manager doesn't get bootstrapped. For the logic for how the class gets bootstrapped, see: https://core.trac.wordpress.org/browser/tags/4.4.2/src/wp-includes/theme.php?marks=1985-2007#L1985

Perhaps the Customizer should also get bootstrapped if the heartbeat_received includes wp_customize=on?

Actually, I just remembered this is something that @valendesigns and I have done in the Customize Concurrency feature plugin.

Here's the heartbeat_settings and heartbeat_received filters: ​https://github.com/xwp/wp-customize-concurrency/blob/dcaf1d421cc2d546ea0cdb9b97b60d8de9327a13/php/class-customize-concurrency.php#L221-L273

Notice the injection of the screenId which would not get set by default when loading heartbeat from the Customizer. And then notice how this screenId=customize is used as a signal to bootstrap the Customizer if not already instantiated in the heartbeat_received filter.

[adamsilverstein]

In 31897.diff​

• Move customizer nonce refresh to hearbeat api
• Bootstrap customizer when heartbeat_received data contains wp-refresh-customizer-nonce data
• Add and apply new nonces if wp_verify_nonce returns 2.

@westonruter I worked on this before reading your comment, open to suggestions for making the bootstrapping cleaner.

[westonruter]

@adamsilverstein nice. The patch is looking good.

• In wp_refresh_customizer_nonces() you can prevent instantiating WP_Customize_manager if the $wp_customize global already exists.
• Add a current_user_can( 'customize' ) cap to the condition along with array_key_exists(). Otherwise, an unprivileged user could potentially obtain nonces.
• Needs @param and @return phpdoc tags.
• It would be useful for other plugins that make use of Heartbeat in the Customizer to have the $screen_id populated to be customize.
• Maybe rename wp_refresh_customizer_nonces() to wp_heartbeat_refresh_customizer_nonces().
• Maybe rename the heartbeat data array key from wp-refresh-customizer-nonce to wp-customize-nonces.

[adamsilverstein]

@westonruter Thanks for the additional feedback; I will work to address these issues.

In 31897.2.diff​ I took an alternate approach: I added support for sending wp_customize=on and moved the heartbeat php code back into class-wp-customize-manager.php. What do you think of this approach? It prevents having to manually bootstrap the customizer in the heartbeat callback.

[adamsilverstein]

@westonruter think I addressed all your points...

In 31897.3.diff​:

• Add current_user_can( 'customize' ) cap check
• Add docblocks
• Add a heartbeat_settings filter to set the $screen_id to customize
• Some renaming as suggested

[westonruter]

@adamsilverstein I think you can remove all references to isCustomizer and instead just do:

if ( 'customize' === settings.screenId ) {
    ajaxData.wp_customize = 'on';
}

This also raises another question: should not the heartbeat sent to the server bring along with it the full context of the current Customizer state? In other words, shouldn't the customized JSON blob of dirty settings and the current theme be sent along in these heartbeat requests? This would ensure that when the Ajax request is received, all of the Customizer state will apply. Something will have to be done here to ensure that the preview nonce can be included alongside the heartbeat nonce.

[westonruter]

I just realized that all nonces now get updated when the preview refreshes as of #35617, making this ticket less important. This ticket now specifically fix the issue where the Customizer is left open for a long time without the preview being refreshed to keep the nonces up to date. Nevertheless, if the user does leave the browser session open for such a long time, it is also likely that their session will expire and they will need to re-login anyway: by default, non-remember user authentication sessions expire after 48 hours (auth_cookie_expiration) and nonces expire after 24 hours (nonce_life).

Nevertheless, the integration of Heartbeat into the Customizer will be useful for plugins generally, so I'd love to see that happen, and keeping nonces up-to-date should be the first application of Heartbeat in the Customizer. That can either be made the scope of this ticket, or another ticket can be made specific for that feature.

[adamsilverstein]

Replying to westonruter:

I just realized that all nonces now get updated when the preview refreshes as of #35617,

Yea, I realized that working on the patch, I thought this was specifically to address the customizer being left open for a long period (with no refresh) and the nonce expiring.

I still think this is useful. A nonce could expire in as little as 12 hours - and leaving the customizer open overnight could easily expire the nonce; this patch would issue a new nonce lasting 24 hours in this case, and keep extending it every 12 hours as long as the heartbeat was running.

Doesn't the user get a warning to log in again before their session expires in the customizer?

[adamsilverstein]

31897.4.diff​

• Check 'customize' === settings.screenId and skip use of isCustomizer

[westonruter]

Replying to adamsilverstein:

Doesn't the user get a warning to log in again before their session expires in the customizer?

Once their session expires, the user gets prompted to re-login. But the nonces generally have a shorter lifespan than the user sessions, so this would ensure the nonces remain valid for the entire session, unless… if they sleep their computer with the Customizer open for a day, and then wake their computer: the nonces will have expired, including the nonce for Heartbeat itself. So in this case, it seems the only solution would be for the user to reload the Customizer, or I suppose a prompt to re-login would have the same effect.

[westonruter]

Patch needs refresh. It doesn't apply cleanly anymore.

[lukecavanagh]

@westonruter 

Would be happy to work on a patch refresh.

[adamsilverstein]

Replying to lukecavanagh:

Would be happy to work on a patch refresh.

Thanks, @lukecavanagh! The patch should still work once we fix the failure applying it. @westonruter any ideas for an approach to a unit test, where it would go with the current tests?

[westonruter]

For QUnit tests, I think it would involve doing jQuery( document ).trigger( 'heartbeat-tick.wp-refresh-nonces', [ { 'wp-customize-nonces':{…} } ] ) and making sure that this results in nonce-refresh being triggered on wp.customize.

Likewise, when doing jQuery( document ).trigger( 'heartbeat-send.wp-refresh-nonces', [ { 'wp-customize-nonces':{…} } ] ) that the current wp.customize.settings.nonce gets populated into the heartbeat data being passed in.

The QUnit tests would go into tests/qunit/wp-admin/js/customize-controls.js, I think.

For PHPUnit, similarly, we'd want to apply filters on heartbeat_received and heartbeat_settings to make sure the supplied arrays get the expected values amended onto them via wp_heartbeat_refresh_customizer_nonces() and wp_heartbeat_settings_customizer_filter() respectively. This would go into tests/phpunit/tests/customize/manager.php

[lukecavanagh]

Patch refresh

[westonruter]

In 41839:

Customize: Add changeset locking in Customizer to prevent users from overriding each other's changes.

• Customization locking is checked when changesets are saved and when heartbeat ticks.
• Lock is lifted immediately upon a user closing the Customizer.
• Heartbeat is introduced into Customizer.
• Changes made to user after it was locked by another user are stored as an autosave revision for restoration.
• Lock notification displays link to preview the other user's changes on the frontend.
• A user loading a locked Customizer changeset will be presented with an option to take over.
• Autosave revisions attached to a published changeset are converted into auto-drafts so that they will be presented to users for restoration.
• Focus constraining is improved in overlay notifications.
• Escape key is stopped from propagating in overlay notifications, and it dismisses dismissible overlay notifications.
• Introduces changesetLocked state which is used to disable the Save button and suppress the AYS dialog when leaving the Customizer.
• Fixes bug where users could be presented with each other's autosave revisions.

Props sayedwp, westonruter, melchoyce.
See #31436, #31897, #39896.
Fixes #42024.

[westonruter]

Note: Now that heartbeat is being used for customization locking (#42024) it will be trivial to pass back refreshed nonces with each tick.

[westonruter]

New feature development is being directed toward the Site Editor, so I'm closing this.