WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 15 months ago

Last modified 15 months ago

#22415 closed defect (bug) (wontfix)

media-upload.php should check whether current user can edit a particular post type

Reported by: danielbachhuber Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Role/Capability Keywords:
Focuses: Cc:

Description

In /wp-admin/media-upload.php, there are a few checks for:

if ( ! empty( $_REQUEST['post_id'] ) && ! current_user_can( 'edit_post' , $_REQUEST['post_id'] ) )
     wp_die( __( 'Cheatin’ uh?' ) );

These should instead be (something like):

$obj = get_post_type_object( get_post_type( $_REQUEST['ID'] ) );
if ( ! current_user_can( $obj->cap->edit_post, $_REQUEST['ID' ) )
     wp_die( __( 'Cheatin’ uh?' ) );

Although $obj->cap->edit_post can map to 'edit_post', sometimes map_meta_cap is bypassed in which case uploading media doesn't work.

Change History (8)

comment:1 nacin17 months ago

  • Component changed from General to Role/Capability
  • Keywords reporter-feedback added

'edit_post' maps internally to cap->edit_post for the post type of the post being passed. Could you give an example of where this would fail?

If custom post types supported meta capabilities in 3.0, we probably wouldn't even have meta capabilities on the cap object. But in 3.1, any inconsistencies were corrected and they work the same.

comment:2 danielbachhuber17 months ago

In Co-Authors Plus, we're filtering 'user_has_cap' to give co-authors permission to edit a post. Here's the relevant, now functional, code:

https://github.com/Automattic/Co-Authors-Plus/blob/da2844c2f7bd401c5ca9066127235ff81a63419b/co-authors-plus.php#L1063

A user reported they created custom roles for editing pages and users with the role weren't able to upload attachments:

https://github.com/Automattic/Co-Authors-Plus/issues/61

The Co-Authors Plus filter kicks in based on the original cap check passed to current_user_can(). In this particular instance, the filter was failing this check:

https://github.com/Automattic/Co-Authors-Plus/blob/83b8fd293bcbfe75b2ca47724c88bcf9e4d78bf1/co-authors-plus.php#L1091

because $args[0] was 'edit_post' and $post_type_object->cap->edit_post was 'edit_page'.

I've since found a somewhat reliable workaround, but I think the semantics identified are off and should be fixed.

comment:3 follow-up: nacin17 months ago

It would make more sense to leverage map_meta_cap in the plugin, since that's how we control permissions for authors.

If this code ran through map_meta_cap(), everything should work just fine. user_has_cap is a fairly powerful hook as well, but when you're dealing with meta caps, you really need to go through map_meta_cap for the bulk of it.

In short, I don't see a bug here. For some unit tests on how edit_post and $post_type->cap->edit_post are the same, see http://unit-tests.trac.wordpress.org/browser/trunk/tests/user/mapMetaCap.php.

comment:4 follow-up: TXC17 months ago

Recently I faced with this issue too.
This part of code fails when you add post_id for custom forms for uploading images.

It seems it's typing error at the row №125
Instead of:

if ( ! empty( $_REQUEST['post_id'] ) && ! current_user_can( 'edit_post' , $_REQUEST['post_id'] )

it should be:

if ( ! empty( $_REQUEST['post_id'] ) && ! current_user_can( 'edit_posts' , $_REQUEST['post_id'] )

Because WP haven't got the edit_post capability, but only edit_posts.

Last edited 17 months ago by TXC (previous) (diff)

comment:5 in reply to: ↑ 4 SergeyBiryukov17 months ago

Replying to TXC:

Because WP haven't got the edit_post capability, but only edit_posts.

edit_post is a meta capability:
http://core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/capabilities.php#L1009

comment:6 in reply to: ↑ 3 westi17 months ago

Replying to nacin:

It would make more sense to leverage map_meta_cap in the plugin, since that's how we control permissions for authors.

If this code ran through map_meta_cap(), everything should work just fine. user_has_cap is a fairly powerful hook as well, but when you're dealing with meta caps, you really need to go through map_meta_cap for the bulk of it.

In short, I don't see a bug here. For some unit tests on how edit_post and $post_type->cap->edit_post are the same, see http://unit-tests.trac.wordpress.org/browser/trunk/tests/user/mapMetaCap.php.

+1

comment:7 danielbachhuber15 months ago

  • Keywords reporter-feedback removed
  • Resolution set to wontfix
  • Status changed from new to closed

I'm going to rewrite the permissions code in CAP to use map_meta_cap: https://github.com/Automattic/Co-Authors-Plus/issues/103

Thanks for the feedback.

comment:8 helen15 months ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.