[wp-trac] [WordPress Trac] #40922: Use finer-grained capabilities with `customize_changeset` post type

[WordPress Trac]

#40922: Use finer-grained capabilities with `customize_changeset` post type
----------------------------------------+------------------
 Reporter:  dlh                         |       Owner:
     Type:  enhancement                 |      Status:  new
 Priority:  normal                      |   Milestone:  4.9
Component:  Customize                   |     Version:  4.7
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |     Focuses:
----------------------------------------+------------------

Comment (by dlh):

 [attachment:40922.4.diff] would move the mapping to a `'map_meta_cap'`
 filter, which would ensure it applies when either
 `'(delete|edit|read)_post'` or `'(delete|edit|read)_customize_changeset'`
 is used with `current_user_can()`.

 We discussed this question in our earlier Slack chat, but after some
 further digging, I'm back to having the impression that `current_user_can(
 'edit_post', $changeset_post_id )` is at least technically incorrect
 because it bypasses the post type's `map_meta_cap` setting.

 I'll outline the explanation as I understand it, but I would be eager for
 additional guidance or corrections:

 If a `book` post type sets `'capability_type' => 'book'` and
 `'map_meta_cap' => false`, then the expectation is that `current_user_can(
 $book->cap->edit_post, $post_id )` (or `'edit_book'`) would fall all the
 way to the bottom of the `switch` statement in `map_meta_cap()`:
 https://github.com/WordPress/WordPress/blob/master/wp-
 includes/capabilities.php#L499.

 The logic in the `default` case that can call `map_meta_cap()` again
 wouldn't fire because `'edit_book'` wouldn't be present in the
 `$post_type_meta_caps` global. Developers would have to apply custom
 mapping through the `'map_meta_cap'` filter.

 Core defends against someone calling `current_user_can( 'edit_post',
 $post_id )` on a `book` by checking the post type's `map_meta_cap`
 property before applying the default mapping.

 If it's `false`, then the `$cap` and `$caps` are changed to what they
 would have been with `current_user_can( $book->cap->edit_post )`, and we
 `break` straight to the `'map_meta_cap'` filter:
 https://github.com/WordPress/WordPress/blob/master/wp-
 includes/capabilities.php#L149.

 That defensive logic ensures the expected `$cap` and `$caps` are passed to
 the `'map_meta_cap'` filter, so plugins and themes don't notice the
 difference if someone calls `current_user_can( 'edit_post' )`.

 But it becomes a problem for core in [attachment:40922.3.diff] because the
 `break` would prevent us from reaching the new `case` statements, whereas
 `current_user_can( $changeset->cap->edit_post )` would reach them.

 For core, unlike the `book` example, the default post types (except
 `custom_css`) either use the `'post'` capability type or have
 `'map_meta_cap' => true`, so I don't think there are `case`s in
 `map_meta_cap()` being missed. But that would no longer be true with
 [attachment:40922.3.diff].

 Even if this description is accurate, though, I agree that
 `current_user_can( 'edit_post', $changeset_post_id )` shouldn't break.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/40922#comment:16>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform