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.
This commit is contained in:
David Taylor 2023-12-01 12:57:11 +00:00 committed by GitHub
parent b72a177fb3
commit ecf7a4f0c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 43 additions and 7 deletions

View File

@ -11,6 +11,8 @@ class Discourse::Cors
end end
def call(env) def call(env)
return @app.call(env) if !GlobalSetting.enable_cors && !GlobalSetting.cdn_url
cors_origins = @global_origins || [] cors_origins = @global_origins || []
cors_origins += SiteSetting.cors_origins.split("|") if SiteSetting.cors_origins.present? cors_origins += SiteSetting.cors_origins.split("|") if SiteSetting.cors_origins.present?
cors_origins = cors_origins.presence cors_origins = cors_origins.presence
@ -32,9 +34,8 @@ class Discourse::Cors
def self.apply_headers(cors_origins, env, headers) def self.apply_headers(cors_origins, env, headers)
request_method = env["REQUEST_METHOD"] request_method = env["REQUEST_METHOD"]
if env["REQUEST_PATH"] =~ %r{/(javascripts|assets)/} && if headers["Access-Control-Allow-Origin"]
Discourse.is_cdn_request?(env, request_method) # Already configured. Probably by ApplicationController#apply_cdn_headers
Discourse.apply_cdn_headers(headers)
elsif cors_origins elsif cors_origins
origin = nil origin = nil
if origin = env["HTTP_ORIGIN"] if origin = env["HTTP_ORIGIN"]
@ -54,6 +55,4 @@ class Discourse::Cors
end end
end end
if GlobalSetting.enable_cors || GlobalSetting.cdn_url Rails.configuration.middleware.insert_before ActionDispatch::Flash, Discourse::Cors
Rails.configuration.middleware.insert_before ActionDispatch::Flash, Discourse::Cors
end

View File

@ -2,6 +2,7 @@
module HighlightJs module HighlightJs
HIGHLIGHTJS_DIR ||= "#{Rails.root}/app/assets/javascripts/node_modules/@highlightjs/cdn-assets/" HIGHLIGHTJS_DIR ||= "#{Rails.root}/app/assets/javascripts/node_modules/@highlightjs/cdn-assets/"
VERSION ||= 1 # bump to invalidate caches following core changes
def self.languages def self.languages
langs = Dir.glob(HIGHLIGHTJS_DIR + "languages/*.js").map { |path| File.basename(path)[0..-8] } langs = Dir.glob(HIGHLIGHTJS_DIR + "languages/*.js").map { |path| File.basename(path)[0..-8] }
@ -36,7 +37,9 @@ module HighlightJs
cache_info = { cache_info = {
lang_string: lang_string, lang_string: lang_string,
digest: 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 cache[RailsMultisite::ConnectionManagement.current_db] = cache_info

View File

@ -73,6 +73,7 @@ RSpec.describe Hijack do
it "handles cors" do it "handles cors" do
SiteSetting.cors_origins = "www.rainbows.com" SiteSetting.cors_origins = "www.rainbows.com"
global_setting :enable_cors, true
app = app =
lambda do |env| lambda do |env|

View File

@ -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