Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport: Enforce permission_callback in REST-API #732

Merged
merged 3 commits into from Oct 21, 2021
Merged

Backport: Enforce permission_callback in REST-API #732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4ac5ce
WP-r48526: REST API: Issue a _doing_it_wrong when registering a route…
TimothyBJacobs Aug 30, 2021
220a249
Fix conflicts from 48526
mattyrob Aug 30, 2021
a0760b1
WP-r48571: REST API: Remove textdomain from doing it wrong message.
TimothyBJacobs Aug 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/wp-includes/class-wp-oembed-controller.php
View file
Open in desktop
Expand Up @@ -35,9 +35,10 @@ public function register_routes() {
array(
'methods' => WP_REST_Server::READABLE,
'callback' => array( $this, 'get_item' ),
'permission_callback' => '__return_true',
'args' => array(
'url' => array(
'required' => true,
'description' => __( 'The URL of the resource for which to fetch oEmbed data.' ),'required' => true,
'sanitize_callback' => 'esc_url_raw',
),
'format' => array(
Expand Down
14 changes: 14 additions & 0 deletions src/wp-includes/rest-api.php
View file
Open in desktop
Expand Up @@ -66,6 +66,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.' ),
'<code>' . $clean_namespace . '/' . trim( $route, '/' ) . '</code>',
'<code>permission_callback</code>',
'<code>__return_true</code>'
),
'5.5.0'
mattyrob marked this conversation as resolved.
Show resolved Hide resolved
);
}
}

$full_route = '/' . trim( $namespace, '/' ) . '/' . trim( $route, '/' );
Expand Down
1 change: 1 addition & 0 deletions src/wp-includes/rest-api/endpoints/class-wp-rest-post-types-controller.php
View file
Open in desktop
Expand Up @@ -55,6 +55,7 @@ public function register_routes() {
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' ) ),
),
Expand Down
1 change: 1 addition & 0 deletions src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php
View file
Open in desktop
Expand Up @@ -106,6 +106,7 @@ public function register_routes() {
register_rest_route( $this->namespace, '/' . $this->rest_base . '/me', array(
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' ) ),
Expand Down
19 changes: 18 additions & 1 deletion tests/phpunit/tests/rest-api.php
View file
Open in desktop
Expand Up @@ -80,6 +80,7 @@ public function test_route_canonicalized() {
register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

// Check the route was registered correctly.
Expand Down Expand Up @@ -112,10 +113,12 @@ public function test_route_canonicalized_multiple() {
array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
),
array(
'methods' => array( 'POST' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
),
) );

Expand Down Expand Up @@ -148,10 +151,12 @@ public function test_route_merge() {
register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );
register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'POST' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

// Check both routes exist.
Expand All @@ -167,11 +172,13 @@ public function test_route_override() {
register_rest_route( 'test-ns', '/test', array(
'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',
'permission_callback' => '__return_true',
'should_exist' => true,
), true );

Expand All @@ -194,7 +201,9 @@ public function test_route_reject_empty_namespace() {
register_rest_route( '', '/test-empty-namespace', array(
'methods' => array( 'POST' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
), true );

$endpoints = $GLOBALS['wp_rest_server']->get_routes();
$this->assertFalse( isset( $endpoints['/test-empty-namespace'] ) );
}
Expand All @@ -208,7 +217,9 @@ public function test_route_reject_empty_route() {
register_rest_route( '/test-empty-route', '', array(
'methods' => array( 'POST' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
), true );

$endpoints = $GLOBALS['wp_rest_server']->get_routes();
$this->assertFalse( isset( $endpoints['/test-empty-route'] ) );
}
Expand All @@ -225,6 +236,7 @@ public function test_route_method() {
register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

$routes = $GLOBALS['wp_rest_server']->get_routes();
Expand All @@ -239,6 +251,7 @@ public function test_route_method_string() {
register_rest_route( 'test-ns', '/test', array(
'methods' => 'GET',
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

$routes = $GLOBALS['wp_rest_server']->get_routes();
Expand All @@ -253,6 +266,7 @@ public function test_route_method_array() {
register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET', 'POST' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

$routes = $GLOBALS['wp_rest_server']->get_routes();
Expand All @@ -267,6 +281,7 @@ public function test_route_method_comma_seperated() {
register_rest_route( 'test-ns', '/test', array(
'methods' => 'GET,POST',
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

$routes = $GLOBALS['wp_rest_server']->get_routes();
Expand All @@ -278,6 +293,7 @@ public function test_options_request() {
register_rest_route( 'test-ns', '/test', array(
'methods' => 'GET,POST',
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

$request = new WP_REST_Request( 'OPTIONS', '/test-ns/test' );
Expand All @@ -296,6 +312,7 @@ public function test_options_request_not_options() {
register_rest_route( 'test-ns', '/test', array(
'methods' => 'GET,POST',
'callback' => '__return_true',
'permission_callback' => '__return_true',
) );

$request = new WP_REST_Request( 'GET', '/test-ns/test' );
Expand Down Expand Up @@ -588,8 +605,8 @@ public function test_register_rest_route_without_server() {
register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
) );

$routes = $GLOBALS['wp_rest_server']->get_routes();
$this->assertEquals( $routes['/test-ns/test'][0]['methods'], array( 'GET' => true ) );
}
Expand Down
17 changes: 15 additions & 2 deletions tests/phpunit/tests/rest-api/rest-server.php
View file
Open in desktop
Expand Up @@ -61,10 +61,10 @@ public function test_envelope() {
}

public function test_default_param() {

register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
'args' => array(
'foo' => array(
'default' => 'bar',
Expand All @@ -79,10 +79,10 @@ public function test_default_param() {
}

public function test_default_param_is_overridden() {

register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
'args' => array(
'foo' => array(
'default' => 'bar',
Expand All @@ -101,6 +101,7 @@ public function test_optional_param() {
register_rest_route( 'optional', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
'args' => array(
'foo' => array(),
),
Expand All @@ -118,6 +119,7 @@ public function test_no_zero_param() {
register_rest_route( 'no-zero', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
'args' => array(
'foo' => array(
'default' => 'bar',
Expand All @@ -133,7 +135,9 @@ public function test_head_request_handled_by_get() {
register_rest_route( 'head-request', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_true',
'permission_callback' => '__return_true',
) );

$request = new WP_REST_Request( 'HEAD', '/head-request/test' );
$response = $this->server->dispatch( $request );
$this->assertEquals( 200, $response->get_status() );
Expand All @@ -150,6 +154,7 @@ public function test_explicit_head_callback() {
array(
'methods' => array( 'HEAD' ),
'callback' => '__return_true',
'permission_callback' => '__return_true',
),
array(
'methods' => array( 'GET' ),
Expand All @@ -166,8 +171,10 @@ public function test_url_params_no_numeric_keys() {

$this->server->register_route( 'test', '/test/(?P<data>.*)', array(
array(

'methods' => WP_REST_Server::READABLE,
'callback' => '__return_false',
'permission_callback' => '__return_true',
'args' => array(
'data' => array(),
),
Expand Down Expand Up @@ -229,6 +236,7 @@ function test_allow_header_sent() {
register_rest_route( 'test-ns', '/test', array(
'methods' => 'GET',
'callback' => '__return_null',
'permission_callback' => '__return_true',
'should_exist' => false,
) );

Expand All @@ -252,12 +260,14 @@ function test_allow_header_sent_with_multiple_methods() {
register_rest_route( 'test-ns', '/test', array(
'methods' => 'GET',
'callback' => '__return_null',
'permission_callback' => '__return_true',
'should_exist' => false,
) );

register_rest_route( 'test-ns', '/test', array(
'methods' => 'POST',
'callback' => '__return_null',
'permission_callback' => '__return_true',
'should_exist' => false,
) );

Expand Down Expand Up @@ -289,6 +299,7 @@ function test_allow_header_send_only_permitted_methods() {
register_rest_route( 'test-ns', '/test', array(
'methods' => 'POST',
'callback' => '__return_null',
'permission_callback' => '__return_true',
'should_exist' => false,
) );

Expand All @@ -308,6 +319,7 @@ public function test_allow_header_sent_on_options_request() {
array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
),
array(
'methods' => array( 'POST' ),
Expand Down Expand Up @@ -990,6 +1002,7 @@ public function test_rest_validate_before_sanitization() {
register_rest_route( 'test-ns', '/test', array(
'methods' => array( 'GET' ),
'callback' => '__return_null',
'permission_callback' => '__return_true',
'args' => array(
'someinteger' => array(
'validate_callback' => array( $this, '_validate_as_integer_123' ),
Expand Down
6 changes: 4 additions & 2 deletions tests/qunit/fixtures/wp-api-generated.js
View file
Open in desktop
Expand Up @@ -78,7 +78,8 @@ mockedApiResponse.Schema = {
],
"args": {
"url": {
"required": true
"required": true,
"description": "The URL of the resource for which to fetch oEmbed data."
},
"format": {
"required": false,
Expand Down Expand Up @@ -3444,7 +3445,8 @@ mockedApiResponse.oembed = {
],
"args": {
"url": {
"required": true
"required": true,
"description": "The URL of the resource for which to fetch oEmbed data."
},
"format": {
"required": false,
Expand Down