Closed tsmith123 closed 1 week ago
Can this be merged?
No worries. So do you want me to add bitcoin.tv to the original list or are we just leaving it and not merging?
Yes I think whitelisting trusted peertube sources will be our preferred approach to doing this, absent some other method for trusting what is being embedded.
Cool, so I've restored the original frame-src values and added bitcointv.com
and peertube.tv
to the list.
Think this needs another review guys 👍
@huumn @ekzyis sorry to ask again guys but is there anything else I can do with this PR to get it over the line? It's been here for a while now.
@tsmith123 it's us, not you. I've been super heads down on https://github.com/stackernews/stacker.news/pull/1195 ... which is taking every bit of working memory I have.
I'll try to give this a review today or tomorrow if @ekzyis doesn't get to it. @kravhen I'll try to get you some feedback too.
In the meantime, if you want to pick up another issue, please do.
Fixes #107 (with a caveat, see notes below)
Description
This PR allows users to embed a video hosted on PeerTube.
Screenshots
Testing
The following urls were used to test in New Post and Comment.
https://peertube.satoshishop.de/w/d2ecb738-bee7-48db-8f7f-0009382cb24d https://peertube.zoz-serv.org/w/aFWFxWPKYxmLFteHNJeBKG?start=0s&subtitle=en
Note the different domains in the urls
Notes
As PeerTube allows videos to be hosted on different domains we can't specify a frame-src domain in the csp frame-src settings. The only way to permit video embeds from PeerTube is to use the allow all (*) value instead. I will leave it you to decide whether this is too big a security risk.