WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 3 months ago

#44581 reviewing enhancement

users without 'edit_posts' capability never get informed that their uploads succeed

Reported by: pbiron Owned by: joemcgill
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-screenshots 2nd-opinion needs-testing
Focuses: Cc:
PR Number:

Description

Steps to reproduce:

  1. create a user with upload_files capability but not edit_posts capability
  2. login as that user
  3. upload a media file
  4. notice that the status of the upload stays at Crunching...
  5. notice also that the filename/post_title eventually ends up blank

Expected behavior:

  1. user gets some sort of "Success" notification (analogous to the Edit link that users with edit_posts capability get)

I discovered this problem on a site with users whose role is basically Subscriber but with upload_files capability. The problem is that /wp-admin/async-upload.php contains:

<?php
if ( ! current_user_can( 'edit_post', $id ) )
        wp_die( __( 'Sorry, you are not allowed to edit this item.' ) );

My current workaround is to hook into user_has_cap and add edit_posts IFF edit_post is being checked from async-upload.php and its an attachment whose post_author is the current user. This workaround is not ideal (because I don't want these users to be able to edit the attachment), but at least they know the upload succeeded.

Attachments (5)

crunching.png (7.2 KB) - added by pbiron 17 months ago.
this is what users without 'edit_posts' capability see after their upload has succeeded
done-crunching.png (5.5 KB) - added by pbiron 17 months ago.
this is what users with 'edit_posts' capability see after their upload has succeeded
44581.diff (1.6 KB) - added by pbiron 17 months ago.
report "Success" for users without 'edit_post' capability
success.png (5.9 KB) - added by pbiron 17 months ago.
this is what users without 'edit_posts' capability see after their upload has succeeded after 44581.diff is applied
44581.2.diff (1.3 KB) - added by mikeschroder 3 months ago.
Refresh of 44581.diff.

Download all attachments as: .zip

Change History (20)

@pbiron
17 months ago

this is what users without 'edit_posts' capability see after their upload has succeeded

@pbiron
17 months ago

this is what users with 'edit_posts' capability see after their upload has succeeded

@pbiron
17 months ago

report "Success" for users without 'edit_post' capability

@pbiron
17 months ago

this is what users without 'edit_posts' capability see after their upload has succeeded after 44581.diff is applied

#1 @pbiron
17 months ago

  • Keywords has-patch has-screenshots added

This ticket was mentioned in Slack in #core-media by pbiron. View the logs.


17 months ago

#3 @mikeschroder
17 months ago

  • Type changed from defect (bug) to enhancement

This ticket was mentioned in Slack in #core-media by pbiron. View the logs.


16 months ago

#5 follow-up: @joemcgill
16 months ago

  • Milestone changed from Awaiting Review to 4.9.9
  • Owner set to joemcgill
  • Status changed from new to reviewing

I want to take a hard look at the capabilities changes to make sure there aren't any issues with this change, but seems sensible.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


15 months ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


15 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


15 months ago

#10 @pento
14 months ago

  • Milestone changed from 4.9.9 to Future Release

This ticket was mentioned in Slack in #core-media by pbiron. View the logs.


4 months ago

#12 in reply to: ↑ 5 @pbiron
4 months ago

Replying to joemcgill:

I want to take a hard look at the capabilities changes to make sure there aren't any issues with this change, but seems sensible.

Just to be clear, the patch does not change any caps...it simply removes the wp_die() when the user doesn't have edit_posts and instead outputs something different in that case.

This ticket was mentioned in Slack in #core-media by pbiron. View the logs.


4 months ago

#14 @joemcgill
4 months ago

  • Milestone changed from Future Release to 5.3

@mikeschroder
3 months ago

Refresh of 44581.diff.

#15 @mikeschroder
3 months ago

  • Keywords 2nd-opinion needs-testing added
  • Milestone changed from 5.3 to Future Release

Refreshed the patch in 44581.2.diff.

As we're right before beta for 5.3 and this hasn't seen movement, moving this to Future Release.

Am interested in seeing this fixed, but would appreciate someone double-checking that nothing unintentional happens when removing the wp_die() noted.

Last edited 3 months ago by mikeschroder (previous) (diff)
Note: See TracTickets for help on using tickets.