Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage): respect WithEndpoint for SignedURLs and PostPolicy #8113

Merged
merged 8 commits into from
Jun 27, 2023
26 changes: 16 additions & 10 deletions storage/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,17 @@ func (b *BucketHandle) Update(ctx context.Context, uattrs BucketAttrsToUpdate) (
// [Overview of access control]: https://cloud.google.com/storage/docs/accesscontrol#signed_urls_query_string_authentication
// [automatic detection of credentials]: https://pkg.go.dev/cloud.google.com/go/storage#hdr-Credential_requirements_for_signing
func (b *BucketHandle) SignedURL(object string, opts *SignedURLOptions) (string, error) {
// Extract the correct endpoint from the readhost set on the client
opts.endpoint = b.c.readHost
// Make a copy of opts so we don't modify the pointer parameter.
newopts := opts.clone()

if newopts.Hostname == "" {
// Extract the correct host from the readhost set on the client
newopts.Hostname = b.c.readHost
}

if opts.GoogleAccessID != "" && (opts.SignBytes != nil || len(opts.PrivateKey) > 0) {
return SignedURL(b.name, object, opts)
return SignedURL(b.name, object, newopts)
}
// Make a copy of opts so we don't modify the pointer parameter.
newopts := opts.clone()

if newopts.GoogleAccessID == "" {
id, err := b.detectDefaultGoogleAccessID()
Expand Down Expand Up @@ -218,14 +221,17 @@ func (b *BucketHandle) SignedURL(object string, opts *SignedURLOptions) (string,
//
// [automatic detection of credentials]: https://pkg.go.dev/cloud.google.com/go/storage#hdr-Credential_requirements_for_signing
func (b *BucketHandle) GenerateSignedPostPolicyV4(object string, opts *PostPolicyV4Options) (*PostPolicyV4, error) {
// Extract the correct endpoint from the readhost set on the client
opts.endpoint = b.c.readHost
// Make a copy of opts so we don't modify the pointer parameter.
newopts := opts.clone()

if newopts.Hostname == "" {
// Extract the correct host from the readhost set on the client
newopts.Hostname = b.c.readHost
}

if opts.GoogleAccessID != "" && (opts.SignRawBytes != nil || opts.SignBytes != nil || len(opts.PrivateKey) > 0) {
return GenerateSignedPostPolicyV4(b.name, object, opts)
return GenerateSignedPostPolicyV4(b.name, object, newopts)
}
// Make a copy of opts so we don't modify the pointer parameter.
newopts := opts.clone()

if newopts.GoogleAccessID == "" {
id, err := b.detectDefaultGoogleAccessID()
Expand Down
19 changes: 19 additions & 0 deletions storage/bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,7 @@ func TestDetectDefaultGoogleAccessID(t *testing.T) {

// TestBucketSignedURL_Endpoint_Emulator_Host tests that Bucket.SignedURl
// respects the host set in STORAGE_EMULATOR_HOST and/or in option.WithEndpoint
// TODO: move this testing to conformance tests.
func TestBucketSignedURL_Endpoint_Emulator_Host(t *testing.T) {
expires, _ := time.Parse(time.RFC3339, "2002-10-02T10:00:00-05:00")
bucketName := "bucket-name"
Expand Down Expand Up @@ -1476,6 +1477,24 @@ func TestBucketSignedURL_Endpoint_Emulator_Host(t *testing.T) {
"&GoogleAccessId=xxx%40clientid" +
"&Signature=oRi3y2tBTmoDto7FezNx4AjC0RXA6fpJjTBa0hINeVroZ%2ByOeRU8MRwJbKg1IkBbV0IjtlPaGwv5YoUH16UYdipBjCXOS%2B1qgRWyzl8AnzvU%2BfwSXSlCk9zPtHHoBkFT7G4cZQOdDTLRrSG%2FmRJ3K09KEHYg%2Fc6R5Dd92inD1tLE2tiFMyHFs5uQHRMsepY4wrWiIQ4u53tPvk%2Fwiq1%2B9yL6x3QGblhdWwjX0BTVBOxexyKTlwczJW0XlWX8wpcTFfzQnJZuujbhanf2g9MGzSmkv3ylyuQdHMJDYp4Bzq%2FmnkNUg0Vp6iEvh9tyVdRNkwXeg3D8qn%2BFSOxcF%2B9vJw%3D%3D",
},
{
desc: "hostname in opts overrides all else",
endpoint: &localhost9000,
emulatorHost: "https://localhost:8000",
now: expires.Add(-24 * time.Hour),
opts: &SignedURLOptions{
GoogleAccessID: "xxx@clientid",
PrivateKey: dummyKey("rsa"),
Method: "POST",
Expires: expires,
Scheme: SigningSchemeV2,
Hostname: "localhost:6000",
},
want: "https://localhost:6000/" + bucketName + "/" + objectName +
"?Expires=1033570800" +
"&GoogleAccessId=xxx%40clientid" +
"&Signature=oRi3y2tBTmoDto7FezNx4AjC0RXA6fpJjTBa0hINeVroZ%2ByOeRU8MRwJbKg1IkBbV0IjtlPaGwv5YoUH16UYdipBjCXOS%2B1qgRWyzl8AnzvU%2BfwSXSlCk9zPtHHoBkFT7G4cZQOdDTLRrSG%2FmRJ3K09KEHYg%2Fc6R5Dd92inD1tLE2tiFMyHFs5uQHRMsepY4wrWiIQ4u53tPvk%2Fwiq1%2B9yL6x3QGblhdWwjX0BTVBOxexyKTlwczJW0XlWX8wpcTFfzQnJZuujbhanf2g9MGzSmkv3ylyuQdHMJDYp4Bzq%2FmnkNUg0Vp6iEvh9tyVdRNkwXeg3D8qn%2BFSOxcF%2B9vJw%3D%3D",
},
}
oldUTCNow := utcNow
defer func() {
Expand Down
11 changes: 8 additions & 3 deletions storage/post_policy_v4.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,13 @@ type PostPolicyV4Options struct {
// Optional.
Conditions []PostPolicyV4Condition

// Hostname sets the host of the signed post policy. This field overrides
// any endpoint set on a storage Client or through STORAGE_EMULATOR_HOST.
// Not compatible with BucketBoundHostname URLStyle.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say "only compatible with PathStyle" because we omitted VirtualHosted also. Same in the docs for SignedURLOptions below.

// Optional.
Hostname string

shouldHashSignBytes bool
endpoint string
}

func (opts *PostPolicyV4Options) clone() *PostPolicyV4Options {
Expand All @@ -129,7 +134,7 @@ func (opts *PostPolicyV4Options) clone() *PostPolicyV4Options {
Fields: opts.Fields,
Conditions: opts.Conditions,
shouldHashSignBytes: opts.shouldHashSignBytes,
endpoint: opts.endpoint,
Hostname: opts.Hostname,
}
}

Expand Down Expand Up @@ -372,7 +377,7 @@ func GenerateSignedPostPolicyV4(bucket, object string, opts *PostPolicyV4Options
u := &url.URL{
Path: path,
RawPath: pathEncodeV4(path),
Host: opts.Style.host(opts.endpoint, bucket),
Host: opts.Style.host(opts.Hostname, bucket),
Scheme: scheme,
}

Expand Down
2 changes: 1 addition & 1 deletion storage/post_policy_v4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestPostPolicyV4OptionsClone(t *testing.T) {
Fields: &PolicyV4Fields{ACL: "test-acl"},
Conditions: []PostPolicyV4Condition{},
shouldHashSignBytes: true,
endpoint: "localhost:9000",
Hostname: "localhost:9000",
}

// Check that all fields are set to a non-zero value, so we can check that
Expand Down
39 changes: 20 additions & 19 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,13 @@ const (
SigningSchemeV4
)

// URLStyle determines the style to use for the signed URL. pathStyle is the
// URLStyle determines the style to use for the signed URL. PathStyle is the
// default. All non-default options work with V4 scheme only. See
// https://cloud.google.com/storage/docs/request-endpoints for details.
type URLStyle interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also update the docs for PathStyle to indicate that the provided hostname is taken into account?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. To-do on virtual hosted style, if we decide to support it.

// host should return the host portion of the signed URL, not including
// the scheme (e.g. storage.googleapis.com).
host(endpoint, bucket string) string
host(hostname, bucket string) string

// path should return the path portion of the signed URL, which may include
// both the bucket and object name or only the object name depending on the
Expand All @@ -284,33 +284,27 @@ type bucketBoundHostname struct {
hostname string
}

func (s pathStyle) host(endpoint, bucket string) string {
if endpoint != "" {
return stripScheme(endpoint)
func (s pathStyle) host(hostname, bucket string) string {
if hostname != "" {
return stripScheme(hostname)
}

// This check is needed for clientless calls to SignedURL
if host := os.Getenv("STORAGE_EMULATOR_HOST"); host != "" {
return stripScheme(host)
}

// Fallback to default endpoint - clientless calls to SignURL/PostPolicy
// will not have an endpoint attached
return "storage.googleapis.com"
}

func (s virtualHostedStyle) host(endpoint, bucket string) string {
if endpoint != "" {
return bucket + "." + stripScheme(endpoint)
func (s virtualHostedStyle) host(hostname, bucket string) string {
if hostname != "" {
return bucket + "." + stripScheme(hostname)
}

// This check is needed for clientless calls to SignedURL
if host := os.Getenv("STORAGE_EMULATOR_HOST"); host != "" {
return bucket + "." + stripScheme(host)
}

// Fallback to default endpoint - clientless calls to SignURL/PostPolicy
// will not have an endpoint attached
return bucket + "." + "storage.googleapis.com"
}

Expand All @@ -335,7 +329,10 @@ func (s bucketBoundHostname) path(bucket, object string) string {
}

// PathStyle is the default style, and will generate a URL of the form
// "storage.googleapis.com/<bucket-name>/<object-name>".
// "<host-name>/<bucket-name>/<object-name>". By default, <host-name> is
// storage.googleapis.com, but setting an endpoint on the storage Client or
// through STORAGE_EMULATOR_HOST overrides this. Setting Hostname on
// SignedURLOptions or PostPolicyV4Options overrides everything else.
func PathStyle() URLStyle {
return pathStyle{}
}
Expand Down Expand Up @@ -457,7 +454,11 @@ type SignedURLOptions struct {
// SigningSchemeV2.
Scheme SigningScheme

endpoint string
// Hostname sets the host of the signed URL. This field overrides any
// endpoint set on a storage Client or through STORAGE_EMULATOR_HOST.
// Not compatible with BucketBoundHostname URLStyle.
// Optional.
Hostname string
}

func (opts *SignedURLOptions) clone() *SignedURLOptions {
Expand All @@ -474,7 +475,7 @@ func (opts *SignedURLOptions) clone() *SignedURLOptions {
Style: opts.Style,
Insecure: opts.Insecure,
Scheme: opts.Scheme,
endpoint: opts.endpoint,
Hostname: opts.Hostname,
}
}

Expand Down Expand Up @@ -733,7 +734,7 @@ func signedURLV4(bucket, name string, opts *SignedURLOptions, now time.Time) (st
fmt.Fprintf(buf, "%s\n", escapedQuery)

// Fill in the hostname based on the desired URL style.
u.Host = opts.Style.host(opts.endpoint, bucket)
u.Host = opts.Style.host(opts.Hostname, bucket)

// Fill in the URL scheme.
if opts.Insecure {
Expand Down Expand Up @@ -867,7 +868,7 @@ func signedURLV2(bucket, name string, opts *SignedURLOptions) (string, error) {
}
encoded := base64.StdEncoding.EncodeToString(b)
u.Scheme = "https"
u.Host = PathStyle().host(opts.endpoint, bucket)
u.Host = PathStyle().host(opts.Hostname, bucket)
q := u.Query()
q.Set("GoogleAccessId", opts.GoogleAccessID)
q.Set("Expires", fmt.Sprintf("%d", opts.Expires.Unix()))
Expand Down
2 changes: 1 addition & 1 deletion storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2337,7 +2337,7 @@ func TestSignedURLOptionsClone(t *testing.T) {
Style: VirtualHostedStyle(),
Insecure: true,
Scheme: SigningSchemeV2,
endpoint: "localhost:8000",
Hostname: "localhost:8000",
}

// Check that all fields are set to a non-zero value, so we can check that
Expand Down