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

Block Bindings: Don't show protected fields that are bound to blocks and post meta #6197

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
105ffe1
Check if the meta field is protected
SantosGuillamot Feb 28, 2024
00b4d65
Check if the meta field is available in the REST API
SantosGuillamot Feb 28, 2024
46a29e4
Remove unnecessary `show_in_rest` conditional
SantosGuillamot Feb 28, 2024
0b106f9
Use `source_args` instead of `source_attrs`
SantosGuillamot Feb 28, 2024
1736c83
Add post meta bindings source tests
SantosGuillamot Feb 28, 2024
1fb2c47
Unregister only the meta fields containing "tests"
SantosGuillamot Feb 28, 2024
81d4e34
Add test for password protected posts
SantosGuillamot Feb 28, 2024
8d0a0be
Add test for posts that are not viewable
SantosGuillamot Feb 28, 2024
ce4aa0c
Move set up tp `wpSetUpBeforeClass`
SantosGuillamot Feb 29, 2024
f1655d6
Add test for non existing meta key
SantosGuillamot Feb 29, 2024
01d45e5
Remove filters in tests
SantosGuillamot Feb 29, 2024
7d25302
Refactor the way post content is modified
SantosGuillamot Feb 29, 2024
dfe02a5
Add test to check unsafe html is sanitized
SantosGuillamot Feb 29, 2024
6414a97
Move unsafe html test to render bindings
SantosGuillamot Feb 29, 2024
193131f
Add post meta unsafe html test again
SantosGuillamot Feb 29, 2024
a0b761b
Change how we modify the global $post variable
SantosGuillamot Feb 29, 2024
4cdf241
Use snake_case
SantosGuillamot Feb 29, 2024
6e37a4c
Use hooks helper function
SantosGuillamot Feb 29, 2024
8389270
Remove outdated function
SantosGuillamot Feb 29, 2024
953058d
Update class methods descriptions
SantosGuillamot Feb 29, 2024
bc81da4
Unset global post variable after class
SantosGuillamot Feb 29, 2024
64f0784
Add tests for bindings modifying html attribute
SantosGuillamot Feb 29, 2024
56beecc
Apply suggestions from code review
swissspidy Mar 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/wp-includes/block-bindings/post-meta.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ function _block_bindings_post_meta_get_value( array $source_args, $block_instanc
return null;
}

// Check if the meta field is protected.
if ( is_protected_meta( $source_args['key'], 'post' ) ) {
return null;
}

// Check if the meta field is registered to be shown in REST.
$meta_keys = get_registered_meta_keys( 'post', $block_instance->context['postType'] );
// Add fields registered for all subtypes.
$meta_keys = array_merge( $meta_keys, get_registered_meta_keys( 'post', '' ) );
if ( empty( $meta_keys[ $source_args['key'] ]['show_in_rest'] ) ) {
return null;
}

return get_post_meta( $post_id, $source_args['key'], true );
}

Expand Down
269 changes: 269 additions & 0 deletions tests/phpunit/tests/block-bindings/postMetaSource.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
<?php
/**
* Tests for Block Bindings API "core/post-meta" source.
*
* @package WordPress
* @subpackage Blocks
* @since 6.5.0
*
* @group blocks
* @group block-bindings
*/
class Tests_Block_Bindings_Post_Meta_Source extends WP_UnitTestCase {
protected static $post;
protected static $wp_meta_keys_saved;

/**
* Modify the post content.
*
* @param string $content The new content.
*/
private function get_modified_post_content( $content ) {
$GLOBALS['post']->post_content = $content;
return apply_filters( 'the_content', $GLOBALS['post']->post_content );
}

/**
* Sets up shared fixtures.
*/
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
self::$post = $factory->post->create_and_get();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per my other comment, the static $post variable is redundant if you make it global anyway.

self::$wp_meta_keys_saved = isset( $GLOBALS['wp_meta_keys'] ) ? $GLOBALS['wp_meta_keys'] : array();
}

/**
* Tear down after class.
*/
public static function wpTearDownAfterClass() {
$GLOBALS['wp_meta_keys'] = self::$wp_meta_keys_saved;
SantosGuillamot marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Set up before each test.
*
* @since 6.5.0
*/
public function set_up() {
parent::set_up();
// Needed because tear_down() will reset it between tests.
$GLOBALS['post'] = self::$post;
}

/**
* Tests that a block connected to a custom field renders its value.
*
* @ticket 60651
*/
public function test_custom_field_value_is_rendered() {
register_meta(
'post',
'tests_custom_field',
array(
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'default' => 'Custom field value',
)
);

$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"tests_custom_field"}}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );
$this->assertSame(
'<p>Custom field value</p>',
$content,
'The post content should show the value of the custom field . '
);
}

/**
* Tests that an html attribute connected to a custom field renders its value.
*
* @ticket 60651
*/
public function test_html_attribute_connected_to_custom_field_value_is_rendered() {
register_meta(
'post',
'tests_url_custom_field',
array(
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'default' => 'https://example.com/foo.png',
)
);

$content = $this->get_modified_post_content( '<!-- wp:image {"metadata":{"bindings":{"url":{"source":"core/post-meta","args":{"key":"tests_url_custom_field"}}}}} --><figure class="wp-block-image"><img alt=""/></figure><!-- /wp:image -->' );
$this->assertSame(
'<figure class="wp-block-image"><img decoding="async" src="https://example.com/foo.png" alt=""/></figure>',
$content,
'The image src should point to the value of the custom field . '
);
}

/**
* Tests that a blocks connected in a password protected post don't render the value.
*
* @ticket 60651
*/
public function test_custom_field_value_is_not_shown_in_password_protected_posts() {
register_meta(
'post',
'tests_custom_field',
array(
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'default' => 'Custom field value',
)
);

add_filter( 'post_password_required', '__return_true' );

$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"tests_custom_field"}}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );

remove_filter( 'post_password_required', '__return_true' );

$this->assertSame(
'<p>Fallback value</p>',
$content,
'The post content should show the fallback value instead of the custom field value.'
);
}

/**
* Tests that a blocks connected in a post that is not publicly viewable don't render the value.
*
* @ticket 60651
*/
public function test_custom_field_value_is_not_shown_in_non_viewable_posts() {
register_meta(
'post',
'tests_custom_field',
array(
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'default' => 'Custom field value',
)
);

add_filter( 'is_post_status_viewable', '__return_false' );

$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"tests_custom_field"}}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );

remove_filter( 'is_post_status_viewable', '__return_false' );

$this->assertSame(
'<p>Fallback value</p>',
$content,
'The post content should show the fallback value instead of the custom field value.'
);
}

/**
* Tests that a block connected to a meta key that doesn't exist renders the fallback.
*
* @ticket 60651
*/
public function test_binding_to_non_existing_meta_key() {
$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"tests_non_existing_field"}}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );

$this->assertSame(
'<p>Fallback value</p>',
$content,
'The post content should show the fallback value.'
);
}

/**
* Tests that a block connected without specifying the custom field renders the fallback.
*
* @ticket 60651
*/
public function test_binding_without_key_renders_the_fallback() {
$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta"}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );

$this->assertSame(
'<p>Fallback value</p>',
$content,
'The post content should show the fallback value.'
);
}

/**
* Tests that a block connected to a protected field doesn't show the value.
*
* @ticket 60651
*/
public function test_protected_field_value_is_not_shown() {
register_meta(
'post',
'_tests_protected_field',
array(
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'default' => 'Protected value',
)
);

$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"_tests_protected_field"}}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );

$this->assertSame(
'<p>Fallback value</p>',
$content,
'The post content should show the fallback value instead of the protected value.'
);
}

/**
* Tests that a block connected to a field not exposed in the REST API doesn't show the value.
*
* @ticket 60651
*/
public function test_custom_field_not_exposed_in_rest_api_is_not_shown() {
register_meta(
'post',
'tests_show_in_rest_false_field',
array(
'show_in_rest' => false,
'single' => true,
'type' => 'string',
'default' => 'Protected value',
)
);

$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"tests_show_in_rest_false_field"}}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );

$this->assertSame(
'<p>Fallback value</p>',
$content,
'The post content should show the fallback value instead of the protected value.'
);
}

/**
* Tests that meta key with unsafe HTML is sanitized.
*
* @ticket 60651
*/
public function test_custom_field_with_unsafe_html_is_sanitized() {
register_meta(
'post',
'tests_unsafe_html_field',
array(
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'default' => '<script>alert("Unsafe HTML")</script>',
)
);

$content = $this->get_modified_post_content( '<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"core/post-meta","args":{"key":"tests_unsafe_html_field"}}}}} --><p>Fallback value</p><!-- /wp:paragraph -->' );

$this->assertSame(
'<p>alert(&#8220;Unsafe HTML&#8221;)</p>',
$content,
'The post content should not include the script tag.'
);
}
}
36 changes: 36 additions & 0 deletions tests/phpunit/tests/block-bindings/render.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,40 @@ public function test_update_block_with_value_from_source_image_placeholder() {
'The block content should be updated with the value returned by the source.'
);
}

/**
* Tests if the block content is sanitized when unsafe HTML is passed.
*
* @ticket 60651
*
* @covers ::register_block_bindings_source
*/
public function test_source_value_with_unsafe_html_is_sanitized() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add another test for a Button block where we put something tricky as the link's URL. Example: javascript:alert('Unsafe HTML'). I'm not sure if that is the best one, but that's definitely how someone could run JS by updating the dynamic source.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering what if we should handle that case in the bindings because passing javascript:alert('1') to the href it is actually valid HTML. We would only be consuming the value stored in the custom field and exposed in the REST API. Not sure if it should be checked in the block bindings API, as part of the meta fields registration, or as part of the button block.

$get_value_callback = function () {
return '<script>alert("Unsafe HTML")</script>';
};

register_block_bindings_source(
self::SOURCE_NAME,
array(
'label' => self::SOURCE_LABEL,
'get_value_callback' => $get_value_callback,
)
);

$block_content = <<<HTML
<!-- wp:paragraph {"metadata":{"bindings":{"content":{"source":"test/source"}}}} -->
<p>This should not appear</p>
<!-- /wp:paragraph -->
HTML;
$parsed_blocks = parse_blocks( $block_content );
$block = new WP_Block( $parsed_blocks[0] );
$result = $block->render();

$this->assertSame(
'<p>alert("Unsafe HTML")</p>',
trim( $result ),
'The block content should be updated with the value returned by the source.'
);
}
}
Loading