From a5923ad60351b58f863362b45da88491181a1ded Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Fri, 29 Jan 2021 07:44:49 +0530 Subject: [PATCH] DEV: apply allow origin response header for CDN requests. (#11893) Currently, it creates a CORS error while accessing those static files. --- app/controllers/application_controller.rb | 4 ++++ app/controllers/highlight_js_controller.rb | 2 ++ app/controllers/static_controller.rb | 2 ++ app/controllers/stylesheets_controller.rb | 2 ++ app/controllers/svg_sprite_controller.rb | 2 ++ .../theme_javascripts_controller.rb | 2 +- app/controllers/uploads_controller.rb | 2 +- app/controllers/user_avatars_controller.rb | 2 ++ config/initializers/008-rack-cors.rb | 11 +++++++---- lib/discourse.rb | 19 +++++++++++++++++++ lib/middleware/enforce_hostname.rb | 1 + spec/requests/static_controller_spec.rb | 19 +++++++++++++++++++ 12 files changed, 62 insertions(+), 6 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c501ff14472..1f0f7c3c2b2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -680,6 +680,10 @@ class ApplicationController < ActionController::Base raise ApplicationController::RenderEmpty.new unless ((request.format && request.format.json?) || request.xhr?) end + def apply_cdn_headers + Discourse.apply_cdn_headers(response.headers) if Discourse.is_cdn_request?(request.env, request.method) + end + def self.requires_login(arg = {}) @requires_login_arg = arg end diff --git a/app/controllers/highlight_js_controller.rb b/app/controllers/highlight_js_controller.rb index 768c4e7286c..655bafd9e87 100644 --- a/app/controllers/highlight_js_controller.rb +++ b/app/controllers/highlight_js_controller.rb @@ -3,6 +3,8 @@ class HighlightJsController < ApplicationController skip_before_action :preload_json, :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show] + before_action :apply_cdn_headers, only: [:show] + def show no_cookies diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index 16f692569a0..5ef7704bafb 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -7,6 +7,8 @@ class StaticController < ApplicationController skip_before_action :preload_json, only: [:brotli_asset, :cdn_asset, :enter, :favicon, :service_worker_asset] skip_before_action :handle_theme, only: [:brotli_asset, :cdn_asset, :enter, :favicon, :service_worker_asset] + before_action :apply_cdn_headers, only: [:brotli_asset, :cdn_asset, :enter, :favicon, :service_worker_asset] + PAGES_WITH_EMAIL_PARAM = ['login', 'password_reset', 'signup'] MODAL_PAGES = ['password_reset', 'signup'] diff --git a/app/controllers/stylesheets_controller.rb b/app/controllers/stylesheets_controller.rb index 163ed9e430f..d9446882a2e 100644 --- a/app/controllers/stylesheets_controller.rb +++ b/app/controllers/stylesheets_controller.rb @@ -3,6 +3,8 @@ class StylesheetsController < ApplicationController skip_before_action :preload_json, :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show, :show_source_map, :color_scheme] + before_action :apply_cdn_headers, only: [:show, :show_source_map, :color_scheme] + def show_source_map show_resource(source_map: true) end diff --git a/app/controllers/svg_sprite_controller.rb b/app/controllers/svg_sprite_controller.rb index 81b970547e1..5851e4b5645 100644 --- a/app/controllers/svg_sprite_controller.rb +++ b/app/controllers/svg_sprite_controller.rb @@ -3,6 +3,8 @@ class SvgSpriteController < ApplicationController skip_before_action :preload_json, :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show, :search, :svg_icon] + before_action :apply_cdn_headers, only: [:show, :search, :svg_icon] + requires_login except: [:show, :svg_icon] def show diff --git a/app/controllers/theme_javascripts_controller.rb b/app/controllers/theme_javascripts_controller.rb index 0fb5f526a5b..25dfd6567db 100644 --- a/app/controllers/theme_javascripts_controller.rb +++ b/app/controllers/theme_javascripts_controller.rb @@ -11,7 +11,7 @@ class ThemeJavascriptsController < ApplicationController only: [:show] ) - before_action :is_asset_path, :no_cookies, only: [:show] + before_action :is_asset_path, :no_cookies, :apply_cdn_headers, only: [:show] def show raise Discourse::NotFound unless last_modified.present? diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 814b65ac39d..f11c1997c63 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -8,7 +8,7 @@ class UploadsController < ApplicationController skip_before_action :preload_json, :check_xhr, :redirect_to_login_if_required, only: [:show, :show_short, :show_secure] protect_from_forgery except: :show - before_action :is_asset_path, only: [:show, :show_short, :show_secure] + before_action :is_asset_path, :apply_cdn_headers, only: [:show, :show_short, :show_secure] SECURE_REDIRECT_GRACE_SECONDS = 5 diff --git a/app/controllers/user_avatars_controller.rb b/app/controllers/user_avatars_controller.rb index 583fc81e653..d65baac29cc 100644 --- a/app/controllers/user_avatars_controller.rb +++ b/app/controllers/user_avatars_controller.rb @@ -4,6 +4,8 @@ class UserAvatarsController < ApplicationController skip_before_action :preload_json, :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show, :show_letter, :show_proxy_letter] + before_action :apply_cdn_headers, only: [:show, :show_letter, :show_proxy_letter] + def refresh_gravatar user = User.find_by(username_lower: params[:username].downcase) guardian.ensure_can_edit!(user) diff --git a/config/initializers/008-rack-cors.rb b/config/initializers/008-rack-cors.rb index e54a451657e..e32bbedc128 100644 --- a/config/initializers/008-rack-cors.rb +++ b/config/initializers/008-rack-cors.rb @@ -25,15 +25,18 @@ class Discourse::Cors status, headers, body = @app.call(env) headers ||= {} - Discourse::Cors.apply_headers(cors_origins, env, headers) if cors_origins + Discourse::Cors.apply_headers(cors_origins, env, headers) [status, headers, body] end def self.apply_headers(cors_origins, env, headers) - origin = nil + request_method = env['REQUEST_METHOD'] - if cors_origins + if env['SCRIPT_NAME'] == "/assets" && Discourse.is_cdn_request?(env, request_method) + Discourse.apply_cdn_headers(headers) + elsif cors_origins + origin = nil if origin = env['HTTP_ORIGIN'] origin = nil unless cors_origins.include?(origin) end @@ -48,6 +51,6 @@ class Discourse::Cors end end -if GlobalSetting.enable_cors +if GlobalSetting.enable_cors || GlobalSetting.cdn_url Rails.configuration.middleware.insert_before ActionDispatch::Flash, Discourse::Cors end diff --git a/lib/discourse.rb b/lib/discourse.rb index d9db43e4e59..5224b2aac2f 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -17,6 +17,7 @@ end module Discourse DB_POST_MIGRATE_PATH ||= "db/post_migrate" + REQUESTED_HOSTNAME ||= "REQUESTED_HOSTNAME" require 'sidekiq/exception_handler' class SidekiqExceptionHandler @@ -917,6 +918,24 @@ module Discourse def self.is_parallel_test? ENV['RAILS_ENV'] == "test" && ENV['TEST_ENV_NUMBER'] end + + CDN_REQUEST_METHODS ||= ["GET", "HEAD", "OPTIONS"] + + def self.is_cdn_request?(env, request_method) + return unless CDN_REQUEST_METHODS.include?(request_method) + + cdn_hostnames = GlobalSetting.cdn_hostnames + return if cdn_hostnames.blank? + + requested_hostname = env[REQUESTED_HOSTNAME] || env[Rack::HTTP_HOST] + cdn_hostnames.include?(requested_hostname) + end + + def self.apply_cdn_headers(headers) + headers['Access-Control-Allow-Origin'] = '*' + headers['Access-Control-Allow-Methods'] = CDN_REQUEST_METHODS.join(", ") + headers + end end # rubocop:enable Style/GlobalVars diff --git a/lib/middleware/enforce_hostname.rb b/lib/middleware/enforce_hostname.rb index 00e8870d57c..c462e5b7bcc 100644 --- a/lib/middleware/enforce_hostname.rb +++ b/lib/middleware/enforce_hostname.rb @@ -17,6 +17,7 @@ module Middleware allowed_hostnames = RailsMultisite::ConnectionManagement.current_db_hostnames requested_hostname = env[Rack::HTTP_HOST] + env[Discourse::REQUESTED_HOSTNAME] = requested_hostname env[Rack::HTTP_HOST] = allowed_hostnames.find { |h| h == requested_hostname } || Discourse.current_hostname @app.call(env) diff --git a/spec/requests/static_controller_spec.rb b/spec/requests/static_controller_spec.rb index 967185f567c..3156b2a7ab1 100644 --- a/spec/requests/static_controller_spec.rb +++ b/spec/requests/static_controller_spec.rb @@ -116,6 +116,25 @@ describe StaticController do File.delete(file_path) end end + + it 'has correct cors headers for brotli assets' do + begin + assets_path = Rails.root.join("public/assets") + + FileUtils.mkdir_p(assets_path) + + file_path = assets_path.join("test.js.br") + File.write(file_path, 'fake brotli file') + GlobalSetting.stubs(:cdn_url).returns("https://www.example.com/") + + get "/brotli_asset/test.js" + + expect(response.status).to eq(200) + expect(response.headers["Access-Control-Allow-Origin"]).to match("*") + ensure + File.delete(file_path) + end + end end context '#cdn_asset' do