XML-RPC "wp_author_id" ignored when changing author to self

[redsweater]

When using the MetaWeblog mw_editPost method to change the author of an existing post from another user to the logged in user, the wp_author_id" field is not read from the content_struct because of a logic error in mw_editPost().

The logic error is in this line:

if ( isset($content_struct['wp_author_id']) && ($user->ID != $content_struct['wp_author_id']) )

Here it is assumed that if the supplied author ID is the same as the logged in user, then the supplied ID does not need to be read. However, this is not true when the post being edited is in fact not currently in the logged in user's authorship.

The attached patch fixes the problem and also clarifies the permissions test so that it will reject any effort to change the author ID either to another user's ID or from another user's ID, unless the logged in user has permission to editor others' posts.

[redsweater]

Patches mw_editPost to correctly convey the supplied author ID even if it matches the logged in user's ID

[redsweater]

Tests to confirm the fix to mw_editPost and to confirm the existing correct behavior of wp_editPost

[redsweater]

I confirmed that the existing behavior of wp_editPost is correct, because the author field for the content is implied as the logged-in user, and doesn't need to be mapped from the legacy MetaWeblog-style content struct to the internal $post_datapost_author? field.

The attached tests TestMetaWeblogChangeAuthorToSelf.patch not only confirm my fix to mw_editPost but also add a test to wp_editPost to confirm the existing correct functionality for the modern wp_editPost API.

[markoheijnen]

In general only use wp.* methods.

[redsweater]

markoheijnen - I understand that the wp.* methods are preferred, but as long as the older methods are present I assume there is not opposition to having them behave correctly?

For what it's worth, as far as I can tell even WordPress's official iOS app still uses metaWeblog.editPost. It doesn't look like it currently supports changing the author of a post but for example, if it tried to add that feature without updating to use wp.editPost, it would run into this failure.

[markoheijnen]

Yes, they should. You can also see it as they missing that feature.
If you want to have this support then you need to move to wp.editPost anyway. So you support WordPress 3.4 > instead of a possible 3.7.

I guess the mobile apps will move to wp.* methods in the near future.

[markoheijnen]

Refreshed the patch and cleanup the code a bit. Moving to 4.1. It's either commit the whole patch or add only the wp.editPost unit tests.

[iseulde]

has-patch 4.2-early so moving to 4.2.

[DrewAPicture]

@markoheijnen: Any interest in pursuing a fix for this in 4.2? If so, the patch needs a refresh in class-wp-xmlrpc-server.php.

[markoheijnen]

I will check it out later today. Thanks for the reminder. Didn't saw that this was moved to 4.2

[markoheijnen]

Refreshed patch

[markoheijnen]

Little bit later then planned but here is the refreshed patch. After reading comments and code It still looks fine for me.

[DrewAPicture]

24916.3.diff​ applies and unit tests pass.

@johnbillion: What do you think? Should we get this in for 4.2 or push to 4.3-early?

[johnbillion]

This change makes sense, but I'm struggling to understand why the two edit_others_* permissions checks are in place here. AFAICT they are redundant due to the if ( ! current_user_can( 'edit_post', $post_ID ) ) check near the beginning of wp_xmlrpc_server::mw_editPost().

Thoughts?

[redsweater]

Replying to johnbillion:

This change makes sense, but I'm struggling to understand why the two edit_others_* permissions checks are in place here. AFAICT they are redundant due to the if ( ! current_user_can( 'edit_post', $post_ID ) ) check near the beginning of wp_xmlrpc_server::mw_editPost().

Thoughts?

In my original patch I just took it for granted that code was correct (and the intent of this bug and patch is to avoid reaching that code in the case the current user is simply supplying their own author ID for their own post).

I assume that although it has been confirmed the user can edit the post, that doesn't necessarily give them the right to change the authorship on the post to e.g. another author. Consider a scenario where the post is currently in the current user's name, and they want to change the authorship to another person. In that case it makes sense that a higher level of permission than just "edit_post" should be required to proceed.

[johnbillion]

In 31983:

Correctly set the post author in wp_xmlrpc_server::mw_editPost() when the current user is not the author of the post.

Props redsweater, markoheijnen, DrewAPicture
Fixes #24916