From 1ba133a6b25eb896f2cf9983f56103fa0b36ba21 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Wed, 16 Jun 2021 15:18:37 -0700 Subject: [PATCH 1/7] internal: [AIP-4111] support scopes for self-signed JWT auth flow --- go.mod | 2 +- go.sum | 3 +- internal/creds.go | 70 +++++++++++++++---------- internal/creds_test.go | 53 +++++++++++++++++-- internal/settings.go | 9 ++++ option/internaloption/internaloption.go | 12 +++++ 6 files changed, 115 insertions(+), 34 deletions(-) diff --git a/go.mod b/go.mod index a9a62d3a204..340c4187037 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // TODO(codyoss): unfreeze after min version of 1.14 golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420 - golang.org/x/oauth2 v0.0.0-20210514164344-f6687ab2804c + golang.org/x/oauth2 v0.0.0-20210615190721-d04028783cf1 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c golang.org/x/sys v0.0.0-20210611083646-a4fc73990273 golang.org/x/tools v0.1.3 diff --git a/go.sum b/go.sum index 591a022799f..51ab019f590 100644 --- a/go.sum +++ b/go.sum @@ -247,8 +247,9 @@ golang.org/x/oauth2 v0.0.0-20201208152858-08078c50e5b5/go.mod h1:KelEdhl1UZF7XfJ golang.org/x/oauth2 v0.0.0-20210218202405-ba52d332ba99/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.0.0-20210220000619-9bb904979d93/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.0.0-20210313182246-cd4f82c27b84/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= -golang.org/x/oauth2 v0.0.0-20210514164344-f6687ab2804c h1:pkQiBZBvdos9qq4wBAHqlzuZHEXo07pqV06ef90u1WI= golang.org/x/oauth2 v0.0.0-20210514164344-f6687ab2804c/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= +golang.org/x/oauth2 v0.0.0-20210615190721-d04028783cf1 h1:x622Z2o4hgCr/4CiKWc51jHVKaWdtVpBNmEI8wI9Qns= +golang.org/x/oauth2 v0.0.0-20210615190721-d04028783cf1/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= diff --git a/internal/creds.go b/internal/creds.go index 1f635e430cc..2d9deee85bc 100644 --- a/internal/creds.go +++ b/internal/creds.go @@ -7,6 +7,7 @@ package internal import ( "context" "encoding/json" + "errors" "fmt" "io/ioutil" @@ -64,36 +65,30 @@ const ( // credentialsFromJSON returns a google.Credentials based on the input. // -// - A self-signed JWT auth flow will be executed if: the data file is a service -// account, no user are scopes provided, an audience is provided, a user -// specified endpoint is not provided, and credentials will not be -// impersonated. +// - A standard OAuth 2.0 flow will be executed if at least one of the +// following conditions is met: +// (1) the scope is non-empty and the scope for self-signed JWT flow is +// disabled. +// (2) Service Account Impersontation // -// - Otherwise, executes a stanard OAuth 2.0 flow. +// - Otherwise, executes a self-signed JWT flow (google.aip.dev/auth/4111) func credentialsFromJSON(ctx context.Context, data []byte, ds *DialSettings) (*google.Credentials, error) { cred, err := google.CredentialsFromJSON(ctx, data, ds.GetScopes()...) if err != nil { return nil, err } - // Standard OAuth 2.0 Flow - if len(data) == 0 || - len(ds.Scopes) > 0 || - (ds.DefaultAudience == "" && len(ds.Audiences) == 0) || - ds.ImpersonationConfig != nil || - ds.Endpoint != "" { + if isOAuthFlow(data, ds) { + // Standard OAuth 2.0 Flow return cred, nil } - // Check if JSON is a service account and if so create a self-signed JWT. - var f struct { - Type string `json:"type"` - // The rest JSON fields are omitted because they are not used. - } - if err := json.Unmarshal(cred.JSON, &f); err != nil { + isJWTFlow, err := isSelfSignedJWTFlow(cred.JSON) + if err != nil { return nil, err } - if f.Type == serviceAccountKey { - ts, err := selfSignedJWTTokenSource(data, ds.DefaultAudience, ds.Audiences) + + if isJWTFlow { + ts, err := selfSignedJWTTokenSource(data, ds.GetAudience(), ds.GetScopes()) if err != nil { return nil, err } @@ -102,16 +97,35 @@ func credentialsFromJSON(ctx context.Context, data []byte, ds *DialSettings) (*g return cred, err } -func selfSignedJWTTokenSource(data []byte, defaultAudience string, audiences []string) (oauth2.TokenSource, error) { - audience := defaultAudience - if len(audiences) > 0 { - // TODO(shinfan): Update golang oauth to support multiple audiences. - if len(audiences) > 1 { - return nil, fmt.Errorf("multiple audiences support is not implemented") - } - audience = audiences[0] +func isOAuthFlow(data []byte, ds *DialSettings) bool { + // Standard OAuth 2.0 Flow + return len(data) == 0 || + (len(ds.GetScopes()) > 0 && !ds.EnableScopeForJWT) || + ds.ImpersonationConfig != nil +} + +func isSelfSignedJWTFlow(data []byte) (bool, error) { + // Check if JSON is a service account and if so create a self-signed JWT. + var f struct { + Type string `json:"type"` + // The rest JSON fields are omitted because they are not used. + } + if err := json.Unmarshal(data, &f); err != nil { + return false, err + } + return f.Type == serviceAccountKey, nil +} + +func selfSignedJWTTokenSource(data []byte, audience string, scopes []string) (oauth2.TokenSource, error) { + if len(scopes) > 0 { + // Scopes are preferred in self-signed JWT + return google.JWTAccessTokenSourceWithScope(data, scopes...) + } else if audience != "" { + // Fallback to audience if scope is not provided + return google.JWTAccessTokenSourceFromJSON(data, audience) + } else { + return nil, errors.New("Neither scopes or audience are provided for the self-signed JWT.") } - return google.JWTAccessTokenSourceFromJSON(data, audience) } // QuotaProjectFromCreds returns the quota project from the JSON blob in the provided credentials. diff --git a/internal/creds_test.go b/internal/creds_test.go index 045c5bc20a6..be0273c3d17 100644 --- a/internal/creds_test.go +++ b/internal/creds_test.go @@ -38,6 +38,7 @@ func TestTokenSource(t *testing.T) { ds = &DialSettings{ TokenSource: ts, CredentialsFile: "testdata/service-account.json", + DefaultScopes: []string{"foo"}, } got, err = Creds(ctx, ds) if err != nil { @@ -54,14 +55,20 @@ func TestDefaultServiceAccount(t *testing.T) { // Load a valid JSON file. No way to really test the contents; we just // verify that there is no error. - ds := &DialSettings{CredentialsFile: "testdata/service-account.json"} + ds := &DialSettings{ + CredentialsFile: "testdata/service-account.json", + DefaultScopes: []string{"foo"}, + } if _, err := Creds(ctx, ds); err != nil { t.Errorf("got %v, wanted no error", err) } // Load valid JSON. No way to really test the contents; we just // verify that there is no error. - ds = &DialSettings{CredentialsJSON: []byte(validServiceAccountJSON)} + ds = &DialSettings{ + CredentialsJSON: []byte(validServiceAccountJSON), + DefaultScopes: []string{"foo"}, + } if _, err := Creds(ctx, ds); err != nil { t.Errorf("got %v, wanted no error", err) } @@ -85,6 +92,32 @@ func TestJWTWithAudience(t *testing.T) { } } +func TestJWTWithScope(t *testing.T) { + ctx := context.Background() + + // Load a valid JSON file. No way to really test the contents; we just + // verify that there is no error. + ds := &DialSettings{ + CredentialsFile: "testdata/service-account.json", + Scopes: []string{"foo"}, + EnableScopeForJWT: true, + } + if _, err := Creds(ctx, ds); err != nil { + t.Errorf("got %v, wanted no error", err) + } + + // Load valid JSON. No way to really test the contents; we just + // verify that there is no error. + ds = &DialSettings{ + CredentialsJSON: []byte(validServiceAccountJSON), + Scopes: []string{"foo"}, + EnableScopeForJWT: true, + } + if _, err := Creds(ctx, ds); err != nil { + t.Errorf("got %v, wanted no error", err) + } +} + func TestOAuth(t *testing.T) { ctx := context.Background() @@ -119,7 +152,13 @@ const validServiceAccountJSON = `{ func TestQuotaProjectFromCreds(t *testing.T) { ctx := context.Background() - cred, err := credentialsFromJSON(ctx, []byte(validServiceAccountJSON), &DialSettings{Endpoint: "foo.googleapis.com"}) + cred, err := credentialsFromJSON( + ctx, + []byte(validServiceAccountJSON), + &DialSettings{ + Endpoint: "foo.googleapis.com", + DefaultScopes: []string{"foo"}, + }) if err != nil { t.Fatalf("got %v, wanted no error", err) } @@ -133,7 +172,13 @@ func TestQuotaProjectFromCreds(t *testing.T) { "quota_project_id": "foobar" }`) - cred, err = credentialsFromJSON(ctx, []byte(quotaProjectJSON), &DialSettings{Endpoint: "foo.googleapis.com"}) + cred, err = credentialsFromJSON( + ctx, + []byte(quotaProjectJSON), + &DialSettings{ + Endpoint: "foo.googleapis.com", + DefaultScopes: []string{"foo"}, + }) if err != nil { t.Fatalf("got %v, wanted no error", err) } diff --git a/internal/settings.go b/internal/settings.go index 0ae1cb9778d..9ca5e6b2a67 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -24,6 +24,7 @@ type DialSettings struct { DefaultMTLSEndpoint string Scopes []string DefaultScopes []string + EnableScopeForJWT bool TokenSource oauth2.TokenSource Credentials *google.Credentials CredentialsFile string // if set, Token Source is ignored. @@ -60,6 +61,14 @@ func (ds *DialSettings) GetScopes() []string { return ds.DefaultScopes } +// GetAudience returns the user-provided audience, if set, or else falls back to the default audience. +func (ds *DialSettings) GetAudience() string { + if len(ds.Audiences) > 0 { + return ds.Audiences[0] + } + return ds.DefaultAudience +} + // Validate reports an error if ds is invalid. func (ds *DialSettings) Validate() error { if ds.SkipValidation { diff --git a/option/internaloption/internaloption.go b/option/internaloption/internaloption.go index 1fff22fd5da..aa97e714425 100644 --- a/option/internaloption/internaloption.go +++ b/option/internaloption/internaloption.go @@ -94,3 +94,15 @@ func (w withDefaultScopes) Apply(o *internal.DialSettings) { o.DefaultScopes = make([]string, len(w)) copy(o.DefaultScopes, w) } + +// EnableScopeForJWT returns a ClientOption that specifies if scope can be used +// with self-signed JWT. +func EnableScopeForJWT(scopeForJWT bool) ClientOption { + return enableScopeForJWT(audience) +} + +type enableScopeForJWT bool + +func (w enableScopeForJWT) Apply(o *internal.DialSettings) { + o.EnableScopeForJWT = w +} From 96893f97dafddc452b94c60b41d7f543800d2736 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Wed, 16 Jun 2021 21:46:44 -0700 Subject: [PATCH 2/7] Fix error format --- internal/creds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/creds.go b/internal/creds.go index 2d9deee85bc..a70eeba7755 100644 --- a/internal/creds.go +++ b/internal/creds.go @@ -124,7 +124,7 @@ func selfSignedJWTTokenSource(data []byte, audience string, scopes []string) (oa // Fallback to audience if scope is not provided return google.JWTAccessTokenSourceFromJSON(data, audience) } else { - return nil, errors.New("Neither scopes or audience are provided for the self-signed JWT.") + return nil, errors.New("neither scopes or audience are provided for the self-signed JWT") } } From 72acd2e08848bea89cbfc3e4edbf52d129795432 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Wed, 16 Jun 2021 22:07:47 -0700 Subject: [PATCH 3/7] Address comment --- internal/creds.go | 52 ++++++++++++------------- internal/creds_test.go | 12 +++--- internal/settings.go | 2 +- option/internaloption/internaloption.go | 12 +++--- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/internal/creds.go b/internal/creds.go index a70eeba7755..87cef98cc36 100644 --- a/internal/creds.go +++ b/internal/creds.go @@ -65,28 +65,26 @@ const ( // credentialsFromJSON returns a google.Credentials based on the input. // -// - A standard OAuth 2.0 flow will be executed if at least one of the -// following conditions is met: -// (1) the scope is non-empty and the scope for self-signed JWT flow is -// disabled. -// (2) Service Account Impersontation +// - A self-signed JWT flow will be executed if the following conditions are +// met: +// (1) Either the scope for self-signed JWT flow is enabled or audiences are +// explicitly provided by users. +// (2) No service account impersontation // -// - Otherwise, executes a self-signed JWT flow (google.aip.dev/auth/4111) +// - Otherwise, executes standard OAuth 2.0 flow +// More details: google.aip.dev/auth/4111 func credentialsFromJSON(ctx context.Context, data []byte, ds *DialSettings) (*google.Credentials, error) { + // By default, a standard OAuth 2.0 token source is created cred, err := google.CredentialsFromJSON(ctx, data, ds.GetScopes()...) if err != nil { return nil, err } - if isOAuthFlow(data, ds) { - // Standard OAuth 2.0 Flow - return cred, nil - } - isJWTFlow, err := isSelfSignedJWTFlow(cred.JSON) + // Override the token source to use self-signed JWT if conditions are met + isJWTFlow, err := isSelfSignedJWTFlow(cred.JSON, ds) if err != nil { return nil, err } - if isJWTFlow { ts, err := selfSignedJWTTokenSource(data, ds.GetAudience(), ds.GetScopes()) if err != nil { @@ -94,26 +92,24 @@ func credentialsFromJSON(ctx context.Context, data []byte, ds *DialSettings) (*g } cred.TokenSource = ts } - return cred, err -} -func isOAuthFlow(data []byte, ds *DialSettings) bool { - // Standard OAuth 2.0 Flow - return len(data) == 0 || - (len(ds.GetScopes()) > 0 && !ds.EnableScopeForJWT) || - ds.ImpersonationConfig != nil + return cred, err } -func isSelfSignedJWTFlow(data []byte) (bool, error) { - // Check if JSON is a service account and if so create a self-signed JWT. - var f struct { - Type string `json:"type"` - // The rest JSON fields are omitted because they are not used. - } - if err := json.Unmarshal(data, &f); err != nil { - return false, err +func isSelfSignedJWTFlow(data []byte, ds *DialSettings) (bool, error) { + if (ds.UseJwtWithScope || len(ds.Audiences) > 0) && + ds.ImpersonationConfig == nil { + // Check if JSON is a service account and if so create a self-signed JWT. + var f struct { + Type string `json:"type"` + // The rest JSON fields are omitted because they are not used. + } + if err := json.Unmarshal(data, &f); err != nil { + return false, err + } + return f.Type == serviceAccountKey, nil } - return f.Type == serviceAccountKey, nil + return false, nil } func selfSignedJWTTokenSource(data []byte, audience string, scopes []string) (oauth2.TokenSource, error) { diff --git a/internal/creds_test.go b/internal/creds_test.go index be0273c3d17..2598254775c 100644 --- a/internal/creds_test.go +++ b/internal/creds_test.go @@ -98,9 +98,9 @@ func TestJWTWithScope(t *testing.T) { // Load a valid JSON file. No way to really test the contents; we just // verify that there is no error. ds := &DialSettings{ - CredentialsFile: "testdata/service-account.json", - Scopes: []string{"foo"}, - EnableScopeForJWT: true, + CredentialsFile: "testdata/service-account.json", + Scopes: []string{"foo"}, + UseJwtWithScope: true, } if _, err := Creds(ctx, ds); err != nil { t.Errorf("got %v, wanted no error", err) @@ -109,9 +109,9 @@ func TestJWTWithScope(t *testing.T) { // Load valid JSON. No way to really test the contents; we just // verify that there is no error. ds = &DialSettings{ - CredentialsJSON: []byte(validServiceAccountJSON), - Scopes: []string{"foo"}, - EnableScopeForJWT: true, + CredentialsJSON: []byte(validServiceAccountJSON), + Scopes: []string{"foo"}, + UseJwtWithScope: true, } if _, err := Creds(ctx, ds); err != nil { t.Errorf("got %v, wanted no error", err) diff --git a/internal/settings.go b/internal/settings.go index 9ca5e6b2a67..ad0dd635a27 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -24,7 +24,7 @@ type DialSettings struct { DefaultMTLSEndpoint string Scopes []string DefaultScopes []string - EnableScopeForJWT bool + UseJwtWithScope bool TokenSource oauth2.TokenSource Credentials *google.Credentials CredentialsFile string // if set, Token Source is ignored. diff --git a/option/internaloption/internaloption.go b/option/internaloption/internaloption.go index aa97e714425..22c6f8dcc32 100644 --- a/option/internaloption/internaloption.go +++ b/option/internaloption/internaloption.go @@ -95,14 +95,14 @@ func (w withDefaultScopes) Apply(o *internal.DialSettings) { copy(o.DefaultScopes, w) } -// EnableScopeForJWT returns a ClientOption that specifies if scope can be used +// UseJwtWithScope returns a ClientOption that specifies if scope can be used // with self-signed JWT. -func EnableScopeForJWT(scopeForJWT bool) ClientOption { - return enableScopeForJWT(audience) +func UseJwtWithScope(useScope bool) option.ClientOption { + return useJwtWithScope(useScope) } -type enableScopeForJWT bool +type useJwtWithScope bool -func (w enableScopeForJWT) Apply(o *internal.DialSettings) { - o.EnableScopeForJWT = w +func (w useJwtWithScope) Apply(o *internal.DialSettings) { + o.UseJwtWithScope = bool(w) } From 23081f69970ed30dc0022245f9198f3422a25445 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Thu, 17 Jun 2021 13:08:38 -0700 Subject: [PATCH 4/7] Address comments --- internal/creds.go | 19 ++++++++++--------- internal/creds_test.go | 12 ++++++------ internal/settings.go | 9 +++++++-- option/internaloption/internaloption.go | 12 ++++++------ 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/internal/creds.go b/internal/creds.go index 87cef98cc36..ebb18f3281f 100644 --- a/internal/creds.go +++ b/internal/creds.go @@ -81,12 +81,12 @@ func credentialsFromJSON(ctx context.Context, data []byte, ds *DialSettings) (*g } // Override the token source to use self-signed JWT if conditions are met - isJWTFlow, err := isSelfSignedJWTFlow(cred.JSON, ds) + isJWTFlow, err := isSelfSignedJWTFlow(data, ds) if err != nil { return nil, err } if isJWTFlow { - ts, err := selfSignedJWTTokenSource(data, ds.GetAudience(), ds.GetScopes()) + ts, err := selfSignedJWTTokenSource(data, ds) if err != nil { return nil, err } @@ -97,7 +97,7 @@ func credentialsFromJSON(ctx context.Context, data []byte, ds *DialSettings) (*g } func isSelfSignedJWTFlow(data []byte, ds *DialSettings) (bool, error) { - if (ds.UseJwtWithScope || len(ds.Audiences) > 0) && + if (ds.EnableJwtWithScope || ds.HasCustomAudience()) && ds.ImpersonationConfig == nil { // Check if JSON is a service account and if so create a self-signed JWT. var f struct { @@ -112,13 +112,14 @@ func isSelfSignedJWTFlow(data []byte, ds *DialSettings) (bool, error) { return false, nil } -func selfSignedJWTTokenSource(data []byte, audience string, scopes []string) (oauth2.TokenSource, error) { - if len(scopes) > 0 { - // Scopes are preferred in self-signed JWT - return google.JWTAccessTokenSourceWithScope(data, scopes...) - } else if audience != "" { +func selfSignedJWTTokenSource(data []byte, ds *DialSettings) (oauth2.TokenSource, error) { + if len(ds.GetScopes()) > 0 && !ds.HasCustomAudience() { + // Scopes are preferred in self-signed JWT unless the scope is not available + // or a custom audience is used. + return google.JWTAccessTokenSourceWithScope(data, ds.GetScopes()...) + } else if ds.GetAudience() != "" { // Fallback to audience if scope is not provided - return google.JWTAccessTokenSourceFromJSON(data, audience) + return google.JWTAccessTokenSourceFromJSON(data, ds.GetAudience()) } else { return nil, errors.New("neither scopes or audience are provided for the self-signed JWT") } diff --git a/internal/creds_test.go b/internal/creds_test.go index 2598254775c..39fe9d11b85 100644 --- a/internal/creds_test.go +++ b/internal/creds_test.go @@ -98,9 +98,9 @@ func TestJWTWithScope(t *testing.T) { // Load a valid JSON file. No way to really test the contents; we just // verify that there is no error. ds := &DialSettings{ - CredentialsFile: "testdata/service-account.json", - Scopes: []string{"foo"}, - UseJwtWithScope: true, + CredentialsFile: "testdata/service-account.json", + Scopes: []string{"foo"}, + EnableJwtWithScope: true, } if _, err := Creds(ctx, ds); err != nil { t.Errorf("got %v, wanted no error", err) @@ -109,9 +109,9 @@ func TestJWTWithScope(t *testing.T) { // Load valid JSON. No way to really test the contents; we just // verify that there is no error. ds = &DialSettings{ - CredentialsJSON: []byte(validServiceAccountJSON), - Scopes: []string{"foo"}, - UseJwtWithScope: true, + CredentialsJSON: []byte(validServiceAccountJSON), + Scopes: []string{"foo"}, + EnableJwtWithScope: true, } if _, err := Creds(ctx, ds); err != nil { t.Errorf("got %v, wanted no error", err) diff --git a/internal/settings.go b/internal/settings.go index ad0dd635a27..89c7bc86fa3 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -24,7 +24,7 @@ type DialSettings struct { DefaultMTLSEndpoint string Scopes []string DefaultScopes []string - UseJwtWithScope bool + EnableJwtWithScope bool TokenSource oauth2.TokenSource Credentials *google.Credentials CredentialsFile string // if set, Token Source is ignored. @@ -63,12 +63,17 @@ func (ds *DialSettings) GetScopes() []string { // GetAudience returns the user-provided audience, if set, or else falls back to the default audience. func (ds *DialSettings) GetAudience() string { - if len(ds.Audiences) > 0 { + if ds.HasCustomAudience() { return ds.Audiences[0] } return ds.DefaultAudience } +// HasCustomAudience returns true if a custom audience is provided by users. +func (ds *DialSettings) HasCustomAudience() bool { + return len(ds.Audiences) > 0 +} + // Validate reports an error if ds is invalid. func (ds *DialSettings) Validate() error { if ds.SkipValidation { diff --git a/option/internaloption/internaloption.go b/option/internaloption/internaloption.go index 22c6f8dcc32..ab81a56a738 100644 --- a/option/internaloption/internaloption.go +++ b/option/internaloption/internaloption.go @@ -95,14 +95,14 @@ func (w withDefaultScopes) Apply(o *internal.DialSettings) { copy(o.DefaultScopes, w) } -// UseJwtWithScope returns a ClientOption that specifies if scope can be used +// EnableJwtWithScope returns a ClientOption that specifies if scope can be used // with self-signed JWT. -func UseJwtWithScope(useScope bool) option.ClientOption { - return useJwtWithScope(useScope) +func EnableJwtWithScope(enableScope bool) option.ClientOption { + return enableJwtWithScope(enableScope) } -type useJwtWithScope bool +type enableJwtWithScope bool -func (w useJwtWithScope) Apply(o *internal.DialSettings) { - o.UseJwtWithScope = bool(w) +func (w enableJwtWithScope) Apply(o *internal.DialSettings) { + o.EnableJwtWithScope = bool(w) } From 2052be61a9a384698217da3f3c587cdf061c0667 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Thu, 17 Jun 2021 14:29:29 -0700 Subject: [PATCH 5/7] Address more comments --- internal/creds.go | 10 +++++---- internal/creds_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/internal/creds.go b/internal/creds.go index ebb18f3281f..b927fae5c57 100644 --- a/internal/creds.go +++ b/internal/creds.go @@ -63,12 +63,14 @@ const ( serviceAccountKey = "service_account" ) -// credentialsFromJSON returns a google.Credentials based on the input. +// credentialsFromJSON returns a google.Credentials from the JSON data // // - A self-signed JWT flow will be executed if the following conditions are // met: -// (1) Either the scope for self-signed JWT flow is enabled or audiences are -// explicitly provided by users. +// (1) At least one of the following is true: +// (a) Scope for self-signed JWT flow is enabled +// (b) Audiences are explicitly provided by users +// (b) No scope is provided // (2) No service account impersontation // // - Otherwise, executes standard OAuth 2.0 flow @@ -97,7 +99,7 @@ func credentialsFromJSON(ctx context.Context, data []byte, ds *DialSettings) (*g } func isSelfSignedJWTFlow(data []byte, ds *DialSettings) (bool, error) { - if (ds.EnableJwtWithScope || ds.HasCustomAudience()) && + if (ds.EnableJwtWithScope || ds.HasCustomAudience() || len(ds.GetScopes()) == 0) && ds.ImpersonationConfig == nil { // Check if JSON is a service account and if so create a self-signed JWT. var f struct { diff --git a/internal/creds_test.go b/internal/creds_test.go index 39fe9d11b85..ecdae563187 100644 --- a/internal/creds_test.go +++ b/internal/creds_test.go @@ -118,6 +118,56 @@ func TestJWTWithScope(t *testing.T) { } } +func TestJWTWithDefaultScopes(t *testing.T) { + ctx := context.Background() + + // Load a valid JSON file. No way to really test the contents; we just + // verify that there is no error. + ds := &DialSettings{ + CredentialsFile: "testdata/service-account.json", + DefaultScopes: []string{"foo"}, + EnableJwtWithScope: true, + } + if _, err := Creds(ctx, ds); err != nil { + t.Errorf("got %v, wanted no error", err) + } + + // Load valid JSON. No way to really test the contents; we just + // verify that there is no error. + ds = &DialSettings{ + CredentialsJSON: []byte(validServiceAccountJSON), + DefaultScopes: []string{"foo"}, + EnableJwtWithScope: true, + } + if _, err := Creds(ctx, ds); err != nil { + t.Errorf("got %v, wanted no error", err) + } +} + +func TestJWTWithDefaultAudience(t *testing.T) { + ctx := context.Background() + + // Load a valid JSON file. No way to really test the contents; we just + // verify that there is no error. + ds := &DialSettings{ + CredentialsFile: "testdata/service-account.json", + DefaultAudience: "foo", + } + if _, err := Creds(ctx, ds); err != nil { + t.Errorf("got %v, wanted no error", err) + } + + // Load valid JSON. No way to really test the contents; we just + // verify that there is no error. + ds = &DialSettings{ + CredentialsJSON: []byte(validServiceAccountJSON), + DefaultAudience: "foo", + } + if _, err := Creds(ctx, ds); err != nil { + t.Errorf("got %v, wanted no error", err) + } +} + func TestOAuth(t *testing.T) { ctx := context.Background() From 7a4986c702d3bc8e7c5a0727499503847b356996 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Mon, 21 Jun 2021 14:58:56 -0700 Subject: [PATCH 6/7] Address comments --- internal/creds.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/creds.go b/internal/creds.go index b927fae5c57..9cb2b74c0ed 100644 --- a/internal/creds.go +++ b/internal/creds.go @@ -68,9 +68,9 @@ const ( // - A self-signed JWT flow will be executed if the following conditions are // met: // (1) At least one of the following is true: -// (a) Scope for self-signed JWT flow is enabled -// (b) Audiences are explicitly provided by users -// (b) No scope is provided +// (a) No scope is provided +// (b) Scope for self-signed JWT flow is enabled +// (c) Audiences are explicitly provided by users // (2) No service account impersontation // // - Otherwise, executes standard OAuth 2.0 flow @@ -123,7 +123,7 @@ func selfSignedJWTTokenSource(data []byte, ds *DialSettings) (oauth2.TokenSource // Fallback to audience if scope is not provided return google.JWTAccessTokenSourceFromJSON(data, ds.GetAudience()) } else { - return nil, errors.New("neither scopes or audience are provided for the self-signed JWT") + return nil, errors.New("neither scopes or audience are available for the self-signed JWT") } } From f036421cc68d99c74b961d9aeb8d102e608f9d66 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Tue, 22 Jun 2021 16:08:44 -0700 Subject: [PATCH 7/7] Update comments --- option/internaloption/internaloption.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/option/internaloption/internaloption.go b/option/internaloption/internaloption.go index ab81a56a738..ed0b7aaf13e 100644 --- a/option/internaloption/internaloption.go +++ b/option/internaloption/internaloption.go @@ -97,8 +97,8 @@ func (w withDefaultScopes) Apply(o *internal.DialSettings) { // EnableJwtWithScope returns a ClientOption that specifies if scope can be used // with self-signed JWT. -func EnableJwtWithScope(enableScope bool) option.ClientOption { - return enableJwtWithScope(enableScope) +func EnableJwtWithScope() option.ClientOption { + return enableJwtWithScope(true) } type enableJwtWithScope bool