Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
shinfan committed Jun 17, 2021
1 parent 72acd2e commit 23081f6
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 23 deletions.
19 changes: 10 additions & 9 deletions internal/creds.go
Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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")
}
Expand Down
12 changes: 6 additions & 6 deletions internal/creds_test.go
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
9 changes: 7 additions & 2 deletions internal/settings.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions option/internaloption/internaloption.go
Expand Up @@ -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)
}

0 comments on commit 23081f6

Please sign in to comment.