FEATURE: Calculate CSP based on active themes (#6976)

approved

#1

FEATURE: Calculate CSP based on active themes (#6976)

diff --git a/lib/content_security_policy.rb b/lib/content_security_policy.rb
index 2f2b4c8..fd38fd5 100644
--- a/lib/content_security_policy.rb
+++ b/lib/content_security_policy.rb
@@ -4,8 +4,8 @@ require_dependency 'content_security_policy/extension'
 
 class ContentSecurityPolicy
   class << self
-    def policy
-      new.build
+    def policy(theme_ids = [])
+      new.build(theme_ids)
     end
 
     def base_url
@@ -14,10 +14,10 @@ class ContentSecurityPolicy
     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
 
diff --git a/lib/content_security_policy/extension.rb b/lib/content_security_policy/extension.rb
index cc31986..6091788 100644
--- a/lib/content_security_policy/extension.rb
+++ b/lib/content_security_policy/extension.rb
@@ -17,12 +17,13 @@ class ContentSecurityPolicy
 
     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
@@ -31,10 +32,10 @@ class ContentSecurityPolicy
       @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
diff --git a/lib/content_security_policy/middleware.rb b/lib/content_security_policy/middleware.rb
index d407ce0..d47babe 100644
--- a/lib/content_security_policy/middleware.rb
+++ b/lib/content_security_policy/middleware.rb
@@ -12,11 +12,11 @@ class ContentSecurityPolicy
       _, 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
diff --git a/spec/lib/content_security_policy_spec.rb b/spec/lib/content_security_policy_spec.rb
index 9795698..df0b66a 100644
--- a/spec/lib/content_security_policy_spec.rb
+++ b/spec/lib/content_security_policy_spec.rb
@@ -120,31 +120,42 @@ describe ContentSecurityPolicy do
     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
@@ -160,7 +171,7 @@ describe ContentSecurityPolicy do
     end.to_h
   end
 
-  def policy
-    ContentSecurityPolicy.policy
+  def policy(theme_ids = [])
+    ContentSecurityPolicy.policy(theme_ids)
   end
 end

GitHub sha: 705c898c


Approved #2