Merge branch 'fix/favicon-cross-origin' into 'master'

Fix: Serve favicon image always from the main GitLab domain to avoid issues with CORS

Closes #47802

See merge request gitlab-org/gitlab-ce!19810

changelogs/unreleased/fix-favicon-cross-origin.yml

+---
+title: Serve favicon image always from the main GitLab domain to avoid issues with CORS
+merge_request: 19810
+author: Alexis Reigel
+type: fixed

lib/gitlab/favicon.rb

@@ -2,10 +2,10 @@ module Gitlab
   class Favicon
     class << self
       def main
-        return appearance_favicon.url if appearance_favicon.exists?
-
         image_name =
-          if Gitlab::Utils.to_boolean(ENV['CANARY'])
+          if appearance_favicon.exists?
+            appearance_favicon.url
+          elsif Gitlab::Utils.to_boolean(ENV['CANARY'])
             'favicon-yellow.png'
           elsif Rails.env.development?
             'favicon-blue.png'
@@ -13,7 +13,7 @@ module Gitlab
             'favicon.png'
           end
 
-        ActionController::Base.helpers.image_path(image_name)
+        ActionController::Base.helpers.image_path(image_name, host: host)
       end
 
       def status_overlay(status_name)
@@ -22,7 +22,7 @@ module Gitlab
           "#{status_name}.png"
         )
 
-        ActionController::Base.helpers.image_path(path)
+        ActionController::Base.helpers.image_path(path, host: host)
       end
 
       def available_status_names
@@ -35,6 +35,16 @@ module Gitlab
 
       private
 
+      # we only want to create full urls when there's a different asset_host
+      # configured.
+      def host
+        if Gitlab::Application.config.asset_host.nil? || Gitlab::Application.config.asset_host == Gitlab.config.gitlab.base_url
+          nil
+        else
+          Gitlab.config.gitlab.base_url
+        end
+      end
+
       def appearance
         RequestStore.store[:appearance] ||= (Appearance.current || Appearance.new)
       end

spec/lib/gitlab/favicon_spec.rb

@@ -21,6 +21,21 @@ RSpec.describe Gitlab::Favicon, :request_store do
       create :appearance, favicon: fixture_file_upload('spec/fixtures/dk.png')
       expect(described_class.main).to match %r{/uploads/-/system/appearance/favicon/\d+/dk.png}
     end
+
+    context 'asset host' do
+      before do
+        allow(Rails).to receive(:env).and_return(ActiveSupport::StringInquirer.new('production'))
+      end
+
+      it 'returns a relative url when the asset host is not configured' do
+        expect(described_class.main).to match %r{^/assets/favicon-(?:\h+).png$}
+      end
+
+      it 'returns a full url when the asset host is configured' do
+        allow(Gitlab::Application.config).to receive(:asset_host).and_return('http://assets.local')
+        expect(described_class.main).to match %r{^http://localhost/assets/favicon-(?:\h+).png$}
+      end
+    end
   end
 
   describe '.status_overlay' do