From 54e7dee6d7dec298d248f2d9d8f3902b9e4cdf39 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 5 Nov 2024 15:56:30 +1000
Subject: [PATCH] FIX: Chat uploads over-secured in some situations (#29586)

In the case where:

* Secure uploads were enabled
* Allow unsecure chat uploads was enabled
* For a site with login required enabled

When a chat upload was created, it was being marked as secure. Since
there is no provision for secure uploads in chat, this would lead to
broken uploads/images shown in the channel.

We can use the "public types" functionality of secure uploads to make
sure we never mark chat uploads as secure, and we can revisit this
whenever we get around to allowing secure uploads in chat.
---
 plugins/chat/plugin.rb           |  4 ++++
 plugins/chat/spec/plugin_spec.rb | 35 ++++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb
index c293627dce1..f95165ec36c 100644
--- a/plugins/chat/plugin.rb
+++ b/plugins/chat/plugin.rb
@@ -548,6 +548,10 @@ after_initialize do
   )
 
   register_bookmarkable(Chat::MessageBookmarkable)
+
+  # When we eventually allow secure_uploads in chat, this will need to be
+  # removed. Depending on the channel, uploads may end up being secure.
+  UploadSecurity.register_custom_public_type("chat-composer")
 end
 
 if Rails.env == "test"
diff --git a/plugins/chat/spec/plugin_spec.rb b/plugins/chat/spec/plugin_spec.rb
index 361b09f60f1..6f057403ef4 100644
--- a/plugins/chat/spec/plugin_spec.rb
+++ b/plugins/chat/spec/plugin_spec.rb
@@ -264,6 +264,8 @@ describe Chat do
   end
 
   describe "secure uploads compatibility" do
+    fab!(:user)
+
     it "disables chat uploads if secure uploads changes from disabled to enabled" do
       enable_secure_uploads
       expect(SiteSetting.chat_allow_uploads).to eq(false)
@@ -275,10 +277,35 @@ describe Chat do
       expect(last_history.context).to eq("Disabled because secure_uploads is enabled")
     end
 
-    it "does not disable chat uploads if the allow_unsecure_chat_uploads global setting is set" do
-      global_setting :allow_unsecure_chat_uploads, true
-      expect { enable_secure_uploads }.not_to change { UserHistory.count }
-      expect(SiteSetting.chat_allow_uploads).to eq(true)
+    context "when the global setting allow_unsecure_chat_uploads is true" do
+      fab!(:filename) { "small.pdf" }
+      fab!(:file) { file_from_fixtures(filename, "pdf") }
+
+      before { global_setting :allow_unsecure_chat_uploads, true }
+
+      it "does not disable chat uploads" do
+        expect { enable_secure_uploads }.not_to change { UserHistory.count }
+        expect(SiteSetting.chat_allow_uploads).to eq(true)
+      end
+
+      it "does not mark chat uploads as secure" do
+        filename = "small.pdf"
+        file = file_from_fixtures(filename, "pdf")
+
+        enable_secure_uploads
+        upload = UploadCreator.new(file, filename, type: "chat-composer").create_for(user.id)
+        expect(upload.secure).to eq(false)
+      end
+
+      context "when login_required is true" do
+        before { SiteSetting.login_required = true }
+
+        it "does not mark chat uploads as secure" do
+          enable_secure_uploads
+          upload = UploadCreator.new(file, filename, type: "chat-composer").create_for(user.id)
+          expect(upload.secure).to eq(false)
+        end
+      end
     end
   end