FIX: Never allow custom emoji to be marked secure (#8965)

* Because custom emoji count as post "uploads" we were
marking them as secure when updating the secure status for post uploads.
* We were also giving them an access control post id, which meant
broken image previews from 403 errors in the admin custom emoji list.
* We now check if an upload is used as a custom emoji and do not
assign the access control post + never mark as secure.

app/models/post.rb

@@ -916,7 +916,11 @@ class Post < ActiveRecord::Base
       end
 
       if SiteSetting.secure_media?
-        Upload.where(id: upload_ids, access_control_post_id: nil).update_all(
+        Upload.where(
+          id: upload_ids, access_control_post_id: nil
+        ).where.not(
+          id: CustomEmoji.pluck(:upload_id)
+        ).update_all(
           access_control_post_id: self.id
         )
       end

app/models/upload.rb

@@ -254,7 +254,6 @@ class Upload < ActiveRecord::Base
   end
 
   def update_secure_status(secure_override_value: nil)
-    return false if self.for_theme || self.for_site_setting
     mark_secure = secure_override_value.nil? ? UploadSecurity.new(self).should_be_secure? : secure_override_value
 
     secure_status_did_change = self.secure? != mark_secure

lib/upload_security.rb

@@ -28,7 +28,7 @@ class UploadSecurity
   private
 
   def uploading_in_public_context?
-    @upload.for_theme || @upload.for_site_setting || @upload.for_gravatar || public_type?
+    @upload.for_theme || @upload.for_site_setting || @upload.for_gravatar || public_type? || used_for_custom_emoji?
   end
 
   def supported_media?
@@ -70,4 +70,9 @@ class UploadSecurity
   def uploading_in_composer?
     @upload_type == "composer"
   end
+
+  def used_for_custom_emoji?
+    return false if @upload.id.blank?
+    CustomEmoji.exists?(upload_id: @upload.id)
+  end
 end

spec/models/post_spec.rb

@@ -1376,6 +1376,16 @@ describe Post do
           expect(image_upload.access_control_post_id).to eq(post.id)
           expect(video_upload.access_control_post_id).not_to eq(post.id)
         end
+
+        context "for custom emoji" do
+          before do
+            CustomEmoji.create(name: "meme", upload: image_upload)
+          end
+          it "never sets an access control post because they should not be secure" do
+            post.link_post_uploads
+            expect(image_upload.reload.access_control_post_id).to eq(nil)
+          end
+        end
       end
     end
 

spec/models/upload_spec.rb

@@ -419,7 +419,17 @@ describe Upload do
         expect { upload.update_secure_status }
           .to change { upload.secure }
 
-        expect(upload.secure).to eq(true)
+        expect(upload.reload.secure).to eq(true)
+      end
+
+      it 'does not mark an upload used for a custom emoji as secure' do
+        SiteSetting.login_required = true
+        upload.update!(secure: false)
+        CustomEmoji.create(name: 'meme', upload: upload)
+        expect { upload.update_secure_status }
+          .not_to change { upload.secure }
+
+        expect(upload.reload.secure).to eq(false)
       end
     end
   end