FIX: Do not mark badge image uploads as secure (#13193)

FIX: Do not mark badge image uploads as secure (#13193)

  • FIX: Do not mark badge image uploads as secure

We do not need badge_image upload types to be marked as secure. Post migration is the same as FIX: Do not mark group_flair images as secure on upload by martin-brennan · Pull Request #12081 · discourse/discourse · GitHub.

See Secure Media Uploads - #122 by danilogit - announcements - Discourse Meta

diff --git a/app/models/badge.rb b/app/models/badge.rb
index ccc95c9..bb9e0a6 100644
--- a/app/models/badge.rb
+++ b/app/models/badge.rb
@@ -340,8 +340,8 @@ end
 #  trigger           :integer
 #  show_posts        :boolean          default(FALSE), not null
 #  system            :boolean          default(FALSE), not null
-#  image             :string(255)
 #  long_description  :text
+#  image_upload_id   :integer
 #
 # Indexes
 #
diff --git a/db/post_migrate/20210528003603_fix_badge_image_avatar_upload_security_and_acls.rb b/db/post_migrate/20210528003603_fix_badge_image_avatar_upload_security_and_acls.rb
new file mode 100644
index 0000000..e41c2f6
--- /dev/null
+++ b/db/post_migrate/20210528003603_fix_badge_image_avatar_upload_security_and_acls.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class FixBadgeImageAvatarUploadSecurityAndAcls < ActiveRecord::Migration[6.1]
+  disable_ddl_transaction!
+
+  def up
+    upload_ids = DB.query_single(<<~SQL
+      SELECT image_upload_id
+      FROM badges
+      INNER JOIN uploads ON uploads.id = badges.image_upload_id
+      WHERE image_upload_id IS NOT NULL AND uploads.secure
+     SQL
+    )
+
+    if upload_ids.any?
+      reason = "badge_image 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)
+      SQL
+
+      if Discourse.store.external?
+        uploads = Upload.where(id: upload_ids)
+        uploads.each do |upload|
+          Discourse.store.update_upload_ACL(upload)
+          upload.touch
+        end
+      end
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/lib/upload_security.rb b/lib/upload_security.rb
index 6ec2bbd..cac17d3 100644
--- a/lib/upload_security.rb
+++ b/lib/upload_security.rb
@@ -26,6 +26,7 @@ class UploadSecurity
     category_logo
     category_background
     group_flair
+    badge_image
   ]
 
   def self.register_custom_public_type(type)
diff --git a/spec/lib/upload_security_spec.rb b/spec/lib/upload_security_spec.rb
index 07e2b54..32ed9ea 100644
--- a/spec/lib/upload_security_spec.rb
+++ b/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 badge_image" do
+          let(:type) { 'badge_image' }
+          it "returns false" do
+            expect(subject.should_be_secure?).to eq(false)
+          end
+        end
         describe "for a public type group_flair" do
           let(:type) { 'group_flair' }
           it "returns false" do

GitHub sha: 501de809

This commit appears in #13193 which was approved by lis2. It was merged by martin.