Make WordPress Core

Opened 7 years ago

Closed 3 months ago

#44940 closed defect (bug) (fixed)

Empty confirm_key property in WP_User_Request when hooking in the user_request_action_email_content

Reported by: dingo_d's profile dingo_d Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 6.9 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: dev-feedback 2nd-opinion has-patch
Focuses: Cc:

Description

I need to change the look (and the url) of the link for the request personal data deletion/export, so I hooked to user_request_action_email_content filter, and inspected the $email_data so that I can get the request ID and the confirm_key in my email.

But upon inspecting the data provided I can see the confirmation key in the confirm_url key of the $email_data, but the field in the object is empty.

This is intentional or a bug?

Array
(
    [request] => WP_User_Request Object
        (
            [ID] => 334
            [user_id] => 20
            [email] => my.email.[at]example.com
            [action_name] => export_personal_data
            [status] => request-pending
            [created_timestamp] => 1536847994
            [modified_timestamp] => 1536847994
            [confirmed_timestamp] => 0
            [completed_timestamp] => 0
            [request_data] => Array
                (
                )

            [confirm_key] => 
        )

    [email] => my.email.[at]example.com
    [description] => Export Personal Data
    [confirm_url] => https://my-site.com/wp-login.php?action=confirmaction&request_id=334&confirm_key=wjjeDD3mx5J02U51F7Zt
    [sitename] => My Cool site
    [siteurl] => https://my-site.com/
)

Attachments (2)

44940-non-breaking.diff (661 bytes) - added by johnjamesjacoby 6 months ago.
44940-breaking.diff (2.2 KB) - added by johnjamesjacoby 6 months ago.

Download all attachments as: .zip

Change History (11)

#1 @birgire
7 years ago

  • Severity changed from major to normal
  • Version changed from 4.9.8 to 4.9.6

Thanks for the ticket @dingo_bastard

Having the confirm key available within these filters seems useful.

Within wp_send_user_request( $request_id ) the request is fetched by request ID with wp_get_user_request_data( $request_id ).

At that moment the confirm key has not been generated for the request.

Then the confirm key is generated by calling wp_generate_user_request_key( $request_id ).

That means the request is updated and stores the confirm key in the post_password field.

But then the request object, in the $email_data, becomes "dirty" (not refreshed) so it's still with an empty confirm key.

At first glance it seems that possible fixes could be to:

  • Re-fetch the request object into the $email_data.
  • Only update the confirm_key attribute, of the request object into the $email_data. But there's currently no set method in WP_User_Request.
  • Set the confirm key explicitly into $email_data['confirm_key'].
  • ...

after the confirm key is generated.

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


7 years ago

#3 @dd32
7 years ago

  • Reporter changed from dingo_bastard to dingo_d

#4 @garrett-eclipse
5 years ago

  • Focuses privacy removed

Dropping privacy focus as it's already in the Privacy component.

#5 @johnjamesjacoby
6 months ago

I see what's happening.

What's needed here is to decide whether a new confirm_key (aka post_password) needs to be generated every time wp_send_user_request() is called, or if it would be acceptable to generate it a single time when wp_create_user_request() is called.

Currently it happens on every (re)send, which is nice because it tumbles the key and invalidates links in previous emails, but that definitely makes the surrounding code somewhat weird like this ticket has uncovered.

I think it is mostly safe to assume that tumbling the key was intentional during the 4.9 release, and I agree that to maintain this behavior will require one of the approaches outlined above by @birgire.

I'll attach 2 patches that address this ticket:

  1. Breaking: Only generate on create
  2. Non-breaking: Regenerating on send

#6 @johnjamesjacoby
6 months ago

  • Milestone changed from Awaiting Review to 6.9
  • Owner set to johnjamesjacoby
  • Status changed from new to assigned

cc @garrett-eclipse for a consult as a Privacy component maintainer.

This ticket was mentioned in PR #9617 on WordPress/wordpress-develop by @sheldorofazeroth.


6 months ago
#7

  • Keywords has-patch added

Trac Link: https://core.trac.wordpress.org/ticket/44940

Problem

When using the user_request_action_email_content filter to customize data request emails, the confirm_key property in the WP_User_Request object is empty, making it impossible to create custom confirmation URLs while maintaining the security flow.

Root Cause

WordPress stores the confirmation key in a hashed format in the database for security reasons. When the WP_User_Request object is created, it loads this hashed value into the confirm_key property. However, the confirmation URL in the email uses a plain-text key generated by wp_generate_user_request_key(). This creates a discrepancy where the URL contains the usable key, but the object property contains only the hashed version.

Solution

The implemented solution adds a filter that:
Extracts the plain text confirmation key from the confirm_url parameter
Adds it to the request object as a new property called plain_confirm_key
Makes this value available to any code using the user_request_action_email_content filter
This approach maintains WordPress's security model while providing developers with the necessary information to create custom confirmation flows.

Expected Behavior After Fix

After applying this fix, developers can:
Access the plain text confirmation key via $email_datarequest?->plain_confirm_key
Create custom confirmation URLs with their own styling and routing
Maintain the security of the confirmation process
Implement custom email templates with branded styling and links

@johnjamesjacoby commented on PR #9617:


4 months ago
#8

This PR appears to be spam.

The code changes do not match the description in any way.

#9 @johnjamesjacoby
3 months ago

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

In 61137:

Privacy: Set $request->confirm_key earlier so it can be reused in 2 places.

This commit updates the wp_send_user_request() function so that the updated confirmation key of the request is available to both the confirmation URL and the subsequent filters (specifically user_request_action_email_content).

This maintains the existing behavior of generating a new key just-in-time before every user request email is sent.

Props birgire, dingo_d, garrett-eclipse, johnjamesjacoby.

Fixes #44940.

Note: See TracTickets for help on using tickets.