Cookie Consent: send a REST nonce from the consent logger so logged-in user IDs are recorded#50068
Open
chihsuan wants to merge 1 commit into
Open
Cookie Consent: send a REST nonce from the consent logger so logged-in user IDs are recorded#50068chihsuan wants to merge 1 commit into
chihsuan wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to WOOA7S-1563 / #49852 (prerequisite for the consent-log personal-data exporter/eraser).
Proposed changes
The consent logger posts to the REST write endpoint with no
X-WP-Nonceheader. For a logged-in visitor the browser still sends the auth cookie, so WordPress core (rest_cookie_check_errors) sees a cookie-authenticated request without a REST nonce and forces it anonymous —wp_set_current_user( 0 ). As a result, every consent row, even for logged-in users, was written withcustomer_id(renameduser_idin #49852)= 0. The POST still returns 200 because the route ispermission_callback => '__return_true', so it failed silently.This is load-bearing for the personal-data exporter/eraser in #49852, which matches consent rows by user ID: with every row stored as
0, it would match nothing for a real logged-in user.Why
user_idwas therefore always0, making per-user export/erasure impossible.How
wp_restnonce intojetpackCookieConsentConfig— only for logged-in visitors. Anonymous visitors deliberately get no nonce: their pages are full-page-cached, and a cached nonce would go stale and make core reject the request withrest_cookie_invalid_nonce(403). Without a nonce, core treats the request as anonymous and storesuser_id = 0, which is correct for them. This keeps anonymous logging — the bulk of the traffic — working unchanged.X-WP-Nonceonly when the nonce is present.What
class-cookie-consent.php— conditionally add awp_restnonce to the localized config whenis_user_logged_in().logger.ts— send theX-WP-Nonceheader when a nonce is present.types.ts— add the optionalnoncefield to the config type.Does this pull request change what data or activity we track or use?
No new data is collected. This fixes attribution of an existing field: consent rows for logged-in users now record the real user ID instead of
0. Anonymous behavior is unchanged.Testing instructions
Environment setup
The cookie-consent code ships inside the Premium Analytics plugin (Composer path dependency), which is the only consumer that boots
Cookie_Consent::init()— registering the consent-log REST routes and enqueuing the logger.window.jetpackCookieConsentConfigis defined in the browser console (this proves the logger is enqueued). The visible banner additionally needs a GDPR region or?preview_cookie_consent=1, but the banner is not required for this test — the steps below POST to the same endpoint the banner uses.Verify
Run both cases on the same site (same IP) and compare the stored user ID. To read rows back as admin:
GET /wp-json/jetpack/v4/cookie-consent/consent-log?per_page=100(needsmanage_privacy_options+ anX-WP-Nonce).window.jetpackCookieConsentConfig.nonceis present. Accept consent (orPOSTto the consent-log endpoint with theX-WP-Nonceheader). Confirm the new row's user ID is your real user ID, not0.noncein the config, thePOSTstill returns200(not403), and the row's user ID is0.Verified live with this branch active (the column is
customer_idon trunk, renameduser_idby #49852):X-WP-Noncesent10