From 244b37fef7ec2ce930a959711aa08084b6e69449 Mon Sep 17 00:00:00 2001
From: Chris Smith <quartzmo@gmail.com>
Date: Wed, 29 Apr 2020 14:04:53 -0600
Subject: [PATCH] fix(storage): Update SignerV4#post_object

* Ensure bucket is not returned in PostObject fields.

refs: #5702
---
 ...ket_generate_signed_post_policy_v4_test.rb | 39 +++++++++++++++++++
 .../google/cloud/storage/file/signer_v4.rb    |  4 +-
 .../signer_v4/post_object_conformance_test.rb |  6 +++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/google-cloud-storage/acceptance/storage/bucket_generate_signed_post_policy_v4_test.rb b/google-cloud-storage/acceptance/storage/bucket_generate_signed_post_policy_v4_test.rb
index 7e47ce1a7230..0e51242dac5e 100644
--- a/google-cloud-storage/acceptance/storage/bucket_generate_signed_post_policy_v4_test.rb
+++ b/google-cloud-storage/acceptance/storage/bucket_generate_signed_post_policy_v4_test.rb
@@ -29,6 +29,15 @@
   it "generates a signed post object v4 simple" do
     post_object = bucket.generate_signed_post_policy_v4 "test-object", expires: 10
 
+    _(post_object.fields.keys.sort).must_equal [
+      "key",
+      "policy",
+      "x-goog-algorithm",
+      "x-goog-credential",
+      "x-goog-date",
+      "x-goog-signature"
+    ]
+
     form_data = [['file', File.open(data)]]
 
     post_object.fields.each do |key, value|
@@ -55,6 +64,15 @@
   it "generates a signed post object v4 virtual hosted style" do
     post_object = bucket.generate_signed_post_policy_v4 "test-object", expires: 10, virtual_hosted_style: true
 
+    _(post_object.fields.keys.sort).must_equal [
+      "key",
+      "policy",
+      "x-goog-algorithm",
+      "x-goog-credential",
+      "x-goog-date",
+      "x-goog-signature"
+    ]
+
     form_data = [['file', File.open(data)]]
 
     post_object.fields.each do |key, value|
@@ -84,6 +102,17 @@
     }
     post_object = bucket.generate_signed_post_policy_v4 "test-object", expires: 10, fields: fields
 
+    _(post_object.fields.keys.sort).must_equal [
+      "acl",
+      "cache-control",
+      "key",
+      "policy",
+      "x-goog-algorithm",
+      "x-goog-credential",
+      "x-goog-date",
+      "x-goog-signature"
+    ]
+
     form_data = [['file', File.open(data)]]
 
     post_object.fields.each do |key, value|
@@ -113,6 +142,16 @@
     }
     post_object = bucket.generate_signed_post_policy_v4 "test-object", expires: 10, fields: fields
 
+    _(post_object.fields.keys.sort).must_equal [
+      "key",
+      "policy",
+      "success_action_status",
+      "x-goog-algorithm",
+      "x-goog-credential",
+      "x-goog-date",
+      "x-goog-signature"
+    ]
+
     form_data = [['file', File.open(data)]]
 
     post_object.fields.each do |key, value|
diff --git a/google-cloud-storage/lib/google/cloud/storage/file/signer_v4.rb b/google-cloud-storage/lib/google/cloud/storage/file/signer_v4.rb
index 72af381a46ad..73fd7a8edbc7 100644
--- a/google-cloud-storage/lib/google/cloud/storage/file/signer_v4.rb
+++ b/google-cloud-storage/lib/google/cloud/storage/file/signer_v4.rb
@@ -63,7 +63,6 @@ def post_object issuer: nil,
             expires ||= 60*60*24
             p["expiration"] = (now + expires).strftime "%Y-%m-%dT%H:%M:%SZ"
 
-
             policy_str = escape_characters p.to_json
 
             policy = Base64.strict_encode64(policy_str).force_encoding "utf-8"
@@ -164,7 +163,6 @@ def escape_special_unicode str
 
           def required_fields issuer, time
             {
-              "bucket" => @bucket_name,
               "key" => @file_name,
               "x-goog-date" => time.strftime("%Y%m%dT%H%M%SZ"),
               "x-goog-credential" => "#{issuer}/#{time.strftime '%Y%m%d'}/auto/storage/goog4_request",
@@ -175,6 +173,8 @@ def required_fields issuer, time
           def policy_conditions base_fields, user_conditions, user_fields
             # Convert each pair in base_fields hash to a single-entry hash in an array.
             conditions = base_fields.to_a.map { |f| Hash[*f] }
+            # Add the bucket to the head of the base_fields. This is not returned in the PostObject fields.
+            conditions.unshift "bucket" => @bucket_name
             # Add user-provided conditions to the head of the conditions array.
             conditions.unshift user_conditions if user_conditions && !user_conditions.empty?
             if user_fields
diff --git a/google-cloud-storage/test/google/cloud/storage/file/signer_v4/post_object_conformance_test.rb b/google-cloud-storage/test/google/cloud/storage/file/signer_v4/post_object_conformance_test.rb
index 9591a2b5b59e..e7606646e9ea 100644
--- a/google-cloud-storage/test/google/cloud/storage/file/signer_v4/post_object_conformance_test.rb
+++ b/google-cloud-storage/test/google/cloud/storage/file/signer_v4/post_object_conformance_test.rb
@@ -50,6 +50,9 @@ def self.signer_v4_test_for description, input, output, index
                                          bucket_bound_hostname: bucket_bound_hostname
 
         _(post_object.url).must_equal output.url
+
+        _(post_object.fields.keys.sort).must_equal output.fields.keys.sort
+
         _(post_object.fields["key"]).must_equal output.fields["key"]
         _(post_object.fields["x-goog-algorithm"]).must_equal output.fields["x-goog-algorithm"]
         _(post_object.fields["x-goog-credential"]).must_equal output.fields["x-goog-credential"]
@@ -84,6 +87,9 @@ def self.bucket_test_for description, input, output, index
                                                             bucket_bound_hostname: bucket_bound_hostname
 
         _(post_object.url).must_equal output.url
+
+        _(post_object.fields.keys.sort).must_equal output.fields.keys.sort
+
         _(post_object.fields["key"]).must_equal output.fields["key"]
         _(post_object.fields["x-goog-algorithm"]).must_equal output.fields["x-goog-algorithm"]
         _(post_object.fields["x-goog-credential"]).must_equal output.fields["x-goog-credential"]