diff --git a/app/models/upload.rb b/app/models/upload.rb index 60b39315c3c..e1fda0ead3a 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -470,7 +470,7 @@ class Upload < ActiveRecord::Base secure_status_did_change = self.secure? != mark_secure self.update(secure_params(mark_secure, reason, source)) - if Discourse.store.external? + if secure_status_did_change && SiteSetting.s3_use_acls && Discourse.store.external? begin Discourse.store.update_upload_ACL(self) rescue Aws::S3::Errors::NotImplemented => err diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index e1fab6b426b..e644146753a 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -444,6 +444,7 @@ RSpec.describe Upload do end it "marks the upload as not secure if its access control post is a public post" do + FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload) upload.update!(secure: true, access_control_post: Fabricate(:post)) upload.update_secure_status expect(upload.secure).to eq(false) @@ -455,6 +456,21 @@ RSpec.describe Upload do expect(upload.secure).to eq(true) end + it "does not attempt to change the ACL if the secure status has not changed" do + FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload).never + upload.update!(secure: true, access_control_post: Fabricate(:private_message_post)) + upload.update_secure_status + end + + it "does not attempt to change the ACL if s3_use_acls is disabled" do + SiteSetting.secure_uploads = false + SiteSetting.s3_use_acls = false + FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload).never + upload.update!(secure: true, access_control_post: Fabricate(:post)) + upload.update_secure_status + expect(upload.secure).to eq(false) + end + it "marks an image upload as secure if login_required is enabled" do SiteSetting.login_required = true upload.update!(secure: false) diff --git a/spec/tasks/uploads_spec.rb b/spec/tasks/uploads_spec.rb index 0f1a3693e1e..f2470377deb 100644 --- a/spec/tasks/uploads_spec.rb +++ b/spec/tasks/uploads_spec.rb @@ -146,7 +146,7 @@ RSpec.describe "tasks/uploads" do end it "does not attempt to update the acl" do - Discourse.store.expects(:update_upload_ACL).with(upload2).never + FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload2).never invoke_task end end