WP 5.3: EDD protected post meta prevents post meta from other plugins from saving

[nickcernis]

Bug Report

Expected behavior

When EDD is active, it should be possible to edit and save other custom post meta via the WP JS API.

Actual behavior

EDD protected custom post meta currently prevents updates made to other post meta via the WP JS API due to the way EDD custom fields have been registered.

When updating non-EDD post meta via the WP JS API and saving changes, the editor reports "Sorry, you are not allowed to edit the _edd_bundled_products custom field”:

There is an additional report of this here: https://wordpress.org/support/topic/possible-conflict-with-easy-digital-downloads-plugins/

Steps to reproduce the behavior

1. Install WP 5.3-RC1 via the WP beta tester plugin by choosing “bleeding edge nightlies” at Tools → Beta Testing.
2. Activate the Twenty Twenty theme (these steps will work with any theme, though).
3. Add this PHP code to the Twenty Twenty theme's functions.php file to add custom post meta exposed to the REST API:

add_action( 'init', 'custom_register_post_meta' );

function custom_register_post_meta() {
    $args = [
        'auth_callback' => '__return_true',
        'type'          => 'boolean',
        'single'        => true,
        'show_in_rest'  => true,
    ];

    register_meta( 'post', '_custom_my_post_meta', $args );
}

4. Edit a post or page. Make a change to the post content and click “Publish” or “Update” to confirm that changes work as expected.
5. In the browser console, display the post meta exposed to the REST API by pasting this command and typing enter: wp.data.select( 'core/editor' ).getEditedPostAttribute( 'meta' );

You should see an object that includes your new _custom_my_post_meta and a subset of EDD post meta:

{edd_variable_prices: Array(0), _edd_bundled_products: Array(0), _edd_button_behavior: Array(0), _custom_my_post_meta: false}

6. Update _custom_my_post_meta to 'true' with this command (this persists the data to the Redux store, but not the database):

wp.data.dispatch( 'core/editor' ).editPost( { meta: { _custom_my_post_meta :  true } } );

This is just a simplified test case. It emulates a plugin or theme updating meta via the WP API, without having to install a plugin or theme that uses custom post meta and Gutenberg.

7. Click Update in the post editor to save your post meta changes to the database.

You'll see “Updating failed. Error message: Sorry, you are not allowed to edit the _edd_bundled_products custom field.”.

Information

• PHP Version: 7.2.23
• EDD Version: 2.9.18
• WordPress Version: 5.3-RC1-46558

One way to solve this is add 'auth_callback' => '__return_true', to all EDD protected post meta (meta starting with an underscore) that's exposed to the REST API (via 'show_in_rest' => true) in includes/class-edd-register-meta.php.

For example:

register_meta(
            'post',
            '_edd_button_behavior',
            array(
                'sanitize_callback' => 'sanitize_text_field',
+               'auth_callback'     => '__return_true',
                'type'              => 'string',
                'description'       => __( "Defines how this product's 'Purchase' button should behave, either add to cart or buy now", 'easy-digital-downloads' ),
                'show_in_rest'      => true,
            )
        );

After making that change to the other protected fields exposed to the REST API in includes/class-edd-register-meta.php (_edd_bundled_products and _edd_default_price_id), I'm able to repeat the steps above without seeing that error. (I'm not familiar enough with EDD to assess the impact on the rest of the codebase, though, which is why I haven't sent a PR for this.)

This seems to work because of the expectation from WordPress that protected post meta fields have an auth_callback key so that they become updateable via the REST API:

Note: If the meta key name starts with an underscore WordPress considers it a protected field. Editing this field requires passing a permission check, which is set as the auth_callback in the register_post_meta function.

From https://developer.wordpress.org/block-editor/tutorials/metabox/meta-block-2-register-meta/.

[cklosowski]

Show in rest cannot simply be True anymore with 5.3, as it throws errors about being incorrectly called.

[cklosowski]

I think we can fix this just by using the subtype argument for register_meta. I'll give that a shot first.

[mintplugins]

Related: https://github.com/easydigitaldownloads/easy-digital-downloads/issues/7422

[robincornett]

I did a quick check locally and adding the object_subtype argument to all of the register_meta calls in EDD does seem to resolve the issue for me.

[cklosowski]

Thanks! I'll be online shortly and commit up a fix

On Tue, Oct 22, 2019, 08:11 Robin Cornett notifications@github.com wrote:

I did a quick check locally and adding the object_subtype argument to all of the register_meta calls in EDD does seem to resolve the issue for me.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/easydigitaldownloads/easy-digital-downloads/issues/7425?email_source=notifications&email_token=AAKTMLGK3LLERNVJVXLWZKDQP4JZRA5CNFSM4JDPTEFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB6DGHI#issuecomment-545010461, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKTMLGGJJ5TA46B5FMRUVTQP4JZRANCNFSM4JDPTEFA .

[mintplugins]

I'm trying to understand this one. Why would calling wp.data.dispatch( 'core/editor' ).editPost( { meta: { _custom_my_post_meta : true } } ); be affected by any register_meta call unless that register_meta call was for _custom_my_post_meta? I would think these 2 pieces of meta should be completely unrelated.

[cklosowski]

@robincornett Did you test with a specific plugin or did you use the above example? I know the above 'simulates' someone using this, but I'd like to test it with a real plugin going through the steps as well.

[nickcernis]

@cklosowski Do you have a copy of Genesis 3.1+? (If not, feel free to email me for a test copy: nick [at] cern.is. The issue is also reproducible in WP 5.3 with Genesis 3.1+ and EDD active by:

1. Activating any Genesis child theme, or Genesis itself.
2. Visiting any page or post.
3. Clicking the Genesis icon in the post editor sidebar.
4. Toggling any of the Genesis checkboxes on, updating the post, then toggling them off and attempting to resave.

@mintplugins

Why would calling wp.data.dispatch( 'core/editor' ).editPost( { meta: { _custom_my_post_meta : true } } ); be affected by any register_meta call unless that register_meta call was for _custom_my_post_meta? I would think these 2 pieces of meta should be completely unrelated.

If the meta changes in the Redux store, that triggers an update of the meta when the post is saved. WP then iterates over all REST-visible meta, and the REST API triggers that “not allowed to edit” failure for the EDD protected keys because of the missing 'auth_callback' that tells WP under what condition protected fields are updateable.

Using object_subtype to restrict the fields only to the downloads post type should solve the issue for regular posts and pages, because EDD post meta would then no longer be visible there. (I think there may still be an issue if plugin devs call wp.data.dispatch( 'core/editor' ).editPost( { meta: {… on an EDD page where EDD meta is available alongside custom meta, but that may be less of a concern.)

[robincornett]

I'm using Genesis 3.1 and trying to edit any Genesis post meta, which is now in a sidebar in the block editor, results in the EDD errors.

We are discussing it in the Genesis repository, and I noted that I do not experience the same errors with Yoast, which also registers a sidebar, and Nick said this:

(Yoast doesn't call .getEditedPostAttribute( 'meta' ), which is why the issue isn't present there.)

[cklosowski]

Ah, yeah I tried with Yoast as well thinking they likely did :).

I've got Genesis 3.1+ so i'll play around with that locally and see what I can come up with as a combination.

[mintplugins]

@nickcernis

If the meta changes in the Redux store, that triggers an update of the meta when the post is saved. WP then iterates over all REST-visible meta, and the REST API triggers that “not allowed to edit” failure for the EDD protected keys

I'm wondering why the EDD protected keys would be changing in the Redux store. If those keys are not being targeted, why would they be getting updated, which is what seems to be triggering this error?

I'm curious because at first glance it sounds like a problem higher up the chain, in that changing one piece of meta should not be dependant on the settings for another unrelated piece of meta.

Note that I say all of this with a very low-level understanding of this code. I am going to dig deeper on the code behind all of this asap.

[mintplugins]

@nickcernis Sidenote, but I wasn't able to replicate this with the initial instructions.

The update of the meta worked as well, as after a refresh of the page, I get the updated value:

[mintplugins]

@nickcernis I did that as well (prior to my screenshot above). I did it all again with a fresh post. I am still not replicating though.

[nickcernis]

I'm wondering why the EDD protected keys would be changing in the Redux store. If those keys are not being targeted, why would they be getting updated, which is what seems to be triggering this error?

Yes, it does seem suboptimal that post meta values get processed as updates even though they don't change.

I haven't looked into it in detail, but it seems that WP applies the update to each field within the post meta if any value in the post meta changes.

After a quick look, it seems that:

1. When WordPress saves the post data, update_item is called: https://github.com/WordPress/WordPress/blob/ed11103f39e2a5736caf09eb9d76bb6711702c04/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L675

2. update_item eventually triggers $this->meta->update_value.

3. update_value loops through every registered meta value.

And eventually routes flow to update_meta_value and update_multi_meta_value depending on the field type. Those functions include a current_user_can check that generate the error message in this report when they fail:

https://github.com/WordPress/WordPress/blob/cfec48cc31c35475e081483d99ee51f9ed98bf66/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php#L258

https://github.com/WordPress/WordPress/blob/cfec48cc31c35475e081483d99ee51f9ed98bf66/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php#L342

It's possible update_value could be optimized to check if the sent value has changed from the stored one to avoid this issue.

[nickcernis]

I am still not replicating though.

That's odd. I can consistently replicate this with the steps above with EDD active and no fixes applied to includes/class-edd-register-meta.php. Happy to provide a copy of Genesis if you'd like to try the other repro steps, though.

[mintplugins]

@nickcernis Nice find. I would agree that the better solution would be that update_value only updates if needed. Not only to fix this, but also to prevent unnecessary calls.

Happy to try with Genesis. phil [at] easydigitaldownloads.com

[mintplugins]

Huh, yeah I am not having any of those problems with Genesis saving either

[nickcernis]

Thanks for testing, Phil, and that's odd. We have a few people who can reproduce this, and I can still do so with the latest EDD/Genesis/WP5.3-RC. Have you ruled out other factors in all the usual ways (disabled other plugins, tried a different WP environment)?

[cklosowski]

@nickcernis @mintplugins I've let the core team know about the discussion that happened above related to trying to save meta keys that were not altered. They said it's something they can try and look at for 5.4.

[cklosowski]

Just a heads up @mintplugins it's likely that whatever solution we come up with here, we'll have to have a deadline at a minimum of November 12th, as that's the scheduled release date for 5.3.

[mintplugins]

@nickcernis I just spun up a brand new testing site and I am replicating there now.

[cklosowski]

I've also replicated this...however I'm struggling to find a native way to replicate this on a Download post edit screen. We need a way to include these types of Genesis checkboxes on the post edit screen for downloads. Is there a way to do this @nickcernis ? It only shows on normal post types for me, not custom ones.

[robincornett]

Is EDD going to switch to the block editor for downloads? I believe this is only an issue in the block editor, not the classic editor--and if EDD does change over, presumably a user editing a download has permission to modify the EDD custom fields.

[cklosowski]

@robincornett As far as I can see in this 5.3 RC...Downloads are forced to be in the new editor:

I don't have our 'blocks' plugin activated or Gutenberg enabled as a plugin...I can't find anything in the 5.3 field notes that says that the block editor is now forced on custom post types...

[robincornett]

Oh that is super weird. I'm not modifying downloads and I still see them in the classic editor in 5.3, Gutenberg is not active...only plugins active are EDD and Query Monitor.

[cklosowski]

@robincornett Weird, I'll have to dig in and see if I've got something setup somewhere I don't remember doing.

[cklosowski]

Whoops:

function eddwp_add_rest_for_dls( $download_args ) {
    $download_args['show_in_rest'] = true;
    $download_args['rest_base'] = 'downloads';

    return $download_args;
}
add_filter( 'edd_download_post_type_args', 'eddwp_add_rest_for_dls', 10, 1 );

gotta love an MU Plugin where you dump all your testing stuff...

[robincornett]

Ha! I have the same kind of MU running too--I was going to check and see if I'd done something. With the editor changes in 5.3, I'm ready to start using the block editor on CPTs a lot more widely; I could be ready to use it with EDD, I think.

[mintplugins]

This is working for me here on branch issue/7425 with the object_subtype additions from @cklosowski when testing with Genesis. This restricts those metas to the downloads post type only, which are also working when I update those metas for an EDD product.

This doesn't solve the deeper problem, which is something wp core will have to tackle, but this should at least solve the issue for posts and pages and Genesis for the 5.3 release.

[mintplugins]

I also opened a ticket on trac relating to this here: https://core.trac.wordpress.org/ticket/48426

[nickcernis]

Thank you very much for digging into this and for the fix! It's working for me to restrict those meta fields to the relevant types too.

[mintplugins]

Thanks for your work on this @nickcernis and @robincornett. I'm going to make sure this goes out prior to WordPress 5.3.

[dashkevych]

I was able to reproduce this issue when adding support for custom-fields in Download post type. The reason of adding this support is because official WP documentation says so in order to be able to save meta fields via PluginDocumentSettingPanel.