WordPress.org

Make WordPress Core

#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:

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 17 months ago.
Patch to make the first cap check post_ID aware
22417.diff (2.1 KB) - added by ryan 17 months ago.
22417.2.diff (2.5 KB) - added by nacin 17 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 SergeyBiryukov17 months ago

  • Component changed from General to Role/Capability

comment:2 nacin17 months 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.

comment:3 nacin17 months ago

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

comment:4 westi17 months ago

Started some basic tests for this code in [UT1147]

comment:5 westi17 months 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.

westi17 months ago

Patch to make the first cap check post_ID aware

comment:6 westi17 months ago

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

comment:7 westi17 months ago

Tests fixed in [UT1150]

comment:8 westi17 months 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

comment:9 nacin17 months 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.

comment:10 nacin17 months ago

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

Back to westi.

ryan17 months ago

nacin17 months ago

comment:11 nacin17 months ago

  • Keywords has-patch added

comment:12 ryan17 months 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.