From ecf7a4f0c606006ab645e9e5632b29b8c11fe33c Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Fri, 1 Dec 2023 12:57:11 +0000
Subject: [PATCH] FIX: Ensure app-cdn CORS is not overridden by cors_origin
 setting (#24661)

We add `Access-Control-Allow-Origin: *` to all asset requests which are requested via a configured CDN. This is particularly important now that we're using browser-native `import()` to load the highlightjs bundle. Unfortunately, user-configurable 'cors_origins' site setting was overriding the wldcard value on CDN assets and causing CORS errors.

This commit updates the logic to give the `*` value precedence, and adds a spec for the situation. It also invalidates the cache of hljs assets (because CDNs will have cached the bad Access-Control-Allow-Origin header).

The rack-cors middleware is also slightly tweaked so that it is always inserted. This makes things easier to test and more consistent.
---
 config/initializers/008-rack-cors.rb         | 11 +++----
 lib/highlight_js.rb                          |  5 ++-
 spec/lib/hijack_spec.rb                      |  1 +
 spec/requests/highlightjs_controller_spec.rb | 33 ++++++++++++++++++++
 4 files changed, 43 insertions(+), 7 deletions(-)
 create mode 100644 spec/requests/highlightjs_controller_spec.rb

diff --git a/config/initializers/008-rack-cors.rb b/config/initializers/008-rack-cors.rb
index d499a38cc8d..d16a6281079 100644
--- a/config/initializers/008-rack-cors.rb
+++ b/config/initializers/008-rack-cors.rb
@@ -11,6 +11,8 @@ class Discourse::Cors
   end
 
   def call(env)
+    return @app.call(env) if !GlobalSetting.enable_cors && !GlobalSetting.cdn_url
+
     cors_origins = @global_origins || []
     cors_origins += SiteSetting.cors_origins.split("|") if SiteSetting.cors_origins.present?
     cors_origins = cors_origins.presence
@@ -32,9 +34,8 @@ class Discourse::Cors
   def self.apply_headers(cors_origins, env, headers)
     request_method = env["REQUEST_METHOD"]
 
-    if env["REQUEST_PATH"] =~ %r{/(javascripts|assets)/} &&
-         Discourse.is_cdn_request?(env, request_method)
-      Discourse.apply_cdn_headers(headers)
+    if headers["Access-Control-Allow-Origin"]
+      # Already configured. Probably by ApplicationController#apply_cdn_headers
     elsif cors_origins
       origin = nil
       if origin = env["HTTP_ORIGIN"]
@@ -54,6 +55,4 @@ class Discourse::Cors
   end
 end
 
-if GlobalSetting.enable_cors || GlobalSetting.cdn_url
-  Rails.configuration.middleware.insert_before ActionDispatch::Flash, Discourse::Cors
-end
+Rails.configuration.middleware.insert_before ActionDispatch::Flash, Discourse::Cors
diff --git a/lib/highlight_js.rb b/lib/highlight_js.rb
index 028bc21c522..8ac83f2c0a0 100644
--- a/lib/highlight_js.rb
+++ b/lib/highlight_js.rb
@@ -2,6 +2,7 @@
 
 module HighlightJs
   HIGHLIGHTJS_DIR ||= "#{Rails.root}/app/assets/javascripts/node_modules/@highlightjs/cdn-assets/"
+  VERSION ||= 1 # bump to invalidate caches following core changes
 
   def self.languages
     langs = Dir.glob(HIGHLIGHTJS_DIR + "languages/*.js").map { |path| File.basename(path)[0..-8] }
@@ -36,7 +37,9 @@ module HighlightJs
     cache_info = {
       lang_string: lang_string,
       digest:
-        Digest::SHA1.hexdigest(bundle(lang_string.split("|")) + "|#{GlobalSetting.asset_url_salt}"),
+        Digest::SHA1.hexdigest(
+          bundle(lang_string.split("|")) + "|#{VERSION}|#{GlobalSetting.asset_url_salt}",
+        ),
     }
 
     cache[RailsMultisite::ConnectionManagement.current_db] = cache_info
diff --git a/spec/lib/hijack_spec.rb b/spec/lib/hijack_spec.rb
index 8e38061c1e7..1c30928872a 100644
--- a/spec/lib/hijack_spec.rb
+++ b/spec/lib/hijack_spec.rb
@@ -73,6 +73,7 @@ RSpec.describe Hijack do
 
   it "handles cors" do
     SiteSetting.cors_origins = "www.rainbows.com"
+    global_setting :enable_cors, true
 
     app =
       lambda do |env|
diff --git a/spec/requests/highlightjs_controller_spec.rb b/spec/requests/highlightjs_controller_spec.rb
new file mode 100644
index 00000000000..63e78aadd73
--- /dev/null
+++ b/spec/requests/highlightjs_controller_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+RSpec.describe HighlightJsController do
+  it "works via the site URL" do
+    get HighlightJs.path
+    expect(response.status).to eq(200)
+    expect(response.body).to include("export default function")
+    expect(response.headers["Access-Control-Allow-Origin"]).to eq(nil)
+  end
+
+  it "works via a CDN" do
+    cdn = "https://original-app-cdn.example.com"
+    set_cdn_url cdn
+
+    get "#{cdn}#{HighlightJs.path}"
+    expect(response.status).to eq(200)
+    expect(response.body).to include("export default function")
+    expect(response.headers["Access-Control-Allow-Origin"]).to eq("*")
+  end
+
+  it "works via a CDN when site has cors configuration" do
+    cdn = "https://original-app-cdn.example.com"
+    set_cdn_url cdn
+
+    global_setting :enable_cors, true
+    SiteSetting.cors_origins = "https://example.com"
+
+    get "#{cdn}#{HighlightJs.path}"
+    expect(response.status).to eq(200)
+    expect(response.body).to include("export default function")
+    expect(response.headers["Access-Control-Allow-Origin"]).to eq("*")
+  end
+end