Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
To view, visit change 407955. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Cody Oss.
1 comment:
Patchset:
I'm not sure who would be good reviewers for this.
To view, visit change 407955. To unsubscribe, or for help writing mail filters, visit settings.
Taahir Ahmed uploaded patch set #2 to this change.
google/sdk: Update SDKConfig to use `gcloud config config-helper`
SDKConfig is currently hard-broken; it looks deep into gcloud
internals that have changed since it was written. The `credentials`
file that it attempts to read has been replaced by a SQLite store at
`credentials.db`.
Gcloud provides a standard integration mechanism for local tools that
want to use the Cloud SDK user: `gcloud config config-helper`.
Invoking this command returns a Google access token, its expiration
time, and additional information about the current SDK configuration.
Moving to this mechanism requires giving up some of the current
features of SDKConfig:
* `gcloud config config-helper` doesn't expose a way to select from among the available Cloud
SDK users. Instead, the user must select the desired user with
`gcloud config set core/account`.
* `gcloud config config-helper` doesn't expose the scopes assigned to
the current access token. Fortunately, these aren't really
configurable anyways.
For now, I have opted to retain source compatibility, but since the
function is fully broken (it has been returning a hard error for
several years), we might consider breaking source compatibility.
This commit is meant to be a starting point for discussion. If the
approach is reasonable, I'll flesh out tests.
Change-Id: I12fa7ae7fe657136120ae09213080659b8cdc081
---
M google/sdk.go
M google/sdk_test.go
2 files changed, 146 insertions(+), 261 deletions(-)
To view, visit change 407955. To unsubscribe, or for help writing mail filters, visit settings.
Taahir Ahmed has uploaded this change for review.
google/sdk: Update SDKConfig to use `gcloud config config-helper`
SDKConfig is currently hard-broken; it looks deep into gcloud
internals that have changed since it was written. The `credentials`
file that it attempts to read has been replaced by a SQLite store at
`credentials.db`.
Gcloud provides a standard integration mechanism for local tools that
want to use the Cloud SDK user: `gcloud config config-helper`.
Invoking this command returns a Google access token, its expiration
time, and additional information about the current SDK configuration.
Moving to this mechanism requires giving up some of the current
features of SDKConfig:
* `gcloud config config-helper` doesn't expose a way to select from among the available Cloud
SDK users. Instead, the user must select the desired user with
`gcloud config set core/account`.
* `gcloud config config-helper` doesn't expose the scopes assigned to
the current access token. Fortunately, these aren't really
configurable anyways.
For now, I have opted to retain source compatibility, but since the
function is fully broken (it has been returning a hard error for
several years), we might consider breaking source compatibility.
Change-Id: I12fa7ae7fe657136120ae09213080659b8cdc081
---
M google/sdk.go
M google/sdk_test.go
2 files changed, 143 insertions(+), 261 deletions(-)
diff --git a/google/sdk.go b/google/sdk.go
index 456224b..01baeba 100644
--- a/google/sdk.go
+++ b/google/sdk.go
@@ -5,197 +5,147 @@
package google
import (
- "bufio"
"context"
"encoding/json"
- "errors"
"fmt"
"io"
"net/http"
"os"
- "os/user"
- "path/filepath"
- "runtime"
- "strings"
+ "os/exec"
+ "sync"
"time"
"golang.org/x/oauth2"
)
-type sdkCredentials struct {
- Data []struct {
- Credential struct {
- ClientID string `json:"client_id"`
- ClientSecret string `json:"client_secret"`
- AccessToken string `json:"access_token"`
- RefreshToken string `json:"refresh_token"`
- TokenExpiry *time.Time `json:"token_expiry"`
- } `json:"credential"`
- Key struct {
- Account string `json:"account"`
- Scope string `json:"scope"`
- } `json:"key"`
+// TokenSource is an oauth2.TokenSource that pulls credentials for the active
+// Cloud SDK user using `gcloud config config-helper`.
+//
+// TokenSource embeds a mutex. It is not safe to copy.
+type TokenSource struct {
+ lock sync.Mutex
+ accessToken string
+ expiry time.Time
+}
+
+type configHelperOutput struct {
+ Credential struct {
+ AccessToken string `json:"access_token"`
+ TokenExpiry string `json:"token_expiry"`
+ } `json:"credential"`
+}
+
+// Token implements oauth2.TokenSource.
+//
+// Warning: Token() connects stdin and stderr to an inferior process running
+// `gcloud`. This is necessary for certain configurations where `gcloud` may
+// prompt the user to re-authenticate themselves using a security key.
+func (t *TokenSource) Token() (*oauth2.Token, error) {
+ // Hold a lock for the entire duration of the refresh. If gcloud
+ // prompts the user for re-authentication, we don't want the user to
+ // potentially be swamped by hundreds of "touch your security key"
+ // requests.
+ t.lock.Lock()
+ defer t.lock.Unlock()
+
+ // Return cached token if it doesn't expire soon.
+ if t.accessToken != "" && t.expiry.Add(-5*time.Minute).After(time.Now()) {
+ return &oauth2.Token{
+ AccessToken: t.accessToken,
+ Expiry: t.expiry,
+ }
}
+
+ cmd := exec.Command("gcloud", "config", "config-helper", "--format=json")
+
+ // Wire up stdin and stderr, so that the user can see prompts from
+ // gcloud, and send input to it.
+ cmd.Stdin = os.Stdin
+ cmd.Stderr = os.Stderr
+
+ stdout, err := cmd.StdoutPipe()
+ if err != nil {
+ return nil, fmt.Errorf("failed to configure gcloud stdout: %w", err)
+ }
+
+ if err := cmd.Start(); err != nil {
+ return nil, fmt.Errorf("failed to start gcloud: %w", err)
+ }
+
+ stdoutBytes, err := io.ReadAll(stdout)
+ if err != nil {
+ return nil, fmt.Errorf("failed to read gcloud stdout: %w", err)
+ }
+
+ if err := cmd.Wait(); err != nil {
+ return nil, fmt.Errorf("failed to wait for gcloud completion: %w", err)
+ }
+
+ var gcloudOutput configHelperOutput
+ if err := json.Unmarshal(stdoutBytes, &gcloudOutput); err != nil {
+ return nil, fmt.Errorf("failed to unmarshal gcloud output: %w", err)
+ }
+
+ expiry, err := time.Parse(time.RFC3339, gcloudOutput.Credential.TokenExpiry)
+ if err != nil {
+ return nil, fmt.Errorf("failed to parse token expiry: %w", err)
+ }
+
+ t.accessToken = accessToken
+ t.expiry = expiry
+ return &oauth2.Token{
+ AccessToken: gcloudOutput.Credential.AccessToken,
+ Expiry: expiry,
+ }, nil
}
// An SDKConfig provides access to tokens from an account already
// authorized via the Google Cloud SDK.
type SDKConfig struct {
- conf oauth2.Config
- initialToken *oauth2.Token
+ tokenSource TokenSource
}
// NewSDKConfig creates an SDKConfig for the given Google Cloud SDK
-// account. If account is empty, the account currently active in
-// Google Cloud SDK properties is used.
-// Google Cloud SDK credentials must be created by running `gcloud auth`
-// before using this function.
-// The Google Cloud SDK is available at https://cloud.google.com/sdk/.
-func NewSDKConfig(account string) (*SDKConfig, error) {
- configPath, err := sdkConfigPath()
- if err != nil {
- return nil, fmt.Errorf("oauth2/google: error getting SDK config path: %v", err)
- }
- credentialsPath := filepath.Join(configPath, "credentials")
- f, err := os.Open(credentialsPath)
- if err != nil {
- return nil, fmt.Errorf("oauth2/google: failed to load SDK credentials: %v", err)
- }
- defer f.Close()
-
- var c sdkCredentials
- if err := json.NewDecoder(f).Decode(&c); err != nil {
- return nil, fmt.Errorf("oauth2/google: failed to decode SDK credentials from %q: %v", credentialsPath, err)
- }
- if len(c.Data) == 0 {
- return nil, fmt.Errorf("oauth2/google: no credentials found in %q, run `gcloud auth login` to create one", credentialsPath)
- }
- if account == "" {
- propertiesPath := filepath.Join(configPath, "properties")
- f, err := os.Open(propertiesPath)
- if err != nil {
- return nil, fmt.Errorf("oauth2/google: failed to load SDK properties: %v", err)
- }
- defer f.Close()
- ini, err := parseINI(f)
- if err != nil {
- return nil, fmt.Errorf("oauth2/google: failed to parse SDK properties %q: %v", propertiesPath, err)
- }
- core, ok := ini["core"]
- if !ok {
- return nil, fmt.Errorf("oauth2/google: failed to find [core] section in %v", ini)
- }
- active, ok := core["account"]
- if !ok {
- return nil, fmt.Errorf("oauth2/google: failed to find %q attribute in %v", "account", core)
- }
- account = active
- }
-
- for _, d := range c.Data {
- if account == "" || d.Key.Account == account {
- if d.Credential.AccessToken == "" && d.Credential.RefreshToken == "" {
- return nil, fmt.Errorf("oauth2/google: no token available for account %q", account)
- }
- var expiry time.Time
- if d.Credential.TokenExpiry != nil {
- expiry = *d.Credential.TokenExpiry
- }
- return &SDKConfig{
- conf: oauth2.Config{
- ClientID: d.Credential.ClientID,
- ClientSecret: d.Credential.ClientSecret,
- Scopes: strings.Split(d.Key.Scope, " "),
- Endpoint: Endpoint,
- RedirectURL: "oob",
- },
- initialToken: &oauth2.Token{
- AccessToken: d.Credential.AccessToken,
- RefreshToken: d.Credential.RefreshToken,
- Expiry: expiry,
- },
- }, nil
- }
- }
- return nil, fmt.Errorf("oauth2/google: no such credentials for account %q", account)
+// account.
+//
+// In general, NewSDKConfig should be used for programs that are acting directly
+// as a human user (command-line clients, for example). Long-lived servers or
+// programs that act as a machine identity should use the Cloud SDK's
+// application-default credentials mechanism.
+//
+// The string parameter is retained for backwards-compatibility, but ignored.
+//
+// Google Cloud SDK credentials must be created by running `gcloud auth` before
+// using this function. The Google Cloud SDK is available at
+// https://cloud.google.com/sdk/.
+//
+// Warning: NewSDKConfig connects stdin and stderr to an inferior process
+// running `gcloud`. This is necessary for certain configurations where
+// `gcloud` may prompt the user to re-authenticate themselves using a security
+// key.
+func NewSDKConfig(_ string) (*SDKConfig, error) {
+ return &SDKConfig{}, nil
}
-// Client returns an HTTP client using Google Cloud SDK credentials to
-// authorize requests. The token will auto-refresh as necessary. The
-// underlying http.RoundTripper will be obtained using the provided
-// context. The returned client and its Transport should not be
-// modified.
+// Client returns an HTTP client using Google Cloud SDK credentials to authorize
+// requests. The token will auto-refresh as necessary. The returned client and
+// its Transport should not be modified.
func (c *SDKConfig) Client(ctx context.Context) *http.Client {
return &http.Client{
Transport: &oauth2.Transport{
- Source: c.TokenSource(ctx),
+ Source: c.tokenSource,
},
}
}
// TokenSource returns an oauth2.TokenSource that retrieve tokens from
-// Google Cloud SDK credentials using the provided context.
-// It will returns the current access token stored in the credentials,
-// and refresh it when it expires, but it won't update the credentials
-// with the new access token.
+// Google Cloud SDK.
func (c *SDKConfig) TokenSource(ctx context.Context) oauth2.TokenSource {
- return c.conf.TokenSource(ctx, c.initialToken)
+ return c.tokenSource
}
-// Scopes are the OAuth 2.0 scopes the current account is authorized for.
+// (Deprecated) Scopes are the OAuth 2.0 scopes the current account is authorized for.
func (c *SDKConfig) Scopes() []string {
- return c.conf.Scopes
-}
-
-func parseINI(ini io.Reader) (map[string]map[string]string, error) {
- result := map[string]map[string]string{
- "": {}, // root section
- }
- scanner := bufio.NewScanner(ini)
- currentSection := ""
- for scanner.Scan() {
- line := strings.TrimSpace(scanner.Text())
- if strings.HasPrefix(line, ";") {
- // comment.
- continue
- }
- if strings.HasPrefix(line, "[") && strings.HasSuffix(line, "]") {
- currentSection = strings.TrimSpace(line[1 : len(line)-1])
- result[currentSection] = map[string]string{}
- continue
- }
- parts := strings.SplitN(line, "=", 2)
- if len(parts) == 2 && parts[0] != "" {
- result[currentSection][strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1])
- }
- }
- if err := scanner.Err(); err != nil {
- return nil, fmt.Errorf("error scanning ini: %v", err)
- }
- return result, nil
-}
-
-// sdkConfigPath tries to guess where the gcloud config is located.
-// It can be overridden during tests.
-var sdkConfigPath = func() (string, error) {
- if runtime.GOOS == "windows" {
- return filepath.Join(os.Getenv("APPDATA"), "gcloud"), nil
- }
- homeDir := guessUnixHomeDir()
- if homeDir == "" {
- return "", errors.New("unable to get current user home directory: os/user lookup failed; $HOME is empty")
- }
- return filepath.Join(homeDir, ".config", "gcloud"), nil
-}
-
-func guessUnixHomeDir() string {
- // Prefer $HOME over user.Current due to glibc bug: golang.org/issue/13470
- if v := os.Getenv("HOME"); v != "" {
- return v
- }
- // Else, fall back to user.Current:
- if u, err := user.Current(); err == nil {
- return u.HomeDir
- }
- return ""
+ // TODO: Use tokeninfo endpoint to retrieve?
+ return []string{}
}
diff --git a/google/sdk_test.go b/google/sdk_test.go
index 52b8eca..4879cdd 100644
--- a/google/sdk_test.go
+++ b/google/sdk_test.go
@@ -3,105 +3,3 @@
// license that can be found in the LICENSE file.
package google
-
-import (
- "reflect"
- "strings"
- "testing"
-)
-
-func TestSDKConfig(t *testing.T) {
- sdkConfigPath = func() (string, error) {
- return "testdata/gcloud", nil
- }
-
- tests := []struct {
- account string
- accessToken string
- err bool
- }{
- {"", "bar_access_token", false},
- {"f...@example.com", "foo_access_token", false},
- {"b...@example.com", "bar_access_token", false},
- {"b...@serviceaccount.example.com", "", true},
- }
- for _, tt := range tests {
- c, err := NewSDKConfig(tt.account)
- if got, want := err != nil, tt.err; got != want {
- if !tt.err {
- t.Errorf("got %v, want nil", err)
- } else {
- t.Errorf("got nil, want error")
- }
- continue
- }
- if err != nil {
- continue
- }
- tok := c.initialToken
- if tok == nil {
- t.Errorf("got nil, want %q", tt.accessToken)
- continue
- }
- if tok.AccessToken != tt.accessToken {
- t.Errorf("got %q, want %q", tok.AccessToken, tt.accessToken)
- }
- }
-}
-
-func TestParseINI(t *testing.T) {
- tests := []struct {
- ini string
- want map[string]map[string]string
- }{
- {
- `root = toor
-[foo]
-bar = hop
-ini = nin
-`,
- map[string]map[string]string{
- "": {"root": "toor"},
- "foo": {"bar": "hop", "ini": "nin"},
- },
- },
- {
- "\t extra \t = whitespace \t\r\n \t [everywhere] \t \r\n here \t = \t there \t \r\n",
- map[string]map[string]string{
- "": {"extra": "whitespace"},
- "everywhere": {"here": "there"},
- },
- },
- {
- `[empty]
-[section]
-empty=
-`,
- map[string]map[string]string{
- "": {},
- "empty": {},
- "section": {"empty": ""},
- },
- },
- {
- `ignore
-[invalid
-=stuff
-;comment=true
-`,
- map[string]map[string]string{
- "": {},
- },
- },
- }
- for _, tt := range tests {
- result, err := parseINI(strings.NewReader(tt.ini))
- if err != nil {
- t.Errorf("parseINI(%q) error %v, want: no error", tt.ini, err)
- continue
- }
- if !reflect.DeepEqual(result, tt.want) {
- t.Errorf("parseINI(%q) = %#v, want: %#v", tt.ini, result, tt.want)
- }
- }
-}
To view, visit change 407955. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Cody Oss.
Taahir Ahmed uploaded patch set #3 to this change.
google/sdk: Update SDKConfig to use `gcloud config config-helper`
SDKConfig is a feature of oauth2's Google support that lets programs
get Google credentials for the active Cloud SDK user (the user logged
in using `gcloud auth login`). This is often used by command-line
clients that are similar to `gcloud` (i.e., those that represent
direct actions of a logged-in human user; for example kubectl and
gsutil).
SDKConfig is currently hard-broken; it looks deep into gcloud
internals that have changed since it was written. The `credentials`
file that it attempts to read has been replaced by a SQLite store at
`credentials.db`.
Gcloud provides a standard integration mechanism for local tools that
want to use the Cloud SDK user: `gcloud config config-helper`.
Invoking this command returns a Google access token, its expiration
time, and additional information about the current SDK configuration.
Moving to this mechanism requires giving up some of the current
features of SDKConfig:
* `gcloud config config-helper` doesn't expose a way to select from
among the available Cloud SDK users. Instead, the user must select
the desired user with `gcloud config set core/account`.
* `gcloud config config-helper` doesn't expose the scopes assigned to
the current access token. Fortunately, these aren't really
configurable anyways.
For now, I have opted to retain source compatibility, but since the
function is fully broken (it has been returning a hard error for
several years), we might consider breaking source compatibility.
This commit is meant to be a starting point for discussion. If the
approach is reasonable, I'll flesh out tests.
Change-Id: I12fa7ae7fe657136120ae09213080659b8cdc081
---
M google/sdk.go
M google/sdk_test.go
2 files changed, 153 insertions(+), 261 deletions(-)
To view, visit change 407955. To unsubscribe, or for help writing mail filters, visit settings.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |