WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#22417 closed defect (bug) (fixed)

_wp_translate_postdata() should use current_user_can( $ptype->cap->edit_post, $post_id )

Reported by: danielbachhuber Owned by: westi
Milestone: 3.5 Priority: low
Severity: normal Version:
Component: Role/Capability Keywords: has-patch
Focuses: Cc:
PR Number:

Description

Similar to #22415, _wp_translate_postdata() should use current_user_can( $ptype->cap->edit_post, $post_id ) here:

$ptype = get_post_type_object( $post_data['post_type'] );
	if ( isset($post_data['user_ID']) && ($post_data['post_author'] != $post_data['user_ID']) ) {
		if ( !current_user_can( $ptype->cap->edit_others_posts ) ) {

I think this is problematic too:

if ( $previous_status != 'publish' || !current_user_can( 'edit_post', $post_id ) )
			$post_data['post_status'] = 'pending';

Switching to current_user_can( $ptype->cap->edit_post, $post_id ) would mean that the context could be appropriately filtered.

Attachments (3)

22417-part1.diff (1.6 KB) - added by westi 7 years ago.
Patch to make the first cap check post_ID aware
22417.diff (2.1 KB) - added by ryan 7 years ago.
22417.2.diff (2.5 KB) - added by nacin 7 years ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
7 years ago

  • Component changed from General to Role/Capability

#2 @nacin
7 years ago

  • Milestone changed from Awaiting Review to 3.5

I imagine this is in place for _wp_translate_postdata( false ), as in — when it is not used for an update.

wp_write_post() uses this, but this code is actually dead now and has been since auto-drafts are introduced, because edit_post() gets called right before it. For more on removing wp_write_post() and making this code more sane, see #21963.

If $update, we should be able to make the more informed capability checks you are proposing.

#3 @nacin
7 years ago

  • Owner set to westi
  • Status changed from new to assigned

#4 @westi
7 years ago

Started some basic tests for this code in [UT1147]

#5 @westi
7 years ago

@danielbachhuber it would be really useful if you could help write some tests which show the issues you are having to add to the above tests.

@westi
7 years ago

Patch to make the first cap check post_ID aware

#6 @westi
7 years ago

That patch shows that my tests are broken too, need to update them to create real posts for the update = true tests ;)

#7 @westi
7 years ago

Tests fixed in [UT1150]

#8 @westi
7 years ago

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

In 22769:

Posting: Improve the capability checking _wp_translate_postdata() when updating posts.

  • Use the specific post_type's 'edit_post' cap
  • Pass the ID of the post being edited.

Fixes #22417

#9 @nacin
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Chatted about this with westi in IRC. A few points:

  • The second part of this bug report can be answered by (and further addressed in) #22415.
  • [22769] looks good, but after it landed, I noticed that 'edit_post' could get away with being outside of the post_author == user_ID block, instead letting the meta capability handle that part. Of course, that check may still be necessary for $update = false (which I'll aim to eliminate in 3.6). So, re-opening for review.

#10 @nacin
7 years ago

  • Priority changed from normal to low
  • Status changed from reopened to assigned

Back to westi.

@ryan
7 years ago

@nacin
7 years ago

#11 @nacin
7 years ago

  • Keywords has-patch added

#12 @ryan
7 years ago

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

In 22950:

Add a create_posts check to _wp_translate_postdata(). Move the edit_post check to the top of the function.

Props nacin
fixes #22417

Note: See TracTickets for help on using tickets.