From c247be8dd158442e5760325c4adf7bc32973597f Mon Sep 17 00:00:00 2001 From: Daniel Bachhuber Date: Sat, 30 Jan 2016 10:11:22 -0800 Subject: [PATCH 1/5] Validate `GET /wp/v2/comments` private query params Error when the user shouldn't be able to use them. --- .../class-wp-rest-comments-controller.php | 36 +++++++++++-------- tests/test-rest-comments-controller.php | 7 ++-- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/lib/endpoints/class-wp-rest-comments-controller.php b/lib/endpoints/class-wp-rest-comments-controller.php index 4578c81d71..2ac0e00a80 100755 --- a/lib/endpoints/class-wp-rest-comments-controller.php +++ b/lib/endpoints/class-wp-rest-comments-controller.php @@ -84,6 +84,23 @@ public function get_items_permissions_check( $request ) { return new WP_Error( 'rest_forbidden_context', __( 'Sorry, you cannot view comments with edit context.' ), array( 'status' => rest_authorization_required_code() ) ); } + if ( ! current_user_can( 'edit_posts' ) ) { + $protected_params = array( 'author', 'karma', 'author_email', 'type', 'status' ); + $forbidden_params = array(); + foreach ( $protected_params as $param ) { + if ( 'status' === $param && 'approved' !== $request[ $param ] ) { + $forbidden_params[] = $param; + } else if ( 'type' === $param && 'comment' !== $request[ $param ] ) { + $forbidden_params[] = $param; + } else if ( ! empty( $request[ $param ] ) ) { + $forbidden_params[] = $param; + } + } + if ( ! empty( $forbidden_params ) ) { + return new WP_Error( 'rest_forbidden_param', sprintf( __( 'Query parameter not permitted: %s' ), implode( ', ', $forbidden_params ) ), array( 'status' => rest_authorization_required_code() ) ); + } + } + return true; } @@ -95,8 +112,10 @@ public function get_items_permissions_check( $request ) { */ public function get_items( $request ) { $prepared_args = array( + 'author_email' => isset( $request['author_email'] ) ? $request['author_email'] : '', 'comment__in' => $request['include'], 'comment__not_in' => $request['exclude'], + 'karma' => isset( $request['karma'] ) ? $request['karma'] : '', 'number' => $request['per_page'], 'post_id' => $request['post'] ? $request['post'] : '', 'parent' => isset( $request['parent'] ) ? $request['parent'] : '', @@ -104,27 +123,16 @@ public function get_items( $request ) { 'offset' => $request['offset'], 'orderby' => $this->normalize_query_param( $request['orderby'] ), 'order' => $request['order'], - 'status' => 'approve', - 'type' => 'comment', + 'status' => $request['status'], + 'type' => $request['type'], 'no_found_rows' => false, + 'user_id' => $request['author'] ? $request['author'] : '', ); if ( empty( $request['offset'] ) ) { $prepared_args['offset'] = $prepared_args['number'] * ( absint( $request['page'] ) - 1 ); } - if ( current_user_can( 'edit_posts' ) ) { - $protected_args = array( - 'user_id' => $request['author'] ? $request['author'] : '', - 'status' => $request['status'], - 'type' => isset( $request['type'] ) ? $request['type'] : '', - 'author_email' => isset( $request['author_email'] ) ? $request['author_email'] : '', - 'karma' => isset( $request['karma'] ) ? $request['karma'] : '', - ); - - $prepared_args = array_merge( $prepared_args, $protected_args ); - } - /** * Filter arguments, before passing to WP_Comment_Query, when querying comments via the REST API. * diff --git a/tests/test-rest-comments-controller.php b/tests/test-rest-comments-controller.php index 08e0703272..b2625ebcb5 100644 --- a/tests/test-rest-comments-controller.php +++ b/tests/test-rest-comments-controller.php @@ -223,6 +223,7 @@ public function test_get_items_private_post_no_permissions() { } public function test_get_items_author_arg() { + // Authorized wp_set_current_user( $this->admin_id ); $args = array( 'comment_approved' => 1, @@ -241,12 +242,10 @@ public function test_get_items_author_arg() { $this->assertEquals( 200, $response->get_status() ); $comments = $response->get_data(); $this->assertCount( 2, $comments ); - // Unavailable to unauthenticated; defauls to all authenticated + // Unavailable to unauthenticated; defauls to error wp_set_current_user( 0 ); $response = $this->server->dispatch( $request ); - $this->assertEquals( 200, $response->get_status() ); - $comments = $response->get_data(); - $this->assertCount( 4, $comments ); + $this->assertErrorResponse( 'rest_forbidden_param', $response, 401 ); } public function test_get_comments_pagination_headers() { From 22e27d70a296cc33cc774f0357d12033ab09a416 Mon Sep 17 00:00:00 2001 From: Daniel Bachhuber Date: Sat, 30 Jan 2016 10:57:03 -0800 Subject: [PATCH 2/5] Fix logical error for sniffing out invalid param --- lib/endpoints/class-wp-rest-comments-controller.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/endpoints/class-wp-rest-comments-controller.php b/lib/endpoints/class-wp-rest-comments-controller.php index e89796cae3..58876c3538 100755 --- a/lib/endpoints/class-wp-rest-comments-controller.php +++ b/lib/endpoints/class-wp-rest-comments-controller.php @@ -88,10 +88,14 @@ public function get_items_permissions_check( $request ) { $protected_params = array( 'author', 'karma', 'author_email', 'type', 'status' ); $forbidden_params = array(); foreach ( $protected_params as $param ) { - if ( 'status' === $param && 'approved' !== $request[ $param ] ) { - $forbidden_params[] = $param; - } else if ( 'type' === $param && 'comment' !== $request[ $param ] ) { - $forbidden_params[] = $param; + if ( 'status' === $param ) { + if ( 'approve' !== $request[ $param ] ) { + $forbidden_params[] = $param; + } + } else if ( 'type' === $param ) { + if ( 'comment' !== $request[ $param ] ) { + $forbidden_params[] = $param; + } } else if ( ! empty( $request[ $param ] ) ) { $forbidden_params[] = $param; } From c99d70a55b6ce8c5e403e019f3a372ecac6a7b06 Mon Sep 17 00:00:00 2001 From: Daniel Bachhuber Date: Sat, 30 Jan 2016 11:18:08 -0800 Subject: [PATCH 3/5] Fix spacing --- lib/endpoints/class-wp-rest-comments-controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/endpoints/class-wp-rest-comments-controller.php b/lib/endpoints/class-wp-rest-comments-controller.php index 58876c3538..9f4f5ed8ec 100755 --- a/lib/endpoints/class-wp-rest-comments-controller.php +++ b/lib/endpoints/class-wp-rest-comments-controller.php @@ -119,7 +119,7 @@ public function get_items( $request ) { 'author_email' => isset( $request['author_email'] ) ? $request['author_email'] : '', 'comment__in' => $request['include'], 'comment__not_in' => $request['exclude'], - 'karma' => isset( $request['karma'] ) ? $request['karma'] : '', + 'karma' => isset( $request['karma'] ) ? $request['karma'] : '', 'number' => $request['per_page'], 'post_id' => $request['post'] ? $request['post'] : '', 'parent' => isset( $request['parent'] ) ? $request['parent'] : '', From e90e390f649c4d8954c1a00f7260570f1275d201 Mon Sep 17 00:00:00 2001 From: Daniel Bachhuber Date: Sat, 30 Jan 2016 12:14:47 -0800 Subject: [PATCH 4/5] Fix typo --- tests/test-rest-comments-controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-rest-comments-controller.php b/tests/test-rest-comments-controller.php index b2625ebcb5..557101df4b 100644 --- a/tests/test-rest-comments-controller.php +++ b/tests/test-rest-comments-controller.php @@ -242,7 +242,7 @@ public function test_get_items_author_arg() { $this->assertEquals( 200, $response->get_status() ); $comments = $response->get_data(); $this->assertCount( 2, $comments ); - // Unavailable to unauthenticated; defauls to error + // Unavailable to unauthenticated; defaults to error wp_set_current_user( 0 ); $response = $this->server->dispatch( $request ); $this->assertErrorResponse( 'rest_forbidden_param', $response, 401 ); From 0a5544a63448057737e2a33e30bfb5a17546771d Mon Sep 17 00:00:00 2001 From: Daniel Bachhuber Date: Sat, 30 Jan 2016 12:16:32 -0800 Subject: [PATCH 5/5] Mention these params require authorization --- .../class-wp-rest-comments-controller.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/endpoints/class-wp-rest-comments-controller.php b/lib/endpoints/class-wp-rest-comments-controller.php index 9f4f5ed8ec..940a4b8e3e 100755 --- a/lib/endpoints/class-wp-rest-comments-controller.php +++ b/lib/endpoints/class-wp-rest-comments-controller.php @@ -915,9 +915,15 @@ public function get_collection_params() { $query_params['context']['default'] = 'view'; + $query_params['author'] = array( + 'description' => __( 'Limit result set to comments assigned to a specific user id. Requires authorization.' ), + 'sanitize_callback' => 'absint', + 'type' => 'integer', + 'validate_callback' => 'rest_validate_request_arg', + ); $query_params['author_email'] = array( 'default' => null, - 'description' => __( 'Limit result set to that from a specific author email.' ), + 'description' => __( 'Limit result set to that from a specific author email. Requires authorization.' ), 'format' => 'email', 'sanitize_callback' => 'sanitize_email', 'validate_callback' => 'rest_validate_request_arg', @@ -939,7 +945,7 @@ public function get_collection_params() { ); $query_params['karma'] = array( 'default' => null, - 'description' => __( 'Limit result set to that of a particular comment karma.' ), + 'description' => __( 'Limit result set to that of a particular comment karma. Requires authorization.' ), 'sanitize_callback' => 'absint', 'type' => 'integer', 'validate_callback' => 'rest_validate_request_arg', @@ -993,24 +999,18 @@ public function get_collection_params() { ); $query_params['status'] = array( 'default' => 'approve', - 'description' => __( 'Limit result set to comments assigned a specific status.' ), + 'description' => __( 'Limit result set to comments assigned a specific status. Requires authorization.' ), 'sanitize_callback' => 'sanitize_key', 'type' => 'string', 'validate_callback' => 'rest_validate_request_arg', ); $query_params['type'] = array( 'default' => 'comment', - 'description' => __( 'Limit result set to comments assigned a specific type.' ), + 'description' => __( 'Limit result set to comments assigned a specific type. Requires authorization.' ), 'sanitize_callback' => 'sanitize_key', 'type' => 'string', 'validate_callback' => 'rest_validate_request_arg', ); - $query_params['author'] = array( - 'description' => __( 'Limit result set to comments assigned to a specific user id.' ), - 'sanitize_callback' => 'absint', - 'type' => 'integer', - 'validate_callback' => 'rest_validate_request_arg', - ); return $query_params; }