FEATURE: enable CSP by default on new sites (#6873)

FEATURE: enable CSP by default on new sites (#6873)

  • adds migration to enable CSP for new sites
  • removes “EXPERIMENTAL” labels from setting names
  • sets CSP violation report to default off
  • adds CSP-related note to GTM setting
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 8b1742f..996f501 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1332,7 +1332,7 @@ en:
     ga_universal_tracking_code: "Google Universal Analytics (analytics.js) tracking code code, eg: UA-12345678-9; see <a href='https://google.com/analytics' target='_blank'>https://google.com/analytics</a>"
     ga_universal_domain_name: "Google Universal Analytics (analytics.js) domain name, eg: mysite.com; see <a href='https://google.com/analytics' target='_blank'>https://google.com/analytics</a>"
     ga_universal_auto_link_domains: "Enable Google Universal Analytics (analytics.js) cross-domain tracking. Outgoing links to these domains will have the client id added to them. See <a href='https://support.google.com/analytics/answer/1034342?hl=en' target='_blank'>Google's Cross-Domain Tracking guide.</a>"
-    gtm_container_id: "Google Tag Manager container id. eg: GTM-ABCDEF"
+    gtm_container_id: "Google Tag Manager container id. eg: GTM-ABCDEF. <br/>Note: third-party scripts loaded by GTM may need to be whitelisted in 'content security policy script src'."
     enable_escaped_fragments: "Fall back to Google's Ajax-Crawling API if no webcrawler is detected. See <a href='https://developers.google.com/webmasters/ajax-crawling/docs/learn-more' target='_blank'>https://developers.google.com/webmasters/ajax-crawling/docs/learn-more</a>"
     allow_moderators_to_create_categories: "Allow moderators to create new categories"
     cors_origins: "Allowed origins for cross-origin requests (CORS). Each origin must include http:// or https://. The DISCOURSE_ENABLE_CORS env variable must be set to true to enable CORS."
@@ -1344,10 +1344,10 @@ en:
     blacklisted_crawler_user_agents: 'Unique case insensitive word in the user agent string identifying web crawlers that should not be allowed to access the site. Does not apply if whitelist is defined.'
     slow_down_crawler_user_agents: 'User agents of web crawlers that should be rate limited in robots.txt using the Crawl-delay directive'
     slow_down_crawler_rate: 'If slow_down_crawler_user_agents is specified this rate will apply to all the crawlers (number of seconds delay between requests)'
-    content_security_policy: EXPERIMENTAL - Turn on Content-Security-Policy
-    content_security_policy_report_only: EXPERIMENTAL - Turn on Content-Security-Policy-Report-Only
-    content_security_policy_collect_reports: Enable CSP violation report collection at /csp_reports
-    content_security_policy_script_src: Additional whitelisted script sources. The current host and CDN are included by default.
+    content_security_policy: "Enable Content-Security-Policy"
+    content_security_policy_report_only: "Enable Content-Security-Policy-Report-Only"
+    content_security_policy_collect_reports: "Enable CSP violation report collection at /csp_reports"
+    content_security_policy_script_src: "Additional whitelisted script sources. The current host and CDN are included by default."
     invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable."
     top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks"
     post_menu: "Determine which items appear on the post menu, and in what order. Example like|edit|flag|delete|share|bookmark|reply"
diff --git a/config/site_settings.yml b/config/site_settings.yml
index a22ce2a..8e66959 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1256,7 +1256,7 @@ security:
   content_security_policy_report_only:
     default: false
   content_security_policy_collect_reports:
-    default: true
+    default: false
   content_security_policy_script_src:
     type: list
     default: ''
diff --git a/db/migrate/20190110142917_enable_content_security_policy_for_new_sites.rb b/db/migrate/20190110142917_enable_content_security_policy_for_new_sites.rb
new file mode 100644
index 0000000..396b514
--- /dev/null
+++ b/db/migrate/20190110142917_enable_content_security_policy_for_new_sites.rb
@@ -0,0 +1,25 @@
+class EnableContentSecurityPolicyForNewSites < ActiveRecord::Migration[5.2]
+  def up
+    return if Rails.env.test?
+    return if row_exists?
+
+    if instance_is_new?
+      execute "INSERT INTO site_settings(name, data_type, value, created_at, updated_at)
+               VALUES ('content_security_policy', 5, 't', now(), now())"
+    end
+  end
+
+  def down
+    # Don't undo, up method only enables CSP if row isn't already there and if instance is new
+  end
+
+  def row_exists?
+    return DB.query("SELECT 1 AS one FROM site_settings where name='content_security_policy'").present?
+  end
+
+  def instance_is_new?
+    post = DB.query_single("SELECT created_at FROM posts ORDER BY created_at ASC LIMIT 1")
+    return post.blank? || (post.present? && post.first > 1.week.ago)
+  end
+
+end

GitHub sha: 1c1fd205

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

This commit has been mentioned on Discourse Meta. There might be relevant details there:

Do we need the return here?

1 Like

Code styling fix

You don’t need the return here either :wink:

2 Likes

I would actually go for a bit clearer syntax here:

dates = DB.query_single("SELECT created_at ...")
dates.empty? || dates.first > 1.week.ago   

query_single is always going to return an array.

2 Likes

Clearer syntax in migration