Make WordPress Core

Opened 6 years ago

Last modified 5 months ago

#45033 reopened defect (bug)

Add TIDAL to oEmbed whitelist

Reported by: tidal's profile tidal Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Embeds Keywords: needs-patch
Focuses: Cc:

Description

At TIDAL we have recently finished our new modern embed player. Compared to the old embed this one is free from Flash, login requirements and has a new fresh design. It's made lean and responsive and optimised to load fast. We've also set up an oEmbed API for it. We would like to get this new embed player whitelisted in WordPress.

I've read through the documentation on whitelisting providers available here: https://make.wordpress.org/core/handbook/contribute/design-decisions/#whitelisting-oembed-providers

TIDAL started back in 2015 and is today one of the biggest music streaming providers, and one of few with music videos and high fidelity sound. So I'd say we meed the criteria for being a well-established, popular, trusted and mainstream service. We implement the oEmbed specification and use the rich and video oEmbed types for our embeds.

As for the security concern it is important for us at TIDAL as well. In our oEmbed API response we only pass an iframe in the HTML field which load a direct embed url. To further touch on the matter we also care for not leaving black holes on the web which can be vectors for malwares or unpleasant user experiences. An example is that our previous embeds are migrated to this new TIDAL embed player.

I'll go through the "things to be considered" list of questions;

Is the service is popular enough for core developers to have heard of it before? Is it "mainstream?"

Probably :)

If similar services are already supported, how does this service compare in terms of size, features, and backing?

Spotify is already supported, in terms of size they have the same restrictions on sizes in their oEmbed API as we do (maxwith/maxheight for rich/video types). For features: TIDAL, apart from Spotify, also serves video embeds. This can be an on demand video or a live stream.

Does this service have a large following on Twitter, Facebook, or other social media? Is its Twitter account verified?

Is its oEmbed endpoint clearly established and properly documented?

The oEmbed endpoint is available at https://oembed.tidal.com and as per the oEmbed documentation it takes an URL as required parameter. As for our case it would be one of the links you get when clicking share in the applications, for example https://tidal.com/track/74695970. ( https://oembed.tidal.com/?url=https://tidal.com/track/74695970 ) There is not documentation on our oEmbed API endpoint separately since the main usage is intended to be via the oEmbed discovery procedure and seamless to the normal user.

Does the oEmbed endpoint work with WordPress' oEmbed auto-discovery?

Yes. Our browser pages have the oEmbed discovery <link> tag. Example: https://tidal.com/browse/track/74695970 which has

<link rel="alternate" type="application/json+oembed" href="http://oembed.tidal.com/?url=https://tidal.com/track/74695970" />

in the source code.

Does the service make an effort to build relationships with developers, such as through robust APIs?

For APIs to be consumed from the outside we stive to create robust APIs. For example the oEmbed API is made in golang and runs on AWS Lambda to scale easily.

How old is the service?

TIDAL, launched in March 2015.

Does it have a well-established Wikipedia article? (Seriously.)

We seriously do. https://en.wikipedia.org/wiki/Tidal_(service)

Has anyone written a WordPress plugin that leverages the service in some way, whether adding it as an oEmbed provider, creating a shortcode, or leveraging other APIs of the service? Do these plugins have any noticeable adoption or traction that would indicate usage and demand?

Bjørn Johansen and Paul Schreiber has written an "Embed Tidal" plugin. This created an embed from a TIDAL link. It creates embeds for the old TIDAL embed though, the one with Flash that is now replaced on our end. And does not use the oEmbed API that came with the new embed we recently launched.
Link: https://wordpress.org/plugins/embed-tidal/

Is the provider frequently proposed?

No idea.


I hope we can get it done soon. Let me know if there is anything I can do to help. :)

Kind regards,
Jeremy Karlsson
TIDAL

Change History (13)

#1 @swissspidy
6 years ago

  • Severity changed from critical to normal

Hey there,

Thanks for opening this ticket!

It's great that the service supports oEmbed discovery and uses a single iframe. That means it should already work without whitelisting the provider. Out of curiosity: is there any reason the discovery URL uses http instead of https?

#2 @swissspidy
6 years ago

  • Keywords reporter-feedback added

It looks like this doesn't work quite yet because WordPress tries to access http://oembed.tidal.com/?url=https://tidal.com/browse/track/34606477 instead of the advertised http://oembed.tidal.com/?url=https://tidal.com/track/34606477. The former results in an "Internal server error" message being returned.

And if the provider would be whitelisted, WordPress only knows the former format, not the latter.

Any chance this could be fixed on your side? Otherwise we'd have to implement a workaround, which I doubt makes sense.

Plus, if you could fix it, there's probably no need for the whitelisting anyway because it just works.

#3 @peterwilsoncc
6 years ago

  • Keywords reporter-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I've tested this locally and confirmed auto-discovery works as the embed only uses allowed tags and attributes. I'll close this ticket accordingly.

In testing I didn't get the 500 error @swissspidy received so there might be a few bugs to iron out with the end-point at your end.

While you're testing, switching the iframe to https always will ensure browsers don't block the embed on sites running over SSL.

#4 @tidal
6 years ago

Hi again!

Thanks for all the feedback!

The oEmbed discovery links on our Browse-pages are constructed like so:

<link rel="alternate" type="application/json+oembed" href="http://oembed.tidal.com/?url=https://tidal.com/track/94560983" />

The URL parameter is set to https://tidal.com/track/94560983, not the browse page URL. (https://tidal.com/browse/track/94560983 )

Why would WordPress attempt to call the endpoint with another URL than what's specified by the discovery tag, like @swissspidy observed? We can easily add support for Browse page URLs in our oEmbed API if needed, but I do not get how WordPress could ever call them?

@peterwilsoncc, the iframe in the html property of the response from the oEmbed API will always be https. Did you mean that the oEmbed discovery link is not? I'll look into why it's not specified as https there, it should be!

Kind regards,
Jeremy Karlsson
TIDAL

Last edited 6 years ago by tidal (previous) (diff)

#5 @peterwilsoncc
6 years ago

Thanks Jeremy,

Yes, I was referring to the oembed link, in the source of the link above I am seeing:

<link rel="alternate" type="application/json+oembed" href="http://oembed.tidal.com/?url=https://tidal.com/track/74695970" />

However, I had a mental blank as the http request is made by WordPress rather than the browser. :)

#6 @tidal
6 years ago

I have now updated the oEmbed API to support Browse URLs and sharing them now work in WordPress as well!

http://oembed.tidal.com/?url=https://tidal.com/browse/track/94560983

But why does WordPress remove the specified url parameter in the <link> with another one (seems to use the href of the page the <link> is on)? That ought to be a WordPress bug?

WordPress should not call http://oembed.tidal.com/?url=https://tidal.com/browse/track/94560983 when http://oembed.tidal.com/?url=https://tidal.com/track/94560983 is specified in the link tag.

Kind regards,
Jeremy Karlsson
TIDAL

#7 follow-up: @swissspidy
6 years ago

That's what I tried to say in my previous comment :-)

See #41746 for a ticket regarding this behavior.

With that being said, Tidal embeds appear to be working just fine with this, even without whitelisting, which is great.

With whitelisting we could save that discovery lookup though.

#8 in reply to: ↑ 7 @tidal
5 months ago

  • Resolution wontfix deleted
  • Severity changed from normal to major
  • Status changed from closed to reopened
  • Type changed from enhancement to defect (bug)

Replying to swissspidy:

With that being said, Tidal embeds appear to be working just fine with this, even without whitelisting, which is great.

Seems like WordPress has changed and now adds a sandbox attribute. This has broken the embed. The sandbox seems to need both allow-scripts and allow-same-origin, else I'm getting CORS errors for our static S3 resources (js, css and fonts).

Examples here: https://codepen.io/enjikaka/pen/JjVZBYX

Kind regards,
Jeremy Karlsson
TIDAL

Last edited 5 months ago by tidal (previous) (diff)

#9 follow-ups: @swissspidy
5 months ago

  • Milestone set to Awaiting Review
  • Severity changed from major to normal

FWIW, WordPress has always added sandboxing (sandbox="allow-scripts" security="restricted"), it's not something new.

With the allow-scripts sandbox I'm getting only CORS errors for the embeds, so presumably if you add a Access-Control-Allow-Origin header to resources such as https://embed.tidal.com/embed-resources/esbuild/js/app.ec5cacdceba9214958f7567feba427fe.js, then the embed should work again. Is this something you could try?

We can certainly re-evaluate adding Tidal to the allowlist, but if fixing the CORS issues would solve the issue already than we'd prefer that.

#10 in reply to: ↑ 9 @tidal
5 months ago

Replying to swissspidy:

We can certainly re-evaluate adding Tidal to the allowlist, but if fixing the CORS issues would solve the issue already than we'd prefer that.

We should be able to add CORS headers on our own stuff but the font is licensed to us and I do not believe we can loosen up the CORS on that.

Kind regards,
Jeremy Karlsson
TIDAL

Last edited 5 months ago by tidal (previous) (diff)

#11 in reply to: ↑ 9 @peterwilsoncc
5 months ago

Replying to swissspidy:

We can certainly re-evaluate adding Tidal to the allowlist, but if fixing the CORS issues would solve the issue already than we'd prefer that.

I think this is worth reconsidering.

My main concern is usefulness to readers of WordPress sites though. Unlike a Spotify embed visitors without an account don't get a track to listen to, they only get a link to try tidal.

This was tested by manually copying the embed code for the track listed in the ticket, <iframe src='https://embed.tidal.com/tracks/74695970' width='500' height='120' frameborder='0' allowfullscreen='allowfullscreen'></iframe>, with a WordPress account permitted to use unfiltered HTML.

TIDAL's licensing and account policies are well beyond the scope of this ticket but the WordPress project does need to consider how useful a supported embed is for site owners and readers.

#12 follow-up: @swissspidy
5 months ago

@peterwilsoncc That's because that song/track no longer exists, so it 404s. See https://tidal.com/browse/track/74695970 how 404s. And you can't oEmbed a non-existing track.

If you try for example <iframe class="wp-embedded-content" src="https://embed.tidal.com/tracks/137457240#?secret=JK0agVf1VT" data-secret="JK0agVf1VT" width="500" height="120" frameborder="0"></iframe> (https://tidal.com/browse/track/137457240) then you'll see the embed works as expected.

So the usefulness is there :)

#13 in reply to: ↑ 12 @peterwilsoncc
5 months ago

Replying to swissspidy:

So the usefulness is there :)

Thanks Pascal, I guess I've just revealed myself to be a user of another streaming service.

@tidal, while folks are discussing whether to add TIDAL to the allow list, are you able to do a couple of things for me:

  • Create a pull request against the WordPress/wordpress-develop GitHub repo linking to this ticket in the description. You can see an example pull request for adding TikTok
  • I'd prefer to credit an account under the name Jeremy rather than a generic company account -- are you able to create a WordPress.org profile under your own name? You can link it to your GitHub account once you do so.

This will speed the process up if it's decided to add TIDAL to the allow list.

Note: See TracTickets for help on using tickets.