Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#27452 closed defect (bug) (fixed)

Contributors can publish privately

Reported by: plocha's profile plocha Owned by: nacin's profile 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 10 years ago.
Draft UI Lock
private.png (8.0 KB) - added by plocha 10 years ago.
Private UI Lock
27452.patch (1.3 KB) - added by bcworkz 10 years ago.

Download all attachments as: .zip

Change History (16)

@plocha
10 years ago

Draft UI Lock

@plocha
10 years ago

Private UI Lock

#1 @knutsp
10 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
10 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 10 years ago by plocha (previous) (diff)

@bcworkz
10 years ago

#3 in reply to: ↑ 2 @bcworkz
10 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
10 years ago

In 27964:

Better checks for contributors when saving posts.

props dd32, kovshenin, plocha.
see #27452.

#5 @nacin
10 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
10 years ago

  • Keywords commit fixed-major added

#7 @nacin
10 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
10 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
10 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
10 years ago

In 27990:

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

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

#11 @nacin
10 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
10 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
10 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.