google/internal/externalaccount: Removed URL validation for google URLs in ADC files
Removes URL validation for token_url, service_account_impersonation_url to allow for TPC urls and adds line to the docs to warn users. See https://github.com/googleapis/google-auth-library-nodejs/pull/1517 for same change in node.js library.
Change-Id: I85fa67ee0b99deed2adb75668a1b5501851c499c
GitHub-Last-Rev: 15d7759884817d0f835768bdb5e5b3fa86e8fdbf
GitHub-Pull-Request: golang/oauth2#627
Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/465696
Reviewed-by: Cody Oss <codyoss@google.com>
Run-TryBot: Cody Oss <codyoss@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Leo Siracusa <leosiracusa@google.com>
diff --git a/google/doc.go b/google/doc.go
index b3e7bc8..8a3349f 100644
--- a/google/doc.go
+++ b/google/doc.go
@@ -57,6 +57,11 @@
// executable-sourced credentials), please check out:
// https://cloud.google.com/iam/docs/using-workload-identity-federation#oidc
//
+// Note that this library does not perform any validation on the token_url, token_info_url,
+// or service_account_impersonation_url fields of the credential configuration.
+// It is not recommended to use a credential configuration that you did not generate with
+// the gcloud CLI unless you verify that the URL fields point to a googleapis.com domain.
+//
// # Credentials
//
// The Credentials type represents Google credentials, including Application Default
@@ -81,4 +86,5 @@
// same as the one obtained from the oauth2.Config returned from ConfigFromJSON or
// JWTConfigFromJSON, but the Credentials may contain additional information
// that is useful is some circumstances.
+//
package google // import "golang.org/x/oauth2/google"
diff --git a/google/internal/externalaccount/basecredentials.go b/google/internal/externalaccount/basecredentials.go
index 3eab8df..dcd252a 100644
--- a/google/internal/externalaccount/basecredentials.go
+++ b/google/internal/externalaccount/basecredentials.go
@@ -67,22 +67,6 @@
// that include all elements in a given list, in that order.
var (
- validTokenURLPatterns = []*regexp.Regexp{
- // The complicated part in the middle matches any number of characters that
- // aren't period, spaces, or slashes.
- regexp.MustCompile(`(?i)^[^\.\s\/\\]+\.sts\.googleapis\.com$`),
- regexp.MustCompile(`(?i)^sts\.googleapis\.com$`),
- regexp.MustCompile(`(?i)^sts\.[^\.\s\/\\]+\.googleapis\.com$`),
- regexp.MustCompile(`(?i)^[^\.\s\/\\]+-sts\.googleapis\.com$`),
- regexp.MustCompile(`(?i)^sts-[^\.\s\/\\]+\.p\.googleapis\.com$`),
- }
- validImpersonateURLPatterns = []*regexp.Regexp{
- regexp.MustCompile(`^[^\.\s\/\\]+\.iamcredentials\.googleapis\.com$`),
- regexp.MustCompile(`^iamcredentials\.googleapis\.com$`),
- regexp.MustCompile(`^iamcredentials\.[^\.\s\/\\]+\.googleapis\.com$`),
- regexp.MustCompile(`^[^\.\s\/\\]+-iamcredentials\.googleapis\.com$`),
- regexp.MustCompile(`^iamcredentials-[^\.\s\/\\]+\.p\.googleapis\.com$`),
- }
validWorkforceAudiencePattern *regexp.Regexp = regexp.MustCompile(`//iam\.googleapis\.com/locations/[^/]+/workforcePools/`)
)
@@ -110,25 +94,13 @@
// TokenSource Returns an external account TokenSource struct. This is to be called by package google to construct a google.Credentials.
func (c *Config) TokenSource(ctx context.Context) (oauth2.TokenSource, error) {
- return c.tokenSource(ctx, validTokenURLPatterns, validImpersonateURLPatterns, "https")
+ return c.tokenSource(ctx, "https")
}
// tokenSource is a private function that's directly called by some of the tests,
// because the unit test URLs are mocked, and would otherwise fail the
// validity check.
-func (c *Config) tokenSource(ctx context.Context, tokenURLValidPats []*regexp.Regexp, impersonateURLValidPats []*regexp.Regexp, scheme string) (oauth2.TokenSource, error) {
- valid := validateURL(c.TokenURL, tokenURLValidPats, scheme)
- if !valid {
- return nil, fmt.Errorf("oauth2/google: invalid TokenURL provided while constructing tokenSource")
- }
-
- if c.ServiceAccountImpersonationURL != "" {
- valid := validateURL(c.ServiceAccountImpersonationURL, impersonateURLValidPats, scheme)
- if !valid {
- return nil, fmt.Errorf("oauth2/google: invalid ServiceAccountImpersonationURL provided while constructing tokenSource")
- }
- }
-
+func (c *Config) tokenSource(ctx context.Context, scheme string) (oauth2.TokenSource, error) {
if c.WorkforcePoolUserProject != "" {
valid := validateWorkforceAudience(c.Audience)
if !valid {
diff --git a/google/internal/externalaccount/basecredentials_test.go b/google/internal/externalaccount/basecredentials_test.go
index 05e0127..bf6be32 100644
--- a/google/internal/externalaccount/basecredentials_test.go
+++ b/google/internal/externalaccount/basecredentials_test.go
@@ -9,7 +9,6 @@
"io/ioutil"
"net/http"
"net/http/httptest"
- "strings"
"testing"
"time"
@@ -208,140 +207,6 @@
}
}
-func TestValidateURLTokenURL(t *testing.T) {
- var urlValidityTests = []struct {
- tokURL string
- expectSuccess bool
- }{
- {"https://east.sts.googleapis.com", true},
- {"https://sts.googleapis.com", true},
- {"https://sts.asfeasfesef.googleapis.com", true},
- {"https://us-east-1-sts.googleapis.com", true},
- {"https://sts.googleapis.com/your/path/here", true},
- {"https://.sts.googleapis.com", false},
- {"https://badsts.googleapis.com", false},
- {"https://sts.asfe.asfesef.googleapis.com", false},
- {"https://sts..googleapis.com", false},
- {"https://-sts.googleapis.com", false},
- {"https://us-ea.st-1-sts.googleapis.com", false},
- {"https://sts.googleapis.com.evil.com/whatever/path", false},
- {"https://us-eas\\t-1.sts.googleapis.com", false},
- {"https:/us-ea/st-1.sts.googleapis.com", false},
- {"https:/us-east 1.sts.googleapis.com", false},
- {"https://", false},
- {"http://us-east-1.sts.googleapis.com", false},
- {"https://us-east-1.sts.googleapis.comevil.com", false},
- {"https://sts-xyz.p.googleapis.com", true},
- {"https://sts.pgoogleapis.com", false},
- {"https://p.googleapis.com", false},
- {"https://sts.p.com", false},
- {"http://sts.p.googleapis.com", false},
- {"https://xyz-sts.p.googleapis.com", false},
- {"https://sts-xyz.123.p.googleapis.com", false},
- {"https://sts-xyz.p1.googleapis.com", false},
- {"https://sts-xyz.p.foo.com", false},
- {"https://sts-xyz.p.foo.googleapis.com", false},
- }
- ctx := context.Background()
- for _, tt := range urlValidityTests {
- t.Run(" "+tt.tokURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
- config := testConfig
- config.TokenURL = tt.tokURL
- _, err := config.TokenSource(ctx)
-
- if tt.expectSuccess && err != nil {
- t.Errorf("got %v but want nil", err)
- } else if !tt.expectSuccess && err == nil {
- t.Errorf("got nil but expected an error")
- }
- })
- }
- for _, el := range urlValidityTests {
- el.tokURL = strings.ToUpper(el.tokURL)
- }
- for _, tt := range urlValidityTests {
- t.Run(" "+tt.tokURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
- config := testConfig
- config.TokenURL = tt.tokURL
- _, err := config.TokenSource(ctx)
-
- if tt.expectSuccess && err != nil {
- t.Errorf("got %v but want nil", err)
- } else if !tt.expectSuccess && err == nil {
- t.Errorf("got nil but expected an error")
- }
- })
- }
-}
-
-func TestValidateURLImpersonateURL(t *testing.T) {
- var urlValidityTests = []struct {
- impURL string
- expectSuccess bool
- }{
- {"https://east.iamcredentials.googleapis.com", true},
- {"https://iamcredentials.googleapis.com", true},
- {"https://iamcredentials.asfeasfesef.googleapis.com", true},
- {"https://us-east-1-iamcredentials.googleapis.com", true},
- {"https://iamcredentials.googleapis.com/your/path/here", true},
- {"https://.iamcredentials.googleapis.com", false},
- {"https://badiamcredentials.googleapis.com", false},
- {"https://iamcredentials.asfe.asfesef.googleapis.com", false},
- {"https://iamcredentials..googleapis.com", false},
- {"https://-iamcredentials.googleapis.com", false},
- {"https://us-ea.st-1-iamcredentials.googleapis.com", false},
- {"https://iamcredentials.googleapis.com.evil.com/whatever/path", false},
- {"https://us-eas\\t-1.iamcredentials.googleapis.com", false},
- {"https:/us-ea/st-1.iamcredentials.googleapis.com", false},
- {"https:/us-east 1.iamcredentials.googleapis.com", false},
- {"https://", false},
- {"http://us-east-1.iamcredentials.googleapis.com", false},
- {"https://us-east-1.iamcredentials.googleapis.comevil.com", false},
- {"https://iamcredentials-xyz.p.googleapis.com", true},
- {"https://iamcredentials.pgoogleapis.com", false},
- {"https://p.googleapis.com", false},
- {"https://iamcredentials.p.com", false},
- {"http://iamcredentials.p.googleapis.com", false},
- {"https://xyz-iamcredentials.p.googleapis.com", false},
- {"https://iamcredentials-xyz.123.p.googleapis.com", false},
- {"https://iamcredentials-xyz.p1.googleapis.com", false},
- {"https://iamcredentials-xyz.p.foo.com", false},
- {"https://iamcredentials-xyz.p.foo.googleapis.com", false},
- }
- ctx := context.Background()
- for _, tt := range urlValidityTests {
- t.Run(" "+tt.impURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
- config := testConfig
- config.TokenURL = "https://sts.googleapis.com" // Setting the most basic acceptable tokenURL
- config.ServiceAccountImpersonationURL = tt.impURL
- _, err := config.TokenSource(ctx)
-
- if tt.expectSuccess && err != nil {
- t.Errorf("got %v but want nil", err)
- } else if !tt.expectSuccess && err == nil {
- t.Errorf("got nil but expected an error")
- }
- })
- }
- for _, el := range urlValidityTests {
- el.impURL = strings.ToUpper(el.impURL)
- }
- for _, tt := range urlValidityTests {
- t.Run(" "+tt.impURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
- config := testConfig
- config.TokenURL = "https://sts.googleapis.com" // Setting the most basic acceptable tokenURL
- config.ServiceAccountImpersonationURL = tt.impURL
- _, err := config.TokenSource(ctx)
-
- if tt.expectSuccess && err != nil {
- t.Errorf("got %v but want nil", err)
- } else if !tt.expectSuccess && err == nil {
- t.Errorf("got nil but expected an error")
- }
- })
- }
-}
-
func TestWorkforcePoolCreation(t *testing.T) {
var audienceValidatyTests = []struct {
audience string
diff --git a/google/internal/externalaccount/impersonate_test.go b/google/internal/externalaccount/impersonate_test.go
index 17e2f6d..8c7f6a9 100644
--- a/google/internal/externalaccount/impersonate_test.go
+++ b/google/internal/externalaccount/impersonate_test.go
@@ -9,7 +9,6 @@
"io/ioutil"
"net/http"
"net/http/httptest"
- "regexp"
"testing"
)
@@ -114,8 +113,7 @@
defer targetServer.Close()
testImpersonateConfig.TokenURL = targetServer.URL
- allURLs := regexp.MustCompile(".+")
- ourTS, err := testImpersonateConfig.tokenSource(context.Background(), []*regexp.Regexp{allURLs}, []*regexp.Regexp{allURLs}, "http")
+ ourTS, err := testImpersonateConfig.tokenSource(context.Background(), "http")
if err != nil {
t.Fatalf("Failed to create TokenSource: %v", err)
}
