From eaa3f813c1fd4c1192b49d69b5a74dd741766593 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Mon, 25 Nov 2024 11:12:00 +0300 Subject: [PATCH] FIX: Don't secure the about banner image (#29889) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uploads that are linked to site settings shouldn't be flagged as secure in login-required sites that enable secure uploads. However, in order for site setting uploads to not be marked secured, the frontend uploader has to include 2 params in the upload request: `for_site_setting: true` and `type: "site_setting"`. Since these 2 params are semantically identical, we want the `type: "site_setting"` param alone to make the upload correctly treated as a site setting upload. To achieve that, we need to include the `site_setting` type in the public types list because the `for_site_setting` param has the same effect — it marks the upload as a public type. https://github.com/discourse/discourse/blob/b138eaf9e58d3207009b5efb9d5ffe37300877e3/lib/upload_security.rb#L128-L131 --- lib/upload_security.rb | 1 + spec/system/admin_about_config_area_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/lib/upload_security.rb b/lib/upload_security.rb index baf387450f9..a0c87e5a901 100644 --- a/lib/upload_security.rb +++ b/lib/upload_security.rb @@ -37,6 +37,7 @@ class UploadSecurity category_background group_flair badge_image + site_setting ] PUBLIC_UPLOAD_REFERENCE_TYPES = %w[ diff --git a/spec/system/admin_about_config_area_spec.rb b/spec/system/admin_about_config_area_spec.rb index 6946794c6e1..a6e26fd20c2 100644 --- a/spec/system/admin_about_config_area_spec.rb +++ b/spec/system/admin_about_config_area_spec.rb @@ -139,6 +139,26 @@ describe "Admin About Config Area Page", type: :system do expect(config_area.general_settings_section).to have_saved_successfully expect(SiteSetting.about_banner_image).to eq(nil) end + + context "when login_required is true" do + before { SiteSetting.login_required = true } + + it "doesn't mark the banner image upload as secure" do + setup_or_skip_s3_system_test(enable_secure_uploads: true) + + config_area.visit + + image_file = file_from_fixtures("logo.png", "images") + config_area.general_settings_section.banner_image_uploader.select_image(image_file.path) + expect(config_area.general_settings_section.banner_image_uploader).to have_uploaded_image + + config_area.general_settings_section.submit + + expect(config_area.general_settings_section).to have_saved_successfully + + expect(SiteSetting.about_banner_image.secure).to eq(false) + end + end end end