diff --git a/app/jobs/regular/update_private_uploads_acl.rb b/app/jobs/regular/update_private_uploads_acl.rb
index 4a95f5a5e4..661f84437c 100644
--- a/app/jobs/regular/update_private_uploads_acl.rb
+++ b/app/jobs/regular/update_private_uploads_acl.rb
@@ -4,7 +4,7 @@ module Jobs
class UpdatePrivateUploadsAcl < ::Jobs::Base
# only runs when SiteSetting.prevent_anons_from_downloading_files is updated
def execute(args)
- return if !SiteSetting.enable_s3_uploads
+ return if !SiteSetting.Upload.enable_s3_uploads
Upload.find_each do |upload|
if !FileHelper.is_supported_media?(upload.original_filename)
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 777895b7dd..73eb81b5e9 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -208,6 +208,7 @@ en:
secure_media_requirements: "S3 uploads must be enabled before enabling secure media."
second_factor_cannot_be_enforced_with_disabled_local_login: "You cannot enforce 2FA if local logins are disabled."
local_login_cannot_be_disabled_if_second_factor_enforced: "You cannot disable local login if 2FA is enforced. Disable enforced 2FA before disabling local logins."
+ cannot_enable_s3_uploads_when_s3_enabled_globally: "You cannot enable S3 uploads because S3 uploads are already globally enabled, and enabling this site-level could cause critical issues with uploads"
conflicting_google_user_id: 'The Google Account ID for this account has changed; staff intervention is required for security reasons. Please contact staff and point them to
https://meta.discourse.org/t/76575'
activemodel:
diff --git a/lib/site_settings/validations.rb b/lib/site_settings/validations.rb
index 65534e0c90..a24da43db5 100644
--- a/lib/site_settings/validations.rb
+++ b/lib/site_settings/validations.rb
@@ -118,11 +118,13 @@ module SiteSettings::Validations
end
def validate_enable_s3_uploads(new_val)
- validate_error :s3_upload_bucket_is_required if new_val == "t" && SiteSetting.s3_upload_bucket.blank?
+ return if new_val == "f"
+ validate_error :cannot_enable_s3_uploads_when_s3_enabled_globally if GlobalSetting.use_s3?
+ validate_error :s3_upload_bucket_is_required if SiteSetting.s3_upload_bucket.blank?
end
def validate_secure_media(new_val)
- validate_error :secure_media_requirements if new_val == "t" && !SiteSetting.enable_s3_uploads?
+ validate_error :secure_media_requirements if new_val == "t" && !SiteSetting.Upload.enable_s3_uploads
end
def validate_enable_s3_inventory(new_val)
diff --git a/spec/jobs/regular/update_private_uploads_acl_spec.rb b/spec/jobs/regular/update_private_uploads_acl_spec.rb
new file mode 100644
index 0000000000..d80713d632
--- /dev/null
+++ b/spec/jobs/regular/update_private_uploads_acl_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Jobs::UpdatePrivateUploadsAcl do
+ let(:args) { [] }
+
+ before do
+ SiteSetting.authorized_extensions = "pdf"
+ end
+
+ describe '#execute' do
+ context "if not SiteSetting.Upload.enable_s3_uploads" do
+ before do
+ SiteSetting.Upload.stubs(:enable_s3_uploads).returns(false)
+ end
+ it "returns early and changes no uploads" do
+ Upload.expects(:find_each).never
+ subject.execute(args)
+ end
+ end
+ context "if SiteSetting.Upload.enable_s3_uploads" do
+ let!(:upload) { Fabricate(:upload_s3, extension: 'pdf', original_filename: "watchmen.pdf", secure: false) }
+ before do
+ SiteSetting.login_required = true
+ SiteSetting.prevent_anons_from_downloading_files = true
+ SiteSetting::Upload.stubs(:enable_s3_uploads).returns(true)
+ Discourse.stubs(:store).returns(stub(external?: false))
+ end
+
+ it "changes the upload to secure" do
+ subject.execute(args)
+ expect(upload.reload.secure).to eq(true)
+ end
+ end
+ end
+end
diff --git a/spec/lib/site_settings/validations_spec.rb b/spec/lib/site_settings/validations_spec.rb
index 6660872806..7f346d4d5c 100644
--- a/spec/lib/site_settings/validations_spec.rb
+++ b/spec/lib/site_settings/validations_spec.rb
@@ -125,7 +125,7 @@ describe SiteSettings::Validations do
end
it "should be ok" do
- expect { subject.validate_enforce_second_factor("t") }.not_to raise_error(Discourse::InvalidParameters, error_message)
+ expect { subject.validate_enforce_second_factor("t") }.not_to raise_error
end
end
end
@@ -150,14 +150,98 @@ describe SiteSettings::Validations do
end
it "should be ok" do
- expect { subject.validate_enable_local_logins("f") }.not_to raise_error(Discourse::InvalidParameters, error_message)
+ expect { subject.validate_enable_local_logins("f") }.not_to raise_error
end
end
end
context "when the new value is true" do
it "should be ok" do
- expect { subject.validate_enable_local_logins("t") }.not_to raise_error(Discourse::InvalidParameters, error_message)
+ expect { subject.validate_enable_local_logins("t") }.not_to raise_error
+ end
+ end
+ end
+
+ describe "#validate_secure_media" do
+ let(:error_message) { I18n.t("errors.site_settings.secure_media_requirements") }
+
+ context "when the new value is true" do
+ context 'if site setting for enable_s3_uploads is enabled' do
+ before do
+ SiteSetting.enable_s3_uploads = true
+ end
+
+ it "should be ok" do
+ expect { subject.validate_secure_media("t") }.not_to raise_error
+ end
+ end
+
+ context 'if site setting for enable_s3_uploads is not enabled' do
+ before do
+ SiteSetting.enable_s3_uploads = false
+ end
+
+ it "is not ok" do
+ expect { subject.validate_secure_media("t") }.to raise_error(Discourse::InvalidParameters, error_message)
+ end
+
+ context "if global s3 setting is enabled" do
+ before do
+ GlobalSetting.stubs(:use_s3?).returns(true)
+ end
+
+ it "should be ok" do
+ expect { subject.validate_secure_media("t") }.not_to raise_error
+ end
+ end
+ end
+ end
+ end
+
+ describe "#validate_enable_s3_uploads" do
+ let(:error_message) { I18n.t("errors.site_settings.cannot_enable_s3_uploads_when_s3_enabled_globally") }
+
+ context "when the new value is true" do
+ context "when s3 uploads are already globally enabled" do
+ before do
+ GlobalSetting.stubs(:use_s3?).returns(true)
+ end
+
+ it "is not ok" do
+ expect { subject.validate_enable_s3_uploads("t") }.to raise_error(Discourse::InvalidParameters, error_message)
+ end
+ end
+
+ context "when s3 uploads are not already globally enabled" do
+ before do
+ GlobalSetting.stubs(:use_s3?).returns(false)
+ end
+
+ it "should be ok" do
+ expect { subject.validate_enable_s3_uploads("t") }.not_to raise_error
+ end
+ end
+
+ context "when the s3_upload_bucket is blank" do
+ let(:error_message) { I18n.t("errors.site_settings.s3_upload_bucket_is_required") }
+
+ before do
+ SiteSetting.s3_upload_bucket = nil
+ end
+
+ it "is not ok" do
+ expect { subject.validate_enable_s3_uploads("t") }.to raise_error(Discourse::InvalidParameters, error_message)
+ end
+ end
+
+ context "when the s3_upload_bucket is not blank" do
+ before do
+ SiteSetting.s3_upload_bucket = "some-bucket"
+ end
+
+ it "should be ok" do
+ expect { subject.validate_enable_s3_uploads("t") }.not_to raise_error
+ end
end
end
end