Introduce REST API endpoint for proxying requests to external oEmbed providers

[westonruter]

This essentially proposes adding the functionality from noembed.com into WordPress.

When doing oEmbed previews in the client, both in the media modal embed frame and in the TinyMCE embed preview, a parse-embed admin-ajax request is currently made to do the parsing. This request requires a post_ID parameter because the WP Embed logic caches oEmbed responses in postmeta (see #34115). There is a need for this in the media widgets to be able to obtain the thumbnail_url for a given oEmbed URL. Unfortunately the oEmbed preview in the media modal fails because there is no post_ID for context. Also, the client cannot reliably make requests to oEmbed providers directly since they inconsistently support CORS.

There is currently an wp-json/oembed/1.0/embed endpoint for serving oEmbed responses for posts on a given site. It would be a useful improvement to extend this endpoint to also allow for proxying the oEmbed provider lookup and fetching, as handled internally in \WP_oEmbed::get_html(): ​https://github.com/WordPress/wordpress-develop/blob/4.7/src/wp-includes/class-oembed.php#L333-L360

I understand there is also a concern for using such an embed endpoint for doing DDoS attacks, so I imagine any external URL lookups would require auth.

Once this is implemented, the use of the embed endpoint can potentially replace the parse-embed request, at least when there is no post_ID for context.

For background, see:
​https://wordpress.slack.com/archives/C02RQC26G/p1492038272597233
​https://github.com/xwp/wp-core-media-widgets/pull/53#issuecomment-294131744

[westonruter]

The patch guards against DDoS by requiring the edit_posts cap. It does't currently cache responses, but I'm guessing this would be added from what lands for #34115.

[westonruter]

Opened a PR for collaboration and running core unit tests via Travis: ​https://github.com/xwp/wordpress-develop/pull/225

[timmydcrawford]

Once this is implemented, the use of the embed endpoint can potentially replace the parse-embed request, at least when there is no post_ID for context.

@westonruter do we have an idea of what might be needed to make this new endpoint work in the context of a particular post? It appears the current logic in the associated admin-ajax function is to verify the user has access to edit the given post : ​https://github.com/xwp/wordpress-develop/blob/master/src/wp-admin/includes/ajax-actions.php#L2945-L2951

Would it be worth exploring supporting a post_ID option for the endpoint too? I think it would be great to ship this new endpoint with all the needed features to replace parse_embed usage from the get-go.

[westonruter]

@timmydcrawford The only reason for the post_ID for context is to that the embed can be cached in a post's postmeta so that when the post gets viewed on the frontend it won't cause the site to crash with external requests. The capability check then I guess is to prevent some unauthorized user from filling up postmeta on a given post with oEmbed data.

To me it seems that for the purposes of the REST API endpoint, we can just use transients instead, if we even really need caching here. The media library JS can then be updated to just go ahead and use the embed proxy endpoint and ignore whether it has a current post as context. Just like this: ​https://github.com/xwp/wordpress-develop/pull/225/commits/2bb963d

[westonruter]

I'm proposing for commit. Any last feedback on the patch prior to doing so?

Ref: ​https://github.com/xwp/wordpress-develop/pull/225

[swissspidy]

40450.diff​ reuses more parts of the oEmbed class to make things more DRY. Otherwise this looks good to me.

Extending Test_oEmbed_Controller would have been great for this, but we can also do that at a later point.

[jnylen0]

The latest version of the PR (​https://github.com/xwp/wordpress-develop/pull/225) has a lot more changes than either of the patches here, including unit tests and updating the media modal to use the new proxy endpoint.

Everything tests out well and looks good to me. I think it's ready for commit.

[westonruter]

The patches attached to this ticket are just copies of older versions of the PR. Now pending Travis build on PR for green light.

[jnylen0]

​https://patch-diff.githubusercontent.com/raw/xwp/wordpress-develop/pull/225.diff

[jnylen0]

In 40628:

REST API: Add endpoint for proxying requests to external oEmbed providers.

This endpoint is a prerequisite for the media widgets work (see ​https://github.com/xwp/wp-core-media-widgets).

Also use the new endpoint in the media modal instead of the parse-embed AJAX action.

Props westonruter, timmydcrawford, swissspidy, jnylen0.
Fixes #40450.

[westonruter]

In 40641:

Widgets: Utilize WP REST API oEmbed proxy endpoint in media widgets.

Props timmydcrawford.
See #32417, #40450.

[westonruter]

In 41035:

REST API: Remove _wpnonce value from being used in hashed oEmbed proxy cache key.

Amends [40628].
Props r-a-y, westonruter.
See #40450.
Fixes #41048.

[westonruter]

In 41047:

REST API: Ensure maxwidth and maxheight params are forwarded to oEmbed provider in proxy requests.

Also correct phpdoc return tag on WP_oEmbed_Controller::get_proxy_item() and remove dead code in oEmbed controller phpunit tests.

Amends [40628].
See #40450.
Fixes #41299.

[westonruter]

In 41048:

REST API: Remove _wpnonce value from being used in hashed oEmbed proxy cache key.

Merges [41035] to 4.8 branch.
Amends [40628].
Props r-a-y, westonruter.
See #40450.
Fixes #41048 for 4.8.1.

[westonruter]

In 41049:

REST API: Ensure maxwidth and maxheight params are forwarded to oEmbed provider in proxy requests.

Also correct phpdoc return tag on WP_oEmbed_Controller::get_proxy_item() and remove dead code in oEmbed controller phpunit tests.

Merges [41047] into 4.8 branch.
Amends [40628].
See #40450.
Fixes #41299 for 4.8.1.

[westonruter]

In 41913:

Widgets: Fix previewing embeds in Text widget by allowing parse-embed admin ajax requests with an empty post_ID just as WP_oEmbed_Controller::get_proxy_item_permissions_check() allows.

As of #34115 if there is no post context the oEmbed will be cached in an oembed_cache custom post type, so having a post as context is no longer a requirement for caching.

Props biskobe, westonruter.
See #34115, #40450.
Fixes #40854.

[Presskopp]

@westonruter Just found a comment regarding this ticket and I wonder if there's a need to do anything:

\wp-admin\js\widgets\media-widgets.js, L.128

// This should be eliminated once #40450 lands of when this is merged into core.
Constructor = ...

[westonruter]

@Presskopp I think you're right. It appears that this comment should have been removed with this commit: ​https://github.com/WordPress/wordpress-develop/commit/42487a5b23b714160da543205c211580130fa464