WordPress.org

Make WordPress Core

Opened 18 months ago

Last modified 3 months ago

#44094 reviewing enhancement

Hook for WP_User data hydration to enable strong data security

Reported by: yguez Owned by: SergeyBiryukov
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch 2nd-opinion
Focuses: Cc:
PR Number:

Description

Hi there! I'm one of the founders of Crypteron, a data-security platform for developers. I'm also active in the WordPress community, an organizer of the [San Diego Advanced WordPress Meetup](https://www.meetup.com/Advanced-WordPress), an admin of the [Advanced WordPress Facebook Group](https://www.facebook.com/groups/advancedwp) (with over 30,000 members) and a past speaker and organizer at WordCamp San Diego.

For the past 8 months I've been working on a free plugin called EncryptWP that brings military-grade data-security to WordPress. It automatically encrypts and decrypts sensitive user data such as names, email addresses and physical addresses and even supports secure, searchable encryption. It's been a labor of love and is available for beta testing at https://github.com/crypteron/encryptwp.

The reason I'm posting a ticket here is that I've had to resort to a non-ideal approach to automatic decryption of native (non-meta) user fields within WordPress. Ideally I would decrypt sensitive user data right after it's loaded from the database as it is hydrated into the WP_User object. Unfortunately, no hook is fired during this process. I strongly believe that one should be.

Instead, I decrypt native user fields using a combination of the filters: edit_user_{{$field}}, {{user_{{$field}}, the_author, and wp_dropdown_users.

This approach works but has some major downsides:

  • The edit_user_{{$field}} and user_{{$field}} filters are only fired when $user->filter is truthy (See line 308 of class-wp-user.php). I do my best to ensure that this value is set, but plenty of plugins interact with WP_User objects without setting their filter property. This results in some plugins outputing encrypted text for fields such as display_name.
  • The edit_user_{{$field}} and user_{{$field}} filters are fired in the sanitize_user_field method which, fundamentally, is not a logical place for this sort of operation.
  • Some native WordPress code bypasses the sanitize_user_field method so I've had to add additional filters for the_author and wp_dropdown_users (using a RegEx!)
  • Rather than decrypting all sensitive data once, I have to decrypt it every time it is fetched which is inefficient.

Despite all of this, EncryptWP works very well. I think the plugin can be a game-changer for WordPress, making HIPAA compliant WordPress sites possible not to mention the new world of GDPR compliance. We have an ambitious, technical road map for the plugin including extending it to handle user-defined meta keys, encrypting wp_post and wp_options data, key management, and more. But I'd really love to get it 100% compatible and performant with all plugins and, in order to do that, we really need a WP_User hydration filter.

Such a filter could benefit many other plugins as well and is guaranteed to be backward compatible with all other sites since it would not change existing behavior. I would envision that the filter could be added within the [WP_User->init](https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-user.php#L170) method:

<?php
public function init( $data, $site_id = '' ) {
    $this->data = apply_filters('init_user_data', $data, $site_id);
    $this->ID   = (int) $data->ID;
    $this->for_site( $site_id );
}

Please let me know your thoughts on adding this new filter. One line of code can make all the difference!

Attachments (4)

44094.patch (522 bytes) - added by macbookandrew 18 months ago.
patch as described by @yguez
44094.2.patch (677 bytes) - added by macbookandrew 18 months ago.
patch as described by @yguez with inline documentation
44094.3.diff (2.0 KB) - added by donmhico 3 months ago.
Added unit test.
44094.4.diff (2.3 KB) - added by donmhico 3 months ago.
Corrected @since to adhere to WPCS.

Download all attachments as: .zip

Change History (18)

This ticket was mentioned in Slack in #gdpr-compliance by yaronguez. View the logs.


18 months ago

@macbookandrew
18 months ago

patch as described by @yguez

#2 @macbookandrew
18 months ago

  • Keywords has-patch added

This ticket was mentioned in Slack in #core by yaronguez. View the logs.


18 months ago

#4 @kraftbj
18 months ago

Thanks for all of the detail @yguez. I don't have an opinion about the filter's name—others can weight in on that—but one thing that will be needed before committing is inline documetation: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#4-hooks-actions-and-filters

@macbookandrew
18 months ago

patch as described by @yguez with inline documentation

#5 @yguez
18 months ago

Thanks, @macbookandrew !

This ticket was mentioned in Slack in #core by yaronguez. View the logs.


18 months ago

#7 @SergeyBiryukov
18 months ago

  • Milestone changed from Awaiting Review to 5.0

This ticket was mentioned in Slack in #core by yaronguez. View the logs.


14 months ago

#9 @SergeyBiryukov
14 months ago

  • Milestone changed from 5.0 to 4.9.9
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#10 @pento
13 months ago

  • Milestone changed from 4.9.9 to 5.1

#11 @pento
10 months ago

  • Milestone changed from 5.1 to 5.2

#12 @desrosj
8 months ago

  • Milestone changed from 5.2 to 5.3

This ticket has not received any attention during the 5.2 cycle. With beta 1 tomorrow, going to punt this to 5.3.

@donmhico
3 months ago

Added unit test.

@donmhico
3 months ago

Corrected @since to adhere to WPCS.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


3 months ago

#14 @johnbillion
3 months ago

  • Keywords 2nd-opinion added
  • Milestone changed from 5.3 to Awaiting Review

This change looks fine but I think the WordPress project needs to consider a unified approach to filtering the hydration of all object types, not just WP_User.

Does this pattern make sense to apply to posts, comments, and terms too? If so, this should be tackled in a holistic manner.

Does this introduce the ability for a WP_User object to contain completely the wrong data compared to what's expected? If so, the implications need consideration.

Note: See TracTickets for help on using tickets.