From 3a6776ada799d857cee190d6f517c4afee7c3170 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 11 Mar 2024 10:07:48 -0400 Subject: [PATCH 1/4] appengine: drop obsolete code for AppEngine envs <=Go 1.11 This library no longer builds on Go versions prior to Go 1.17, so no longer needs to support compilation specific to AppEngine environments on Go versions prior to Go 1.11 Related to #615 Change-Id: Ia9579ea2091cb86ee96065affb920370c4ba33ea Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/570595 Reviewed-by: Matt Hickford LUCI-TryBot-Result: Go LUCI Reviewed-by: Cody Oss Run-TryBot: Matt Hickford TryBot-Result: Gopher Robot --- go.mod | 8 +--- go.sum | 20 --------- google/appengine.go | 20 +++++---- google/appengine_gen1.go | 77 ----------------------------------- google/appengine_gen2_flex.go | 27 ------------ google/default.go | 16 +------- internal/client_appengine.go | 13 ------ internal/transport.go | 5 --- 8 files changed, 14 insertions(+), 172 deletions(-) delete mode 100644 google/appengine_gen1.go delete mode 100644 google/appengine_gen2_flex.go delete mode 100644 internal/client_appengine.go diff --git a/go.mod b/go.mod index a6e553041..fdbe73c10 100644 --- a/go.mod +++ b/go.mod @@ -5,12 +5,6 @@ go 1.18 require ( cloud.google.com/go/compute/metadata v0.2.3 github.com/google/go-cmp v0.5.9 - google.golang.org/appengine v1.6.7 ) -require ( - cloud.google.com/go/compute v1.20.1 // indirect - github.com/golang/protobuf v1.5.3 // indirect - golang.org/x/net v0.22.0 // indirect - google.golang.org/protobuf v1.31.0 // indirect -) +require cloud.google.com/go/compute v1.20.1 // indirect diff --git a/go.sum b/go.sum index 1f42ab621..71362ed3f 100644 --- a/go.sum +++ b/go.sum @@ -2,25 +2,5 @@ cloud.google.com/go/compute v1.20.1 h1:6aKEtlUiwEpJzM001l0yFkpXmUVXaN8W+fbkb2AZN cloud.google.com/go/compute v1.20.1/go.mod h1:4tCnrn48xsqlwSAiLf1HXMQk8CONslYbdiEZc9FEIbM= cloud.google.com/go/compute/metadata v0.2.3 h1:mg4jlk7mCAj6xXp9UJ4fjI9VUI5rubuGBW5aJ7UnBMY= cloud.google.com/go/compute/metadata v0.2.3/go.mod h1:VAV5nSsACxMJvgaAuX6Pk2AawlZn8kiOGuCv6gTkwuA= -github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= -github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= -github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= -github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc= -golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= -google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= -google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= -google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= -google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= diff --git a/google/appengine.go b/google/appengine.go index feb1157b1..564920bd4 100644 --- a/google/appengine.go +++ b/google/appengine.go @@ -6,16 +6,13 @@ package google import ( "context" - "time" + "log" + "sync" "golang.org/x/oauth2" ) -// Set at init time by appengine_gen1.go. If nil, we're not on App Engine standard first generation (<= Go 1.9) or App Engine flexible. -var appengineTokenFunc func(c context.Context, scopes ...string) (token string, expiry time.Time, err error) - -// Set at init time by appengine_gen1.go. If nil, we're not on App Engine standard first generation (<= Go 1.9) or App Engine flexible. -var appengineAppIDFunc func(c context.Context) string +var logOnce sync.Once // only spam about deprecation once // AppEngineTokenSource returns a token source that fetches tokens from either // the current application's service account or from the metadata server, @@ -23,8 +20,10 @@ var appengineAppIDFunc func(c context.Context) string // details. If you are implementing a 3-legged OAuth 2.0 flow on App Engine that // involves user accounts, see oauth2.Config instead. // -// First generation App Engine runtimes (<= Go 1.9): -// AppEngineTokenSource returns a token source that fetches tokens issued to the +// The current version of this library requires at least Go 1.17 to build, +// so first generation App Engine runtimes (<= Go 1.9) are unsupported. +// Previously, on first generation App Engine runtimes, AppEngineTokenSource +// returned a token source that fetches tokens issued to the // current App Engine application's service account. The provided context must have // come from appengine.NewContext. // @@ -34,5 +33,8 @@ var appengineAppIDFunc func(c context.Context) string // context and scopes are not used. Please use DefaultTokenSource (or ComputeTokenSource, // which DefaultTokenSource will use in this case) instead. func AppEngineTokenSource(ctx context.Context, scope ...string) oauth2.TokenSource { - return appEngineTokenSource(ctx, scope...) + logOnce.Do(func() { + log.Print("google: AppEngineTokenSource is deprecated on App Engine standard second generation runtimes (>= Go 1.11) and App Engine flexible. Please use DefaultTokenSource or ComputeTokenSource.") + }) + return ComputeTokenSource("") } diff --git a/google/appengine_gen1.go b/google/appengine_gen1.go deleted file mode 100644 index e61587945..000000000 --- a/google/appengine_gen1.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build appengine - -// This file applies to App Engine first generation runtimes (<= Go 1.9). - -package google - -import ( - "context" - "sort" - "strings" - "sync" - - "golang.org/x/oauth2" - "google.golang.org/appengine" -) - -func init() { - appengineTokenFunc = appengine.AccessToken - appengineAppIDFunc = appengine.AppID -} - -// See comment on AppEngineTokenSource in appengine.go. -func appEngineTokenSource(ctx context.Context, scope ...string) oauth2.TokenSource { - scopes := append([]string{}, scope...) - sort.Strings(scopes) - return &gaeTokenSource{ - ctx: ctx, - scopes: scopes, - key: strings.Join(scopes, " "), - } -} - -// aeTokens helps the fetched tokens to be reused until their expiration. -var ( - aeTokensMu sync.Mutex - aeTokens = make(map[string]*tokenLock) // key is space-separated scopes -) - -type tokenLock struct { - mu sync.Mutex // guards t; held while fetching or updating t - t *oauth2.Token -} - -type gaeTokenSource struct { - ctx context.Context - scopes []string - key string // to aeTokens map; space-separated scopes -} - -func (ts *gaeTokenSource) Token() (*oauth2.Token, error) { - aeTokensMu.Lock() - tok, ok := aeTokens[ts.key] - if !ok { - tok = &tokenLock{} - aeTokens[ts.key] = tok - } - aeTokensMu.Unlock() - - tok.mu.Lock() - defer tok.mu.Unlock() - if tok.t.Valid() { - return tok.t, nil - } - access, exp, err := appengineTokenFunc(ts.ctx, ts.scopes...) - if err != nil { - return nil, err - } - tok.t = &oauth2.Token{ - AccessToken: access, - Expiry: exp, - } - return tok.t, nil -} diff --git a/google/appengine_gen2_flex.go b/google/appengine_gen2_flex.go deleted file mode 100644 index 9c79aa0a0..000000000 --- a/google/appengine_gen2_flex.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !appengine - -// This file applies to App Engine second generation runtimes (>= Go 1.11) and App Engine flexible. - -package google - -import ( - "context" - "log" - "sync" - - "golang.org/x/oauth2" -) - -var logOnce sync.Once // only spam about deprecation once - -// See comment on AppEngineTokenSource in appengine.go. -func appEngineTokenSource(ctx context.Context, scope ...string) oauth2.TokenSource { - logOnce.Do(func() { - log.Print("google: AppEngineTokenSource is deprecated on App Engine standard second generation runtimes (>= Go 1.11) and App Engine flexible. Please use DefaultTokenSource or ComputeTokenSource.") - }) - return ComputeTokenSource("") -} diff --git a/google/default.go b/google/default.go index 18f369851..4b55b3f5a 100644 --- a/google/default.go +++ b/google/default.go @@ -199,9 +199,7 @@ func DefaultTokenSource(ctx context.Context, scope ...string) (oauth2.TokenSourc // 2. A JSON file in a location known to the gcloud command-line tool. // On Windows, this is %APPDATA%/gcloud/application_default_credentials.json. // On other systems, $HOME/.config/gcloud/application_default_credentials.json. -// 3. On Google App Engine standard first generation runtimes (<= Go 1.9) it uses -// the appengine.AccessToken function. -// 4. On Google Compute Engine, Google App Engine standard second generation runtimes +// 3. On Google Compute Engine, Google App Engine standard second generation runtimes // (>= Go 1.11), and Google App Engine flexible environment, it fetches // credentials from the metadata server. func FindDefaultCredentialsWithParams(ctx context.Context, params CredentialsParams) (*Credentials, error) { @@ -224,17 +222,7 @@ func FindDefaultCredentialsWithParams(ctx context.Context, params CredentialsPar return CredentialsFromJSONWithParams(ctx, b, params) } - // Third, if we're on a Google App Engine standard first generation runtime (<= Go 1.9) - // use those credentials. App Engine standard second generation runtimes (>= Go 1.11) - // and App Engine flexible use ComputeTokenSource and the metadata server. - if appengineTokenFunc != nil { - return &Credentials{ - ProjectID: appengineAppIDFunc(ctx), - TokenSource: AppEngineTokenSource(ctx, params.Scopes...), - }, nil - } - - // Fourth, if we're on Google Compute Engine, an App Engine standard second generation runtime, + // Third, if we're on Google Compute Engine, an App Engine standard second generation runtime, // or App Engine flexible, use the metadata server. if metadata.OnGCE() { id, _ := metadata.ProjectID() diff --git a/internal/client_appengine.go b/internal/client_appengine.go deleted file mode 100644 index d28140f78..000000000 --- a/internal/client_appengine.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build appengine - -package internal - -import "google.golang.org/appengine/urlfetch" - -func init() { - appengineClientHook = urlfetch.Client -} diff --git a/internal/transport.go b/internal/transport.go index 572074a63..b9db01ddf 100644 --- a/internal/transport.go +++ b/internal/transport.go @@ -18,16 +18,11 @@ var HTTPClient ContextKey // because nobody else can create a ContextKey, being unexported. type ContextKey struct{} -var appengineClientHook func(context.Context) *http.Client - func ContextClient(ctx context.Context) *http.Client { if ctx != nil { if hc, ok := ctx.Value(HTTPClient).(*http.Client); ok { return hc } } - if appengineClientHook != nil { - return appengineClientHook(ctx) - } return http.DefaultClient } From 5a05c654f92c675eb6bfe782f61b601b0efb77ea Mon Sep 17 00:00:00 2001 From: Jin Qin Date: Mon, 11 Mar 2024 23:09:23 +0000 Subject: [PATCH 2/4] oauth2/google: fix remove content-type header from idms get requests This is a fix on the https://github.com/googleapis/google-cloud-go/pull/9508. The aws provider in that library is a ported dependency from here. Change-Id: I28e1efa4fdb8292210b695a164a55060c83dae88 GitHub-Last-Rev: c425f2d3b12082bdd477100648a9e46cab026da0 GitHub-Pull-Request: golang/oauth2#711 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/570875 Reviewed-by: Cody Oss Reviewed-by: Chris Smith LUCI-TryBot-Result: Go LUCI --- google/externalaccount/aws.go | 1 - 1 file changed, 1 deletion(-) diff --git a/google/externalaccount/aws.go b/google/externalaccount/aws.go index da61d0c0e..ca27c2e98 100644 --- a/google/externalaccount/aws.go +++ b/google/externalaccount/aws.go @@ -520,7 +520,6 @@ func (cs *awsCredentialSource) getMetadataSecurityCredentials(roleName string, h if err != nil { return result, err } - req.Header.Add("Content-Type", "application/json") for name, value := range headers { req.Header.Add(name, value) From 3c9c1f6d00e8761389cd2c50bc4179459d6320b5 Mon Sep 17 00:00:00 2001 From: Jin Qin Date: Mon, 11 Mar 2024 22:50:33 +0000 Subject: [PATCH 3/4] oauth2/google: fix the logic of sts 0 value of expires_in The sts response contains an optional field of `expires_in` and the value can be any integer. https://github.com/golang/oauth2/blob/master/google/internal/externalaccount/basecredentials.go#L246-L248 In the case of less than `0`, we are going to throw an error. But in the case of equals to `0` practically it means "never expire" instead of "instantly expire" which doesn't make sense. So we need to not set the expiration value for Token object. The current else if greater or equal is wrong. It's never triggered only because we are sending positive `3600` in sts response. Change-Id: Id227ca71130855235572b65ab178681e80d0da3a GitHub-Last-Rev: a95c923d6a5d256fa92629a1fcb908495d7b1338 GitHub-Pull-Request: golang/oauth2#687 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/545895 Reviewed-by: Shin Fan LUCI-TryBot-Result: Go LUCI Auto-Submit: Cody Oss Reviewed-by: Cody Oss --- google/externalaccount/basecredentials.go | 7 +- .../externalaccount/basecredentials_test.go | 123 ++++++++++++++---- 2 files changed, 103 insertions(+), 27 deletions(-) diff --git a/google/externalaccount/basecredentials.go b/google/externalaccount/basecredentials.go index 400aa0a07..6c81a6872 100644 --- a/google/externalaccount/basecredentials.go +++ b/google/externalaccount/basecredentials.go @@ -471,11 +471,12 @@ func (ts tokenSource) Token() (*oauth2.Token, error) { AccessToken: stsResp.AccessToken, TokenType: stsResp.TokenType, } - if stsResp.ExpiresIn < 0 { + + // The RFC8693 doesn't define the explicit 0 of "expires_in" field behavior. + if stsResp.ExpiresIn <= 0 { return nil, fmt.Errorf("oauth2/google/externalaccount: got invalid expiry from security token service") - } else if stsResp.ExpiresIn >= 0 { - accessToken.Expiry = now().Add(time.Duration(stsResp.ExpiresIn) * time.Second) } + accessToken.Expiry = now().Add(time.Duration(stsResp.ExpiresIn) * time.Second) if stsResp.RefreshToken != "" { accessToken.RefreshToken = stsResp.RefreshToken diff --git a/google/externalaccount/basecredentials_test.go b/google/externalaccount/basecredentials_test.go index 33314c3f0..8f165cdb0 100644 --- a/google/externalaccount/basecredentials_test.go +++ b/google/externalaccount/basecredentials_test.go @@ -6,6 +6,7 @@ package externalaccount import ( "context" + "encoding/json" "fmt" "io/ioutil" "net/http" @@ -101,15 +102,18 @@ func run(t *testing.T, config *Config, tets *testExchangeTokenServer) (*oauth2.T return ts.Token() } -func validateToken(t *testing.T, tok *oauth2.Token) { - if got, want := tok.AccessToken, correctAT; got != want { +func validateToken(t *testing.T, tok *oauth2.Token, expectToken *oauth2.Token) { + if expectToken == nil { + return + } + if got, want := tok.AccessToken, expectToken.AccessToken; got != want { t.Errorf("Unexpected access token: got %v, but wanted %v", got, want) } - if got, want := tok.TokenType, "Bearer"; got != want { + if got, want := tok.TokenType, expectToken.TokenType; got != want { t.Errorf("Unexpected TokenType: got %v, but wanted %v", got, want) } - if got, want := tok.Expiry, testNow().Add(time.Duration(3600)*time.Second); got != want { + if got, want := tok.Expiry, expectToken.Expiry; got != want { t.Errorf("Unexpected Expiry: got %v, but wanted %v", got, want) } } @@ -173,30 +177,91 @@ func getExpectedMetricsHeader(source string, saImpersonation bool, configLifetim } func TestToken(t *testing.T) { - config := Config{ - Audience: "32555940559.apps.googleusercontent.com", - SubjectTokenType: "urn:ietf:params:oauth:token-type:id_token", - ClientSecret: "notsosecret", - ClientID: "rbrgnognrhongo3bi4gb9ghg9g", - CredentialSource: &testBaseCredSource, - Scopes: []string{"https://www.googleapis.com/auth/devstorage.full_control"}, + type MockSTSResponse struct { + AccessToken string `json:"access_token"` + IssuedTokenType string `json:"issued_token_type"` + TokenType string `json:"token_type"` + ExpiresIn int32 `json:"expires_in,omitempty"` + Scope string `json:"scopre,omitenpty"` } - server := testExchangeTokenServer{ - url: "/", - authorization: "Basic cmJyZ25vZ25yaG9uZ28zYmk0Z2I5Z2hnOWc6bm90c29zZWNyZXQ=", - contentType: "application/x-www-form-urlencoded", - metricsHeader: getExpectedMetricsHeader("file", false, false), - body: baseCredsRequestBody, - response: baseCredsResponseBody, + testCases := []struct { + name string + responseBody MockSTSResponse + expectToken *oauth2.Token + expectErrorMsg string + }{ + { + name: "happy case", + responseBody: MockSTSResponse{ + AccessToken: correctAT, + IssuedTokenType: "urn:ietf:params:oauth:token-type:access_token", + TokenType: "Bearer", + ExpiresIn: 3600, + Scope: "https://www.googleapis.com/auth/cloud-platform", + }, + expectToken: &oauth2.Token{ + AccessToken: correctAT, + TokenType: "Bearer", + Expiry: testNow().Add(time.Duration(3600) * time.Second), + }, + }, + { + name: "no expiry time on token", + responseBody: MockSTSResponse{ + AccessToken: correctAT, + IssuedTokenType: "urn:ietf:params:oauth:token-type:access_token", + TokenType: "Bearer", + Scope: "https://www.googleapis.com/auth/cloud-platform", + }, + expectToken: nil, + expectErrorMsg: "oauth2/google/externalaccount: got invalid expiry from security token service", + }, + { + name: "negative expiry time", + responseBody: MockSTSResponse{ + AccessToken: correctAT, + IssuedTokenType: "urn:ietf:params:oauth:token-type:access_token", + TokenType: "Bearer", + ExpiresIn: -1, + Scope: "https://www.googleapis.com/auth/cloud-platform", + }, + expectToken: nil, + expectErrorMsg: "oauth2/google/externalaccount: got invalid expiry from security token service", + }, } - tok, err := run(t, &config, &server) + for _, testCase := range testCases { + config := Config{ + Audience: "32555940559.apps.googleusercontent.com", + SubjectTokenType: "urn:ietf:params:oauth:token-type:id_token", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + CredentialSource: &testBaseCredSource, + Scopes: []string{"https://www.googleapis.com/auth/devstorage.full_control"}, + } - if err != nil { - t.Fatalf("Unexpected error: %e", err) + responseBody, err := json.Marshal(testCase.responseBody) + if err != nil { + t.Errorf("Invalid response received.") + } + + server := testExchangeTokenServer{ + url: "/", + authorization: "Basic cmJyZ25vZ25yaG9uZ28zYmk0Z2I5Z2hnOWc6bm90c29zZWNyZXQ=", + contentType: "application/x-www-form-urlencoded", + metricsHeader: getExpectedMetricsHeader("file", false, false), + body: baseCredsRequestBody, + response: string(responseBody), + } + + tok, err := run(t, &config, &server) + + if err != nil && err.Error() != testCase.expectErrorMsg { + t.Errorf("Error not as expected: got = %v, and want = %v", err, testCase.expectErrorMsg) + } + validateToken(t, tok, testCase.expectToken) } - validateToken(t, tok) } func TestWorkforcePoolTokenWithClientID(t *testing.T) { @@ -224,7 +289,12 @@ func TestWorkforcePoolTokenWithClientID(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %e", err) } - validateToken(t, tok) + expectToken := oauth2.Token{ + AccessToken: correctAT, + TokenType: "Bearer", + Expiry: testNow().Add(time.Duration(3600) * time.Second), + } + validateToken(t, tok, &expectToken) } func TestWorkforcePoolTokenWithoutClientID(t *testing.T) { @@ -251,7 +321,12 @@ func TestWorkforcePoolTokenWithoutClientID(t *testing.T) { if err != nil { t.Fatalf("Unexpected error: %e", err) } - validateToken(t, tok) + expectToken := oauth2.Token{ + AccessToken: correctAT, + TokenType: "Bearer", + Expiry: testNow().Add(time.Duration(3600) * time.Second), + } + validateToken(t, tok, &expectToken) } func TestNonworkforceWithWorkforcePoolUserProject(t *testing.T) { From d0e617c58cf747cf27df9762003502f814dd524c Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 2 Apr 2024 16:20:57 -0600 Subject: [PATCH 4/4] google: add Credentials.UniverseDomainProvider * move MDS universe retrieval within Compute credentials Change-Id: I847d2075ca11bde998a06220307626e902230c23 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/575936 Reviewed-by: Cody Oss Auto-Submit: Cody Oss Run-TryBot: Cody Oss TryBot-Result: Gopher Robot LUCI-TryBot-Result: Go LUCI --- google/default.go | 74 ++++++++++++++++++++++-------------------- google/default_test.go | 25 +++++++++++--- 2 files changed, 58 insertions(+), 41 deletions(-) diff --git a/google/default.go b/google/default.go index 4b55b3f5a..df958359a 100644 --- a/google/default.go +++ b/google/default.go @@ -42,6 +42,17 @@ type Credentials struct { // running on Google Cloud Platform. JSON []byte + // UniverseDomainProvider returns the default service domain for a given + // Cloud universe. Optional. + // + // On GCE, UniverseDomainProvider should return the universe domain value + // from Google Compute Engine (GCE)'s metadata server. See also [The attached service + // account](https://cloud.google.com/docs/authentication/application-default-credentials#attached-sa). + // If the GCE metadata server returns a 404 error, the default universe + // domain value should be returned. If the GCE metadata server returns an + // error other than 404, the error should be returned. + UniverseDomainProvider func() (string, error) + udMu sync.Mutex // guards universeDomain // universeDomain is the default service domain for a given Cloud universe. universeDomain string @@ -64,54 +75,32 @@ func (c *Credentials) UniverseDomain() string { } // GetUniverseDomain returns the default service domain for a given Cloud -// universe. +// universe. If present, UniverseDomainProvider will be invoked and its return +// value will be cached. // // The default value is "googleapis.com". -// -// It obtains the universe domain from the attached service account on GCE when -// authenticating via the GCE metadata server. See also [The attached service -// account](https://cloud.google.com/docs/authentication/application-default-credentials#attached-sa). -// If the GCE metadata server returns a 404 error, the default value is -// returned. If the GCE metadata server returns an error other than 404, the -// error is returned. func (c *Credentials) GetUniverseDomain() (string, error) { c.udMu.Lock() defer c.udMu.Unlock() - if c.universeDomain == "" && metadata.OnGCE() { - // If we're on Google Compute Engine, an App Engine standard second - // generation runtime, or App Engine flexible, use the metadata server. - err := c.computeUniverseDomain() + if c.universeDomain == "" && c.UniverseDomainProvider != nil { + // On Google Compute Engine, an App Engine standard second generation + // runtime, or App Engine flexible, use an externally provided function + // to request the universe domain from the metadata server. + ud, err := c.UniverseDomainProvider() if err != nil { return "", err } + c.universeDomain = ud } - // If not on Google Compute Engine, or in case of any non-error path in - // computeUniverseDomain that did not set universeDomain, set the default - // universe domain. + // If no UniverseDomainProvider (meaning not on Google Compute Engine), or + // in case of any (non-error) empty return value from + // UniverseDomainProvider, set the default universe domain. if c.universeDomain == "" { c.universeDomain = defaultUniverseDomain } return c.universeDomain, nil } -// computeUniverseDomain fetches the default service domain for a given Cloud -// universe from Google Compute Engine (GCE)'s metadata server. It's only valid -// to use this method if your program is running on a GCE instance. -func (c *Credentials) computeUniverseDomain() error { - var err error - c.universeDomain, err = metadata.Get("universe/universe_domain") - if err != nil { - if _, ok := err.(metadata.NotDefinedError); ok { - // http.StatusNotFound (404) - c.universeDomain = defaultUniverseDomain - return nil - } else { - return err - } - } - return nil -} - // DefaultCredentials is the old name of Credentials. // // Deprecated: use Credentials instead. @@ -226,10 +215,23 @@ func FindDefaultCredentialsWithParams(ctx context.Context, params CredentialsPar // or App Engine flexible, use the metadata server. if metadata.OnGCE() { id, _ := metadata.ProjectID() + universeDomainProvider := func() (string, error) { + universeDomain, err := metadata.Get("universe/universe_domain") + if err != nil { + if _, ok := err.(metadata.NotDefinedError); ok { + // http.StatusNotFound (404) + return defaultUniverseDomain, nil + } else { + return "", err + } + } + return universeDomain, nil + } return &Credentials{ - ProjectID: id, - TokenSource: computeTokenSource("", params.EarlyTokenRefresh, params.Scopes...), - universeDomain: params.UniverseDomain, + ProjectID: id, + TokenSource: computeTokenSource("", params.EarlyTokenRefresh, params.Scopes...), + UniverseDomainProvider: universeDomainProvider, + universeDomain: params.UniverseDomain, }, nil } diff --git a/google/default_test.go b/google/default_test.go index 7352ffcce..c8465e94f 100644 --- a/google/default_test.go +++ b/google/default_test.go @@ -10,6 +10,8 @@ import ( "net/http/httptest" "strings" "testing" + + "cloud.google.com/go/compute/metadata" ) var saJSONJWT = []byte(`{ @@ -255,9 +257,14 @@ func TestCredentialsFromJSONWithParams_User_UniverseDomain_Params_UniverseDomain func TestComputeUniverseDomain(t *testing.T) { universeDomainPath := "/computeMetadata/v1/universe/universe_domain" universeDomainResponseBody := "example.com" + var requests int s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ if r.URL.Path != universeDomainPath { - t.Errorf("got %s, want %s", r.URL.Path, universeDomainPath) + t.Errorf("bad path, got %s, want %s", r.URL.Path, universeDomainPath) + } + if requests > 1 { + t.Errorf("too many requests, got %d, want 1", requests) } w.Write([]byte(universeDomainResponseBody)) })) @@ -268,11 +275,19 @@ func TestComputeUniverseDomain(t *testing.T) { params := CredentialsParams{ Scopes: []string{scope}, } + universeDomainProvider := func() (string, error) { + universeDomain, err := metadata.Get("universe/universe_domain") + if err != nil { + return "", err + } + return universeDomain, nil + } // Copied from FindDefaultCredentialsWithParams, metadata.OnGCE() = true block creds := &Credentials{ - ProjectID: "fake_project", - TokenSource: computeTokenSource("", params.EarlyTokenRefresh, params.Scopes...), - universeDomain: params.UniverseDomain, // empty + ProjectID: "fake_project", + TokenSource: computeTokenSource("", params.EarlyTokenRefresh, params.Scopes...), + UniverseDomainProvider: universeDomainProvider, + universeDomain: params.UniverseDomain, // empty } c := make(chan bool) go func() { @@ -285,7 +300,7 @@ func TestComputeUniverseDomain(t *testing.T) { } c <- true }() - got, err := creds.GetUniverseDomain() // Second conflicting access. + got, err := creds.GetUniverseDomain() // Second conflicting (and potentially uncached) access. <-c if err != nil { t.Error(err)