Skip to content

Commit c82d0e1

Browse files
aeitzmancodyoss
authored andcommitted
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 googleapis/google-auth-library-nodejs#1517 for same change in node.js library. Change-Id: I85fa67ee0b99deed2adb75668a1b5501851c499c GitHub-Last-Rev: 15d7759 GitHub-Pull-Request: #627 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/465696 Reviewed-by: Cody Oss <[email protected]> Run-TryBot: Cody Oss <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Leo Siracusa <[email protected]>
1 parent adbaf66 commit c82d0e1

File tree

4 files changed

+9
-168
lines changed

4 files changed

+9
-168
lines changed

google/doc.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@
5757
// executable-sourced credentials), please check out:
5858
// https://cloud.google.com/iam/docs/using-workload-identity-federation#oidc
5959
//
60+
// Note that this library does not perform any validation on the token_url, token_info_url,
61+
// or service_account_impersonation_url fields of the credential configuration.
62+
// It is not recommended to use a credential configuration that you did not generate with
63+
// the gcloud CLI unless you verify that the URL fields point to a googleapis.com domain.
64+
//
6065
// # Credentials
6166
//
6267
// The Credentials type represents Google credentials, including Application Default
@@ -81,4 +86,5 @@
8186
// same as the one obtained from the oauth2.Config returned from ConfigFromJSON or
8287
// JWTConfigFromJSON, but the Credentials may contain additional information
8388
// that is useful is some circumstances.
89+
//
8490
package google // import "golang.org/x/oauth2/google"

google/internal/externalaccount/basecredentials.go

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -67,22 +67,6 @@ type Config struct {
6767
// that include all elements in a given list, in that order.
6868

6969
var (
70-
validTokenURLPatterns = []*regexp.Regexp{
71-
// The complicated part in the middle matches any number of characters that
72-
// aren't period, spaces, or slashes.
73-
regexp.MustCompile(`(?i)^[^\.\s\/\\]+\.sts\.googleapis\.com$`),
74-
regexp.MustCompile(`(?i)^sts\.googleapis\.com$`),
75-
regexp.MustCompile(`(?i)^sts\.[^\.\s\/\\]+\.googleapis\.com$`),
76-
regexp.MustCompile(`(?i)^[^\.\s\/\\]+-sts\.googleapis\.com$`),
77-
regexp.MustCompile(`(?i)^sts-[^\.\s\/\\]+\.p\.googleapis\.com$`),
78-
}
79-
validImpersonateURLPatterns = []*regexp.Regexp{
80-
regexp.MustCompile(`^[^\.\s\/\\]+\.iamcredentials\.googleapis\.com$`),
81-
regexp.MustCompile(`^iamcredentials\.googleapis\.com$`),
82-
regexp.MustCompile(`^iamcredentials\.[^\.\s\/\\]+\.googleapis\.com$`),
83-
regexp.MustCompile(`^[^\.\s\/\\]+-iamcredentials\.googleapis\.com$`),
84-
regexp.MustCompile(`^iamcredentials-[^\.\s\/\\]+\.p\.googleapis\.com$`),
85-
}
8670
validWorkforceAudiencePattern *regexp.Regexp = regexp.MustCompile(`//iam\.googleapis\.com/locations/[^/]+/workforcePools/`)
8771
)
8872

@@ -110,25 +94,13 @@ func validateWorkforceAudience(input string) bool {
11094

11195
// TokenSource Returns an external account TokenSource struct. This is to be called by package google to construct a google.Credentials.
11296
func (c *Config) TokenSource(ctx context.Context) (oauth2.TokenSource, error) {
113-
return c.tokenSource(ctx, validTokenURLPatterns, validImpersonateURLPatterns, "https")
97+
return c.tokenSource(ctx, "https")
11498
}
11599

116100
// tokenSource is a private function that's directly called by some of the tests,
117101
// because the unit test URLs are mocked, and would otherwise fail the
118102
// validity check.
119-
func (c *Config) tokenSource(ctx context.Context, tokenURLValidPats []*regexp.Regexp, impersonateURLValidPats []*regexp.Regexp, scheme string) (oauth2.TokenSource, error) {
120-
valid := validateURL(c.TokenURL, tokenURLValidPats, scheme)
121-
if !valid {
122-
return nil, fmt.Errorf("oauth2/google: invalid TokenURL provided while constructing tokenSource")
123-
}
124-
125-
if c.ServiceAccountImpersonationURL != "" {
126-
valid := validateURL(c.ServiceAccountImpersonationURL, impersonateURLValidPats, scheme)
127-
if !valid {
128-
return nil, fmt.Errorf("oauth2/google: invalid ServiceAccountImpersonationURL provided while constructing tokenSource")
129-
}
130-
}
131-
103+
func (c *Config) tokenSource(ctx context.Context, scheme string) (oauth2.TokenSource, error) {
132104
if c.WorkforcePoolUserProject != "" {
133105
valid := validateWorkforceAudience(c.Audience)
134106
if !valid {

google/internal/externalaccount/basecredentials_test.go

Lines changed: 0 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"io/ioutil"
1010
"net/http"
1111
"net/http/httptest"
12-
"strings"
1312
"testing"
1413
"time"
1514

@@ -208,140 +207,6 @@ func TestNonworkforceWithWorkforcePoolUserProject(t *testing.T) {
208207
}
209208
}
210209

211-
func TestValidateURLTokenURL(t *testing.T) {
212-
var urlValidityTests = []struct {
213-
tokURL string
214-
expectSuccess bool
215-
}{
216-
{"https://east.sts.googleapis.com", true},
217-
{"https://sts.googleapis.com", true},
218-
{"https://sts.asfeasfesef.googleapis.com", true},
219-
{"https://us-east-1-sts.googleapis.com", true},
220-
{"https://sts.googleapis.com/your/path/here", true},
221-
{"https://.sts.googleapis.com", false},
222-
{"https://badsts.googleapis.com", false},
223-
{"https://sts.asfe.asfesef.googleapis.com", false},
224-
{"https://sts..googleapis.com", false},
225-
{"https://-sts.googleapis.com", false},
226-
{"https://us-ea.st-1-sts.googleapis.com", false},
227-
{"https://sts.googleapis.com.evil.com/whatever/path", false},
228-
{"https://us-eas\\t-1.sts.googleapis.com", false},
229-
{"https:/us-ea/st-1.sts.googleapis.com", false},
230-
{"https:/us-east 1.sts.googleapis.com", false},
231-
{"https://", false},
232-
{"http://us-east-1.sts.googleapis.com", false},
233-
{"https://us-east-1.sts.googleapis.comevil.com", false},
234-
{"https://sts-xyz.p.googleapis.com", true},
235-
{"https://sts.pgoogleapis.com", false},
236-
{"https://p.googleapis.com", false},
237-
{"https://sts.p.com", false},
238-
{"http://sts.p.googleapis.com", false},
239-
{"https://xyz-sts.p.googleapis.com", false},
240-
{"https://sts-xyz.123.p.googleapis.com", false},
241-
{"https://sts-xyz.p1.googleapis.com", false},
242-
{"https://sts-xyz.p.foo.com", false},
243-
{"https://sts-xyz.p.foo.googleapis.com", false},
244-
}
245-
ctx := context.Background()
246-
for _, tt := range urlValidityTests {
247-
t.Run(" "+tt.tokURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
248-
config := testConfig
249-
config.TokenURL = tt.tokURL
250-
_, err := config.TokenSource(ctx)
251-
252-
if tt.expectSuccess && err != nil {
253-
t.Errorf("got %v but want nil", err)
254-
} else if !tt.expectSuccess && err == nil {
255-
t.Errorf("got nil but expected an error")
256-
}
257-
})
258-
}
259-
for _, el := range urlValidityTests {
260-
el.tokURL = strings.ToUpper(el.tokURL)
261-
}
262-
for _, tt := range urlValidityTests {
263-
t.Run(" "+tt.tokURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
264-
config := testConfig
265-
config.TokenURL = tt.tokURL
266-
_, err := config.TokenSource(ctx)
267-
268-
if tt.expectSuccess && err != nil {
269-
t.Errorf("got %v but want nil", err)
270-
} else if !tt.expectSuccess && err == nil {
271-
t.Errorf("got nil but expected an error")
272-
}
273-
})
274-
}
275-
}
276-
277-
func TestValidateURLImpersonateURL(t *testing.T) {
278-
var urlValidityTests = []struct {
279-
impURL string
280-
expectSuccess bool
281-
}{
282-
{"https://east.iamcredentials.googleapis.com", true},
283-
{"https://iamcredentials.googleapis.com", true},
284-
{"https://iamcredentials.asfeasfesef.googleapis.com", true},
285-
{"https://us-east-1-iamcredentials.googleapis.com", true},
286-
{"https://iamcredentials.googleapis.com/your/path/here", true},
287-
{"https://.iamcredentials.googleapis.com", false},
288-
{"https://badiamcredentials.googleapis.com", false},
289-
{"https://iamcredentials.asfe.asfesef.googleapis.com", false},
290-
{"https://iamcredentials..googleapis.com", false},
291-
{"https://-iamcredentials.googleapis.com", false},
292-
{"https://us-ea.st-1-iamcredentials.googleapis.com", false},
293-
{"https://iamcredentials.googleapis.com.evil.com/whatever/path", false},
294-
{"https://us-eas\\t-1.iamcredentials.googleapis.com", false},
295-
{"https:/us-ea/st-1.iamcredentials.googleapis.com", false},
296-
{"https:/us-east 1.iamcredentials.googleapis.com", false},
297-
{"https://", false},
298-
{"http://us-east-1.iamcredentials.googleapis.com", false},
299-
{"https://us-east-1.iamcredentials.googleapis.comevil.com", false},
300-
{"https://iamcredentials-xyz.p.googleapis.com", true},
301-
{"https://iamcredentials.pgoogleapis.com", false},
302-
{"https://p.googleapis.com", false},
303-
{"https://iamcredentials.p.com", false},
304-
{"http://iamcredentials.p.googleapis.com", false},
305-
{"https://xyz-iamcredentials.p.googleapis.com", false},
306-
{"https://iamcredentials-xyz.123.p.googleapis.com", false},
307-
{"https://iamcredentials-xyz.p1.googleapis.com", false},
308-
{"https://iamcredentials-xyz.p.foo.com", false},
309-
{"https://iamcredentials-xyz.p.foo.googleapis.com", false},
310-
}
311-
ctx := context.Background()
312-
for _, tt := range urlValidityTests {
313-
t.Run(" "+tt.impURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
314-
config := testConfig
315-
config.TokenURL = "https://sts.googleapis.com" // Setting the most basic acceptable tokenURL
316-
config.ServiceAccountImpersonationURL = tt.impURL
317-
_, err := config.TokenSource(ctx)
318-
319-
if tt.expectSuccess && err != nil {
320-
t.Errorf("got %v but want nil", err)
321-
} else if !tt.expectSuccess && err == nil {
322-
t.Errorf("got nil but expected an error")
323-
}
324-
})
325-
}
326-
for _, el := range urlValidityTests {
327-
el.impURL = strings.ToUpper(el.impURL)
328-
}
329-
for _, tt := range urlValidityTests {
330-
t.Run(" "+tt.impURL, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability.
331-
config := testConfig
332-
config.TokenURL = "https://sts.googleapis.com" // Setting the most basic acceptable tokenURL
333-
config.ServiceAccountImpersonationURL = tt.impURL
334-
_, err := config.TokenSource(ctx)
335-
336-
if tt.expectSuccess && err != nil {
337-
t.Errorf("got %v but want nil", err)
338-
} else if !tt.expectSuccess && err == nil {
339-
t.Errorf("got nil but expected an error")
340-
}
341-
})
342-
}
343-
}
344-
345210
func TestWorkforcePoolCreation(t *testing.T) {
346211
var audienceValidatyTests = []struct {
347212
audience string

google/internal/externalaccount/impersonate_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"io/ioutil"
1010
"net/http"
1111
"net/http/httptest"
12-
"regexp"
1312
"testing"
1413
)
1514

@@ -114,8 +113,7 @@ func TestImpersonation(t *testing.T) {
114113
defer targetServer.Close()
115114
testImpersonateConfig.TokenURL = targetServer.URL
116115

117-
allURLs := regexp.MustCompile(".+")
118-
ourTS, err := testImpersonateConfig.tokenSource(context.Background(), []*regexp.Regexp{allURLs}, []*regexp.Regexp{allURLs}, "http")
116+
ourTS, err := testImpersonateConfig.tokenSource(context.Background(), "http")
119117
if err != nil {
120118
t.Fatalf("Failed to create TokenSource: %v", err)
121119
}

0 commit comments

Comments
 (0)