From 74cc64d74e6fbe62c4911ccfe16ddcb5ccbda812 Mon Sep 17 00:00:00 2001 From: Timothy Jacobs Date: Tue, 21 Jul 2020 12:01:10 +0000 Subject: [PATCH] REST API: Issue a _doing_it_wrong when registering a route without a permission callback. The REST API treats routes without a permission_callback as public. Because this happens without any warning to the user, if the permission callback is unintentionally omitted or misspelled, the endpoint can end up being available to the public. Such a scenario has happened multiple times in the wild, and the results can be catostrophic when it occurs. For REST API routes that are intended to be public, it is recommended to set the permission callback to the `__return_true` built in function. Fixes #50075. Props rmccue, sorenbronsted, whyisjake, SergeyBiryukov, TimothyBlynJacobs. git-svn-id: https://develop.svn.wordpress.org/trunk@48526 602fd350-edb4-49c9-b593-d223f7449a82 --- .../class-wp-oembed-controller.php | 7 +- src/wp-includes/rest-api.php | 14 ++ .../class-wp-rest-post-types-controller.php | 7 +- .../class-wp-rest-users-controller.php | 7 +- tests/phpunit/tests/rest-api.php | 147 +++++++++++++----- tests/phpunit/tests/rest-api/rest-server.php | 105 +++++++------ 6 files changed, 199 insertions(+), 88 deletions(-) diff --git a/src/wp-includes/class-wp-oembed-controller.php b/src/wp-includes/class-wp-oembed-controller.php index 3ab621408e..912e2d131b 100644 --- a/src/wp-includes/class-wp-oembed-controller.php +++ b/src/wp-includes/class-wp-oembed-controller.php @@ -36,9 +36,10 @@ final class WP_oEmbed_Controller { '/embed', array( array( - 'methods' => WP_REST_Server::READABLE, - 'callback' => array( $this, 'get_item' ), - 'args' => array( + 'methods' => WP_REST_Server::READABLE, + 'callback' => array( $this, 'get_item' ), + 'permission_callback' => '__return_true', + 'args' => array( 'url' => array( 'description' => __( 'The URL of the resource for which to fetch oEmbed data.' ), 'required' => true, diff --git a/src/wp-includes/rest-api.php b/src/wp-includes/rest-api.php index d3bb80a3d6..e9a5799b60 100644 --- a/src/wp-includes/rest-api.php +++ b/src/wp-includes/rest-api.php @@ -88,6 +88,20 @@ function register_rest_route( $namespace, $route, $args = array(), $override = f $arg_group = array_merge( $defaults, $arg_group ); $arg_group['args'] = array_merge( $common_args, $arg_group['args'] ); + + if ( ! isset( $arg_group['permission_callback'] ) ) { + _doing_it_wrong( + __FUNCTION__, + sprintf( + /* translators: 1. The REST API route being registered. 2. The argument name. 3. The suggested function name. */ + __( 'The REST API route definition for %1$s is missing the required %2$s argument. For REST API routes that are intended to be public, use %3$s as the permission callback.', 'LION' ), + '' . $clean_namespace . '/' . trim( $route, '/' ) . '', + 'permission_callback', + '__return_true' + ), + '5.5.0' + ); + } } $full_route = '/' . $clean_namespace . '/' . trim( $route, '/' ); diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-post-types-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-post-types-controller.php index fd9759d79e..938697157b 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-post-types-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-post-types-controller.php @@ -60,9 +60,10 @@ class WP_REST_Post_Types_Controller extends WP_REST_Controller { ), ), array( - 'methods' => WP_REST_Server::READABLE, - 'callback' => array( $this, 'get_item' ), - 'args' => array( + 'methods' => WP_REST_Server::READABLE, + 'callback' => array( $this, 'get_item' ), + 'permission_callback' => '__return_true', + 'args' => array( 'context' => $this->get_context_param( array( 'default' => 'view' ) ), ), ), diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php index 88d233d4af..2320668222 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php @@ -116,9 +116,10 @@ class WP_REST_Users_Controller extends WP_REST_Controller { '/' . $this->rest_base . '/me', array( array( - 'methods' => WP_REST_Server::READABLE, - 'callback' => array( $this, 'get_current_item' ), - 'args' => array( + 'methods' => WP_REST_Server::READABLE, + 'permission_callback' => '__return_true', + 'callback' => array( $this, 'get_current_item' ), + 'args' => array( 'context' => $this->get_context_param( array( 'default' => 'view' ) ), ), ), diff --git a/tests/phpunit/tests/rest-api.php b/tests/phpunit/tests/rest-api.php index 789c4e19b3..123e872f07 100644 --- a/tests/phpunit/tests/rest-api.php +++ b/tests/phpunit/tests/rest-api.php @@ -84,8 +84,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -120,12 +121,14 @@ class Tests_REST_API extends WP_UnitTestCase { '/test', array( array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ), array( - 'methods' => array( 'POST' ), - 'callback' => '__return_null', + 'methods' => array( 'POST' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ), ) ); @@ -160,16 +163,18 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); register_rest_route( 'test-ns', '/test', array( - 'methods' => array( 'POST' ), - 'callback' => '__return_null', + 'methods' => array( 'POST' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -187,18 +192,20 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', - 'should_exist' => false, + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'should_exist' => false, ) ); register_rest_route( 'test-ns', '/test', array( - 'methods' => array( 'POST' ), - 'callback' => '__return_null', - 'should_exist' => true, + 'methods' => array( 'POST' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'should_exist' => true, ), true ); @@ -223,8 +230,9 @@ class Tests_REST_API extends WP_UnitTestCase { '', '/test-empty-namespace', array( - 'methods' => array( 'POST' ), - 'callback' => '__return_null', + 'methods' => array( 'POST' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ), true ); @@ -242,8 +250,9 @@ class Tests_REST_API extends WP_UnitTestCase { '/test-empty-route', '', array( - 'methods' => array( 'POST' ), - 'callback' => '__return_null', + 'methods' => array( 'POST' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ), true ); @@ -264,8 +273,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -282,8 +292,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => 'GET', - 'callback' => '__return_null', + 'methods' => 'GET', + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -300,8 +311,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET', 'POST' ), - 'callback' => '__return_null', + 'methods' => array( 'GET', 'POST' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -324,8 +336,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => 'GET,POST', - 'callback' => '__return_null', + 'methods' => 'GET,POST', + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -345,8 +358,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => 'GET,POST', - 'callback' => '__return_null', + 'methods' => 'GET,POST', + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -367,8 +381,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => 'GET,POST', - 'callback' => '__return_true', + 'methods' => 'GET,POST', + 'callback' => '__return_true', + 'permission_callback' => '__return_true', ) ); @@ -910,8 +925,9 @@ class Tests_REST_API extends WP_UnitTestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ) ); @@ -996,7 +1012,8 @@ class Tests_REST_API extends WP_UnitTestCase { '/my-namespace/v1/', '/my-route', array( - 'callback' => '__return_true', + 'callback' => '__return_true', + 'permission_callback' => '__return_true', ) ); @@ -1006,6 +1023,66 @@ class Tests_REST_API extends WP_UnitTestCase { $this->assertTrue( rest_do_request( '/my-namespace/v1/my-route' )->get_data() ); } + /** + * @ticket 50075 + */ + public function test_register_route_with_missing_permission_callback_top_level_route() { + $this->setExpectedIncorrectUsage( 'register_rest_route' ); + + $registered = register_rest_route( + 'my-ns/v1', + '/my-route', + array( + 'callback' => '__return_true', + ) + ); + + $this->assertTrue( $registered ); + } + + /** + * @ticket 50075 + */ + public function test_register_route_with_missing_permission_callback_single_wrapped_route() { + $this->setExpectedIncorrectUsage( 'register_rest_route' ); + + $registered = register_rest_route( + 'my-ns/v1', + '/my-route', + array( + array( + 'callback' => '__return_true', + ), + ) + ); + + $this->assertTrue( $registered ); + } + + + /** + * @ticket 50075 + */ + public function test_register_route_with_missing_permission_callback_multiple_wrapped_route() { + $this->setExpectedIncorrectUsage( 'register_rest_route' ); + + $registered = register_rest_route( + 'my-ns/v1', + '/my-route', + array( + array( + 'callback' => '__return_true', + ), + array( + 'callback' => '__return_true', + 'permission_callback' => '__return_true', + ), + ) + ); + + $this->assertTrue( $registered ); + } + public function _dp_rest_filter_response_by_context() { return array( 'default' => array( diff --git a/tests/phpunit/tests/rest-api/rest-server.php b/tests/phpunit/tests/rest-api/rest-server.php index 8f178bff2b..5870e8cb07 100644 --- a/tests/phpunit/tests/rest-api/rest-server.php +++ b/tests/phpunit/tests/rest-api/rest-server.php @@ -66,9 +66,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', - 'args' => array( + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'args' => array( 'foo' => array( 'default' => 'bar', ), @@ -88,9 +89,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', - 'args' => array( + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'args' => array( 'foo' => array( 'default' => 'bar', ), @@ -110,9 +112,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'optional', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', - 'args' => array( + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'args' => array( 'foo' => array(), ), ) @@ -131,9 +134,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'no-zero', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', - 'args' => array( + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'args' => array( 'foo' => array( 'default' => 'bar', ), @@ -150,8 +154,9 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'head-request', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_true', + 'methods' => array( 'GET' ), + 'callback' => '__return_true', + 'permission_callback' => '__return_true', ) ); $request = new WP_REST_Request( 'HEAD', '/head-request/test' ); @@ -171,8 +176,9 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { '/test', array( array( - 'methods' => array( 'HEAD' ), - 'callback' => '__return_true', + 'methods' => array( 'HEAD' ), + 'callback' => '__return_true', + 'permission_callback' => '__return_true', ), array( 'methods' => array( 'GET' ), @@ -193,9 +199,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { '/test/(?P.*)', array( array( - 'methods' => WP_REST_Server::READABLE, - 'callback' => '__return_false', - 'args' => array( + 'methods' => WP_REST_Server::READABLE, + 'callback' => '__return_false', + 'permission_callback' => '__return_true', + 'args' => array( 'data' => array(), ), ), @@ -266,9 +273,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => 'GET', - 'callback' => '__return_null', - 'should_exist' => false, + 'methods' => 'GET', + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'should_exist' => false, ) ); @@ -293,9 +301,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => 'GET', - 'callback' => '__return_null', - 'should_exist' => false, + 'methods' => 'GET', + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'should_exist' => false, ) ); @@ -303,9 +312,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => 'POST', - 'callback' => '__return_null', - 'should_exist' => false, + 'methods' => 'POST', + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'should_exist' => false, ) ); @@ -342,9 +352,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => 'POST', - 'callback' => '__return_null', - 'should_exist' => false, + 'methods' => 'POST', + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'should_exist' => false, ) ); @@ -365,8 +376,9 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { '/test', array( array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', ), array( 'methods' => array( 'POST' ), @@ -1327,9 +1339,10 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => '__return_null', - 'args' => array( + 'methods' => array( 'GET' ), + 'callback' => '__return_null', + 'permission_callback' => '__return_true', + 'args' => array( 'someinteger' => array( 'validate_callback' => array( $this, '_validate_as_integer_123' ), 'sanitize_callback' => 'absint', @@ -1362,10 +1375,11 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => function () { + 'methods' => array( 'GET' ), + 'callback' => function () { return new WP_REST_Response(); }, + 'permission_callback' => '__return_true', ) ); @@ -1383,10 +1397,11 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => function () { + 'methods' => array( 'GET' ), + 'callback' => function () { return new WP_REST_Response( 'data', 204 ); }, + 'permission_callback' => '__return_true', ) ); @@ -1473,20 +1488,22 @@ class Tests_REST_Server extends WP_Test_REST_TestCase { 'test-ns', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => function() { + 'methods' => array( 'GET' ), + 'callback' => function() { return new WP_REST_Response( 'data', 204 ); }, + 'permission_callback' => '__return_true', ) ); register_rest_route( 'test-ns/v1', '/test', array( - 'methods' => array( 'GET' ), - 'callback' => function() { + 'methods' => array( 'GET' ), + 'callback' => function() { return new WP_REST_Response( 'data', 204 ); }, + 'permission_callback' => '__return_true', ) );