FIX: strip the trailing slash (/) of cors origins. (#10996)

Strips trailing `/` from global settings
Provides a validation for site settings to ensure a trailing `/` is not added

config/initializers/008-rack-cors.rb

@@ -6,7 +6,7 @@ class Discourse::Cors
   def initialize(app, options = nil)
     @app = app
     if GlobalSetting.enable_cors && GlobalSetting.cors_origin.present?
-      @global_origins = GlobalSetting.cors_origin.split(',').map(&:strip)
+      @global_origins = GlobalSetting.cors_origin.split(',').map { |x| x.strip.chomp('/') }
     end
   end
 

config/locales/server.en.yml

@@ -199,6 +199,7 @@ en:
       second_factor_cannot_be_enforced_with_sso_enabled: "You cannot enforce 2FA if SSO is enabled."
       local_login_cannot_be_disabled_if_second_factor_enforced: "You cannot disable local login if 2FA is enforced. Disable enforced 2FA before disabling local logins."
       cannot_enable_s3_uploads_when_s3_enabled_globally: "You cannot enable S3 uploads because S3 uploads are already globally enabled, and enabling this site-level could cause critical issues with uploads"
+      cors_origins_should_not_have_trailing_slash: "You should not add the trailing slash (/) to CORS origins."
     conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to <br><a href="https://meta.discourse.org/t/76575">https://meta.discourse.org/t/76575</a>'
 
   activemodel:

lib/site_settings/validations.rb

@@ -191,6 +191,12 @@ module SiteSettings::Validations
     validate_error :local_login_cannot_be_disabled_if_second_factor_enforced
   end
 
+  def validate_cors_origins(new_val)
+    return if new_val.blank?
+    return unless new_val.split('|').any?(/\/$/)
+    validate_error :cors_origins_should_not_have_trailing_slash
+  end
+
   private
 
   def validate_bucket_setting(setting_name, upload_bucket, backup_bucket)

spec/components/hijack_spec.rb

@@ -116,6 +116,43 @@ describe Hijack do
     expect(headers).to eq(expected)
   end
 
+  it "removes trailing slash in cors origin" do
+    GlobalSetting.stubs(:enable_cors).returns(true)
+    GlobalSetting.stubs(:cors_origin).returns("https://www.rainbows.com/")
+
+    app = lambda do |env|
+      tester = Hijack::Tester.new(env)
+      tester.hijack_test do
+        render body: "hello", status: 201
+      end
+
+      expect(tester.io.string).to include("Access-Control-Allow-Origin: https://www.rainbows.com")
+    end
+
+    env = {}
+    middleware = Discourse::Cors.new(app)
+    middleware.call(env)
+
+    # it can do pre-flight
+    env = {
+      'REQUEST_METHOD' => 'OPTIONS',
+      'HTTP_ACCESS_CONTROL_REQUEST_METHOD' => 'GET'
+    }
+
+    status, headers, _body = middleware.call(env)
+
+    expect(status).to eq(200)
+
+    expected = {
+      "Access-Control-Allow-Origin" => "https://www.rainbows.com",
+      "Access-Control-Allow-Headers" => "Content-Type, Cache-Control, X-Requested-With, X-CSRF-Token, Discourse-Present, User-Api-Key, User-Api-Client-Id, Authorization",
+      "Access-Control-Allow-Credentials" => "true",
+      "Access-Control-Allow-Methods" => "POST, PUT, GET, OPTIONS, DELETE"
+    }
+
+    expect(headers).to eq(expected)
+  end
+
   it "handles transfers headers" do
     tester.response.headers["Hello-World"] = "sam"
     tester.hijack_test do

spec/lib/site_settings/validations_spec.rb

@@ -195,6 +195,16 @@ describe SiteSettings::Validations do
       end
     end
 
+    describe "#validate_cors_origins" do
+      let(:error_message) { I18n.t("errors.site_settings.cors_origins_should_not_have_trailing_slash") }
+
+      context "when the new value has trailing slash" do
+        it "should raise an error" do
+          expect { subject.validate_cors_origins("https://www.rainbows.com/") }.to raise_error(Discourse::InvalidParameters, error_message)
+        end
+      end
+    end
+
     describe "#validate_secure_media" do
       let(:error_message) { I18n.t("errors.site_settings.secure_media_requirements") }