Commit 7036f75f authored by Stan Hu's avatar Stan Hu

Use system paths for appearance logos

When object storage is enabled, the logos used to customize a GitLab
appearance causes the time-limited URLs to be used. We fix this
by forcing all of these URLs to use the /uploads/-/system prefix
so that they will always be proxied through GitLab.

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/6778
parent 5dc656fc
...@@ -11,7 +11,7 @@ module AppearancesHelper ...@@ -11,7 +11,7 @@ module AppearancesHelper
end end
def brand_image def brand_image
image_tag(current_appearance.logo) if current_appearance&.logo? image_tag(current_appearance.logo_path) if current_appearance&.logo?
end end
def brand_text def brand_text
...@@ -28,7 +28,7 @@ module AppearancesHelper ...@@ -28,7 +28,7 @@ module AppearancesHelper
def brand_header_logo def brand_header_logo
if current_appearance&.header_logo? if current_appearance&.header_logo?
image_tag current_appearance.header_logo image_tag current_appearance.header_logo_path
else else
render 'shared/logo.svg' render 'shared/logo.svg'
end end
......
...@@ -58,7 +58,7 @@ module EmailsHelper ...@@ -58,7 +58,7 @@ module EmailsHelper
def header_logo def header_logo
if current_appearance&.header_logo? if current_appearance&.header_logo?
image_tag( image_tag(
current_appearance.header_logo, current_appearance.header_logo_path,
style: 'height: 50px' style: 'height: 50px'
) )
else else
......
...@@ -28,4 +28,32 @@ class Appearance < ActiveRecord::Base ...@@ -28,4 +28,32 @@ class Appearance < ActiveRecord::Base
errors.add(:single_appearance_row, 'Only 1 appearances row can exist') errors.add(:single_appearance_row, 'Only 1 appearances row can exist')
end end
end end
def logo_path
logo_system_path(logo, 'logo')
end
def header_logo_path
logo_system_path(header_logo, 'header_logo')
end
def favicon_path
logo_system_path(favicon, 'favicon')
end
private
def logo_system_path(logo, mount_type)
return unless logo&.upload
# If we're using a CDN, we need to use the full URL
asset_host = ActionController::Base.asset_host
local_path = Gitlab::Routing.url_helpers.appearance_upload_path(
filename: logo.filename,
id: logo.upload.model_id,
model: 'appearance',
mounted_as: mount_type)
Gitlab::Utils.append_path(asset_host, local_path)
end
end end
...@@ -17,7 +17,8 @@ scope path: :uploads do ...@@ -17,7 +17,8 @@ scope path: :uploads do
# Appearance # Appearance
get "-/system/:model/:mounted_as/:id/:filename", get "-/system/:model/:mounted_as/:id/:filename",
to: "uploads#show", to: "uploads#show",
constraints: { model: /appearance/, mounted_as: /logo|header_logo|favicon/, filename: /.+/ } constraints: { model: /appearance/, mounted_as: /logo|header_logo|favicon/, filename: /.+/ },
as: 'appearance_upload'
# Project markdown uploads # Project markdown uploads
get ":namespace_id/:project_id/:secret/:filename", get ":namespace_id/:project_id/:secret/:filename",
......
...@@ -15,6 +15,10 @@ FactoryBot.define do ...@@ -15,6 +15,10 @@ FactoryBot.define do
header_logo { fixture_file_upload('spec/fixtures/dk.png') } header_logo { fixture_file_upload('spec/fixtures/dk.png') }
end end
trait :with_favicon do
favicon { fixture_file_upload('spec/fixtures/dk.png') }
end
trait :with_logos do trait :with_logos do
with_logo with_logo
with_header_logo with_header_logo
......
...@@ -26,4 +26,34 @@ describe Appearance do ...@@ -26,4 +26,34 @@ describe Appearance do
let(:uploader_class) { AttachmentUploader } let(:uploader_class) { AttachmentUploader }
end end
end end
shared_examples 'logo paths' do |logo_type|
let(:appearance) { create(:appearance, "with_#{logo_type}".to_sym) }
let(:filename) { 'dk.png' }
let(:expected_path) { "/uploads/-/system/appearance/#{logo_type}/#{appearance.id}/#{filename}" }
it 'returns nil when there is no upload' do
expect(subject.send("#{logo_type}_path")).to be_nil
end
it 'returns a local path using the system route' do
expect(appearance.send("#{logo_type}_path")).to eq(expected_path)
end
describe 'with asset host configured' do
let(:asset_host) { 'https://gitlab-assets.example.com' }
before do
allow(ActionController::Base).to receive(:asset_host) { asset_host }
end
it 'returns a full URL with the system path' do
expect(appearance.send("#{logo_type}_path")).to eq("#{asset_host}#{expected_path}")
end
end
end
%i(logo header_logo favicon).each do |logo_type|
it_behaves_like 'logo paths', logo_type
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment