FIX: Change additional public uploads to not be secure (#8738)

Custom emoji, profile background, and card background were being set to secure, which we do not want as they are always in a public context and result in a 403 error from the ACL if linked directly.

lib/upload_security.rb

@@ -17,6 +17,7 @@ class UploadSecurity
   def initialize(upload, opts = {})
     @upload = upload
     @opts = opts
+    @upload_type = @opts[:type]
   end
 
   def should_be_secure?
@@ -27,7 +28,7 @@ class UploadSecurity
   private
 
   def uploading_in_public_context?
-    @upload.for_theme || @upload.for_site_setting || avatar?
+    @upload.for_theme || @upload.for_site_setting || @upload.for_gravatar || public_type?
   end
 
   def supported_media?
@@ -47,7 +48,7 @@ class UploadSecurity
     if @upload.access_control_post_id.present?
       return access_control_post_has_secure_media?
     end
-    composer? || @upload.for_private_message || @upload.secure?
+    uploading_in_composer? || @upload.for_private_message || @upload.for_group_message || @upload.secure?
   end
 
   # whether the upload should remain secure or not after posting depends on its context,
@@ -62,11 +63,11 @@ class UploadSecurity
     Post.find_by(id: @upload.access_control_post_id).with_secure_media?
   end
 
-  def avatar?
-    @opts[:type] == "avatar"
+  def public_type?
+    %w[avatar custom_emoji profile_background card_background].include?(@upload_type)
   end
 
-  def composer?
-    @opts[:type] == "composer"
+  def uploading_in_composer?
+    @upload_type == "composer"
   end
 end

spec/lib/upload_creator_spec.rb

@@ -295,12 +295,16 @@ RSpec.describe UploadCreator do
           expect(result.original_sha1).not_to eq(nil)
         end
 
-        context "when uploading in a public context (theme, site setting, avatar)" do
-          it "does not set the upload to secure" do
+        context "when uploading in a public context (theme, site setting, avatar, custom_emoji, profile_background, card_background)" do
+          def expect_no_public_context_uploads_to_be_secure
             upload = UploadCreator.new(file_from_fixtures(filename), filename, for_site_setting: true).create_for(user.id)
             expect(upload.secure).to eq(false)
             upload.destroy!
 
+            upload = UploadCreator.new(file_from_fixtures(filename), filename, for_gravatar: true).create_for(user.id)
+            expect(upload.secure).to eq(false)
+            upload.destroy!
+
             upload = UploadCreator.new(file_from_fixtures(filename), filename, for_theme: true).create_for(user.id)
             expect(upload.secure).to eq(false)
             upload.destroy!
@@ -308,6 +312,32 @@ RSpec.describe UploadCreator do
             upload = UploadCreator.new(file_from_fixtures(filename), filename, type: "avatar").create_for(user.id)
             expect(upload.secure).to eq(false)
             upload.destroy!
+
+            upload = UploadCreator.new(file_from_fixtures(filename), filename, type: "custom_emoji").create_for(user.id)
+            expect(upload.secure).to eq(false)
+            upload.destroy!
+
+            upload = UploadCreator.new(file_from_fixtures(filename), filename, type: "profile_background").create_for(user.id)
+            expect(upload.secure).to eq(false)
+            upload.destroy!
+
+            upload = UploadCreator.new(file_from_fixtures(filename), filename, type: "card_background").create_for(user.id)
+            expect(upload.secure).to eq(false)
+            upload.destroy!
+          end
+
+          it "does not set the upload to secure" do
+            expect_no_public_context_uploads_to_be_secure
+          end
+
+          context "when login required" do
+            before do
+              SiteSetting.login_required = true
+            end
+
+            it "does not set the upload to secure" do
+              expect_no_public_context_uploads_to_be_secure
+            end
           end
         end
 
@@ -327,6 +357,22 @@ RSpec.describe UploadCreator do
           end
         end
 
+        context "if the upload is for a group message" do
+          let(:opts) { { for_group_message: true } }
+          it "sets the upload to secure and sets the original_sha1" do
+            expect(result.secure).to eq(true)
+            expect(result.original_sha1).not_to eq(nil)
+          end
+        end
+
+        context "if the upload is for a PM" do
+          let(:opts) { { for_private_message: true } }
+          it "sets the upload to secure and sets the original_sha1" do
+            expect(result.secure).to eq(true)
+            expect(result.original_sha1).not_to eq(nil)
+          end
+        end
+
         context "if SiteSetting.login_required" do
           before do
             SiteSetting.login_required = true