Merge branch '44775-avatar-on-os-fails-with-cdn' into 'master'

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

Closes #44775

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

4 files changed, 26 insertions, 4 deletions

diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index 7677891b9ce..13246a774e3 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -31,12 +31,13 @@ module Avatarable
 
     asset_host = ActionController::Base.asset_host
     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,
     # 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
     # 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
       only_path = false
     end
@@ -49,6 +50,6 @@ module Avatarable
       url_base << gitlab_config.relative_url_root
     end
 
-    url_base + avatar.url
+    url_base + avatar.local_url
   end
 end
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index f12f0466a1d..f8a237178d9 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -65,6 +65,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
     !!model
   end
 
+  def local_url
+    File.join('/', self.class.base_dir, dynamic_segment, filename)
+  end
+
   private
 
   # Designed to be overridden by child uploaders that have a dynamic path
diff --git a/changelogs/unreleased/44775-avatar-on-os-fails-with-cdn.yml b/changelogs/unreleased/44775-avatar-on-os-fails-with-cdn.yml
new file mode 100644
index 00000000000..80b5b4a8abe
--- /dev/null
+++ b/changelogs/unreleased/44775-avatar-on-os-fails-with-cdn.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed wrong avatar URL when the avatar is on object storage.
+merge_request: 18092
+author:
+type: fixed
diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb
index 3696e6f62fd..9faf21bfbbd 100644
--- a/spec/models/concerns/avatarable_spec.rb
+++ b/spec/models/concerns/avatarable_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 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(:relative_url_root) { "/gitlab" }
@@ -37,11 +37,23 @@ describe Avatarable do
         project.visibility_level = visibility_level
       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
         expect(project.avatar_path(only_path: only_path)).to eq(avatar_path)
       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