From 501de809daed7ba88c26463a3db7024d4357f1a1 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Fri, 28 May 2021 12:35:52 +1000
Subject: [PATCH] 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
https://github.com/discourse/discourse/pull/12081.

See
https://meta.discourse.org/t/secure-media-uploads/140017/122?u=martin
---
 app/models/badge.rb                           |  2 +-
 ...e_image_avatar_upload_security_and_acls.rb | 35 +++++++++++++++++++
 lib/upload_security.rb                        |  1 +
 spec/lib/upload_security_spec.rb              |  6 ++++
 4 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 db/post_migrate/20210528003603_fix_badge_image_avatar_upload_security_and_acls.rb

diff --git a/app/models/badge.rb b/app/models/badge.rb
index ccc95c99c89..bb9e0a67ccc 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 00000000000..e41c2f6d3df
--- /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 6ec2bbd5bd2..cac17d3bba7 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 07e2b544022..32ed9ead8f8 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