From 4b791ecf5f057b5dd97135f846ef771033ee8532 Mon Sep 17 00:00:00 2001 From: Daniel Bachhuber Date: Thu, 15 Nov 2018 12:56:54 +0000 Subject: [PATCH] REST API: Avoid using 'parent' as path argument name for autosaves. When 'parent' is set as the path argument name, it gets passed down through to the `create_item()` method and can erroneously reset the 'parent' value on the post itself. Instead, we rename the argument to 'id' and replicate the revision controller's `get_items_permissions_check()` to instead reference 'id'. Also ensures revision query params (of which there are many) aren't exposed as the query params for autosaves (of which there are two). Props TimothyBlynJacobs. See #43316. git-svn-id: https://develop.svn.wordpress.org/branches/5.0@43897 602fd350-edb4-49c9-b593-d223f7449a82 --- .../class-wp-rest-autosaves-controller.php | 45 +- .../rest-api/rest-autosaves-controller.php | 76 ++- .../tests/rest-api/rest-schema-setup.php | 6 +- tests/qunit/fixtures/wp-api-generated.js | 433 +++++++----------- 4 files changed, 295 insertions(+), 265 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php index 79529213c3..bf5b180df8 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php @@ -79,7 +79,7 @@ class WP_REST_Autosaves_Controller extends WP_REST_Revisions_Controller { public function register_routes() { register_rest_route( $this->rest_namespace, - '/' . $this->parent_base . '/(?P[\d]+)/' . $this->rest_base, + '/' . $this->parent_base . '/(?P[\d]+)/' . $this->rest_base, array( 'args' => array( 'parent' => array( @@ -90,14 +90,14 @@ class WP_REST_Autosaves_Controller extends WP_REST_Revisions_Controller { array( 'methods' => WP_REST_Server::READABLE, 'callback' => array( $this, 'get_items' ), - 'permission_callback' => array( $this->revisions_controller, 'get_items_permissions_check' ), + 'permission_callback' => array( $this, 'get_items_permissions_check' ), 'args' => $this->get_collection_params(), ), array( 'methods' => WP_REST_Server::CREATABLE, 'callback' => array( $this, 'create_item' ), 'permission_callback' => array( $this, 'create_item_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( WP_REST_Server::CREATABLE ), + 'args' => $this->parent_controller->get_endpoint_args_for_item_schema( WP_REST_Server::EDITABLE ), ), 'schema' => array( $this, 'get_public_item_schema' ), ) @@ -143,6 +143,28 @@ class WP_REST_Autosaves_Controller extends WP_REST_Revisions_Controller { return $this->revisions_controller->get_parent( $parent_id ); } + /** + * Checks if a given request has access to get autosaves. + * + * @since 5.0.0 + * + * @param WP_REST_Request $request Full data about the request. + * @return true|WP_Error True if the request has read access, WP_Error object otherwise. + */ + public function get_items_permissions_check( $request ) { + $parent = $this->get_parent( $request['id'] ); + if ( is_wp_error( $parent ) ) { + return $parent; + } + + $parent_post_type_obj = get_post_type_object( $parent->post_type ); + if ( ! current_user_can( $parent_post_type_obj->cap->edit_post, $parent->ID ) ) { + return new WP_Error( 'rest_cannot_read', __( 'Sorry, you are not allowed to view autosaves of this post.' ), array( 'status' => rest_authorization_required_code() ) ); + } + + return true; + } + /** * Checks if a given request has access to create an autosave revision. * @@ -177,7 +199,7 @@ class WP_REST_Autosaves_Controller extends WP_REST_Revisions_Controller { define( 'DOING_AUTOSAVE', true ); } - $post = get_post( $request->get_param( 'id' ) ); + $post = get_post( $request['id'] ); if ( is_wp_error( $post ) ) { return $post; @@ -245,7 +267,7 @@ class WP_REST_Autosaves_Controller extends WP_REST_Revisions_Controller { * @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure. */ public function get_items( $request ) { - $parent = $this->get_parent( $request->get_param( 'parent' ) ); + $parent = $this->get_parent( $request['id'] ); if ( is_wp_error( $parent ) ) { return $parent; } @@ -389,4 +411,17 @@ class WP_REST_Autosaves_Controller extends WP_REST_Revisions_Controller { */ return apply_filters( 'rest_prepare_autosave', $response, $post, $request ); } + + /** + * Retrieves the query params for the autosaves collection. + * + * @since 5.0.0 + * + * @return array Collection parameters. + */ + public function get_collection_params() { + return array( + 'context' => $this->get_context_param( array( 'default' => 'view' ) ), + ); + } } diff --git a/tests/phpunit/tests/rest-api/rest-autosaves-controller.php b/tests/phpunit/tests/rest-api/rest-autosaves-controller.php index ede81c5f25..c0217028ef 100644 --- a/tests/phpunit/tests/rest-api/rest-autosaves-controller.php +++ b/tests/phpunit/tests/rest-api/rest-autosaves-controller.php @@ -13,6 +13,7 @@ class WP_Test_REST_Autosaves_Controller extends WP_Test_REST_Post_Type_Controller_Testcase { protected static $post_id; protected static $page_id; + protected static $draft_page_id; protected static $autosave_post_id; protected static $autosave_page_id; @@ -20,6 +21,10 @@ class WP_Test_REST_Autosaves_Controller extends WP_Test_REST_Post_Type_Controlle protected static $editor_id; protected static $contributor_id; + protected static $parent_page_id; + protected static $child_page_id; + protected static $child_draft_page_id; + protected function set_post_data( $args = array() ) { $defaults = array( 'title' => 'Post Title', @@ -76,6 +81,25 @@ class WP_Test_REST_Autosaves_Controller extends WP_Test_REST_Post_Type_Controlle ) ); + self::$draft_page_id = $factory->post->create( array( + 'post_type' => 'page', + 'post_status' => 'draft', + ) ); + self::$parent_page_id = $factory->post->create( array( + 'post_type' => 'page', + ) ); + self::$child_page_id = $factory->post->create( array( + 'post_type' => 'page', + 'post_parent' => self::$parent_page_id, + ) ); + self::$child_draft_page_id = $factory->post->create( array( + 'post_type' => 'page', + 'post_parent' => self::$parent_page_id, + // The "update post" behavior of the autosave endpoint only occurs + // when saving a draft/auto-draft authored by the current user. + 'post_status' => 'draft', + 'post_author' => self::$editor_id, + ) ); } public static function wpTearDownAfterClass() { @@ -96,9 +120,9 @@ class WP_Test_REST_Autosaves_Controller extends WP_Test_REST_Post_Type_Controlle public function test_register_routes() { $routes = rest_get_server()->get_routes(); - $this->assertArrayHasKey( '/wp/v2/posts/(?P[\d]+)/autosaves', $routes ); + $this->assertArrayHasKey( '/wp/v2/posts/(?P[\d]+)/autosaves', $routes ); $this->assertArrayHasKey( '/wp/v2/posts/(?P[\d]+)/autosaves/(?P[\d]+)', $routes ); - $this->assertArrayHasKey( '/wp/v2/pages/(?P[\d]+)/autosaves', $routes ); + $this->assertArrayHasKey( '/wp/v2/pages/(?P[\d]+)/autosaves', $routes ); $this->assertArrayHasKey( '/wp/v2/pages/(?P[\d]+)/autosaves/(?P[\d]+)', $routes ); } @@ -119,6 +143,18 @@ class WP_Test_REST_Autosaves_Controller extends WP_Test_REST_Post_Type_Controlle $this->assertEqualSets( array( 'view', 'edit', 'embed' ), $data['endpoints'][0]['args']['context']['enum'] ); } + public function test_registered_query_params() { + $request = new WP_REST_Request( 'OPTIONS', '/wp/v2/posts/' . self::$post_id . '/autosaves' ); + $response = $this->server->dispatch( $request ); + $data = $response->get_data(); + $keys = array_keys( $data['endpoints'][0]['args'] ); + sort( $keys ); + $this->assertEquals( array( + 'context', + 'parent', + ), $keys ); + } + public function test_get_items() { wp_set_current_user( self::$editor_id ); $request = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/autosaves' ); @@ -517,4 +553,40 @@ class WP_Test_REST_Autosaves_Controller extends WP_Test_REST_Post_Type_Controlle $this->assertEquals( $parent_post_id, self::$post_id ); } + public function test_update_item_draft_page_with_parent() { + wp_set_current_user( self::$editor_id ); + $request = new WP_REST_Request( 'POST', '/wp/v2/pages/' . self::$child_draft_page_id . '/autosaves' ); + $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); + + $params = $this->set_post_data( + array( + 'id' => self::$child_draft_page_id, + 'author' => self::$editor_id, + ) + ); + + $request->set_body_params( $params ); + $response = rest_get_server()->dispatch( $request ); + $data = $response->get_data(); + + $this->assertEquals( self::$child_draft_page_id, $data['id'] ); + $this->assertEquals( self::$parent_page_id, $data['parent'] ); + } + + public function test_schema_validation_is_applied() { + wp_set_current_user( self::$editor_id ); + + $request = new WP_REST_Request( 'POST', '/wp/v2/pages/' . self::$draft_page_id . '/autosaves' ); + $request->add_header( 'content-type', 'application/x-www-form-urlencoded' ); + + $params = $this->set_post_data( array( + 'id' => self::$draft_page_id, + 'comment_status' => 'garbage', + ) ); + + $request->set_body_params( $params ); + + $response = rest_get_server()->dispatch( $request ); + $this->assertNotEquals( 'garbage', get_post( self::$draft_page_id )->comment_status ); + } } diff --git a/tests/phpunit/tests/rest-api/rest-schema-setup.php b/tests/phpunit/tests/rest-api/rest-schema-setup.php index 7672b373d9..1d1b7b971f 100644 --- a/tests/phpunit/tests/rest-api/rest-schema-setup.php +++ b/tests/phpunit/tests/rest-api/rest-schema-setup.php @@ -89,19 +89,19 @@ class WP_Test_REST_Schema_Initialization extends WP_Test_REST_TestCase { '/wp/v2/posts/(?P[\\d]+)', '/wp/v2/posts/(?P[\\d]+)/revisions', '/wp/v2/posts/(?P[\\d]+)/revisions/(?P[\\d]+)', - '/wp/v2/posts/(?P[\\d]+)/autosaves', + '/wp/v2/posts/(?P[\\d]+)/autosaves', '/wp/v2/posts/(?P[\\d]+)/autosaves/(?P[\\d]+)', '/wp/v2/pages', '/wp/v2/pages/(?P[\\d]+)', '/wp/v2/pages/(?P[\\d]+)/revisions', '/wp/v2/pages/(?P[\\d]+)/revisions/(?P[\\d]+)', - '/wp/v2/pages/(?P[\\d]+)/autosaves', + '/wp/v2/pages/(?P[\\d]+)/autosaves', '/wp/v2/pages/(?P[\\d]+)/autosaves/(?P[\\d]+)', '/wp/v2/media', '/wp/v2/media/(?P[\\d]+)', '/wp/v2/blocks', '/wp/v2/blocks/(?P[\d]+)', - '/wp/v2/blocks/(?P[\d]+)/autosaves', + '/wp/v2/blocks/(?P[\d]+)/autosaves', '/wp/v2/blocks/(?P[\d]+)/autosaves/(?P[\d]+)', '/wp/v2/types', '/wp/v2/types/(?P[\\w-]+)', diff --git a/tests/qunit/fixtures/wp-api-generated.js b/tests/qunit/fixtures/wp-api-generated.js index 092896b08d..eaf5c754c1 100644 --- a/tests/qunit/fixtures/wp-api-generated.js +++ b/tests/qunit/fixtures/wp-api-generated.js @@ -850,7 +850,7 @@ mockedApiResponse.Schema = { } ] }, - "/wp/v2/posts/(?P[\\d]+)/autosaves": { + "/wp/v2/posts/(?P[\\d]+)/autosaves": { "namespace": "wp/v2", "methods": [ "GET", @@ -877,70 +877,6 @@ mockedApiResponse.Schema = { ], "description": "Scope under which the request is made; determines fields present in response.", "type": "string" - }, - "page": { - "required": false, - "default": 1, - "description": "Current page of the collection.", - "type": "integer" - }, - "per_page": { - "required": false, - "description": "Maximum number of items to be returned in result set.", - "type": "integer" - }, - "search": { - "required": false, - "description": "Limit results to those matching a string.", - "type": "string" - }, - "exclude": { - "required": false, - "default": [], - "description": "Ensure result set excludes specific IDs.", - "type": "array", - "items": { - "type": "integer" - } - }, - "include": { - "required": false, - "default": [], - "description": "Limit result set to specific IDs.", - "type": "array", - "items": { - "type": "integer" - } - }, - "offset": { - "required": false, - "description": "Offset the result set by a specific number of items.", - "type": "integer" - }, - "order": { - "required": false, - "default": "desc", - "enum": [ - "asc", - "desc" - ], - "description": "Order sort attribute ascending or descending.", - "type": "string" - }, - "orderby": { - "required": false, - "default": "date", - "enum": [ - "date", - "id", - "include", - "relevance", - "slug", - "include_slugs", - "title" - ], - "description": "Sort collection by object attribute.", - "type": "string" } } }, @@ -954,11 +890,6 @@ mockedApiResponse.Schema = { "description": "The ID for the parent of the object.", "type": "integer" }, - "author": { - "required": false, - "description": "The ID for the author of the object.", - "type": "integer" - }, "date": { "required": false, "description": "The date the object was published, in the site's timezone.", @@ -969,26 +900,28 @@ mockedApiResponse.Schema = { "description": "The date the object was published, as GMT.", "type": "string" }, - "id": { - "required": false, - "description": "Unique identifier for the object.", - "type": "integer" - }, - "modified": { - "required": false, - "description": "The date the object was last modified, in the site's timezone.", - "type": "string" - }, - "modified_gmt": { - "required": false, - "description": "The date the object was last modified, as GMT.", - "type": "string" - }, "slug": { "required": false, "description": "An alphanumeric identifier for the object unique to its type.", "type": "string" }, + "status": { + "required": false, + "enum": [ + "publish", + "future", + "draft", + "pending", + "private" + ], + "description": "A named status for the object.", + "type": "string" + }, + "password": { + "required": false, + "description": "A password to protect access to the content and excerpt.", + "type": "string" + }, "title": { "required": false, "description": "The title for the object.", @@ -999,10 +932,86 @@ mockedApiResponse.Schema = { "description": "The content for the object.", "type": "object" }, + "author": { + "required": false, + "description": "The ID for the author of the object.", + "type": "integer" + }, "excerpt": { "required": false, "description": "The excerpt for the object.", "type": "object" + }, + "featured_media": { + "required": false, + "description": "The ID of the featured media for the object.", + "type": "integer" + }, + "comment_status": { + "required": false, + "enum": [ + "open", + "closed" + ], + "description": "Whether or not comments are open on the object.", + "type": "string" + }, + "ping_status": { + "required": false, + "enum": [ + "open", + "closed" + ], + "description": "Whether or not the object can be pinged.", + "type": "string" + }, + "format": { + "required": false, + "enum": [ + "standard", + "aside", + "chat", + "gallery", + "link", + "image", + "quote", + "status", + "video", + "audio" + ], + "description": "The format for the object.", + "type": "string" + }, + "meta": { + "required": false, + "description": "Meta fields.", + "type": "object" + }, + "sticky": { + "required": false, + "description": "Whether or not the object should be treated as sticky.", + "type": "boolean" + }, + "template": { + "required": false, + "description": "The theme file to use to display the object.", + "type": "string" + }, + "categories": { + "required": false, + "description": "The terms assigned to the object in the category taxonomy.", + "type": "array", + "items": { + "type": "integer" + } + }, + "tags": { + "required": false, + "description": "The terms assigned to the object in the post_tag taxonomy.", + "type": "array", + "items": { + "type": "integer" + } } } } @@ -1650,7 +1659,7 @@ mockedApiResponse.Schema = { } ] }, - "/wp/v2/pages/(?P[\\d]+)/autosaves": { + "/wp/v2/pages/(?P[\\d]+)/autosaves": { "namespace": "wp/v2", "methods": [ "GET", @@ -1677,70 +1686,6 @@ mockedApiResponse.Schema = { ], "description": "Scope under which the request is made; determines fields present in response.", "type": "string" - }, - "page": { - "required": false, - "default": 1, - "description": "Current page of the collection.", - "type": "integer" - }, - "per_page": { - "required": false, - "description": "Maximum number of items to be returned in result set.", - "type": "integer" - }, - "search": { - "required": false, - "description": "Limit results to those matching a string.", - "type": "string" - }, - "exclude": { - "required": false, - "default": [], - "description": "Ensure result set excludes specific IDs.", - "type": "array", - "items": { - "type": "integer" - } - }, - "include": { - "required": false, - "default": [], - "description": "Limit result set to specific IDs.", - "type": "array", - "items": { - "type": "integer" - } - }, - "offset": { - "required": false, - "description": "Offset the result set by a specific number of items.", - "type": "integer" - }, - "order": { - "required": false, - "default": "desc", - "enum": [ - "asc", - "desc" - ], - "description": "Order sort attribute ascending or descending.", - "type": "string" - }, - "orderby": { - "required": false, - "default": "date", - "enum": [ - "date", - "id", - "include", - "relevance", - "slug", - "include_slugs", - "title" - ], - "description": "Sort collection by object attribute.", - "type": "string" } } }, @@ -1754,11 +1699,6 @@ mockedApiResponse.Schema = { "description": "The ID for the parent of the object.", "type": "integer" }, - "author": { - "required": false, - "description": "The ID for the author of the object.", - "type": "integer" - }, "date": { "required": false, "description": "The date the object was published, in the site's timezone.", @@ -1769,26 +1709,28 @@ mockedApiResponse.Schema = { "description": "The date the object was published, as GMT.", "type": "string" }, - "id": { - "required": false, - "description": "Unique identifier for the object.", - "type": "integer" - }, - "modified": { - "required": false, - "description": "The date the object was last modified, in the site's timezone.", - "type": "string" - }, - "modified_gmt": { - "required": false, - "description": "The date the object was last modified, as GMT.", - "type": "string" - }, "slug": { "required": false, "description": "An alphanumeric identifier for the object unique to its type.", "type": "string" }, + "status": { + "required": false, + "enum": [ + "publish", + "future", + "draft", + "pending", + "private" + ], + "description": "A named status for the object.", + "type": "string" + }, + "password": { + "required": false, + "description": "A password to protect access to the content and excerpt.", + "type": "string" + }, "title": { "required": false, "description": "The title for the object.", @@ -1799,10 +1741,53 @@ mockedApiResponse.Schema = { "description": "The content for the object.", "type": "object" }, + "author": { + "required": false, + "description": "The ID for the author of the object.", + "type": "integer" + }, "excerpt": { "required": false, "description": "The excerpt for the object.", "type": "object" + }, + "featured_media": { + "required": false, + "description": "The ID of the featured media for the object.", + "type": "integer" + }, + "comment_status": { + "required": false, + "enum": [ + "open", + "closed" + ], + "description": "Whether or not comments are open on the object.", + "type": "string" + }, + "ping_status": { + "required": false, + "enum": [ + "open", + "closed" + ], + "description": "Whether or not the object can be pinged.", + "type": "string" + }, + "menu_order": { + "required": false, + "description": "The order of the object in relation to other object of its type.", + "type": "integer" + }, + "meta": { + "required": false, + "description": "Meta fields.", + "type": "object" + }, + "template": { + "required": false, + "description": "The theme file to use to display the object.", + "type": "string" } } } @@ -2584,7 +2569,7 @@ mockedApiResponse.Schema = { } ] }, - "/wp/v2/blocks/(?P[\\d]+)/autosaves": { + "/wp/v2/blocks/(?P[\\d]+)/autosaves": { "namespace": "wp/v2", "methods": [ "GET", @@ -2611,70 +2596,6 @@ mockedApiResponse.Schema = { ], "description": "Scope under which the request is made; determines fields present in response.", "type": "string" - }, - "page": { - "required": false, - "default": 1, - "description": "Current page of the collection.", - "type": "integer" - }, - "per_page": { - "required": false, - "description": "Maximum number of items to be returned in result set.", - "type": "integer" - }, - "search": { - "required": false, - "description": "Limit results to those matching a string.", - "type": "string" - }, - "exclude": { - "required": false, - "default": [], - "description": "Ensure result set excludes specific IDs.", - "type": "array", - "items": { - "type": "integer" - } - }, - "include": { - "required": false, - "default": [], - "description": "Limit result set to specific IDs.", - "type": "array", - "items": { - "type": "integer" - } - }, - "offset": { - "required": false, - "description": "Offset the result set by a specific number of items.", - "type": "integer" - }, - "order": { - "required": false, - "default": "desc", - "enum": [ - "asc", - "desc" - ], - "description": "Order sort attribute ascending or descending.", - "type": "string" - }, - "orderby": { - "required": false, - "default": "date", - "enum": [ - "date", - "id", - "include", - "relevance", - "slug", - "include_slugs", - "title" - ], - "description": "Sort collection by object attribute.", - "type": "string" } } }, @@ -2688,11 +2609,6 @@ mockedApiResponse.Schema = { "description": "The ID for the parent of the object.", "type": "integer" }, - "author": { - "required": false, - "description": "The ID for the author of the object.", - "type": "integer" - }, "date": { "required": false, "description": "The date the object was published, in the site's timezone.", @@ -2703,26 +2619,28 @@ mockedApiResponse.Schema = { "description": "The date the object was published, as GMT.", "type": "string" }, - "id": { - "required": false, - "description": "Unique identifier for the object.", - "type": "integer" - }, - "modified": { - "required": false, - "description": "The date the object was last modified, in the site's timezone.", - "type": "string" - }, - "modified_gmt": { - "required": false, - "description": "The date the object was last modified, as GMT.", - "type": "string" - }, "slug": { "required": false, "description": "An alphanumeric identifier for the object unique to its type.", "type": "string" }, + "status": { + "required": false, + "enum": [ + "publish", + "future", + "draft", + "pending", + "private" + ], + "description": "A named status for the object.", + "type": "string" + }, + "password": { + "required": false, + "description": "A password to protect access to the content and excerpt.", + "type": "string" + }, "title": { "required": false, "description": "The title for the object.", @@ -2732,6 +2650,11 @@ mockedApiResponse.Schema = { "required": false, "description": "The content for the object.", "type": "object" + }, + "template": { + "required": false, + "description": "The theme file to use to display the object.", + "type": "string" } } }