Skip to content

Commit

Permalink
fix(storage): don't assume auth from a client option (#4982)
Browse files Browse the repository at this point in the history
* 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

* switch ordering of some lines

* add some docs
  • Loading branch information
codyoss committed Oct 14, 2021
1 parent 0f7457c commit e17334d
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 9 deletions.
7 changes: 5 additions & 2 deletions storage/bucket.go
Expand Up @@ -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)
Expand All @@ -260,7 +263,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"`
}
Expand All @@ -285,7 +288,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"`
}
Expand Down
14 changes: 7 additions & 7 deletions storage/storage.go
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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 {
creds = c
opts = append(opts, internaloption.WithCredentials(creds))
}
creds = c

opts = append(opts, internaloption.WithCredentials(creds))

} else {
var hostURL *url.URL

Expand Down

0 comments on commit e17334d

Please sign in to comment.