FIX: Do not mark group_flair images as secure on upload (#12081)

See https://meta.discourse.org/t/secure-media-uploads-breaks-group-flair-image/173671/4

Group flair image uploads definitely do not need to be secure.

app/assets/javascripts/discourse/app/templates/components/group-flair-inputs.hbs

@@ -2,13 +2,13 @@
   <label class="control-label" for="flair_url">{{i18n "groups.flair_url"}}</label>
 
   <div class="radios">
-    <label class="radio-label" for="avatar-flair">
-      {{radio-button name="avatar-flair" value="icon" selection=model.flair_type}}
+    <label class="radio-label" for="avatar-flair-icon">
+      {{radio-button name="avatar-flair-icon" id="avatar-flair-icon" value="icon" selection=model.flair_type}}
       <b>{{i18n "groups.flair_type.icon"}}</b>
     </label>
 
-    <label class="radio-label" for="avatar-flair">
-      {{radio-button name="avatar-flair" value="image" selection=model.flair_type}}
+    <label class="radio-label" for="avatar-flair-image">
+      {{radio-button name="avatar-flair-image" id="avatar-flair-image" value="image" selection=model.flair_type}}
       <b>{{i18n "groups.flair_type.image"}}</b>
     </label>
   </div>

db/post_migrate/20210215231312_fix_group_flair_avatar_upload_security_and_acls.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class FixGroupFlairAvatarUploadSecurityAndAcls < ActiveRecord::Migration[6.0]
+  disable_ddl_transaction!
+
+  def up
+    upload_ids = DB.query_single(<<~SQL
+      SELECT flair_upload_id
+      FROM groups
+      WHERE flair_upload_id IS NOT NULL
+     SQL
+    )
+
+    if upload_ids.any?
+      reason = "group_flair fixup migration"
+      DB.exec(<<~SQL, upload_ids: upload_ids, reason: reason, now: Time.zone.now)
+        UPDATE uploads SET secure = false, security_last_changed_at = :now, updated_at = :now, security_last_changed_reason = :reason
+        WHERE id IN (:upload_ids) AND uploads.secure
+      SQL
+
+      if Discourse.store.external?
+        uploads = Upload.where(id: upload_ids, secure: false).where("updated_at = security_last_changed_at")
+        uploads.each do |upload|
+          Discourse.store.update_upload_ACL(upload)
+          upload.touch
+        end
+      end
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/upload_security.rb

@@ -25,6 +25,7 @@ class UploadSecurity
     card_background
     category_logo
     category_background
+    group_flair
   ]
 
   def self.register_custom_public_type(type)

spec/lib/upload_security_spec.rb

@@ -25,6 +25,12 @@ RSpec.describe UploadSecurity do
       end
 
       context "when uploading in public context" do
+        describe "for a public type group_flair" do
+          let(:type) { 'group_flair' }
+          it "returns false" do
+            expect(subject.should_be_secure?).to eq(false)
+          end
+        end
         describe "for a public type avatar" do
           let(:type) { 'avatar' }
           it "returns false" do