Refresh nonce with heartbeat#2790
Conversation
ellatrix
left a comment
There was a problem hiding this comment.
@adamsilverstein This doesn't solve anything as far as I can see. The way I test it is setting the nonce life to 10 seconds. This is low enough to ensure that by the first heartbeat after page load the nonce will be invalid, so it's just like being away for over 24 hours and coming back to an invalid nonce. With this patch, nothing will be sent back. You'll need to hook into wp_refresh_nonces (see https://github.com/WordPress/wordpress-develop/blob/4.8.2/src/wp-admin/includes/ajax-actions.php#L2794). heartbeat_send is too late.
I wouldn't use jQuery here, we don't want a dependency on jQuery as well. You can just use that native browser API as we don't need to support lower IE versions.
Additionally there are some listing errors.
|
@iseulde thanks for testing and the feedback, also for explaining how you tested - I wasn't sure how to do that. I saw I thought I saw jQuery already used, I'll switch that and fix the linting errors. :) |
|
@iseulde actually for the jquery, I am listening for the event triggered by heartbeat:
|
Codecov Report
@@ Coverage Diff @@
## master #2790 +/- ##
==========================================
- Coverage 34.43% 34.42% -0.01%
==========================================
Files 196 196
Lines 5782 5783 +1
Branches 1020 1021 +1
==========================================
Hits 1991 1991
Misses 3206 3206
- Partials 585 586 +1
Continue to review full report at Codecov.
|
|
@iseulde I switched the hook to wp_refresh_nonces and verified this actually works correctly - i set my nonce expiration (30 sec) and heartbeat rate (15 sec) low and watched as the nonce expired and got renewed. Can you please re-test? |
Ah ok, that's unfortunate. :) Do we have proper dependencies set for heartbeat.js? |
Yes, in script-loader, we add hearbeat with a jQuery dependency: Is that what you meant? heartbeat.js has 6 $document.trigger() calls. If we swap out these for wp.hooks, we could remove the jQuery dependency. |
|
@adamsilverstein I meant here, in Gutenberg. I don't see a Heartbeat dependency. :) |
Ah, good point - I was testing on the post page so it must be loading there. i'll check/add the dependency. |
|
@adamsilverstein I fixed the failing tests, and replaced |
62c1d0f to
6e2bdc1
Compare
6e2bdc1 to
91211d5
Compare
* master: (45 commits) Parser: Hide the core namespace in serialised data (WordPress#2950) fix: Undefined index warning (WordPress#2982) navigation via keydown PHPCS improvements (WordPress#2914) Update/html block description (WordPress#2917) Framework: Merge EditorSettingsProvider into EditorProvider Framework: Move the store initialization to a dedicated component Rotate header ellipsis to match block ellipsis add/459: Added support for ins, sub and sup Refresh nonce with heartbeat (WordPress#2790) Paste: add table support Bump version to 1.4.0. (WordPress#2954) Blocks: The custom classname should be persisted properly in the block's comment Editor: Fix the scroll position when reordering blocks Polish spacing in block ellipsis menu (WordPress#2955) Editor: HTML Editor per block (WordPress#2797) Show most frequently used blocks next to inserter (WordPress#2877) add/459: Added light grey background to selected inline boundaries Extend inline boundaries to other elements Move markdown fix to own plugin-compatibility file ...
* master: (45 commits) Parser: Hide the core namespace in serialised data (WordPress#2950) fix: Undefined index warning (WordPress#2982) navigation via keydown PHPCS improvements (WordPress#2914) Update/html block description (WordPress#2917) Framework: Merge EditorSettingsProvider into EditorProvider Framework: Move the store initialization to a dedicated component Rotate header ellipsis to match block ellipsis add/459: Added support for ins, sub and sup Refresh nonce with heartbeat (WordPress#2790) Paste: add table support Bump version to 1.4.0. (WordPress#2954) Blocks: The custom classname should be persisted properly in the block's comment Editor: Fix the scroll position when reordering blocks Polish spacing in block ellipsis menu (WordPress#2955) Editor: HTML Editor per block (WordPress#2797) Show most frequently used blocks next to inserter (WordPress#2877) add/459: Added light grey background to selected inline boundaries Extend inline boundaries to other elements Move markdown fix to own plugin-compatibility file ...
* master: (45 commits) Parser: Hide the core namespace in serialised data (WordPress#2950) fix: Undefined index warning (WordPress#2982) navigation via keydown PHPCS improvements (WordPress#2914) Update/html block description (WordPress#2917) Framework: Merge EditorSettingsProvider into EditorProvider Framework: Move the store initialization to a dedicated component Rotate header ellipsis to match block ellipsis add/459: Added support for ins, sub and sup Refresh nonce with heartbeat (WordPress#2790) Paste: add table support Bump version to 1.4.0. (WordPress#2954) Blocks: The custom classname should be persisted properly in the block's comment Editor: Fix the scroll position when reordering blocks Polish spacing in block ellipsis menu (WordPress#2955) Editor: HTML Editor per block (WordPress#2797) Show most frequently used blocks next to inserter (WordPress#2877) add/459: Added light grey background to selected inline boundaries Extend inline boundaries to other elements Move markdown fix to own plugin-compatibility file ...
@iseulde Thanks for merging! I think I was testing in trunk and you were maybe on 4.8? The nonce handling changes a little in 4.9 and I don't think this will work since https://core.trac.wordpress.org/changeset/41553 - I am going to open a new PR that will use more flexible code to use the right approach in all cases. |
|
Yeah, I'm just on the latest version. :) |
|
Ok, makes sense and we need it to work there as well, so I'l work on a follow up PR. |
|
I'm having this issue. Is there a different place where it's being tracked? |
|
@getsource yes, see #9260 |
Summary
Refresh the wp-api nonce when a new nonce is available, keeping the nonce fresh and the client authorized. Fixes #2088.
Description
Add a wp_rest nonce to the heartbeat response when in Gutenberg.
Add a JavaScript callback on jQuery( document ) 'heartbeat-tick' which heartbeat triggers. Update the wp-api nonce if it has changed.
How Has This Been Tested?
Verified heartbeat returns nonce.
Verified if nonce is different, logic sets it on root endpoint.
Notes
Wasn't sure about the best place to put the JS callback, feedback appreciated!