FIX: Chat uploads over-secured in some situations (#29586)
Some checks are pending
Licenses / run (push) Waiting to run
Linting / run (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (annotations, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (backend, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (backend, plugins) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (frontend, plugins) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (frontend, themes) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, chat) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, core) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, plugins) (push) Waiting to run
Tests / ${{ matrix.target }} ${{ matrix.build_type }} (system, themes) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Chrome) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Firefox ESR) (push) Waiting to run
Tests / core frontend (${{ matrix.browser }}) (Firefox Evergreen) (push) Waiting to run

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.
This commit is contained in:
Martin Brennan 2024-11-05 15:56:30 +10:00 committed by GitHub
parent 3b0332ef6c
commit 54e7dee6d7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 35 additions and 4 deletions

View File

@ -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"

View File

@ -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