FEATURE: Support for a whitelist for embeddable host paths

This commit is contained in:
Robin Ward 2016-08-23 14:55:52 -04:00
parent 43a3210c20
commit c3a3aff120
12 changed files with 51 additions and 31 deletions

View file

@ -30,10 +30,11 @@ export default Ember.Component.extend(bufferedProperty('host'), {
save() {
if (this.get('cantSave')) { return; }
const props = this.get('buffered').getProperties('host');
const props = this.get('buffered').getProperties('host', 'path_whitelist');
props.category_id = this.get('categoryId');
const host = this.get('host');
host.save(props).then(() => {
host.set('category', Discourse.Category.findById(this.get('categoryId')));
this.set('editToggled', false);

View file

@ -2,6 +2,9 @@
<td>
{{input value=buffered.host placeholder="example.com" enter="save" class="host-name"}}
</td>
<td>
{{input value=buffered.path_whitelist placeholder="/blog/.*" enter="save" class="path-whitelist"}}
</td>
<td>
{{category-chooser value=categoryId}}
</td>
@ -11,6 +14,7 @@
</td>
{{else}}
<td>{{host.host}}</td>
<td>{{host.path_whitelist}}</td>
<td>{{category-badge host.category}}</td>
<td>
{{d-button icon="pencil" action="edit"}}

View file

@ -2,9 +2,10 @@
{{#if embedding.embeddable_hosts}}
<table class='embedding'>
<tr>
<th style='width: 50%'>{{i18n "admin.embedding.host"}}</th>
<th style='width: 30%'>{{i18n "admin.embedding.host"}}</th>
<th style='width: 30%'>{{i18n "admin.embedding.path_whitelist"}}</th>
<th style='width: 30%'>{{i18n "admin.embedding.category"}}</th>
<th style='width: 20%'>&nbsp;</th>
<th style='width: 10%'>&nbsp;</th>
</tr>
{{#each embedding.embeddable_hosts as |host|}}
{{embeddable-host host=host deleteHost="deleteHost"}}

View file

@ -21,6 +21,7 @@ class Admin::EmbeddableHostsController < Admin::AdminController
def save_host(host)
host.host = params[:embeddable_host][:host]
host.path_whitelist = params[:embeddable_host][:path_whitelist]
host.category_id = params[:embeddable_host][:category_id]
host.category_id = SiteSetting.uncategorized_category_id if host.category_id.blank?

View file

@ -85,7 +85,7 @@ class EmbedController < ApplicationController
def ensure_embeddable
if !(Rails.env.development? && current_user.try(:admin?))
raise Discourse::InvalidAccess.new('invalid referer host') unless EmbeddableHost.host_allowed?(request.referer)
raise Discourse::InvalidAccess.new('invalid referer host') unless EmbeddableHost.url_allowed?(request.referer)
end
response.headers['X-Frame-Options'] = "ALLOWALL"

View file

@ -7,8 +7,11 @@ class EmbeddableHost < ActiveRecord::Base
self.host.sub!(/\/.*$/, '')
end
def self.record_for_host(host)
uri = URI(host) rescue nil
def self.record_for_url(uri)
if uri.is_a?(String)
uri = URI(uri) rescue nil
end
return false unless uri.present?
host = uri.host
@ -17,8 +20,13 @@ class EmbeddableHost < ActiveRecord::Base
where("lower(host) = ?", host).first
end
def self.host_allowed?(host)
record_for_host(host).present?
def self.url_allowed?(url)
uri = URI(url) rescue nil
return false unless uri.present?
host = record_for_url(uri)
return host.present? &&
(host.path_whitelist.blank? || !Regexp.new(host.path_whitelist).match(uri.path).nil?)
end
private

View file

@ -33,7 +33,7 @@ class TopicEmbed < ActiveRecord::Base
# If there is no embed, create a topic, post and the embed.
if embed.blank?
Topic.transaction do
eh = EmbeddableHost.record_for_host(url)
eh = EmbeddableHost.record_for_url(url)
creator = PostCreator.new(user,
title: title,

View file

@ -1,16 +1,11 @@
class EmbeddableHostSerializer < ApplicationSerializer
attributes :id, :host, :category_id
def id
object.id
TO_SERIALIZE = [:id, :host, :path_whitelist, :category_id]
attributes *TO_SERIALIZE
TO_SERIALIZE.each do |attr|
define_method(attr) { object.send(attr) }
end
def host
object.host
end
def category_id
object.category_id
end
end

View file

@ -3113,6 +3113,7 @@ en:
sample: "Use the following HTML code into your site to create and embed discourse topics. Replace <b>REPLACE_ME</b> with the canonical URL of the page you are embedding it on."
title: "Embedding"
host: "Allowed Hosts"
path_whitelist: "Path Whitelist"
edit: "edit"
category: "Post to Category"
add_host: "Add Host"

View file

@ -7,13 +7,13 @@ class TopicRetriever
end
def retrieve
perform_retrieve unless (invalid_host? || retrieved_recently?)
perform_retrieve unless (invalid_url? || retrieved_recently?)
end
private
def invalid_host?
!EmbeddableHost.host_allowed?(@embed_url)
def invalid_url?
!EmbeddableHost.url_allowed?(@embed_url)
end
def retrieved_recently?

View file

@ -10,7 +10,7 @@ describe TopicRetriever do
describe "#retrieve" do
context "when host is invalid" do
before do
topic_retriever.stubs(:invalid_host?).returns(true)
topic_retriever.stubs(:invalid_url?).returns(true)
end
it "does not perform_retrieve" do
@ -32,7 +32,7 @@ describe TopicRetriever do
context "when host is not invalid" do
before do
topic_retriever.stubs(:invalid_host?).returns(false)
topic_retriever.stubs(:invalid_url?).returns(false)
end
context "when topics have been retrieived recently" do

View file

@ -49,19 +49,28 @@ describe EmbeddableHost do
expect(eh).not_to be_valid
end
describe "allows_embeddable_host" do
describe "url_allowed?" do
let!(:host) { Fabricate(:embeddable_host) }
it 'works as expected' do
expect(EmbeddableHost.host_allowed?('http://eviltrout.com')).to eq(true)
expect(EmbeddableHost.host_allowed?('https://eviltrout.com')).to eq(true)
expect(EmbeddableHost.host_allowed?('https://not-eviltrout.com')).to eq(false)
expect(EmbeddableHost.url_allowed?('http://eviltrout.com')).to eq(true)
expect(EmbeddableHost.url_allowed?('https://eviltrout.com')).to eq(true)
expect(EmbeddableHost.url_allowed?('https://not-eviltrout.com')).to eq(false)
end
it 'works with multiple hosts' do
Fabricate(:embeddable_host, host: 'discourse.org')
expect(EmbeddableHost.host_allowed?('http://eviltrout.com')).to eq(true)
expect(EmbeddableHost.host_allowed?('http://discourse.org')).to eq(true)
expect(EmbeddableHost.url_allowed?('http://eviltrout.com')).to eq(true)
expect(EmbeddableHost.url_allowed?('http://discourse.org')).to eq(true)
end
end
describe "path_whitelist" do
let!(:host) { Fabricate(:embeddable_host, path_whitelist: '^/fp/\d{4}/\d{2}/\d{2}/.*$') }
it "matches the path" do
expect(EmbeddableHost.url_allowed?('http://eviltrout.com')).to eq(false)
expect(EmbeddableHost.url_allowed?('http://eviltrout.com/fp/2016/08/25/test-page')).to eq(true)
end
end