WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#7070 closed defect (bug) (fixed)

users with 'edit_published_posts' can't edit posts without unpublishing them.

Reported by: jeremyclarke Owned by: jeremyclarke
Milestone: 2.6 Priority: high
Severity: major Version: 2.5.1
Component: General Keywords: has-patch,
Focuses: Cc:

Description

Right now the save box display and post edit processing functions don't take into account the existence of the 'edit_published_posts' capability (since 2.5).

Effectively both bits of code only check for the 'publish_posts' capability and deny publishing rights (hiding it from the pulldown menu and changing the status during processing to pending) if the user doesn't have it.

The problem is that users with 'edit_published_posts' (the ability to edit your own posts once they have been published, seperate from 'publish_posts' and 'edit_others_posts'), are only given the option of 'pending' when they edit their posts. This is not only frustrating, but because the screen only shows the 'save' button, it means that loading your article, making a change, and saving (without looking at the status dropdown) results in unpublishing your article, which you are unlikely to even notice potentially forever.

The solution is to not only check for

current_user_can('publish_posts')

but also for

current_user_can('edit_post', $post->ID)

which automatically checks whether a user is allowed to edit a post based on whether they are the author, it is published etc. That way users with the 'edit_published_posts' capability are able to use it in those situations where it's appropriate.

The code is affected in two places:

1) wp-admin/edit-form-advanced.php ~line 94 (which controls whether the 'published' item displays in the status dropdown menu)


<?php if ( current_user_can('publish_posts') ) :?>


becomes


if ( current_user_can('publish_posts') OR ( $post->post_status == 'publish' AND current_user_can('edit_post', $post->ID)) ) :


2) wp-admin/includes/post.php ~line 64 (which controls whether the post_status in $_POST is shunted from publish down to 'pending')

if ( 'publish' == $_POST['post_status'] && !current_user_can( 'publish_posts' ) )


becomes

if ('publish' == $_POST['post_status'] && !current_user_can( 'publish_posts' ) && !current_user_can( 'edit_post', $_POST['ID'] ))

That way users can 1) choose the publish option when they edit their published posts (and it's selected by default) and 2) the posts aren't downgraded while being processed after saving (otherwise the post is silently unpublished!).

In 2.6 the second part is in a function called _wp_translate_postdata(), whereas in <2.6 its in edit_post(). My personal impression is that if the permissions check is going to be moved out of edit_posts it should live in wp_insert_post(), which gets called at the same times as _wp_translate_postdata() but contains other similar processing (unlike _wp_translate_postdata() who's main purpose is validation and cleaning of the raw $_POST contents). Ryan Boren said that you want to keep capability conditions out of the api, not sure what that means, but figured I'd float another idea anyway. The changes will work in their current positions either way (the 2.6 patch is for the current location in _wp_translate_postdata()).

I tried to fix the same thing for PAGES but in 2.6 the capability checks for pages didn't seem to be working at all (before i changed anything anyone could publish pages). i think fixing posts is more urgent either way, please let me konw if you want help fixing the pages as well (though the exact same changes but with 'edit_page' should work in wp-admin/includes/post.php a few lines above and in wp-admin/edit-page-form.php).

Attachments (4)

posting_permissions_2-6_may30.diff (2.0 KB) - added by jeremyclarke 12 years ago.
fixes edit_published_posts perms for trunk (2.6)
posting_permissions_2-5_may30.diff (1.8 KB) - added by jeremyclarke 12 years ago.
fix 'edit_published_posts' permissions for 2.5 head (may30, for 2.5.2)
posting_permissions_2-6_jun2.diff (2.3 KB) - added by jeremyclarke 12 years ago.
trunk patch to fix publishing/pending status based on capabilities
posting_permissions_2-5_jun2.diff (2.6 KB) - added by jeremyclarke 12 years ago.
2.5 branch patch to fix publishing/pending status based on capabilities

Download all attachments as: .zip

Change History (14)

@jeremyclarke
12 years ago

fixes edit_published_posts perms for trunk (2.6)

@jeremyclarke
12 years ago

fix 'edit_published_posts' permissions for 2.5 head (may30, for 2.5.2)

#1 @jeremyclarke
12 years ago

  • Keywords added

Okay, so there's a problem with this patch. When users without publishing rights submit a post for review the post gets published instead (found out the hard way on my live site). I'll upload a new patch when I figure it out.

@jeremyclarke
12 years ago

trunk patch to fix publishing/pending status based on capabilities

#2 @jeremyclarke
12 years ago

Okay, so the jun2 patch for trunk fixes the problem above. The edit-form-advanced.php part is the same, but I added to the wp-admin/includes/post.php part.

In the _wp_translate_postdata() function, where the permissions checking for post publishing/pending status is done while screening the other $_POST values, it recieves both requests to PUBLISH and to SUBMIT FOR APPROVAL as

 $_POST['post_status'] = 'publish'

Personally, i would expect pending submissions to be submitted to _POST with their own status (i.e. 'pending'), but as of now it just claims to want to be published. What _wp_translate_postdata() does is switch that 'publish' status to 'pending' IF the person submitting doesn't have the right permissions.

This wasn't serving the 'edit_published_posts' capability very well because it was only checking if the NEW status (in $_POST) was 'publish' and switching it to 'pending' if the user didn't have the 'publish_posts' capability.

Instead what needs to happen is another layer of checking, where even if the person can't publish posts but is asking to publish, it checks to make sure the old status wasn't already 'publish' (i.e. that it isn't a previously published post) and if WAS aldready published, and the person is allowed to 'edit_published_posts' then it leaves the status as 'published' rather than switching it to pending.

	
$previous_status = get_post_field('post_status',  $_POST['ID']);

if ( 'publish' == $_POST['post_status'] && !current_user_can( 'publish_posts' ) ) :
	// Stop attempts to publish new posts, but allow already published posts to be saved if appropriate.
	if ( $previous_status != 'publish' OR !current_user_can( 'edit_published_posts') )
		$_POST['post_status'] = 'pending';
endif;

I think that it's pretty solid and should be implemented ASAP. I will make a 2.5 patch of these changes soon.

For the record: The edit-page-form.php file is completely screwed up in the way it handles the status dropdown menu if someone has edit_pages but not 'publish_pages', it marks things as private when you ask to publish and then tells it was published, I'm pretty sure that there was just never anyone who tried giving that privilege combination, but it's worth taking a look at. The changes in this patch won't fix the problesm with pages until that is sorted out, I might do it eventually, but post permissions is my priority atm.

Feedback very welcome, i'd love to hear from a core dev.

@jeremyclarke
12 years ago

2.5 branch patch to fix publishing/pending status based on capabilities

#3 @ryan
12 years ago

(In [8032]) Don't unpublish posts when a user edit who can edit publised posts but not publih new posts edits a post. Props jeremyclarke. see #7070 for trunk

#4 @ryan
12 years ago

jQuery changes snuck in there. Ignore them.

#5 @jeremyclarke
12 years ago

Just posted a patch for the page form side of this to [6943].

#6 @jeremyclarke
12 years ago

Oops, meant #6943, ticket not changeset.

#7 @ryan
12 years ago

(In [8034]) Don't unpublish pages when a user who can edit publised pages but not publish new pages edits a page. Props jeremyclarke. see #6943 #7070 for trunk

#8 @ryan
12 years ago

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

(In [8102]) Don't unpublish pages when a user who can edit publised pages but not publish new pages edits a page. Props jeremyclarke. fixes #6943 #7070 for 2.5

#9 @ryan
12 years ago

  • Milestone changed from 2.5.2 to 2.9

Milestone 2.5.2 deleted

#10 @westi
12 years ago

  • Milestone changed from 2.9 to 2.6
Note: See TracTickets for help on using tickets.