From 7ad5ac4faa5b2b91325f2404f64eafcd709ce9c3 Mon Sep 17 00:00:00 2001 From: Cody Oss Date: Wed, 13 Oct 2021 14:03:42 -0600 Subject: [PATCH 1/3] fix(storage): don't assume auth from a client option There are cases where a user might not auth with ADC or a client option such as passing in their own client. In these cases we should not error, but instead just not make use of the internal cred lookup. Fixes: #4980 --- storage/bucket.go | 4 ++-- storage/storage.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index d8747e58c95..271687818e7 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -260,7 +260,7 @@ func (b *BucketHandle) SignedURL(object string, opts *SignedURLOptions) (string, newopts.GoogleAccessID = id } if newopts.SignBytes == nil && len(newopts.PrivateKey) == 0 { - if len(b.c.creds.JSON) > 0 { + if b.c.creds != nil && len(b.c.creds.JSON) > 0 { var sa struct { PrivateKey string `json:"private_key"` } @@ -285,7 +285,7 @@ func (b *BucketHandle) SignedURL(object string, opts *SignedURLOptions) (string, func (b *BucketHandle) detectDefaultGoogleAccessID() (string, error) { returnErr := errors.New("no credentials found on client and not on GCE (Google Compute Engine)") - if len(b.c.creds.JSON) > 0 { + if b.c.creds != nil && len(b.c.creds.JSON) > 0 { var sa struct { ClientEmail string `json:"client_email"` } diff --git a/storage/storage.go b/storage/storage.go index 98a0389d0fc..e224025565d 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -100,7 +100,8 @@ type Client struct { scheme string // ReadHost is the default host used on the reader. readHost string - creds *google.Credentials + // May be nil. + creds *google.Credentials // gc is an optional gRPC-based, GAPIC client. // @@ -131,14 +132,13 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error opts = append(opts, internaloption.WithDefaultEndpoint("https://storage.googleapis.com/storage/v1/")) opts = append(opts, internaloption.WithDefaultMTLSEndpoint("https://storage.mtls.googleapis.com/storage/v1/")) + // Don't error out here. The user may have passed in their own HTTP + // client which does not auth with ADC or other common conventions. c, err := transport.Creds(ctx, opts...) - if err != nil { - return nil, err + if err == nil { + opts = append(opts, internaloption.WithCredentials(creds)) + creds = c } - creds = c - - opts = append(opts, internaloption.WithCredentials(creds)) - } else { var hostURL *url.URL From c6189e3314af62da765a73da441539ab999938cc Mon Sep 17 00:00:00 2001 From: Cody Oss Date: Wed, 13 Oct 2021 15:18:31 -0600 Subject: [PATCH 2/3] switch ordering of some lines --- storage/storage.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/storage.go b/storage/storage.go index e224025565d..81ff43c6d60 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -136,8 +136,8 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error // client which does not auth with ADC or other common conventions. c, err := transport.Creds(ctx, opts...) if err == nil { - opts = append(opts, internaloption.WithCredentials(creds)) creds = c + opts = append(opts, internaloption.WithCredentials(creds)) } } else { var hostURL *url.URL From 45f309c331419133f50950e9bbf870527023a7b5 Mon Sep 17 00:00:00 2001 From: Cody Oss Date: Wed, 13 Oct 2021 21:50:29 -0600 Subject: [PATCH 3/3] add some docs --- storage/bucket.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/storage/bucket.go b/storage/bucket.go index 271687818e7..ec7dcb5c322 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -239,6 +239,9 @@ func (b *BucketHandle) newPatchCall(uattrs *BucketAttrsToUpdate) (*raw.BucketsPa // This method only requires the Method and Expires fields in the specified // SignedURLOptions opts to be non-nil. If not provided, it attempts to fill the // GoogleAccessID and PrivateKey from the GOOGLE_APPLICATION_CREDENTIALS environment variable. +// If you are authenticating with a custom HTTP client, Service Account based +// auto-detection will be hindered. +// // If no private key is found, it attempts to use the GoogleAccessID to sign the URL. // This requires the IAM Service Account Credentials API to be enabled // (https://console.developers.google.com/apis/api/iamcredentials.googleapis.com/overview)