Commit 741f333d authored by Micaël Bergeron's avatar Micaël Bergeron Committed by John Northrup

Resolve "Avatar URLs are wrong when using a CDN path and Object Storage"

parent c0eabb84
...@@ -31,12 +31,13 @@ module Avatarable ...@@ -31,12 +31,13 @@ module Avatarable
asset_host = ActionController::Base.asset_host asset_host = ActionController::Base.asset_host
use_asset_host = asset_host.present? use_asset_host = asset_host.present?
use_authentication = respond_to?(:public?) && !public?
# Avatars for private and internal groups and projects require authentication to be viewed, # Avatars for private and internal groups and projects require authentication to be viewed,
# which means they can only be served by Rails, on the regular GitLab host. # which means they can only be served by Rails, on the regular GitLab host.
# If an asset host is configured, we need to return the fully qualified URL # If an asset host is configured, we need to return the fully qualified URL
# instead of only the avatar path, so that Rails doesn't prefix it with the asset host. # instead of only the avatar path, so that Rails doesn't prefix it with the asset host.
if use_asset_host && respond_to?(:public?) && !public? if use_asset_host && use_authentication
use_asset_host = false use_asset_host = false
only_path = false only_path = false
end end
...@@ -49,6 +50,6 @@ module Avatarable ...@@ -49,6 +50,6 @@ module Avatarable
url_base << gitlab_config.relative_url_root url_base << gitlab_config.relative_url_root
end end
url_base + avatar.url url_base + avatar.local_url
end end
end end
...@@ -65,6 +65,10 @@ class GitlabUploader < CarrierWave::Uploader::Base ...@@ -65,6 +65,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
!!model !!model
end end
def local_url
File.join('/', self.class.base_dir, dynamic_segment, filename)
end
private private
# Designed to be overridden by child uploaders that have a dynamic path # Designed to be overridden by child uploaders that have a dynamic path
......
---
title: Fixed wrong avatar URL when the avatar is on object storage.
merge_request: 18092
author:
type: fixed
require 'spec_helper' require 'spec_helper'
describe Avatarable do describe Avatarable do
set(:project) { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) } let(:project) { create(:project, :with_avatar) }
let(:gitlab_host) { "https://gitlab.example.com" } let(:gitlab_host) { "https://gitlab.example.com" }
let(:relative_url_root) { "/gitlab" } let(:relative_url_root) { "/gitlab" }
...@@ -37,11 +37,23 @@ describe Avatarable do ...@@ -37,11 +37,23 @@ describe Avatarable do
project.visibility_level = visibility_level project.visibility_level = visibility_level
end end
let(:avatar_path) { (avatar_path_prefix + [project.avatar.url]).join } let(:avatar_path) { (avatar_path_prefix + [project.avatar.local_url]).join }
it 'returns the expected avatar path' do it 'returns the expected avatar path' do
expect(project.avatar_path(only_path: only_path)).to eq(avatar_path) expect(project.avatar_path(only_path: only_path)).to eq(avatar_path)
end end
context "when avatar is stored remotely" do
before do
stub_uploads_object_storage(AvatarUploader)
project.avatar.migrate!(ObjectStorage::Store::REMOTE)
end
it 'returns the expected avatar path' do
expect(project.avatar_url(only_path: only_path)).to eq(avatar_path)
end
end
end end
end 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