WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#27452 closed defect (bug) (fixed)

Contributors can publish privately

Reported by: plocha Owned by: nacin
Milestone: 3.8.2 Priority: normal
Severity: normal Version: 2.7
Component: Posts, Post Types Keywords: has-patch commit fixed-major
Focuses: ui Cc:

Description

If user doesn't own the 'publish_posts' capability, he can't set a post status to 'private' in edit screen even if he has the capabilities to edit this post. But in Quick Edit he can. The same problem appears with pages. You can check the problem with a user of the role 'contributor'.

In my humble opinion such a user should be able to set the status from draft to 'private' and vice versa.

Attachments (3)

draft.png (9.1 KB) - added by plocha 6 years ago.
Draft UI Lock
private.png (8.0 KB) - added by plocha 6 years ago.
Private UI Lock
27452.patch (1.3 KB) - added by bcworkz 6 years ago.

Download all attachments as: .zip

Change History (16)

@plocha
6 years ago

Draft UI Lock

@plocha
6 years ago

Private UI Lock

#1 @knutsp
6 years ago

A post with status 'private' is also published (as indicated on private.png) and can be viewed by users with 'read_private_posts' capability. Moving to status 'private' is a publishing action.

I would not want my contributors to "publish" any draft post they have written as private, surprising my editors when they arrive and view the site. It seems to that Quick Edit is the place to fix this, if inconsistent.

#2 follow-up: @plocha
6 years ago

  • Keywords needs-patch added
  • Summary changed from Can't set status to 'private' without publishing capability to Contributors can publish privately

Okay thanks, I interpreted the private status incorrectly. In that case we shouldn't only implement the restriction in Quick Edit but also in edit processing. The latter should be done in wp_insert_post imho.

Btw I'm sure the server-side post edit processing contains more bugs of this type. That's the second time I found an input validation bug in wp_insert_post unintentionally.

Last edited 6 years ago by plocha (previous) (diff)

@bcworkz
6 years ago

#3 in reply to: ↑ 2 @bcworkz
6 years ago

  • Keywords has-patch added; needs-patch removed

Replying to knutsp:

It seems to that Quick Edit is the place to fix this, if inconsistent.

Exactly! All it takes is adding a "disabled" attribute to the checkbox element if the user does not have publish_post capability. Then checking user capability again when the form is submitted.

Replying to plocha:

...we shouldn't only implement the restriction in Quick Edit but also in edit processing. The latter should be done in wp_insert_post imho.

No capabilities are ever checked in wp_insert_post(). It must be able function properly without restriction in order for remote procedure calls to work. In WordPress, capabilities are always verified at the user interface level, never at system calls.

We do need to check user capability when the form is submitted, disabling the form element is not enough, that can be easily circumvented. The first code to process quick edit submits is wp_ajax_inline_save(). The patch checks user capability here.

Until now, it's not totally clear what steps to take to replicate this error. Here they are:

  • Ensure the Contributor role only has the default capabilities: read, edit_posts, delete_posts
  • Log in as a user with Contributor role and create a post, then submit for review.
  • In the posts list table, open the post just created for quick edit.
  • Check the "Private" checkbox and click Update
  • The post is now published as a private post and is visible on the main index page to any logged in editor or administrator.

After applying the patch, contributors can see the private condition but cannot change it.

#4 @nacin
6 years ago

In 27964:

Better checks for contributors when saving posts.

props dd32, kovshenin, plocha.
see #27452.

#5 @nacin
6 years ago

  • Milestone changed from Awaiting Review to 3.8.2
  • Owner set to nacin
  • Status changed from new to reviewing
  • Version changed from 3.8.1 to 2.7

#6 @nacin
6 years ago

  • Keywords commit fixed-major added

#7 @nacin
6 years ago

In 27975:

Apply checks in [27964] to wp_write_post(), which is unused and due for dismantling and deprecation.

see #27452.

#8 @nacin
6 years ago

In 27976:

Better checks for contributors when saving posts.

Merges [27964] and [27975] to the 3.8 branch.

props dd32, kovshenin, plocha.
see #27452.

#9 @nacin
6 years ago

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

In 27977:

Better checks for contributors when saving posts.

Merges [27976] from the 3.8 branch to the 3.7 branch.

props dd32, kovshenin, plocha.
fixes #27452.

#10 @nacin
6 years ago

In 27990:

Avoid stomping of bulk postdata inside the bulk_edit_posts() loop.

props kovshenin.
see [27964], see #27452.

#11 @nacin
6 years ago

[27990] looks a bit weird, but basically, $post_data holds information used for all posts being looped over. The information coming from _wp_translate_postdata() is specific to that post. If we overwrite it, things can happen like all subsequent posts getting published, if the initial post was already published, even if "No Change" was chosen during bulk edit.

#12 @nacin
6 years ago

In 27991:

Avoid stomping of bulk postdata inside the bulk_edit_posts() loop.

Merges [27990] to the 3.8 branch.

props kovshenin.
see [27964], see #27452.

#13 @nacin
6 years ago

In 27992:

Avoid stomping of bulk postdata inside the bulk_edit_posts() loop.

Merges [27990] to the 3.7 branch.

props kovshenin.
see [27964], see #27452.

Note: See TracTickets for help on using tickets.