Sticky flag gets unset if author doesn't have publish_posts permission

[archon810]

I'm observing a bug with the sticky flag. I set up a special user with a role of "Grammar Nazi" who should only have access to editing of other people's posts, but not publishing his own.

The permissions given to this role are:

• edit_published_posts
• edit_others_posts
• edit_posts
• read
• read_private_posts

This user works out great - he's limited to only editing errors in other authors' posts.

However, there is a bug with sticky posts. If a grammar nazi edits a stickied post, the sticky flag gets unset. As a possibly related observation, there's no Edit button on the post edit page next to the Visibility area.

This bug is worked around by adding the publish_posts permission. However, this permission is unwanted in this case as grammar nazis shouldn't be able to post their own posts. Adding publish_posts enables the Edit button next to Visibility, and saves retain the sticky bit correctly.

So, in short: the sticky bit should be retained even when users without the publish_posts permission update a post.

[SergeyBiryukov]

Sticky flag indeed requires both publish_posts and edit_others_posts capabilities:
​http://core.trac.wordpress.org/browser/tags/3.5.1/wp-admin/includes/meta-boxes.php#L127

Related: [8546], [8577], ticket:7457:19.

[archon810]

Thanks for tracking down the source references and the original ticket with relevant comments. It's clear that this is a bug at this point, right?

[chriscct7]

Confirmed this still exists. Since the setting requires publish_post to edit, requiring publish_post when checking whether to change the status when saving the post solves the bug.

[archon810]

Wow, I forgot all about this bug. Been 2 years now. :o

[chriscct7]

@archon810 Yep, I'm part of a group working on reducing the length tickets are staying open without a response (called ancient tickets ). We've got it down from 4 1/2 years to 2 years so far. You just happened to be next on the queue :)

Thanks for your work on reporting this :-)

[obenland]

We should discuss the patch a it more before it's ready for commit. By default Authors can't edit others posts, but they can publish posts.

[chriscct7]

Replying to obenland:

We should discuss the patch a it more before it's ready for commit. By default Authors can't edit others posts, but they can publish posts.

That actually works fine as is, and the patch doesn't affect it. When mentioning "author", we're not talking about the author role but rather an author of a post who can't publish a post. A better description would be "if user has edit_post but not publish_post, and edits a post, sticky gets unset"

[obenland]

If we change the cap from edit_others_posts, to publish_posts, users with the Author role can pass a true value for sticky in the post array to make that post sticky. Admittedly an edge case, but hey, devtools FTW!

[chriscct7]

Replying to obenland:

If we change the cap from edit_others_posts, to publish_posts, users with the Author role can pass a true value for sticky in the post array to make that post sticky. Admittedly an edge case, but hey, devtools FTW!

how about

current_user_can( $ptype->cap->publish_posts ) && current_user_can( $ptype->cap->edit_others_posts)

then?

[chriscct7]

current_user_can( $ptype->cap->edit_others_posts ) && current_user_can( $ptype->cap->publish_posts ) implemented

[chriscct7]

@obenland thoughts on 24153.2.patch?

[obenland]

Replying to chriscct7:

how about

current_user_can( $ptype->cap->publish_posts ) && current_user_can( $ptype->cap->edit_others_posts)

then?

Would that fix it for the scenario outlined in the ticket description?

[chriscct7]

Replying to obenland:

Replying to chriscct7:

how about

current_user_can( $ptype->cap->publish_posts ) && current_user_can( $ptype->cap->edit_others_posts)

then?

Would that fix it for the scenario outlined in the ticket description?

Yes because the updated patch does not allow the stickiness status to be updated without both caps.

[obenland]

But Grammar Nazi doesn't have publish_posts, only edit_others_posts.

The problem lies in the Publish meta box, where the sticky option only gets added when the user has both caps. If they lack publish_post the sticky input is never added to the form. The cap check in edit_post() goes through, but 'sticky' is not set in the $_POST array, hence removing the sticky flag.

[chriscct7]

Replying to obenland:

But Grammar Nazi doesn't have publish_posts, only edit_others_posts.

That's correct. The problem is in the save routine, right now Grammar Nazi can change the stickiness of the post because since he doesn't have both caps, the save routine will assume the post was unstickied. By wrapping the check in a cap check for both capabilities, Gramar Nazi cannot change the stickiness of the post by editing the post because the part that checks whether or not the post is sticky or not is circumvented by the capability check

[chriscct7]

Replying to obenland:

The problem lies in the Publish meta box, where the sticky option only gets added when the user has both caps. If they lack publish_post the sticky input is never added to the form. The cap check in edit_post() goes through, but 'sticky' is not set in the $_POST array, hence removing the sticky flag.

We should do a permissions check before changing as the patch does as opposed to adding a hidden field or something can be manipulated by editing the source output of a page or using a POST replay

[obenland]

Replying to chriscct7:

We should do a permissions check before changing as the patch does as opposed to adding a hidden field or something can be manipulated by editing the source output of a page or using a POST replay

Agreed. Could you check the other instances where we stick/unstick posts for permissions? This would also benefit from getting unit tests.

[ericmann]

I wrote a test that replicates the reported behavior with both wp_update_post and edit_post:

• User with edit_others_posts but lacking publish_posts edits a sticky post
• Make sure the post is still sticky after it's been edited

As things turn out, the wp_update_post routine worked just fine without any changes - it's merely the presence of the $_POST array that mucked things up in edit_post. I've added tests for both update/save routines just to be sure nothing breaks.

My new patch also refreshes the existing patch.

[ericmann]

Updated patch file, with tests

[chriscct7]

@ericmann Thanks!

Completely forgot about this ticket, but nevertheless, Eric's patch 3 looks good

[obenland]

In 33096:

Check for all required caps before (un)sticking a post.

In cases where a user has the edit_others_posts capability but not
publish_posts, it was possible for that user to unstick a post after editing,
since the input field was never made available in that context.

Props ericmann, chriscct7.
Fixes #24153.

[johnbillion]

In 35705:

Clean up the grammarian role so it doesn't pollute other tests.

See #24153