Make WordPress Core

Opened 7 years ago

Last modified 7 years ago

#40340 reopened defect (bug)

"Attach to existing content" modal shows posts and pages of other users

Reported by: menakas's profile menakas Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Role/Capability Keywords: has-screenshots needs-patch needs-unit-tests
Focuses: Cc:

Description

From the Media library, a user can attach media (through the Attach link) that were uploaded by himself only. The first image shows the Attach links appearing for certain media only.

In the same way, the "Attach to existing content" should also list only those posts/pages for which the user has the permissions to attach.

Instead it lists all posts (including private) and pages of all users, and then, upon selection, gives the message "Sorry, you are not allowed to edit this post."

Attachments (2)

attach1.png (46.3 KB) - added by menakas 7 years ago.
Attach link available for media for which user has permissions - Correct behaviour
attach2.png (25.6 KB) - added by menakas 7 years ago.
All posts (including private) and pages for which user does not have edit permissions listed - Expected different behaviour

Download all attachments as: .zip

Change History (8)

@menakas
7 years ago

Attach link available for media for which user has permissions - Correct behaviour

@menakas
7 years ago

All posts (including private) and pages for which user does not have edit permissions listed - Expected different behaviour

#1 @menakas
7 years ago

  • Focuses ui added
  • Keywords has-screenshots needs-patch added

#2 @subrataemfluence
7 years ago

  • Keywords close added; needs-patch removed
  • Resolution set to maybelater
  • Status changed from new to closed

To my understanding the current setting is doing good.

If posts/user to be filtered out in the popup then there would be another check to be added - like Admin has the power to do everything, a Moderator have different privilege etc. meaning which posts to be listed based on current user's role would be an additional burden and to achieve this a series of if...else statements will have to be incorporated which I believe is not very necessary here.

#3 @swissspidy
7 years ago

  • Keywords close removed
  • Resolution maybelater deleted
  • Status changed from closed to reopened

meaning which posts to be listed based on current user's role would be an additional burden and to achieve this a series of if...else statements will have to be incorporated which I believe is not very necessary here.

Not really. WordPress has a robust capabilities API. This would be as simple as current_user_can( 'edit_post', $post_id ) for example.

#4 @subrataemfluence
7 years ago

Yes, that's true. But when listing posts in popup why should I loop through all posts to find out what posts the 'current_user_can' edit? Don't you think it creates overhead if there are say 5000 posts and WP has to go though all of them in order to just decide whether or not to place it in the popup?

This should be checked when the action is being performed, i.e. attaching a media to a post which the current_user_can NOT edit (as per the issue reported here), and I believe this is how it is working now.

The issue as reported here is to filter out only "user's own posts" in the popup when somebody clicks on Attach link of a specific media. So if that has to be achieved current_user_can check needs to check every single post available in the database. I believe this is an extra burden. I may be wrong!

#5 @menakas
7 years ago

Firstly, at the very least, I would expect the private posts of other users to not be displayed. As of now, other users' private posts are also displayed.

Secondly, should we let implementation concerns affect the interface design/behavior?

Thirdly, whether user has permissions to attach an image/media or not has been checked for each image; so why not check for posts too?

Fourth,
a) an editor can attach an image to any public post, so pick all posts and pages that are not private.

b) an author can attach an image to his posts only, so we can use the query arg 'author'
to be current user's id. Perhaps, using current_user_can for each post is not necessary?

#6 @johnbillion
7 years ago

  • Focuses ui removed
  • Keywords needs-patch needs-unit-tests added
  • Version 4.7.3 deleted

WP_Query has the perm => editable parameter just for this (which can be added to the query args in wp_ajax_find_posts()), but it doesn't appear to work in conjunction with the status parameter. Needs some more investigation.

Note: See TracTickets for help on using tickets.