cannot update or delete user, term or comment meta via REST API

[tharsheblows]

The capability checks used when updating meta for all object types are currently "edit_post_meta" which doesn't work for users, terms or comments. Adding in eg "edit_user_meta" is straightforward, however this and the other object type capabilities don't exist. (see #38303)

Attached is a patch which could be used if those capabilities did exist, otherwise we'll need to work around them. That's possible, just need to remember to apply_filters(auth_{$object_type}_meta_{$meta_key}); as these can be added in register_meta()

I think it'd make sense to fix it properly now though.

[tharsheblows]

check meta update and delete capabilities using correct object type

[rmccue]

Add meta caps for other object types

[rmccue]

Added a patch that adds in the other relevant meta caps for other object meta, which completes the other half of this.

This could probably be split out into a separate, entirely new function.

[tharsheblows]

This includes a patch to class-wp-rest-comments-controller.php where you couldn't update comment meta if you didn't update the comment.

There are also working unit tests for each object type. I haven't gone through and pruned these, there is definitely a massive amount of redundancy. I'm not sure if you'd rather be redundant or combine these into one file with only test_set_value() for each object type. The meta passes through the object type update_item function on its way to being updated so that should stay in as those update_item functions are all a little different (eg the patch needed in class WP_REST_Comments_Controller)

@boonebgorges I was unsure how to make the test in tests/rest-api/rest-term-metafields.php lovely like these https://core.trac.wordpress.org/changeset/38975 and I apologise for that and the decided lack of frugal gaze in general.

[tharsheblows]

unit tests and patch to comments controller

[boonebgorges]

Thanks for the ping, and sorry for the delay.

The general approach of the map_meta_cap() part of the patch looks good to me.

38412.4.diff​ does the following:

• Adds core (non-REST-API) tests for each of the new caps
• Casts mine frugal gaze on the tests written by @tharsheblows
• Fixes the user-meta-fields tests for multisite, where we have to dance around the fact that you need manage_network_users, but granting super-admin means that the metadata cap check will automatically pass
• Some minor documentation and formatting cleanup

I haven't spent a huge amount of time looking at the endpoint-specific tests. They look good at a glance, but they should have a readthrough by someone more familiar with the pattern of the API tests.

One last note - I'm slightly uneasy about making so deep a change to the capability API at this point in the development cycle. Technically, this is a compat break - people are likely relying on edit_post_meta etc for non-post-related meta checks. Not a good situation to be sure, and something that very much should be fixed, but it should be carefully documented for public consumption. I'm OK with the change, but a signoff from @helen as the release lead would put my mind at ease somewhat.

[jeremyfelt]

It seems like this also fixes #38303. @tharsheblows, is that correct?

[tharsheblows]

@jeremyfelt yes, pretty much! I've just looked back at it - I'm sorry I didn't realise this earlier and whilst I think it's an issue, it might not be.

It's my understanding that the auth_{$object_type}_meta_{$meta_key} filter is, if set, what determines whether a user can update a meta key. If it isn't set, then there's a check to see if it's a protected meta key and then, if it's not a protected meta key, then you can update the meta key if you can update the object type.

(The auth_callback in register_meta is one thing that adds this filter.)

So the check should be:

1. is there a filter on auth_{$object_type}_meta_{$meta_key})?
   • Yes -> do whatever it says
   • No -> proceed
2. is the meta key protected?
   • Yes -> set the capability to edit_{$object_type}_meta
   • No -> set the capability to edit_{$object_type}

It currently doesn't work like this, the filter is applied after the edit_{$object_type} capability is set so you can't allow a user who doesn't have that capability to update meta even if you'd like to do so.

The change would affect this edge case: if someone is using current_user_can( 'edit_post_meta' ) with a filter on auth_{$object_type}_meta_{$meta_key} that returns true but they really don't want the person to update the post meta so are relying on the fact that they don't have the edit_post capability, then that will change and that person will be able to update the post meta. It will only affect post_meta because that's the only filter in there currently.

It's a straightforward fix and I have tests for the auth_callback filter that I can add to the capabilities tests but thought I'd ask about it now before cluttering up with another patch. One thing it does require is setting the auth_callback default to null rather than __return_true in register_meta. I don't think that would be an issue but want to mention.

[rmccue]

In 39179:

Roles/Capabilities: Add meta-caps for comment, term, and user meta.

Additionally, use these meta-caps in the REST API endpoints.

Previously, register_meta()'s auth_callback had no effect for non-post meta. This introduces {add,edit,delete}_{comment,term,user}_meta meta-caps to match the existing post meta capabilities. These are currently only used in the REST API.

Props tharsheblows, boonebgorges.
Fixes #38303, fixes #38412.

[rmccue]

@tharsheblows Re: 11, is this a change that needs to be made in 4.7?

[tharsheblows]

@rmccue Yes, I think so. Otherwise register_meta( 'post', $meta_key, array( 'auth_callback' => 'new_callback' ) ) doesn't work as intended. It'd be best if it did from now when the REST API has just been included in core and people are starting to use it, especially since meta has been separate from the WP-API plugin up until not too long ago.

Attached is patch with an additional unit test.

[tharsheblows]

respect filter on object meta, unit test

[helen]

I reopened this for 15 - who is owning review and further commit here?

[rmccue]

That'd be on me for review. :)

The remaining issue is actually #38303, so I think I was wrong to close that. Going to move over there, since it makes more sense than in this ticket.