Skip to content

Commit

Permalink
XML-RPC: Improve error messages for unprivileged users.
Browse files Browse the repository at this point in the history 
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
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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.