Revert "Set a public ACL for files uploaded to a public GCS service"

This reverts commit 43503bdf.

activestorage/CHANGELOG.md

-*   Add support for `upload` options with `GCSService`.
-
-    For example, to add `Cache-Control` headers to uploaded files, modify
-    `config/storage.yml` with the `upload` key and corresponding Hash:
-
-    ```yml
-    google:
-      service: GCS
-      ...
-      upload:
-        cache_control: "public, max-age=60"
-    ```
-
-    *Brendan Abbott*
-
 *   Add `config.active_storage.web_image_content_types` to allow applications
     to add content types (like `image/webp`) in which variants can be processed,
     instead of letting those images be converted to the fallback PNG format.

activestorage/lib/active_storage/service/gcs_service.rb

@@ -7,13 +7,9 @@ module ActiveStorage
   # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API
   # documentation that applies to all services.
   class Service::GCSService < Service
-    attr_reader :upload_options
-
-    def initialize(public: false, upload: {}, **config)
+    def initialize(public: false, **config)
       @config = config
       @public = public
-      @upload_options = upload
-      @upload_options[:acl] = "public_read" if public?
     end
 
     def upload(key, io, checksum: nil, content_type: nil, disposition: nil, filename: nil)
@@ -23,7 +19,7 @@ def upload(key, io, checksum: nil, content_type: nil, disposition: nil, filename
         # binary and attachment when the file's content type requires it. The only way to force them is to
         # store them as object's metadata.
         content_disposition = content_disposition_with(type: disposition, filename: filename) if disposition && filename
-        bucket.create_file(io, key, md5: checksum, content_type: content_type, content_disposition: content_disposition, **upload_options)
+        bucket.create_file(io, key, md5: checksum, content_type: content_type, content_disposition: content_disposition)
       rescue Google::Cloud::InvalidArgumentError
         raise ActiveStorage::IntegrityError
       end
@@ -88,7 +84,7 @@ def exist?(key)
 
     def url_for_direct_upload(key, expires_in:, checksum:, **)
       instrument :url, key: key do |payload|
-        generated_url = bucket.signed_url key, method: "PUT", expires: expires_in, content_md5: checksum, **upload_options
+        generated_url = bucket.signed_url key, method: "PUT", expires: expires_in, content_md5: checksum
 
         payload[:url] = generated_url
 

activestorage/test/service/gcs_public_service_test.rb

@@ -9,34 +9,6 @@ class ActiveStorage::Service::GCSPublicServiceTest < ActiveSupport::TestCase
 
     include ActiveStorage::Service::SharedServiceTests
 
-    test "public acl options" do
-      assert_equal "public_read", @service.upload_options[:acl]
-    end
-
-    test "uploaded file is accessible by all users" do
-      assert_includes @service.bucket.find_file(@key).acl.readers, "allUsers"
-    end
-
-    test "direct upload file is accessible by all users" do
-      key      = SecureRandom.base58(24)
-      data     = "Something else entirely!"
-      checksum = Digest::MD5.base64digest(data)
-      url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
-
-      uri = URI.parse url
-      request = Net::HTTP::Put.new uri.request_uri
-      request.body = data
-      request.add_field "Content-Type", ""
-      request.add_field "Content-MD5", checksum
-      Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
-        http.request request
-      end
-
-      assert_includes @service.bucket.find_file(key).acl.readers, "allUsers"
-    ensure
-      @service.delete key
-    end
-
     test "public URL generation" do
       url = @service.url(@key, filename: ActiveStorage::Filename.new("avatar.png"))
 

activestorage/test/service/gcs_service_test.rb

@@ -57,33 +57,6 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
       @service.delete key
     end
 
-    test "direct upload with custom upload options" do
-      cache_control = "public, max-age=60"
-      service = build_service(upload: { cache_control: cache_control })
-
-      key      = SecureRandom.base58(24)
-      data     = "Something else entirely!"
-      checksum = Digest::MD5.base64digest(data)
-      url      = service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
-
-      uri = URI.parse url
-      request = Net::HTTP::Put.new uri.request_uri
-      request.body = data
-      service.headers_for_direct_upload(key, checksum: checksum, filename: ActiveStorage::Filename.new("test.txt"), disposition: :attachment).each do |k, v|
-        request.add_field k, v
-      end
-      request.add_field "Content-Type", ""
-      Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
-        http.request request
-      end
-
-      url = service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
-      response = Net::HTTP.get_response(URI(url))
-      assert_equal(cache_control, response["Cache-Control"])
-    ensure
-      service.delete key
-    end
-
     test "upload with content_type and content_disposition" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
@@ -112,21 +85,6 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
       @service.delete key
     end
 
-    test "upload with custom upload options" do
-      key = SecureRandom.base58(24)
-      data = "Something else entirely!"
-      cache_control = "public, max-age=60"
-      service = build_service(upload: { cache_control: cache_control })
-
-      begin
-        service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
-
-        assert_equal cache_control, service.bucket.find_file(key).cache_control
-      ensure
-        service.delete key
-      end
-    end
-
     test "update metadata" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"
@@ -146,11 +104,6 @@ class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase
       assert_match(/storage\.googleapis\.com\/.*response-content-disposition=inline.*test\.txt.*response-content-type=text%2Fplain/,
         @service.url(@key, expires_in: 2.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain"))
     end
-
-    private
-      def build_service(configuration)
-        ActiveStorage::Service.configure(:gcs, SERVICE_CONFIGURATIONS.deep_merge(gcs: configuration))
-      end
   end
 else
   puts "Skipping GCS Service tests because no GCS configuration was supplied"

guides/source/active_storage_overview.md

@@ -118,7 +118,6 @@ amazon:
   secret_access_key: ""
   region: ""
   bucket: ""
-  public: false
 ```
 
 Optionally provide a Hash of upload options:
@@ -130,7 +129,7 @@ amazon:
   secret_access_key: ""
   region: ""
   bucket: ""
-  upload:
+  upload: 
     server_side_encryption: "" # 'aws:kms' or 'AES256'
 ```
 
@@ -177,7 +176,6 @@ google:
   credentials: <%= Rails.root.join("path/to/keyfile.json") %>
   project: ""
   bucket: ""
-  public: false
 ```
 
 Optionally provide a Hash of credentials instead of a keyfile path:
@@ -200,22 +198,6 @@ google:
   bucket: ""
 ```
 
-Optionally provide a Hash of upload options:
-
-```yaml
-google:
-  service: GCS
-  credentials: <%= Rails.root.join("path/to/keyfile.json") %>
-  project: ""
-  bucket: ""
-  upload:
-    acl: "" # will be set to `public_read` on public buckets
-    cache_control: ""
-    storage_class: ""
-```
-
-The [Google Cloud Storage SDK docs](https://googleapis.dev/ruby/google-cloud-storage/latest/Google/Cloud/Storage/Bucket.html#create_file-instance_method) detail other possible upload options.
-
 Add the [`google-cloud-storage`](https://github.com/GoogleCloudPlatform/google-cloud-ruby/tree/master/google-cloud-storage) gem to your `Gemfile`:
 
 ```ruby