google: findDefaultCredentials fallback to well-known file only passes through when the file is not found. Consider falling through also when permission is denied.

[michalklempa]

In findDefaultCredentials, the 2nd fallback, when the credentials are searched in well known file.
The attempt is passed by if the file is not found. There exist at least one situation as we have found out, when the error permission denied is returned from OS and it would be useful to gracefully continue and try other authentication methods.

Steps to reproduce permission denied:

1. Have an instance in GCE with service account
2. Install some service which uses this library
3. Start the service using systemd with sandboxing (https://www.freedesktop.org/software/systemd/man/systemd.exec.html#Sandboxing), the option ProtectHome= is set to Yes.
4. Code at https://github.com/golang/oauth2/blob/master/google/default.go#L55 will cause error Permission Denied which is normal, the authentication can succeed though, using the metadata server (as described in code comment here https://github.com/golang/oauth2/blob/master/google/go19.go#L38 or as in documentation here https://cloud.google.com/docs/authentication/production#auth-cloud-implicit-go

Desired behavior:

• when the well known credentials file cannot be read for whatever reason, the code should log a warning and try next authentication method and only return the error when all available authentication methods failed (return composite error of all errors encountered?).

The code segment which is responsible for this behavior is https://github.com/golang/oauth2/blob/master/google/default.go#L55-L60:

	filename := wellKnownFile()
	if creds, err := readCredentialsFile(ctx, filename, scopes); err == nil {
		return creds, nil
	} else if !os.IsNotExist(err) {
		return nil, fmt.Errorf("google: error getting credentials using well-known file (%v): %v", filename, err)
}

Please consider keeping the err and only returning it on line https://github.com/golang/oauth2/blob/master/google/default.go#L80 when all other methods failed.

Thanks.

[bradfitz]

What does the default credentials spec say about this? What do the other language implementations do?

/cc @zombiezen

[zombiezen]

There's not clear guidance from the spec.

/cc @enocom @jadekler @tbpg to see if they have more thoughts.

[jeanbza]

I'm not in favour of authentication code doing any kind of logging, but apart from that I don't have any sophisticated thoughts here. It seems like it would be nice to try all possible methods, though on the other hand changing the interface of the method (returning a composite error) seems like it might not be ideal.