Skip to content
Permalink
Browse files Browse the repository at this point in the history
XML-RPC: Improve error messages for unprivileged users.
Add specific permission checks to avoid ambiguous failure messages.

Props zieladam, peterwilsoncc, xknown, whyisjake.

git-svn-id: https://develop.svn.wordpress.org/trunk@49380 602fd350-edb4-49c9-b593-d223f7449a82
  • Loading branch information
desrosj committed Oct 29, 2020
1 parent ed2b1a4 commit c9e6b98
Show file tree
Hide file tree
Showing 2 changed files with 186 additions and 14 deletions.
15 changes: 15 additions & 0 deletions src/wp-includes/class-wp-xmlrpc-server.php
Expand Up @@ -3876,6 +3876,21 @@ public function wp_newComment( $args ) {
return new IXR_Error( 403, __( 'Sorry, comments are closed for this item.' ) );
}

if (
'publish' === get_post_status( $post_id ) &&
! current_user_can( 'edit_post', $post_id ) &&
post_password_required( $post_id )
) {
return new IXR_Error( 403, __( 'Sorry, you are not allowed to comment on this post.' ) );
}

if (
'private' === get_post_status( $post_id ) &&
! current_user_can( 'read_post', $post_id )
) {
return new IXR_Error( 403, __( 'Sorry, you are not allowed to comment on this post.' ) );
}

$comment = array(
'comment_post_ID' => $post_id,
'comment_content' => trim( $content_struct['content'] ),
Expand Down
185 changes: 171 additions & 14 deletions tests/phpunit/tests/xmlrpc/wp/newComment.php
Expand Up @@ -6,15 +6,45 @@
class Tests_XMLRPC_wp_newComment extends WP_XMLRPC_UnitTestCase {

/**
* Post object for shared fixture.
* Array of posts.
*
* @var WP_Post
* @var WP_Post[]
*/
public static $post;
public static $posts;

/**
* User IDs.
*
* Array of user IDs keyed by role.
*
* @var int[]
*/
public static $user_ids;

public static function wpSetUpBeforeClass( $factory ) {
self::make_user_by_role( 'administrator' );
self::$post = $factory->post->create_and_get();
self::$user_ids = array(
'administrator' => self::make_user_by_role( 'administrator' ),
'contributor' => self::make_user_by_role( 'contributor' ),
);
self::$posts['publish'] = $factory->post->create_and_get();
self::$posts['password'] = $factory->post->create_and_get(
array(
'post_password' => 'xmlrpc',
'post_author' => self::$user_ids['administrator'],
)
);
self::$posts['private'] = $factory->post->create_and_get(
array(
'post_status' => 'private',
'post_author' => self::$user_ids['administrator'],
)
);
self::$posts['private_contributor'] = $factory->post->create_and_get(
array(
'post_status' => 'private',
'post_author' => self::$user_ids['contributor'],
)
);
}

function test_valid_comment() {
Expand All @@ -23,7 +53,7 @@ function test_valid_comment() {
1,
'administrator',
'administrator',
self::$post->ID,
self::$posts['publish']->ID,
array(
'content' => rand_str( 100 ),
),
Expand All @@ -39,7 +69,7 @@ function test_empty_comment() {
1,
'administrator',
'administrator',
self::$post->ID,
self::$posts['publish']->ID,
array(
'content' => '',
),
Expand All @@ -59,7 +89,7 @@ public function test_empty_content_multiple_spaces() {
1,
'administrator',
'administrator',
self::$post->ID,
self::$posts['publish']->ID,
array(
'content' => ' ',
),
Expand All @@ -79,7 +109,7 @@ public function test_valid_comment_0_content() {
1,
'administrator',
'administrator',
self::$post->ID,
self::$posts['publish']->ID,
array(
'content' => '0',
),
Expand All @@ -99,7 +129,7 @@ public function test_valid_comment_allow_empty_content() {
1,
'administrator',
'administrator',
self::$post->ID,
self::$posts['publish']->ID,
array(
'content' => ' ',
),
Expand Down Expand Up @@ -139,7 +169,7 @@ function test_new_comment_duplicated() {
1,
'administrator',
'administrator',
self::$post->ID,
self::$posts['publish']->ID,
array(
'content' => rand_str( 100 ),
),
Expand Down Expand Up @@ -168,7 +198,7 @@ function test_allowed_anon_comments() {
1,
'',
'',
self::$post->ID,
self::$posts['publish']->ID,
array(
'author' => 'WordPress',
'author_email' => 'noreply@wordpress.org',
Expand All @@ -193,7 +223,7 @@ function test_anon_comments_require_email() {
1,
'',
'',
self::$post->ID,
self::$posts['publish']->ID,
array(
'author' => 'WordPress',
'author_email' => 'noreply at wordpress.org',
Expand All @@ -218,7 +248,7 @@ function test_username_avoids_anon_flow() {
1,
'administrator',
'administrator',
self::$post->ID,
self::$posts['publish']->ID,
array(
'author' => 'WordPress',
'author_email' => 'noreply at wordpress.org',
Expand All @@ -232,4 +262,131 @@ function test_username_avoids_anon_flow() {

$this->assertSame( $user_id, (int) $comment->user_id );
}

/**
* Ensure users can only comment on posts they're permitted to access.
*
* @dataProvider data_comments_observe_post_permissions
*
* @param string $post_key Post identifier from the self::$posts array.
* @param string $username Username leaving comment.
* @param bool $expected Expected result. True: successfull comment. False: Refused comment.
* @param string $anon_callback Optional. Allow anonymous comment callback. Default __return_false.
*/
function test_comments_observe_post_permissions( $post_key, $username, $expected, $anon_callback = '__return_false' ) {
add_filter( 'xmlrpc_allow_anonymous_comments', $anon_callback );

$comment_args = array(
1,
$username,
$username,
self::$posts[ $post_key ]->ID,
array(
'author' => 'WordPress',
'author_email' => 'noreply@wordpress.org',
'content' => 'Test Comment',
),
);

$result = $this->myxmlrpcserver->wp_newComment( $comment_args );
if ( $expected ) {
$this->assertInternalType( 'int', $result );
return;
}

$this->assertIXRError( $result );
$this->assertSame( 403, $result->code );
}

/**
* Data provider for test_comments_observe_post_permissions.
*
* @return array[] {
* @type string Post identifier from the self::$posts array.
* @type string Username leaving comment.
* @type bool Expected result. True: successfull comment. False: Refused comment.
* @type string Optional. Allow anonymous comment callback. Default __return_false.
* }
*/
function data_comments_observe_post_permissions() {
return array(
// 0: Post author, password protected public post.
array(
'password',
'administrator',
true,
),
// 1: Low privileged non-author, password protected public post.
array(
'password',
'contributor',
false,
),
// 2: Anonymous user, password protected public post.
array(
'password',
'', // Anonymous user.
false,
),
// 3: Anonymous user, anon comments allowed, password protected public post.
array(
'password',
'', // Anonymous user.
false,
'__return_true',
),

// 4: Post author, private post.
array(
'private',
'administrator',
true,
),
// 5: Low privileged non-author, private post.
array(
'private',
'contributor',
false,
),
// 6: Anonymous user, private post.
array(
'private',
'', // Anonymous user.
false,
),
// 7: Anonymous user, anon comments allowed, private post.
array(
'private',
'', // Anonymous user.
false,
'__return_true',
),

// 8: High privileged non-author, private post.
array(
'private_contributor',
'administrator',
true,
),
// 9: Low privileged author, private post.
array(
'private_contributor',
'contributor',
true,
),
// 10: Anonymous user, private post.
array(
'private_contributor',
'', // Anonymous user.
false,
),
// 11: Anonymous user, anon comments allowed, private post.
array(
'private_contributor',
'', // Anonymous user.
false,
'__return_true',
),
);
}
}

0 comments on commit c9e6b98

Please sign in to comment.