WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 2 months ago

#48556 new defect (bug)

Query for multiple post types not considering user permission to retrieve private posts

Reported by: leogermani Owned by:
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Query Keywords: has-patch
Focuses: Cc:
PR Number:

Description

When you query for posts not informing a specific post_status, WordPress will return posts that the current user can read (if there is a user logged in).

However, if you query for multiple post types, passing an array, or if you query for any post type, WordPress will ignore this behavior and won't return any private posts at all.

Expected behavior is that it would return posts with private status if they belong to a post type for which the user has the read_private_posts capability.

An existing, and rather undocumented, workaround is to grant the user the read_multiple_post_types capability. But this, again, will not check the permission current user have in each queried post type and will simply return all private posts for all queried post types.

Proposal

The proposed solution for this is to change the SQL query when querying for multiple post types without informing a post status, and combining the post_status and post_type WHERE clauses, checking user capability for each post type and returning the appropriate query in the very same way WordPress already does when you query for only one post type.

Sample Query when querying for posts and pages, for a user that HAS read_private_posts cap but DOES NOT HAVE read_private_pages:

SELECT SQL_CALC_FOUND_ROWS  wptests_posts.ID FROM wptests_posts  WHERE 1=1  AND 
(
  (wptests_posts.post_type = 'post' AND 
    (wptests_posts.post_status = 'publish' OR wptests_posts.post_status = 'private')
  ) 
  OR 
  (wptests_posts.post_type = 'page' AND 
    (wptests_posts.post_status = 'publish' 
     OR wptests_posts.post_author = 4 
     AND wptests_posts.post_status = 'private'
    )
  )
)  ORDER BY wptests_posts.post_date DESC LIMIT 0, 10 }}}



Attachments (1)

48556.diff (9.2 KB) - added by leogermani 3 months ago.

Download all attachments as: .zip

Change History (8)

@leogermani
3 months ago

#1 @leogermani
3 months ago

  • Keywords has-patch added

Attached a patch with the change and some tests.

I made an additional elseif statement so it do not touch all other cases and acts only in this very specific case.

Would love to have some feedback.

git branch: https://github.com/leogermani/wordpress-develop/tree/48556

#2 @leogermani
3 months ago

This is a related ticket: #44737

If either of these tickets are merged, the other have to be updated

Last edited 3 months ago by SergeyBiryukov (previous) (diff)

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


3 months ago

#4 @SergeyBiryukov
3 months ago

  • Milestone changed from Awaiting Review to 5.4

#5 @leogermani
2 months ago

Few observations on this ticket:

1) It changes the query results, so it needs Dev Notes

2) It also changes the SQL query, so it might break things for people filtering the query with "posts_where" filter, for example. Also needs to be in Dev Notes

3) I made it in a way it only changes the query in the very specific case of this ticket. The code could be cleaner if we changed the way the query is built in every case, but I thought this would bring less problems for developers

#6 @leogermani
2 months ago

There is another case where the same situation described in this ticket happens.

If you query for multiple post_types, informing a private post_status, WP_Query will not check permissions for each post type and again rely on the read_multiple_post_types capability.

All of this only if perm=readable.

The expected behavior is for the query to check read_private_posts permission for each post type consider the informed post_status only when current user has permission.

I don't think it is the case of opening another ticket because both things should be done together and the ticket title still applies. I'll edit the description.

#7 @leogermani
2 months ago

As a last note for today, I will have a look at all the tests we have for WP_Query. I think we should write more tests to cover as many situations as possible before doing such changes.

Note: See TracTickets for help on using tickets.