From 43503bdfecb86ed7386eecc54a75ccf3744b5dc2 Mon Sep 17 00:00:00 2001 From: Brendan Abbott Date: Wed, 29 Apr 2020 05:44:17 +1000 Subject: [PATCH] Set a public ACL for files uploaded to a public GCS service --- activestorage/CHANGELOG.md | 15 ++++++ .../lib/active_storage/service/gcs_service.rb | 10 ++-- .../test/service/gcs_public_service_test.rb | 28 +++++++++++ .../test/service/gcs_service_test.rb | 47 +++++++++++++++++++ guides/source/active_storage_overview.md | 20 +++++++- 5 files changed, 116 insertions(+), 4 deletions(-) diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 8e43acc97ec93..cffb2b26cecbf 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,18 @@ +* 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. diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index f166aee07ec66..ec63a55a4421c 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -7,9 +7,13 @@ 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 - def initialize(public: false, **config) + attr_reader :upload_options + + def initialize(public: false, upload: {}, **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) @@ -19,7 +23,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) + bucket.create_file(io, key, md5: checksum, content_type: content_type, content_disposition: content_disposition, **upload_options) rescue Google::Cloud::InvalidArgumentError raise ActiveStorage::IntegrityError end @@ -84,7 +88,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 + generated_url = bucket.signed_url key, method: "PUT", expires: expires_in, content_md5: checksum, **upload_options payload[:url] = generated_url diff --git a/activestorage/test/service/gcs_public_service_test.rb b/activestorage/test/service/gcs_public_service_test.rb index 9ab440122d52b..759546b37dd98 100644 --- a/activestorage/test/service/gcs_public_service_test.rb +++ b/activestorage/test/service/gcs_public_service_test.rb @@ -9,6 +9,34 @@ 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")) diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb index 4a982372773f4..57fb8f68c16e1 100644 --- a/activestorage/test/service/gcs_service_test.rb +++ b/activestorage/test/service/gcs_service_test.rb @@ -57,6 +57,33 @@ 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!" @@ -85,6 +112,21 @@ 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!" @@ -104,6 +146,11 @@ 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" diff --git a/guides/source/active_storage_overview.md b/guides/source/active_storage_overview.md index f5d2521778302..29e002fe81d45 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -118,6 +118,7 @@ amazon: secret_access_key: "" region: "" bucket: "" + public: false ``` Optionally provide a Hash of upload options: @@ -129,7 +130,7 @@ amazon: secret_access_key: "" region: "" bucket: "" - upload: + upload: server_side_encryption: "" # 'aws:kms' or 'AES256' ``` @@ -176,6 +177,7 @@ google: credentials: <%= Rails.root.join("path/to/keyfile.json") %> project: "" bucket: "" + public: false ``` Optionally provide a Hash of credentials instead of a keyfile path: @@ -198,6 +200,22 @@ 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