FIX: Handle storage providers not implementing ACLs (#13675)

FIX: Handle storage providers not implementing ACLs (#13675)

When secure media is enabled or when upload secure status is updated, we also try and update the upload ACL. However if the object storage provider does not implement this we get an Aws::S3::Errors::NotImplemented error. This PR handles this error so the update_secure_status method does not error out and still returns whether the secure status changed.

diff --git a/app/models/upload.rb b/app/models/upload.rb
index e660073..5b4b4be 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -361,7 +361,13 @@ class Upload < ActiveRecord::Base
     secure_status_did_change = self.secure? != mark_secure
     self.update(secure_params(mark_secure, reason, source))
 
-    Discourse.store.update_upload_ACL(self) if Discourse.store.external?
+    if Discourse.store.external?
+      begin
+        Discourse.store.update_upload_ACL(self)
+      rescue Aws::S3::Errors::NotImplemented => err
+        Discourse.warn_exception(err, message: "The file store object storage provider does not support setting ACLs")
+      end
+    end
 
     secure_status_did_change
   end
diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb
index a628d2e..c614b28 100644
--- a/spec/models/upload_spec.rb
+++ b/spec/models/upload_spec.rb
@@ -478,6 +478,14 @@ describe Upload do
         upload.update_secure_status
         expect(upload.reload.secure).to eq(false)
       end
+
+      it "does not throw an error if the object storage provider does not support ACLs" do
+        FileStore::S3Store.any_instance.stubs(:update_upload_ACL).raises(
+          Aws::S3::Errors::NotImplemented.new("A header you provided implies functionality that is not implemented", "")
+        )
+        upload.update!(secure: true, access_control_post: Fabricate(:private_message_post))
+        expect { upload.update_secure_status }.not_to raise_error
+      end
     end
   end
 

GitHub sha: 9f275c12ab97fef63c9251c059f6965138af48c8

This commit appears in #13675 which was approved by techAPJ. It was merged by martin.