From cd749add62c55086845f8f9e3314a75b8d179553 Mon Sep 17 00:00:00 2001 From: aeitzman Date: Tue, 11 Jun 2024 11:54:48 -0700 Subject: [PATCH 1/9] feat: adds new provider type for X509 credentials and new client creation logic --- .../externalaccount/externalaccount.go | 24 ++- .../testdata/certificate_config_workload.json | 8 + .../testdata/workload_cert.pem | 22 +++ .../externalaccount/testdata/workload_key.pem | 28 ++++ .../internal/externalaccount/url_provider.go | 1 + .../internal/externalaccount/x509_provider.go | 58 +++++++ .../externalaccount/x509_provider_test.go | 141 ++++++++++++++++++ auth/internal/credsfile/filetype.go | 28 ++-- 8 files changed, 297 insertions(+), 13 deletions(-) create mode 100644 auth/credentials/internal/externalaccount/testdata/certificate_config_workload.json create mode 100644 auth/credentials/internal/externalaccount/testdata/workload_cert.pem create mode 100644 auth/credentials/internal/externalaccount/testdata/workload_key.pem create mode 100644 auth/credentials/internal/externalaccount/x509_provider.go create mode 100644 auth/credentials/internal/externalaccount/x509_provider_test.go diff --git a/auth/credentials/internal/externalaccount/externalaccount.go b/auth/credentials/internal/externalaccount/externalaccount.go index b19c6edeae5a..53c0442fe5cb 100644 --- a/auth/credentials/internal/externalaccount/externalaccount.go +++ b/auth/credentials/internal/externalaccount/externalaccount.go @@ -204,11 +204,22 @@ func NewTokenProvider(opts *Options) (auth.TokenProvider, error) { if err != nil { return nil, err } + + client := opts.Client + xp, _ := stp.(*x509Provider) + if xp != nil { + client, err = xp.client() + if err != nil { + return nil, err + } + } + tp := &tokenProvider{ - client: opts.Client, + client: client, opts: opts, stp: stp, } + if opts.ServiceAccountImpersonationURL == "" { return auth.NewCachedTokenProvider(tp, nil), nil } @@ -218,7 +229,7 @@ func NewTokenProvider(opts *Options) (auth.TokenProvider, error) { // needed for impersonation tp.opts.Scopes = []string{"https://www.googleapis.com/auth/cloud-platform"} imp, err := impersonate.NewTokenProvider(&impersonate.Options{ - Client: opts.Client, + Client: client, URL: opts.ServiceAccountImpersonationURL, Scopes: scopes, Tp: auth.NewCachedTokenProvider(tp, nil), @@ -353,6 +364,15 @@ func newSubjectTokenProvider(o *Options) (subjectTokenProvider, error) { execProvider.opts = o execProvider.env = runtimeEnvironment{} return execProvider, nil + } else if o.CredentialSource.Certificate != nil { + cert := o.CredentialSource.Certificate + if cert.UseDefaultCertificateConfig == false && cert.CertificateConfigLocation == "" { + return nil, errors.New("credentials: \"certificate\" object must either specify a certificate_config_location or use_default_certificate_config should be true") + } + if cert.UseDefaultCertificateConfig == true && cert.CertificateConfigLocation != "" { + return nil, errors.New("credentials: \"certificate\" object cannot specify both a certificate_config_location and use_default_certificate_config=true") + } + return &x509Provider{certificateConfigLocation: cert.CertificateConfigLocation}, nil } return nil, errors.New("credentials: unable to parse credential source") } diff --git a/auth/credentials/internal/externalaccount/testdata/certificate_config_workload.json b/auth/credentials/internal/externalaccount/testdata/certificate_config_workload.json new file mode 100644 index 000000000000..e61375c9bd3a --- /dev/null +++ b/auth/credentials/internal/externalaccount/testdata/certificate_config_workload.json @@ -0,0 +1,8 @@ +{ + "cert_configs": { + "workload": { + "cert_path": "testdata/workload_cert.pem", + "key_path": "testdata/workload_key.pem" + } + } +} diff --git a/auth/credentials/internal/externalaccount/testdata/workload_cert.pem b/auth/credentials/internal/externalaccount/testdata/workload_cert.pem new file mode 100644 index 000000000000..21d2b8738e1b --- /dev/null +++ b/auth/credentials/internal/externalaccount/testdata/workload_cert.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDujCCAqICCQD+yrCYuiC8djANBgkqhkiG9w0BAQsFADCBnTELMAkGA1UEBhMC +VVMxEzARBgNVBAgMCldhc2hpbmd0b24xETAPBgNVBAcMCEtpcmtsYW5kMQ8wDQYD +VQQKDAZHb29nbGUxDjAMBgNVBAsMBUNsb3VkMRswGQYDVQQDDBJnb29nbGVhcGlz +dGVzdC5jb20xKDAmBgkqhkiG9w0BCQEWGWdvb2dsZWFwaXN0ZXN0QGdvb2dsZS5j +b20wIBcNMjAxMDIzMjEyNTU1WhgPMjEyMDA5MjkyMTI1NTVaMIGdMQswCQYDVQQG +EwJVUzETMBEGA1UECAwKV2FzaGluZ3RvbjERMA8GA1UEBwwIS2lya2xhbmQxDzAN +BgNVBAoMBkdvb2dsZTEOMAwGA1UECwwFQ2xvdWQxGzAZBgNVBAMMEmdvb2dsZWFw +aXN0ZXN0LmNvbTEoMCYGCSqGSIb3DQEJARYZZ29vZ2xlYXBpc3Rlc3RAZ29vZ2xl +LmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKnzFX97VP4XSQ8l +4/Z08eajnAiGpK+ZQTV9k7Qy2tpo5+iFFiL0JLGP9+GRILuDGQufYlPLDhLLho9V +YXIR9UOhhapmQJqUAUFhvZlBEixLxcfwa2LecNiJ6+8gvJCoRbrPIrz91crY+t59 +aY/09vmsCbFDX8d8WWVnww4285dfKwE2IDinqZ1VuT4zYR66f4lL8qj6t5TXeGAW +Nkd6O3yuAVO8RLiXBRRABP5217mq0jNL+kJUormzhuKgvP+oxRsi56XHPGiq7l2e +54PS/cqa4atjqbhZI1xV27y0sVr0/CmBsfeM3TwLbCSjv7r0lCz64xtCJa8R45MA +22or9z8CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAnwLY9qBIQ2IYDLNLx16av8C6 +9vca8gOzMpYZ4UKHDN+Qk2CidpmFamXWDXqmOLNZYlmEoGY5n8zg8rwYK+vauqwb +o94HzxLmQcQ4kmAI4xJnMqKZAbukRdWw2GCuvdVqG4Osngz4WBIHrAsl4btogdJy +ACU/YUA3K0tLjwe6wUYYF6eu5sb6zJkF4cfLpqECWtF9XG6nkJbo2GomHFuHm+6t +gOj7YiqU/cHCyU4FQF9/2jDLzFHxt2Bb30zi602YjuIZhYp35ktI66XwsE4kFmwo +iHCEG0fXMNN7OMFmNg2YVLhaHxrQNFxbzOQdfKg2gi2qzX4AiCo1tx5LCg6aGw== +-----END CERTIFICATE----- diff --git a/auth/credentials/internal/externalaccount/testdata/workload_key.pem b/auth/credentials/internal/externalaccount/testdata/workload_key.pem new file mode 100644 index 000000000000..1f85f955a69d --- /dev/null +++ b/auth/credentials/internal/externalaccount/testdata/workload_key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCp8xV/e1T+F0kP +JeP2dPHmo5wIhqSvmUE1fZO0MtraaOfohRYi9CSxj/fhkSC7gxkLn2JTyw4Sy4aP +VWFyEfVDoYWqZkCalAFBYb2ZQRIsS8XH8Gti3nDYievvILyQqEW6zyK8/dXK2Pre +fWmP9Pb5rAmxQ1/HfFllZ8MONvOXXysBNiA4p6mdVbk+M2Eeun+JS/Ko+reU13hg +FjZHejt8rgFTvES4lwUUQAT+dte5qtIzS/pCVKK5s4bioLz/qMUbIuelxzxoqu5d +nueD0v3KmuGrY6m4WSNcVdu8tLFa9PwpgbH3jN08C2wko7+69JQs+uMbQiWvEeOT +ANtqK/c/AgMBAAECggEAYjeE3hb1yJ7Gb0WzmDR/tI4rV9YQiRcl03cOjJ6zUnQ8 +SmnXoD2+kwuj8y1/YD7kk436MnjwWjZbPqzWUylDuGE5sX/EqFEO5K1K+K3dhdII +rIMqXIo3Zz1WJ+2gbG2DVvHsnpKIIuIBIeISxsqIjUQ6mcJZMR2RQISV+roRTxIU +1Ga0xWrExcKL8FSjs8ih0DWU4vHoSYH4DFXB1/ViyLn+DEljnOlo8Q+7DG0uQQnX +ixfYMbXSJcZxFm1iwuZv8SESjqbTsogNny5Wi6H9Vp0JFasAPUjnc+QuD/U1HTDn +PCX3eBNMcxvVJDhu/7nnO7kcU1Cx0gJeN+1bklrAcQKBgQDURl0Ac8N94I82n4Lg +wjGLWj3AMxSEHNcZuomCvoYcLTmJdd2tOnunXhh1jANnx6q8P8aR5fiTthokIUdx +bOmWwFAbP6kMe0WFWQhXjX4mXLRmJ4mWayWCE7hstnDb3/Fr7LuJeg5L3OU4ss3b +j4UvhtuQ9Qh8piVhKwFkQh3tOQKBgQDM9NSkRDVW3Q37lMUdyn8B2FBF78e/9ck+ +5bHOs52G2hXJ4tyLYNjBoLXPpMp9VWRTXxUaii+gHSa4DkHTkFwIg34hLgrCX7Gc +a0rldvkpX0xWSANfvO9bvavPgKnLSP8j3mjDiwqJuy3L5TBThIHDvPV9F/akpLne +bdcywa4ANwKBgHlvAzcGAniZJPRXjfRrwxH3/slbr0nggcDLMG0l9uxZhse3MKgv +g5t8PbvI7A3LcEWeqka+a1R84Tl3/DnL11kRDQJ5iYiFYIDnLNmBLQBfGigySAhP +pTZjd6ZhO/DcjGx0EdiUhWcqp8qmpxMKaGOG30ZulntQRKPwiSxEkoApAoGBAJ1o +h4ulawXMfnmyt3T62XJ0TKp5zoKqZSYuSNIEdr5j7goAdvuApNiI8jmISY/arlOt +mcqpSIyC9wKyyHGQ1G4hdxRKhS7lScZlTL9REWlp7HnzksvLklV2JWcXXNBovrMw +lGth9PT00eZfni72fKb1D+FEL0Qh0zJ2T6mGwHkfAoGAMOy8bbyCASCYG9MYzqaP +Lf+AKKNEYUvUGspyJUqu5ERudr5stmei6PrchxFiKjm5Qg7B/M1VnKsCtL9kk8Z9 +lHgwU5mOATZvd9k/5oiuRxzXyrWqFoT/mivI2rZE+g5cLTLytCTnyLjHm5B/aTy8 +1AmbAh5hvWYs+EMKZAlQ5GM= +-----END PRIVATE KEY----- diff --git a/auth/credentials/internal/externalaccount/url_provider.go b/auth/credentials/internal/externalaccount/url_provider.go index 22b8af1c11b8..a9429abbc581 100644 --- a/auth/credentials/internal/externalaccount/url_provider.go +++ b/auth/credentials/internal/externalaccount/url_provider.go @@ -30,6 +30,7 @@ const ( fileTypeJSON = "json" urlProviderType = "url" programmaticProviderType = "programmatic" + x509ProviderType = "x509" ) type urlSubjectProvider struct { diff --git a/auth/credentials/internal/externalaccount/x509_provider.go b/auth/credentials/internal/externalaccount/x509_provider.go new file mode 100644 index 000000000000..21187bdbf8da --- /dev/null +++ b/auth/credentials/internal/externalaccount/x509_provider.go @@ -0,0 +1,58 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package externalaccount + +import ( + "context" + "crypto/tls" + "net/http" + "time" + + "cloud.google.com/go/auth/internal/transport/cert" +) + +type x509Provider struct { + certificateConfigLocation string + cachedClient *http.Client +} + +func (xp *x509Provider) providerType() string { + return x509ProviderType +} + +func (xp *x509Provider) subjectToken(ctx context.Context) (string, error) { + return "", nil +} + +func (xp *x509Provider) client() (*http.Client, error) { + if xp.cachedClient == nil { + certProvider, err := cert.NewWorkloadX509CertProvider(xp.certificateConfigLocation) + if err != nil { + return nil, err + } + + trans := http.DefaultTransport.(*http.Transport).Clone() + + trans.TLSClientConfig = &tls.Config{ + GetClientCertificate: certProvider, + } + + xp.cachedClient = &http.Client{ + Transport: trans, + Timeout: 30 * time.Second, + } + } + return xp.cachedClient, nil +} diff --git a/auth/credentials/internal/externalaccount/x509_provider_test.go b/auth/credentials/internal/externalaccount/x509_provider_test.go new file mode 100644 index 000000000000..add3ec76276f --- /dev/null +++ b/auth/credentials/internal/externalaccount/x509_provider_test.go @@ -0,0 +1,141 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package externalaccount + +import ( + "context" + "testing" + + "cloud.google.com/go/auth/internal" + "cloud.google.com/go/auth/internal/credsfile" +) + +func TestCreateX509Credential(t *testing.T) { + var tests = []struct { + name string + certificateConfig credsfile.CertificateConfig + wantErr bool + }{ + { + name: "Basic Creation", + certificateConfig: credsfile.CertificateConfig{ + UseDefaultCertificateConfig: true, + }, + }, + { + name: "Specific location", + certificateConfig: credsfile.CertificateConfig{ + CertificateConfigLocation: "test", + }, + }, + { + name: "Default and location provided", + certificateConfig: credsfile.CertificateConfig{ + UseDefaultCertificateConfig: true, + CertificateConfigLocation: "test", + }, + wantErr: true, + }, + { + name: "Neither default or location provided", + certificateConfig: credsfile.CertificateConfig{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := newSubjectTokenProvider(&Options{ + Client: internal.CloneDefaultClient(), + CredentialSource: &credsfile.CredentialSource{ + Certificate: &tt.certificateConfig, + }, + }) + if tt.wantErr == true { + if err == nil { + t.Fatalf("got nil, want an error") + } + } else if err != nil { + t.Errorf("got error: %v, expected no error", err) + } + }) + } +} + +func TestRetrieveSubjectToken_X509(t *testing.T) { + opts := cloneTestOpts() + opts.CredentialSource = &credsfile.CredentialSource{ + Certificate: &credsfile.CertificateConfig{ + UseDefaultCertificateConfig: true, + }, + } + + base, err := newSubjectTokenProvider(opts) + if err != nil { + t.Fatalf("newSubjectTokenProvider(): %v", err) + } + + got, err := base.subjectToken(context.Background()) + if err != nil { + t.Fatalf("subjectToken(): %v", err) + } + + if want := ""; got != want { + t.Errorf("got %q, want %q", got, want) + } + if got, want := base.providerType(), x509ProviderType; got != want { + t.Fatalf("got %q, want %q", got, want) + } +} + +func TestGetClient_Success(t *testing.T) { + opts := cloneTestOpts() + opts.CredentialSource = &credsfile.CredentialSource{ + Certificate: &credsfile.CertificateConfig{ + CertificateConfigLocation: "testdata/certificate_config_workload.json", + }, + } + + base, err := newSubjectTokenProvider(opts) + if err != nil { + t.Fatalf("newSubjectTokenProvider(): %v", err) + } + + client, err := base.(*x509Provider).client() + if err != nil { + t.Fatalf("getclient(): %v", err) + } + + if client == nil { + t.Error("client returned was nil") + } +} + +func TestGetClient_error(t *testing.T) { + opts := cloneTestOpts() + opts.CredentialSource = &credsfile.CredentialSource{ + Certificate: &credsfile.CertificateConfig{ + CertificateConfigLocation: "testdata/bad_file.json", + }, + } + + base, err := newSubjectTokenProvider(opts) + if err != nil { + t.Fatalf("newSubjectTokenProvider(): %v", err) + } + + if _, err = base.(*x509Provider).client(); err == nil { + t.Errorf("got nil, want an error") + } +} diff --git a/auth/internal/credsfile/filetype.go b/auth/internal/credsfile/filetype.go index 69e30779f987..0c92c35ef265 100644 --- a/auth/internal/credsfile/filetype.go +++ b/auth/internal/credsfile/filetype.go @@ -90,19 +90,20 @@ type ExternalAccountAuthorizedUserFile struct { // CredentialSource stores the information necessary to retrieve the credentials for the STS exchange. // -// One field amongst File, URL, and Executable should be filled, depending on the kind of credential in question. +// One field amongst File, URL, Certificate, and Executable should be filled, depending on the kind of credential in question. // The EnvironmentID should start with AWS if being used for an AWS credential. type CredentialSource struct { - File string `json:"file"` - URL string `json:"url"` - Headers map[string]string `json:"headers"` - Executable *ExecutableConfig `json:"executable,omitempty"` - EnvironmentID string `json:"environment_id"` - RegionURL string `json:"region_url"` - RegionalCredVerificationURL string `json:"regional_cred_verification_url"` - CredVerificationURL string `json:"cred_verification_url"` - IMDSv2SessionTokenURL string `json:"imdsv2_session_token_url"` - Format *Format `json:"format,omitempty"` + File string `json:"file"` + URL string `json:"url"` + Headers map[string]string `json:"headers"` + Executable *ExecutableConfig `json:"executable,omitempty"` + Certificate *CertificateConfig `json:"certificate"` + EnvironmentID string `json:"environment_id"` // TODO: Make type for this + RegionURL string `json:"region_url"` + RegionalCredVerificationURL string `json:"regional_cred_verification_url"` + CredVerificationURL string `json:"cred_verification_url"` + IMDSv2SessionTokenURL string `json:"imdsv2_session_token_url"` + Format *Format `json:"format,omitempty"` } // Format describes the format of a [CredentialSource]. @@ -121,6 +122,11 @@ type ExecutableConfig struct { OutputFile string `json:"output_file"` } +type CertificateConfig struct { + UseDefaultCertificateConfig bool `json:"use_default_certificate_config"` + CertificateConfigLocation string `json:"certificate_config_location"` +} + // ServiceAccountImpersonationInfo has impersonation configuration. type ServiceAccountImpersonationInfo struct { TokenLifetimeSeconds int `json:"token_lifetime_seconds"` From e85be852b18452fbf46292b6d4e080fda618aa17 Mon Sep 17 00:00:00 2001 From: aeitzman Date: Wed, 12 Jun 2024 09:34:45 -0700 Subject: [PATCH 2/9] adds comments, minor fixes --- auth/credentials/internal/externalaccount/externalaccount.go | 4 ++-- auth/credentials/internal/externalaccount/x509_provider.go | 2 ++ .../internal/externalaccount/x509_provider_test.go | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/auth/credentials/internal/externalaccount/externalaccount.go b/auth/credentials/internal/externalaccount/externalaccount.go index 53c0442fe5cb..0ae3d7635ce3 100644 --- a/auth/credentials/internal/externalaccount/externalaccount.go +++ b/auth/credentials/internal/externalaccount/externalaccount.go @@ -366,10 +366,10 @@ func newSubjectTokenProvider(o *Options) (subjectTokenProvider, error) { return execProvider, nil } else if o.CredentialSource.Certificate != nil { cert := o.CredentialSource.Certificate - if cert.UseDefaultCertificateConfig == false && cert.CertificateConfigLocation == "" { + if !cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation == "" { return nil, errors.New("credentials: \"certificate\" object must either specify a certificate_config_location or use_default_certificate_config should be true") } - if cert.UseDefaultCertificateConfig == true && cert.CertificateConfigLocation != "" { + if cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation != "" { return nil, errors.New("credentials: \"certificate\" object cannot specify both a certificate_config_location and use_default_certificate_config=true") } return &x509Provider{certificateConfigLocation: cert.CertificateConfigLocation}, nil diff --git a/auth/credentials/internal/externalaccount/x509_provider.go b/auth/credentials/internal/externalaccount/x509_provider.go index 21187bdbf8da..4ada6d7577b1 100644 --- a/auth/credentials/internal/externalaccount/x509_provider.go +++ b/auth/credentials/internal/externalaccount/x509_provider.go @@ -37,6 +37,7 @@ func (xp *x509Provider) subjectToken(ctx context.Context) (string, error) { } func (xp *x509Provider) client() (*http.Client, error) { + // Create client if it doesn't already exist if xp.cachedClient == nil { certProvider, err := cert.NewWorkloadX509CertProvider(xp.certificateConfigLocation) if err != nil { @@ -49,6 +50,7 @@ func (xp *x509Provider) client() (*http.Client, error) { GetClientCertificate: certProvider, } + // Create client with default settings plus the X509 workload certs xp.cachedClient = &http.Client{ Transport: trans, Timeout: 30 * time.Second, diff --git a/auth/credentials/internal/externalaccount/x509_provider_test.go b/auth/credentials/internal/externalaccount/x509_provider_test.go index add3ec76276f..175d3b07606d 100644 --- a/auth/credentials/internal/externalaccount/x509_provider_test.go +++ b/auth/credentials/internal/externalaccount/x509_provider_test.go @@ -99,7 +99,7 @@ func TestRetrieveSubjectToken_X509(t *testing.T) { } } -func TestGetClient_Success(t *testing.T) { +func TestClient_Success(t *testing.T) { opts := cloneTestOpts() opts.CredentialSource = &credsfile.CredentialSource{ Certificate: &credsfile.CertificateConfig{ @@ -114,7 +114,7 @@ func TestGetClient_Success(t *testing.T) { client, err := base.(*x509Provider).client() if err != nil { - t.Fatalf("getclient(): %v", err) + t.Fatalf("client(): %v", err) } if client == nil { From 0e14d3ccf82bf06864efaebcf9ce174097813ebf Mon Sep 17 00:00:00 2001 From: aeitzman Date: Wed, 12 Jun 2024 09:41:33 -0700 Subject: [PATCH 3/9] adds comment --- auth/credentials/internal/externalaccount/externalaccount.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/auth/credentials/internal/externalaccount/externalaccount.go b/auth/credentials/internal/externalaccount/externalaccount.go index 0ae3d7635ce3..4dde9970e677 100644 --- a/auth/credentials/internal/externalaccount/externalaccount.go +++ b/auth/credentials/internal/externalaccount/externalaccount.go @@ -206,6 +206,8 @@ func NewTokenProvider(opts *Options) (auth.TokenProvider, error) { } client := opts.Client + // If the credential is using X509, we need to create a different client that uses + // the certs specified in the credential configuration for a mTLS request. xp, _ := stp.(*x509Provider) if xp != nil { client, err = xp.client() From 005b56e5d9ff36d5d252f0a2120fa8f109440bce Mon Sep 17 00:00:00 2001 From: aeitzman Date: Wed, 12 Jun 2024 09:58:25 -0700 Subject: [PATCH 4/9] Add comment for linter --- auth/internal/credsfile/filetype.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/auth/internal/credsfile/filetype.go b/auth/internal/credsfile/filetype.go index 0c92c35ef265..52670f4b2a06 100644 --- a/auth/internal/credsfile/filetype.go +++ b/auth/internal/credsfile/filetype.go @@ -122,6 +122,8 @@ type ExecutableConfig struct { OutputFile string `json:"output_file"` } +// CertificateConfig reqpresents the options used to set up X509 based workload +// [CredentialSource] type CertificateConfig struct { UseDefaultCertificateConfig bool `json:"use_default_certificate_config"` CertificateConfigLocation string `json:"certificate_config_location"` From 9bdb4579af378f88bdf74ded9bef4fc2c6dc4fdd Mon Sep 17 00:00:00 2001 From: aeitzman Date: Fri, 14 Jun 2024 11:56:25 -0700 Subject: [PATCH 5/9] Added default client logic so the external account credentials have visibility into whether the client should be overriden --- .../externalaccount/externalaccount.go | 1 + auth/credentials/filetypes.go | 1 + .../externalaccount/externalaccount.go | 45 +++--- .../externalaccount/externalaccount_test.go | 136 ++++++++++++++++++ .../internal/externalaccount/x509_provider.go | 39 +++-- .../externalaccount/x509_provider_test.go | 30 +--- 6 files changed, 186 insertions(+), 66 deletions(-) diff --git a/auth/credentials/externalaccount/externalaccount.go b/auth/credentials/externalaccount/externalaccount.go index f350e1973803..cae0a300c1fc 100644 --- a/auth/credentials/externalaccount/externalaccount.go +++ b/auth/credentials/externalaccount/externalaccount.go @@ -242,6 +242,7 @@ func (o *Options) toInternalOpts() *iexacc.Options { SubjectTokenProvider: toInternalSubjectTokenProvider(o.SubjectTokenProvider), AwsSecurityCredentialsProvider: toInternalAwsSecurityCredentialsProvider(o.AwsSecurityCredentialsProvider), Client: o.client(), + IsDefaultClient: o.Client == nil, } if o.CredentialSource != nil { cs := o.CredentialSource diff --git a/auth/credentials/filetypes.go b/auth/credentials/filetypes.go index fe93557389d2..b426e16d2975 100644 --- a/auth/credentials/filetypes.go +++ b/auth/credentials/filetypes.go @@ -174,6 +174,7 @@ func handleExternalAccount(f *credsfile.ExternalAccountFile, opts *DetectOptions Scopes: opts.scopes(), WorkforcePoolUserProject: f.WorkforcePoolUserProject, Client: opts.client(), + IsDefaultClient: opts.Client == nil, } if f.ServiceAccountImpersonation != nil { externalOpts.ServiceAccountImpersonationLifetimeSeconds = f.ServiceAccountImpersonation.TokenLifetimeSeconds diff --git a/auth/credentials/internal/externalaccount/externalaccount.go b/auth/credentials/internal/externalaccount/externalaccount.go index 4dde9970e677..555ce5d1fab1 100644 --- a/auth/credentials/internal/externalaccount/externalaccount.go +++ b/auth/credentials/internal/externalaccount/externalaccount.go @@ -100,6 +100,10 @@ type Options struct { AwsSecurityCredentialsProvider AwsSecurityCredentialsProvider // Client for token request. Client *http.Client + // IsDefaultClient marks whether the client passed in is a default client that can be overriden. + // This is important for X509 credentials which should create a new client if the default was used + // but should respect a client explicitly passed in by the user. + IsDefaultClient bool } // SubjectTokenProvider can be used to supply a subject token to exchange for a @@ -181,6 +185,26 @@ func (o *Options) validate() error { return nil } +// client returns the http client that should be used for the token exchange. If a non-default client +// is provided, then the client configured in the options will always be returned. If a default client +// is provided and the options are configured for X509 credentials, a new client will be created. +func (o *Options) client() (*http.Client, error) { + // If a client was explicitly provided or if there is no certificate configuration, return the client + // from the options. + if !o.IsDefaultClient || o.CredentialSource == nil || o.CredentialSource.Certificate == nil { + return o.Client, nil + } + + cert := o.CredentialSource.Certificate + if !cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation == "" { + return nil, errors.New("credentials: \"certificate\" object must either specify a certificate_config_location or use_default_certificate_config should be true") + } + if cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation != "" { + return nil, errors.New("credentials: \"certificate\" object cannot specify both a certificate_config_location and use_default_certificate_config=true") + } + return createX509Client(cert.CertificateConfigLocation) +} + // resolveTokenURL sets the default STS token endpoint with the configured // universe domain. func (o *Options) resolveTokenURL() { @@ -205,15 +229,9 @@ func NewTokenProvider(opts *Options) (auth.TokenProvider, error) { return nil, err } - client := opts.Client - // If the credential is using X509, we need to create a different client that uses - // the certs specified in the credential configuration for a mTLS request. - xp, _ := stp.(*x509Provider) - if xp != nil { - client, err = xp.client() - if err != nil { - return nil, err - } + client, err := opts.client() + if err != nil { + return nil, err } tp := &tokenProvider{ @@ -367,14 +385,7 @@ func newSubjectTokenProvider(o *Options) (subjectTokenProvider, error) { execProvider.env = runtimeEnvironment{} return execProvider, nil } else if o.CredentialSource.Certificate != nil { - cert := o.CredentialSource.Certificate - if !cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation == "" { - return nil, errors.New("credentials: \"certificate\" object must either specify a certificate_config_location or use_default_certificate_config should be true") - } - if cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation != "" { - return nil, errors.New("credentials: \"certificate\" object cannot specify both a certificate_config_location and use_default_certificate_config=true") - } - return &x509Provider{certificateConfigLocation: cert.CertificateConfigLocation}, nil + return &x509Provider{}, nil } return nil, errors.New("credentials: unable to parse credential source") } diff --git a/auth/credentials/internal/externalaccount/externalaccount_test.go b/auth/credentials/internal/externalaccount/externalaccount_test.go index 6e32c98a910f..2f0386c09b8a 100644 --- a/auth/credentials/internal/externalaccount/externalaccount_test.go +++ b/auth/credentials/internal/externalaccount/externalaccount_test.go @@ -33,10 +33,12 @@ import ( const ( textBaseCredPath = "testdata/3pi_cred.txt" jsonBaseCredPath = "testdata/3pi_cred.json" + certConfigCredPath = "testdata/certificate_config_workload.json" 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%3Aid_token" 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%3Aid_token" 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%3Aid_token" + x509CredsRequestBody = "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=&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Amtls" correctAT = "Sample.Access.Token" expiry int64 = 234852 ) @@ -463,6 +465,140 @@ func TestOptionsValidate(t *testing.T) { } } +func TestClient(t *testing.T) { + goodCertConfig := credsfile.CertificateConfig{ + CertificateConfigLocation: "testdata/certificate_config_workload.json", + } + badCertConfig := credsfile.CertificateConfig{ + CertificateConfigLocation: "bad_file.json", + } + client := internal.CloneDefaultClient() + + tests := []struct { + name string + o *Options + wantErr bool + wantClientChanged bool + }{ + { + name: "isDefault true with no certificate config", + o: &Options{ + Audience: "32555940559.apps.googleusercontent.com", + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: true, + CredentialSource: testBaseCredSource, + }, + wantErr: false, + wantClientChanged: false, + }, + { + name: "isDefault false with no certificate config", + o: &Options{ + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: false, + CredentialSource: testBaseCredSource, + }, + wantErr: false, + wantClientChanged: false, + }, + { + name: "isDefault false with certificate config", + o: &Options{ + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: false, + CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfig}, + }, + wantErr: false, + wantClientChanged: false, + }, + { + name: "isDefault true with certificate config", + o: &Options{ + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: true, + CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfig}, + }, + wantErr: false, + wantClientChanged: true, + }, + { + name: "isDefault false with bad certificate config", + o: &Options{ + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: false, + CredentialSource: &credsfile.CredentialSource{Certificate: &badCertConfig}, + }, + wantErr: false, + wantClientChanged: false, + }, + { + name: "isDefault true with bad certificate config", + o: &Options{ + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: true, + CredentialSource: &credsfile.CredentialSource{Certificate: &badCertConfig}, + }, + wantErr: true, + wantClientChanged: true, + }, + } + for _, tc := range tests { + actualClient, err := tc.o.client() + if err == nil && tc.wantErr { + t.Fatalf("o.validate() = nil, want error") + } + if err != nil && !tc.wantErr { + t.Fatalf("o.validate() = non-nil error, want error") + } + + if tc.wantClientChanged { + if actualClient == client { + t.Fatalf("wanted client to change, but default was used") + } + } else { + if actualClient != client { + t.Fatalf("wanted default client, but client was changed. %s", tc.name) + } + } + } +} + func TestOptionsResolveTokenURL(t *testing.T) { tests := []struct { name string diff --git a/auth/credentials/internal/externalaccount/x509_provider.go b/auth/credentials/internal/externalaccount/x509_provider.go index 4ada6d7577b1..af8cb477c9f4 100644 --- a/auth/credentials/internal/externalaccount/x509_provider.go +++ b/auth/credentials/internal/externalaccount/x509_provider.go @@ -24,8 +24,6 @@ import ( ) type x509Provider struct { - certificateConfigLocation string - cachedClient *http.Client } func (xp *x509Provider) providerType() string { @@ -36,25 +34,22 @@ func (xp *x509Provider) subjectToken(ctx context.Context) (string, error) { return "", nil } -func (xp *x509Provider) client() (*http.Client, error) { - // Create client if it doesn't already exist - if xp.cachedClient == nil { - certProvider, err := cert.NewWorkloadX509CertProvider(xp.certificateConfigLocation) - if err != nil { - return nil, err - } - - trans := http.DefaultTransport.(*http.Transport).Clone() - - trans.TLSClientConfig = &tls.Config{ - GetClientCertificate: certProvider, - } - - // Create client with default settings plus the X509 workload certs - xp.cachedClient = &http.Client{ - Transport: trans, - Timeout: 30 * time.Second, - } +func createX509Client(certificateConfigLocation string) (*http.Client, error) { + certProvider, err := cert.NewWorkloadX509CertProvider(certificateConfigLocation) + if err != nil { + return nil, err } - return xp.cachedClient, nil + trans := http.DefaultTransport.(*http.Transport).Clone() + + trans.TLSClientConfig = &tls.Config{ + GetClientCertificate: certProvider, + } + + // Create client with default settings plus the X509 workload certs + client := &http.Client{ + Transport: trans, + Timeout: 30 * time.Second, + } + + return client, nil } diff --git a/auth/credentials/internal/externalaccount/x509_provider_test.go b/auth/credentials/internal/externalaccount/x509_provider_test.go index 175d3b07606d..8ae45492bdac 100644 --- a/auth/credentials/internal/externalaccount/x509_provider_test.go +++ b/auth/credentials/internal/externalaccount/x509_provider_test.go @@ -100,21 +100,9 @@ func TestRetrieveSubjectToken_X509(t *testing.T) { } func TestClient_Success(t *testing.T) { - opts := cloneTestOpts() - opts.CredentialSource = &credsfile.CredentialSource{ - Certificate: &credsfile.CertificateConfig{ - CertificateConfigLocation: "testdata/certificate_config_workload.json", - }, - } - - base, err := newSubjectTokenProvider(opts) - if err != nil { - t.Fatalf("newSubjectTokenProvider(): %v", err) - } - - client, err := base.(*x509Provider).client() + client, err := createX509Client("testdata/certificate_config_workload.json") if err != nil { - t.Fatalf("client(): %v", err) + t.Fatalf("createX509Client(): %v", err) } if client == nil { @@ -123,19 +111,7 @@ func TestClient_Success(t *testing.T) { } func TestGetClient_error(t *testing.T) { - opts := cloneTestOpts() - opts.CredentialSource = &credsfile.CredentialSource{ - Certificate: &credsfile.CertificateConfig{ - CertificateConfigLocation: "testdata/bad_file.json", - }, - } - - base, err := newSubjectTokenProvider(opts) - if err != nil { - t.Fatalf("newSubjectTokenProvider(): %v", err) - } - - if _, err = base.(*x509Provider).client(); err == nil { + if _, err := createX509Client("testdata/bad_file.json"); err == nil { t.Errorf("got nil, want an error") } } From ec03913f4ba06e940e4d5acaf13376019d35660e Mon Sep 17 00:00:00 2001 From: aeitzman Date: Thu, 27 Jun 2024 10:29:03 -0700 Subject: [PATCH 6/9] fix: added more comments --- .../internal/externalaccount/externalaccount.go | 2 ++ .../internal/externalaccount/x509_provider.go | 10 +++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/auth/credentials/internal/externalaccount/externalaccount.go b/auth/credentials/internal/externalaccount/externalaccount.go index 555ce5d1fab1..a73d422fd5dc 100644 --- a/auth/credentials/internal/externalaccount/externalaccount.go +++ b/auth/credentials/internal/externalaccount/externalaccount.go @@ -195,6 +195,8 @@ func (o *Options) client() (*http.Client, error) { return o.Client, nil } + // If the client was a default client, and a certificate source is present, validate and + // use that certificate source to create a new mTLS client. cert := o.CredentialSource.Certificate if !cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation == "" { return nil, errors.New("credentials: \"certificate\" object must either specify a certificate_config_location or use_default_certificate_config should be true") diff --git a/auth/credentials/internal/externalaccount/x509_provider.go b/auth/credentials/internal/externalaccount/x509_provider.go index af8cb477c9f4..115df5881f12 100644 --- a/auth/credentials/internal/externalaccount/x509_provider.go +++ b/auth/credentials/internal/externalaccount/x509_provider.go @@ -23,6 +23,12 @@ import ( "cloud.google.com/go/auth/internal/transport/cert" ) +// x509Provider implements the subjectTokenProvider type for +// x509 workload identity credentials. Because x509 credentials +// rely on an mTLS connection to represent the 3rd party identity +// rather than a subject token, this provider will always return +// an empty string when a subject token is requested by the external account +// token provider. type x509Provider struct { } @@ -34,6 +40,8 @@ func (xp *x509Provider) subjectToken(ctx context.Context) (string, error) { return "", nil } +// createX509Client creates a new client that is configured with mTLS, using the +// certificate configuration specified in the credential source. func createX509Client(certificateConfigLocation string) (*http.Client, error) { certProvider, err := cert.NewWorkloadX509CertProvider(certificateConfigLocation) if err != nil { @@ -45,7 +53,7 @@ func createX509Client(certificateConfigLocation string) (*http.Client, error) { GetClientCertificate: certProvider, } - // Create client with default settings plus the X509 workload certs + // Create a client with default settings plus the X509 workload cert and key. client := &http.Client{ Transport: trans, Timeout: 30 * time.Second, From 4c051b3ea3373e83261b4c276a68af7548b49150 Mon Sep 17 00:00:00 2001 From: aeitzman Date: Mon, 1 Jul 2024 09:36:43 -0700 Subject: [PATCH 7/9] Add cred source validation back in in the constructor --- .../internal/externalaccount/externalaccount.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/auth/credentials/internal/externalaccount/externalaccount.go b/auth/credentials/internal/externalaccount/externalaccount.go index a73d422fd5dc..8bb6207180af 100644 --- a/auth/credentials/internal/externalaccount/externalaccount.go +++ b/auth/credentials/internal/externalaccount/externalaccount.go @@ -387,6 +387,13 @@ func newSubjectTokenProvider(o *Options) (subjectTokenProvider, error) { execProvider.env = runtimeEnvironment{} return execProvider, nil } else if o.CredentialSource.Certificate != nil { + cert := o.CredentialSource.Certificate + if !cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation == "" { + return nil, errors.New("credentials: \"certificate\" object must either specify a certificate_config_location or use_default_certificate_config should be true") + } + if cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation != "" { + return nil, errors.New("credentials: \"certificate\" object cannot specify both a certificate_config_location and use_default_certificate_config=true") + } return &x509Provider{}, nil } return nil, errors.New("credentials: unable to parse credential source") From d360a46cd974fa3a0852a2cefefe38a9b147120a Mon Sep 17 00:00:00 2001 From: Alex Eitzman Date: Wed, 7 Aug 2024 11:32:43 -0700 Subject: [PATCH 8/9] Fix typo and consider default client from api lib changes --- .../internal/externalaccount/externalaccount.go | 8 +++----- auth/internal/credsfile/filetype.go | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/auth/credentials/internal/externalaccount/externalaccount.go b/auth/credentials/internal/externalaccount/externalaccount.go index 8bb6207180af..112186a9e6ef 100644 --- a/auth/credentials/internal/externalaccount/externalaccount.go +++ b/auth/credentials/internal/externalaccount/externalaccount.go @@ -189,14 +189,12 @@ func (o *Options) validate() error { // is provided, then the client configured in the options will always be returned. If a default client // is provided and the options are configured for X509 credentials, a new client will be created. func (o *Options) client() (*http.Client, error) { - // If a client was explicitly provided or if there is no certificate configuration, return the client - // from the options. - if !o.IsDefaultClient || o.CredentialSource == nil || o.CredentialSource.Certificate == nil { + // If a client was provided and no override certificate config location was provided, use the provided client. + if o.CredentialSource == nil || o.CredentialSource.Certificate == nil || (!o.IsDefaultClient && o.CredentialSource.Certificate.CertificateConfigLocation == "") { return o.Client, nil } - // If the client was a default client, and a certificate source is present, validate and - // use that certificate source to create a new mTLS client. + // If a new client should be created, validate and use the certificate source to create a new mTLS client. cert := o.CredentialSource.Certificate if !cert.UseDefaultCertificateConfig && cert.CertificateConfigLocation == "" { return nil, errors.New("credentials: \"certificate\" object must either specify a certificate_config_location or use_default_certificate_config should be true") diff --git a/auth/internal/credsfile/filetype.go b/auth/internal/credsfile/filetype.go index 52670f4b2a06..3be6e5bbb418 100644 --- a/auth/internal/credsfile/filetype.go +++ b/auth/internal/credsfile/filetype.go @@ -122,7 +122,7 @@ type ExecutableConfig struct { OutputFile string `json:"output_file"` } -// CertificateConfig reqpresents the options used to set up X509 based workload +// CertificateConfig represents the options used to set up X509 based workload // [CredentialSource] type CertificateConfig struct { UseDefaultCertificateConfig bool `json:"use_default_certificate_config"` From f38ccda45bf3e79031e5a220d69fdf6a953ae6f0 Mon Sep 17 00:00:00 2001 From: Alex Eitzman Date: Wed, 7 Aug 2024 12:13:13 -0700 Subject: [PATCH 9/9] fix unit test --- .../externalaccount/externalaccount_test.go | 53 ++++++++++++++++--- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/auth/credentials/internal/externalaccount/externalaccount_test.go b/auth/credentials/internal/externalaccount/externalaccount_test.go index 2f0386c09b8a..7ef664bf3cff 100644 --- a/auth/credentials/internal/externalaccount/externalaccount_test.go +++ b/auth/credentials/internal/externalaccount/externalaccount_test.go @@ -466,12 +466,17 @@ func TestOptionsValidate(t *testing.T) { } func TestClient(t *testing.T) { - goodCertConfig := credsfile.CertificateConfig{ + goodCertConfigFileLocation := credsfile.CertificateConfig{ CertificateConfigLocation: "testdata/certificate_config_workload.json", } + goodCertConfigEnvLocation := credsfile.CertificateConfig{ + UseDefaultCertificateConfig: true, + } badCertConfig := credsfile.CertificateConfig{ CertificateConfigLocation: "bad_file.json", } + t.Setenv("GOOGLE_API_CERTIFICATE_CONFIG", "testdata/certificate_config_workload.json") + client := internal.CloneDefaultClient() tests := []struct { @@ -514,7 +519,39 @@ func TestClient(t *testing.T) { wantClientChanged: false, }, { - name: "isDefault false with certificate config", + name: "isDefault false with override certificate config", + o: &Options{ + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: false, + CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfigFileLocation}, + }, + wantErr: false, + wantClientChanged: true, + }, + { + name: "isDefault true with override certificate config", + o: &Options{ + SubjectTokenType: jwtTokenType, + TokenURL: "http://localhost:8080/v1/token", + TokenInfoURL: "http://localhost:8080/v1/tokeninfo", + ServiceAccountImpersonationURL: "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/service-gcs-admin@$PROJECT_ID.iam.gserviceaccount.com:generateAccessToken", + ClientSecret: "notsosecret", + ClientID: "rbrgnognrhongo3bi4gb9ghg9g", + Client: client, + IsDefaultClient: true, + CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfigFileLocation}, + }, + wantErr: false, + wantClientChanged: true, + }, + { + name: "isDefault false with default certificate config", o: &Options{ SubjectTokenType: jwtTokenType, TokenURL: "http://localhost:8080/v1/token", @@ -524,13 +561,13 @@ func TestClient(t *testing.T) { ClientID: "rbrgnognrhongo3bi4gb9ghg9g", Client: client, IsDefaultClient: false, - CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfig}, + CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfigEnvLocation}, }, wantErr: false, wantClientChanged: false, }, { - name: "isDefault true with certificate config", + name: "isDefault true with default certificate config", o: &Options{ SubjectTokenType: jwtTokenType, TokenURL: "http://localhost:8080/v1/token", @@ -540,7 +577,7 @@ func TestClient(t *testing.T) { ClientID: "rbrgnognrhongo3bi4gb9ghg9g", Client: client, IsDefaultClient: true, - CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfig}, + CredentialSource: &credsfile.CredentialSource{Certificate: &goodCertConfigEnvLocation}, }, wantErr: false, wantClientChanged: true, @@ -558,8 +595,8 @@ func TestClient(t *testing.T) { IsDefaultClient: false, CredentialSource: &credsfile.CredentialSource{Certificate: &badCertConfig}, }, - wantErr: false, - wantClientChanged: false, + wantErr: true, + wantClientChanged: true, }, { name: "isDefault true with bad certificate config", @@ -584,7 +621,7 @@ func TestClient(t *testing.T) { t.Fatalf("o.validate() = nil, want error") } if err != nil && !tc.wantErr { - t.Fatalf("o.validate() = non-nil error, want error") + t.Fatalf(err.Error()) } if tc.wantClientChanged {