Skip to content

Commit

Permalink
FEATURE: Calculate CSP based on active themes (discourse#6976)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidtaylorhq authored Feb 11, 2019
1 parent e0c16d3 commit 705c898
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 33 deletions.
8 changes: 4 additions & 4 deletions lib/content_security_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

class ContentSecurityPolicy
class << self
def policy
new.build
def policy(theme_ids = [])
new.build(theme_ids)
end

def base_url
Expand All @@ -14,10 +14,10 @@ def base_url
attr_writer :base_url
end

def build
def build(theme_ids)
builder = Builder.new

Extension.theme_extensions.each { |extension| builder << extension }
Extension.theme_extensions(theme_ids).each { |extension| builder << extension }
Extension.plugin_extensions.each { |extension| builder << extension }
builder << Extension.site_setting_extension

Expand Down
11 changes: 6 additions & 5 deletions lib/content_security_policy/extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ def plugin_extensions

THEME_SETTING = 'extend_content_security_policy'

def theme_extensions
cache['theme_extensions'] ||= find_theme_extensions
def theme_extensions(theme_ids)
key = "theme_extensions_#{Theme.transform_ids(theme_ids).join(',')}"
cache[key] ||= find_theme_extensions(theme_ids)
end

def clear_theme_extensions_cache!
cache['theme_extensions'] = nil
cache.clear
end

private
Expand All @@ -31,10 +32,10 @@ def cache
@cache ||= DistributedCache.new('csp_extensions')
end

def find_theme_extensions
def find_theme_extensions(theme_ids)
extensions = []

Theme.find_each do |theme|
Theme.where(id: Theme.transform_ids(theme_ids)).find_each do |theme|
theme.cached_settings.each do |setting, value|
extensions << build_theme_extension(value) if setting.to_s == THEME_SETTING
end
Expand Down
6 changes: 3 additions & 3 deletions lib/content_security_policy/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ def call(env)
_, headers, _ = response = @app.call(env)

return response unless html_response?(headers)

ContentSecurityPolicy.base_url = request.host_with_port if Rails.env.development?

headers['Content-Security-Policy'] = policy if SiteSetting.content_security_policy
headers['Content-Security-Policy-Report-Only'] = policy if SiteSetting.content_security_policy_report_only
theme_ids = env[:resolved_theme_ids]
headers['Content-Security-Policy'] = policy(theme_ids) if SiteSetting.content_security_policy
headers['Content-Security-Policy-Report-Only'] = policy(theme_ids) if SiteSetting.content_security_policy_report_only

response
end
Expand Down
53 changes: 32 additions & 21 deletions spec/lib/content_security_policy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,31 +120,42 @@ def enabled?
Discourse.plugins.pop
end

it 'can be extended by themes' do
policy # call this first to make sure further actions clear the cache
context "with a theme" do
let!(:theme) {
Fabricate(:theme).tap do |t|
settings = <<~YML
extend_content_security_policy:
type: list
default: 'script-src: from-theme.com'
YML
t.set_field(target: :settings, name: :yaml, value: settings)
t.save!
end
}

def theme_policy
policy([theme.id])
end

theme = Fabricate(:theme)
settings = <<~YML
extend_content_security_policy:
type: list
default: 'script-src: from-theme.com'
YML
theme.set_field(target: :settings, name: :yaml, value: settings)
theme.save!
it 'can be extended by themes' do
policy # call this first to make sure further actions clear the cache

expect(parse(policy)['script-src']).to include('from-theme.com')
expect(parse(policy)['script-src']).not_to include('from-theme.com')

theme.update_setting(:extend_content_security_policy, "script-src: https://from-theme.net|worker-src: from-theme.com")
theme.save!
expect(parse(theme_policy)['script-src']).to include('from-theme.com')

expect(parse(policy)['script-src']).to_not include('from-theme.com')
expect(parse(policy)['script-src']).to include('https://from-theme.net')
expect(parse(policy)['worker-src']).to include('from-theme.com')
theme.update_setting(:extend_content_security_policy, "script-src: https://from-theme.net|worker-src: from-theme.com")
theme.save!

theme.destroy!
expect(parse(theme_policy)['script-src']).to_not include('from-theme.com')
expect(parse(theme_policy)['script-src']).to include('https://from-theme.net')
expect(parse(theme_policy)['worker-src']).to include('from-theme.com')

expect(parse(policy)['script-src']).to_not include('https://from-theme.net')
expect(parse(policy)['worker-src']).to_not include('from-theme.com')
theme.destroy!

expect(parse(theme_policy)['script-src']).to_not include('https://from-theme.net')
expect(parse(theme_policy)['worker-src']).to_not include('from-theme.com')
end
end

it 'can be extended by site setting' do
Expand All @@ -160,7 +171,7 @@ def parse(csp_string)
end.to_h
end

def policy
ContentSecurityPolicy.policy
def policy(theme_ids = [])
ContentSecurityPolicy.policy(theme_ids)
end
end

0 comments on commit 705c898

Please sign in to comment.