From 72acd2e08848bea89cbfc3e4edbf52d129795432 Mon Sep 17 00:00:00 2001 From: Shin Fan Date: Wed, 16 Jun 2021 22:07:47 -0700 Subject: [PATCH] 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) }