From 930d60d82d07c23b19c530d9a33fb0221489580a Mon Sep 17 00:00:00 2001 From: Ryan Kohler Date: Mon, 4 Oct 2021 14:45:06 -0700 Subject: [PATCH] more tests, refactoring tests, not submitting userProject when unneeded --- .../externalaccount/basecredentials.go | 6 +- .../externalaccount/basecredentials_test.go | 193 ++++++++++++------ 2 files changed, 132 insertions(+), 67 deletions(-) diff --git a/google/internal/externalaccount/basecredentials.go b/google/internal/externalaccount/basecredentials.go index 38fb253..a1e36c0 100644 --- a/google/internal/externalaccount/basecredentials.go +++ b/google/internal/externalaccount/basecredentials.go @@ -127,7 +127,7 @@ func (c *Config) tokenSource(ctx context.Context, tokenURLValidPats []*regexp.Re if c.WorkforcePoolUserProject != "" { valid := validateWorkforceAudience(c.Audience) if !valid { - return nil, fmt.Errorf("oauth2/google: invalid Workforce Pool Audience provided while constructing tokenSource") + return nil, fmt.Errorf("oauth2/google: workforce_pool_user_project should not be set for non-workforce pool credentials") } } @@ -241,7 +241,9 @@ func (ts tokenSource) Token() (*oauth2.Token, error) { ClientSecret: conf.ClientSecret, } var options map[string]interface{} - if conf.WorkforcePoolUserProject != "" { + // Do not pass workforce_pool_user_project when client authentication is used. + // The client ID is sufficient for determining the user project. + if conf.WorkforcePoolUserProject != "" && conf.ClientID == "" { options = map[string]interface{}{ "userProject": conf.WorkforcePoolUserProject, } diff --git a/google/internal/externalaccount/basecredentials_test.go b/google/internal/externalaccount/basecredentials_test.go index b269b0c..7b9e07b 100644 --- a/google/internal/externalaccount/basecredentials_test.go +++ b/google/internal/externalaccount/basecredentials_test.go @@ -12,6 +12,8 @@ import ( "strings" "testing" "time" + + "golang.org/x/oauth2" ) const ( @@ -35,55 +37,64 @@ var testConfig = Config{ } var ( - baseCredsRequestBody = "audience=32555940559.apps.googleusercontent.com&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange&requested_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control&subject_token=street123&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt" - baseCredsResponseBody = `{"access_token":"Sample.Access.Token","issued_token_type":"urn:ietf:params:oauth:token-type:access_token","token_type":"Bearer","expires_in":3600,"scope":"https://www.googleapis.com/auth/cloud-platform"}` - workforcePoolRequestBody = "audience=%2F%2Fiam.googleapis.com%2Flocations%2Feu%2FworkforcePools%2Fpool-id%2Fproviders%2Fprovider-id&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange&options=%7B%22userProject%22%3A%22myProject%22%7D&requested_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control&subject_token=street123&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt" - correctAT = "Sample.Access.Token" - expiry int64 = 234852 + baseCredsRequestBody = "audience=32555940559.apps.googleusercontent.com&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange&requested_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control&subject_token=street123&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt" + baseCredsResponseBody = `{"access_token":"Sample.Access.Token","issued_token_type":"urn:ietf:params:oauth:token-type:access_token","token_type":"Bearer","expires_in":3600,"scope":"https://www.googleapis.com/auth/cloud-platform"}` + workforcePoolRequestBodyWithClientId = "audience=%2F%2Fiam.googleapis.com%2Flocations%2Feu%2FworkforcePools%2Fpool-id%2Fproviders%2Fprovider-id&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange&requested_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control&subject_token=street123&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt" + workforcePoolRequestBodyWithoutClientId = "audience=%2F%2Fiam.googleapis.com%2Flocations%2Feu%2FworkforcePools%2Fpool-id%2Fproviders%2Fprovider-id&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange&options=%7B%22userProject%22%3A%22myProject%22%7D&requested_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Aaccess_token&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fdevstorage.full_control&subject_token=street123&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt" + correctAT = "Sample.Access.Token" + expiry int64 = 234852 ) var ( testNow = func() time.Time { return time.Unix(expiry, 0) } ) -func TestToken(t *testing.T) { - targetServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if got, want := r.URL.String(), "/"; got != want { +type testExchangeTokenServer struct { + url string + authorization string + contentType string + body string + response string +} + +func run(t *testing.T, config *Config, tets *testExchangeTokenServer) (*oauth2.Token, error) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got, want := r.URL.String(), tets.url; got != want { t.Errorf("URL.String(): got %v but want %v", got, want) } headerAuth := r.Header.Get("Authorization") - if got, want := headerAuth, "Basic cmJyZ25vZ25yaG9uZ28zYmk0Z2I5Z2hnOWc6bm90c29zZWNyZXQ="; got != want { + if got, want := headerAuth, tets.authorization; got != want { t.Errorf("got %v but want %v", got, want) } headerContentType := r.Header.Get("Content-Type") - if got, want := headerContentType, "application/x-www-form-urlencoded"; got != want { + if got, want := headerContentType, tets.contentType; got != want { t.Errorf("got %v but want %v", got, want) } body, err := ioutil.ReadAll(r.Body) if err != nil { t.Fatalf("Failed reading request body: %s.", err) } - if got, want := string(body), baseCredsRequestBody; got != want { + if got, want := string(body), tets.body; got != want { t.Errorf("Unexpected exchange payload: got %v but want %v", got, want) } w.Header().Set("Content-Type", "application/json") - w.Write([]byte(baseCredsResponseBody)) + w.Write([]byte(tets.response)) })) - defer targetServer.Close() - - testConfig.TokenURL = targetServer.URL - ourTS := tokenSource{ - ctx: context.Background(), - conf: &testConfig, - } + defer server.Close() + config.TokenURL = server.URL oldNow := now defer func() { now = oldNow }() now = testNow - tok, err := ourTS.Token() - if err != nil { - t.Fatalf("Unexpected error: %e", err) + ts := tokenSource{ + ctx: context.Background(), + conf: config, } + + return ts.Token() +} + +func validateToken(t *testing.T, tok *oauth2.Token) { if got, want := tok.AccessToken, correctAT; got != want { t.Errorf("Unexpected access token: got %v, but wanted %v", got, want) } @@ -91,61 +102,113 @@ func TestToken(t *testing.T) { t.Errorf("Unexpected TokenType: got %v, but wanted %v", got, want) } - if got, want := tok.Expiry, now().Add(time.Duration(3600)*time.Second); got != want { + if got, want := tok.Expiry, testNow().Add(time.Duration(3600)*time.Second); got != want { t.Errorf("Unexpected Expiry: got %v, but wanted %v", got, want) } } -func TestWorkforcePoolToken(t *testing.T) { - targetServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if got, want := r.URL.String(), "/"; got != want { - t.Errorf("URL.String(): got %v but want %v", got, want) - } - headerAuth := r.Header.Get("Authorization") - if got, want := headerAuth, "Basic cmJyZ25vZ25yaG9uZ28zYmk0Z2I5Z2hnOWc6bm90c29zZWNyZXQ="; got != want { - t.Errorf("got %v but want %v", got, want) - } - headerContentType := r.Header.Get("Content-Type") - if got, want := headerContentType, "application/x-www-form-urlencoded"; got != want { - t.Errorf("got %v but want %v", got, want) - } - body, err := ioutil.ReadAll(r.Body) - if err != nil { - t.Fatalf("Failed reading request body: %s.", err) - } - if got, want := string(body), workforcePoolRequestBody; got != want { - t.Errorf("Unexpected exchange payload: got %v but want %v", got, want) - } - w.Header().Set("Content-Type", "application/json") - w.Write([]byte(baseCredsResponseBody)) - })) - defer targetServer.Close() - - testConfig.TokenURL = targetServer.URL - testConfig.WorkforcePoolUserProject = "myProject" - testConfig.Audience = "//iam.googleapis.com/locations/eu/workforcePools/pool-id/providers/provider-id" - ourTS := tokenSource{ - ctx: context.Background(), - conf: &testConfig, +func TestToken(t *testing.T) { + config := Config{ + Audience: "32555940559.apps.googleusercontent.com", + SubjectTokenType: "urn:ietf:params:oauth:token-type:jwt", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + CredentialSource: testBaseCredSource, + Scopes: []string{"https://www.googleapis.com/auth/devstorage.full_control"}, } - oldNow := now - defer func() { now = oldNow }() - now = testNow + server := testExchangeTokenServer{ + url: "/", + authorization: "Basic cmJyZ25vZ25yaG9uZ28zYmk0Z2I5Z2hnOWc6bm90c29zZWNyZXQ=", + contentType: "application/x-www-form-urlencoded", + body: baseCredsRequestBody, + response: baseCredsResponseBody, + } + + tok, err := run(t, &config, &server) - tok, err := ourTS.Token() if err != nil { t.Fatalf("Unexpected error: %e", err) } - if got, want := tok.AccessToken, correctAT; got != want { - t.Errorf("Unexpected access token: got %v, but wanted %v", got, want) - } - if got, want := tok.TokenType, "Bearer"; got != want { - t.Errorf("Unexpected TokenType: got %v, but wanted %v", got, want) + validateToken(t, tok) +} + +func TestWorkforcePoolTokenWithClientID(t *testing.T) { + config := Config{ + Audience: "//iam.googleapis.com/locations/eu/workforcePools/pool-id/providers/provider-id", + SubjectTokenType: "urn:ietf:params:oauth:token-type:jwt", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + CredentialSource: testBaseCredSource, + Scopes: []string{"https://www.googleapis.com/auth/devstorage.full_control"}, + WorkforcePoolUserProject: "myProject", } - if got, want := tok.Expiry, now().Add(time.Duration(3600)*time.Second); got != want { - t.Errorf("Unexpected Expiry: got %v, but wanted %v", got, want) + server := testExchangeTokenServer{ + url: "/", + authorization: "Basic cmJyZ25vZ25yaG9uZ28zYmk0Z2I5Z2hnOWc6bm90c29zZWNyZXQ=", + contentType: "application/x-www-form-urlencoded", + body: workforcePoolRequestBodyWithClientId, + response: baseCredsResponseBody, + } + + tok, err := run(t, &config, &server) + + if err != nil { + t.Fatalf("Unexpected error: %e", err) + } + validateToken(t, tok) +} + +func TestWorkforcePoolTokenWithoutClientID(t *testing.T) { + config := Config{ + Audience: "//iam.googleapis.com/locations/eu/workforcePools/pool-id/providers/provider-id", + SubjectTokenType: "urn:ietf:params:oauth:token-type:jwt", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ClientSecret: "notsosecret", + CredentialSource: testBaseCredSource, + Scopes: []string{"https://www.googleapis.com/auth/devstorage.full_control"}, + WorkforcePoolUserProject: "myProject", + } + + server := testExchangeTokenServer{ + url: "/", + authorization: "", + contentType: "application/x-www-form-urlencoded", + body: workforcePoolRequestBodyWithoutClientId, + response: baseCredsResponseBody, + } + + tok, err := run(t, &config, &server) + + if err != nil { + t.Fatalf("Unexpected error: %e", err) + } + validateToken(t, tok) +} + +func TestNonworkforceWithWorkforcePoolUserProject(t *testing.T) { + config := Config{ + Audience: "32555940559.apps.googleusercontent.com", + SubjectTokenType: "urn:ietf:params:oauth:token-type:jwt", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + TokenURL: "https://sts.googleapis.com", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + CredentialSource: testBaseCredSource, + Scopes: []string{"https://www.googleapis.com/auth/devstorage.full_control"}, + WorkforcePoolUserProject: "myProject", + } + + _, err := config.TokenSource(context.Background()) + + if err == nil { + t.Fatalf("Expected error but found none") + } + if got, want := err.Error(), "oauth2/google: workforce_pool_user_project should not be set for non-workforce pool credentials"; got != want { + t.Errorf("Incorrect error received.\nExpected: %s\nRecieved: %s", want, got) } }