WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 7 weeks ago

Last modified 4 weeks ago

#44581 closed enhancement (fixed)

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

Reported by: pbiron Owned by: joemcgill
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-screenshots commit
Focuses: Cc:

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 2 years ago.
this is what users without 'edit_posts' capability see after their upload has succeeded
done-crunching.png (5.5 KB) - added by pbiron 2 years ago.
this is what users with 'edit_posts' capability see after their upload has succeeded
44581.diff (1.6 KB) - added by pbiron 2 years ago.
report "Success" for users without 'edit_post' capability
success.png (5.9 KB) - added by pbiron 2 years 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 11 months ago.
Refresh of 44581.diff.

Download all attachments as: .zip

Change History (29)

@pbiron
2 years ago

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

@pbiron
2 years ago

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

@pbiron
2 years ago

report "Success" for users without 'edit_post' capability

@pbiron
2 years ago

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

#1 @pbiron
2 years ago

  • Keywords has-patch has-screenshots added

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


2 years ago

#3 @mikeschroder
2 years ago

  • Type changed from defect (bug) to enhancement

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


2 years ago

#5 follow-up: @joemcgill
2 years 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.

#6 @SergeyBiryukov
2 years ago

Previously: #8234

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


2 years ago

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


2 years ago

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


23 months ago

#10 @pento
22 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.


12 months ago

#12 in reply to: ↑ 5 @pbiron
12 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.


12 months ago

#14 @joemcgill
12 months ago

  • Milestone changed from Future Release to 5.3

@mikeschroder
11 months ago

Refresh of 44581.diff.

#15 @mikeschroder
11 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 11 months ago by mikeschroder (previous) (diff)

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


6 months ago

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


6 months ago

#19 @pbiron
6 months ago

  • Milestone changed from Future Release to 5.5

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


8 weeks ago

#21 @rebasaurus
8 weeks ago

  • Keywords 2nd-opinion needs-testing removed

Thanks for the patch @pbiron. Works as expected on a clean install!

#22 @pbiron
8 weeks ago

  • Keywords commit added

Thanx for testing @rebasaurus!

#23 @whyisjake
7 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 48128:

Media: Ensure that uploaded images get a success notification by users with upload_files capability.

There was an early wp_die that was preventing the success notification from being sent in the upload process.

Fixes #44581.

Props pbiron, mikeschroder, joemcgill, rebasaurus, whyisjake.

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


4 weeks ago

Note: See TracTickets for help on using tickets.