Opened 8 years ago
Last modified 3 days ago
#44399 new defect (bug)
Add unique capability for oembed
| Reported by: | jason_the_adams | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Embeds | Version: | |
| Severity: | normal | Keywords: | dev-feedback has-patch has-unit-tests |
| Cc: | Focuses: |
Description
Ran into a very specific use case today wherein I have a custom user role with a custom WYSIWYG editor on their profile. The user role has unique capabilities for a couple custom post types with unique capabilities. The user needs to be able to paste a YouTube URL in their profile editor, but I found that it doesn't work.
After some digging, I found that the oEmbed ajax function wp_ajax_parse_embed() checks for current_user_can('edit_posts'). Also, the oEmbed REST API does the same thing in the WP_oEmbed_Controller::wp_ajax_parse_embed() method.
This is a problem for custom post types with custom capabilities. We don't want the user to have the edit_posts permission, but they do have the edit_custom_posts equivalent. While this isn't a problem for current_user_can('edit_post', $post_id) calls, as it uses the post to grab the post object and thereby post object capabilities, the edit_posts primitive check has no context.
Since we can't rely on being able to gather post object context (as, in my case, there may be no post object as we're on the user profile), I propose creating a single (or group) of oEmbed capabilities. Something like create_oembeds.
Wanted to gather some feedback and thoughts from the community before putting together a patch. Let me know what you think! :)
Change History (7)
This ticket was mentioned in Slack in #core by jasontheadams. View the logs.
8 years ago
#4
in reply to: ↑ 3
@
8 years ago
Hi!
Replying to joyously:
Is this ticket reporting a problem with the capability check for embeds or is it proposing a new capability? Or maybe both.
Both. There is an issue with oembed capability checks as they require either a post context or the user to have edit_posts which doesn't fit custom post types with capabilities.
It seems to me that the existing capability check assumes a post context, which your use case has a problem with. Would putting a filter on that capability check work? Are there other places where embeds might be wanted, but checking
edit_postswouldn't work? Like comments or custom widgets?
Regarding the filter, that could work but the issue is that there isn't really any further context that a hook could do anything with. If there's a post, it's working fine; if there's no post, there's not much context.
You're right on with the other scenarios. That's why, along with my last point, I'm suggesting a new capability, similar to upload_files or other one-off capabilities. I think it's good to check if a user has a capability, I just don't think edit_posts is the right one as it impacts far too much else.
Is it just embeds that has this problem? I've seen some other discussions about other places where core checks for a capability that doesn't work in all contexts. (I also have a use case for custom role with custom post type created with a custom editor, and I don't want the user to
edit_posts. This seems likely to be common for sites with user-created content.)
I'm trying to keep the scope of this issue to just embeds. I think other scenarios are worth considering, but I don't think there's a fundamental issue with the capability system, just niche situations that could use some tweaking.
---
Thanks for chiming in! I hope this helps clarify my thinking. :)
This ticket was mentioned in PR #14124 on WordPress/wordpress-develop by @abhishekfdd.
3 days ago
#6
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
Adds an embed_url meta capability and uses it in the two places that decide whether a user can resolve an embed:
wp_ajax_parse_embed()WP_oEmbed_Controller::get_proxy_item_permissions_check()(the/oembed/1.0/proxyendpoint the block editor uses)
map_meta_cap() resolves it as:
embed_urlwith a post ID maps toedit_postfor that postembed_urlwithout a post ID maps toedit_posts
Both checks fall back to a bare edit_posts when there's no post context. That blocks roles that only have custom post type capabilities from embedding anything, for example in a CPT's block editor (the REST proxy never sends a post ID) or on the profile screen as described in the ticket. The only workaround today is a user_has_cap filter that grants edit_posts for specific requests, which leaks far more access than intended.
Backward compatibility
Default behavior is unchanged. Every role that could embed before can embed now, and nobody gains access. The difference is that sites can now remap the capability in one place:
add_filter( 'map_meta_cap', function ( $caps, $cap, $user_id, $args ) { if ( 'embed_url' === $cap && empty( $args ) ) { return array( 'edit_products' ); } return $caps; }, 10, 4 );
Testing
New tests:
Tests_User_Capabilities: default mapping with and without a post, invalid post ID, and the custom post type case from the ticket including a filtered remap.embed_urlis also added to the single site and multisite meta cap fixtures.Test_oEmbed_Controller: the proxy honors a remapped and a revokedembed_url.- New
Tests_Ajax_wpAjaxParseEmbed: allowed and denied with no post, denied for a post the user can't edit, and a filtered remap. Uses a local embed handler, so no HTTP requests are made.
Six of the nine new tests fail on trunk without the source change. The other three lock in the existing default behavior.
phpunit --group oembed,ajax,user --filter 'embed_url|ParseEmbed' phpunit tests/phpunit/tests/user/capabilities.php phpunit -c tests/phpunit/multisite.xml tests/phpunit/tests/user/capabilities.php
All passing, and the changed files are clean under phpcs.xml.dist.
Open question for reviewers
Naming. embed_url is a placeholder and I'm happy to change it.
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5.5
Used for: Investigation, implementation, and tests were written with Claude Code. I reviewed the approach, verified the test results, and take responsibility for the change.
#7
@
3 days ago
Confirmed this is still an issue on trunk. Both wp_ajax_parse_embed() and WP_oEmbed_Controller::get_proxy_item_permissions_check() fall back to a bare edit_posts check when there's no post context. This matters more now than in 2018, because the block editor resolves embeds through /oembed/1.0/proxy, which never sends a post ID.
I've opened a PR that takes a slightly different approach from the original proposal. Instead of a new primitive capability, which existing roles would need to be granted, it adds an embed_url meta capability resolved in map_meta_cap():
- With a post ID, it maps to
edit_postfor that post. - Without one, it maps to
edit_posts.
Default behavior is unchanged. Sites with custom post type capabilities can remap it with a single map_meta_cap filter instead of granting edit_posts.
The PR includes unit tests for the capability mapping, the REST proxy, and the Ajax handler.
Feedback welcome on the approach and the capability name. A possible follow-up is letting the proxy accept an optional post ID so the block editor gets post context, but I've kept that out of scope here.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Is this ticket reporting a problem with the capability check for embeds or is it proposing a new capability? Or maybe both.
It seems to me that the existing capability check assumes a post context, which your use case has a problem with. Would putting a filter on that capability check work? Are there other places where embeds might be wanted, but checking
edit_postswouldn't work? Like comments or custom widgets?Is it just embeds that has this problem? I've seen some other discussions about other places where core checks for a capability that doesn't work in all contexts. (I also have a use case for custom role with custom post type created with a custom editor, and I don't want the user to
edit_posts. This seems likely to be common for sites with user-created content.)