auth(fix): remove incorrect validation from impersonate.CredentialsOp…

…tions

* add credentials validation to idtoken.Options

auth/credentials/idtoken/idtoken.go

@@ -51,6 +51,12 @@ const (
 	ComputeTokenFormatFullWithLicense
 )
 
+var (
+	errMissingOpts     = errors.New("idtoken: opts must be provided")
+	errMissingAudience = errors.New("idtoken: Audience must be provided")
+	errBothFileAndJSON = errors.New("idtoken: CredentialsFile and CredentialsJSON must not both be provided")
+)
+
 // Options for the configuration of creation of an ID token with
 // [NewCredentials].
 type Options struct {
@@ -64,14 +70,14 @@ type Options struct {
 	// Optional.
 	CustomClaims map[string]interface{}
 
-	// CredentialsFile overrides detection logic and sources a credential file
-	// from the provided filepath. Optional.
+	// CredentialsFile sources a JSON credential file from the provided
+	// filepath. If provided, do not provide CredentialsJSON. Optional.
 	CredentialsFile string
-	// CredentialsJSON overrides detection logic and uses the JSON bytes as the
-	// source for the credential. Optional.
+	// CredentialsJSON sources a JSON credential file from the provided bytes.
+	// If provided, do not provide CredentialsJSON. Optional.
 	CredentialsJSON []byte
 	// Client configures the underlying client used to make network requests
-	// when fetching tokens. If provided this should be a fully authenticated
+	// when fetching tokens. If provided this should be a fully-authenticated
 	// client. Optional.
 	Client *http.Client
 }
@@ -85,17 +91,24 @@ func (o *Options) client() *http.Client {
 
 func (o *Options) validate() error {
 	if o == nil {
-		return errors.New("idtoken: opts must be provided")
+		return errMissingOpts
 	}
 	if o.Audience == "" {
-		return errors.New("idtoken: audience must be specified")
+		return errMissingAudience
+	}
+	if o.CredentialsFile != "" && len(o.CredentialsJSON) > 0 {
+		return errBothFileAndJSON
 	}
 	return nil
 }
 
-// NewCredentials creates a [cloud.google.com/go/auth.Credentials] that
-// returns ID tokens configured by the opts provided. The parameter
-// opts.Audience may not be empty.
+// NewCredentials creates a [cloud.google.com/go/auth.Credentials] that returns
+// ID tokens configured by the opts provided. The parameter opts.Audience must
+// not be empty. If both opts.CredentialsFile and opts.CredentialsJSON are
+// empty, an attempt will be made to detect credentials from the environment
+// (see [cloud.google.com/go/auth/credentials.DetectDefault]). Only service
+// account, impersonated service account, external account and Compute
+// credentials are supported.
 func NewCredentials(opts *Options) (*auth.Credentials, error) {
 	if err := opts.validate(); err != nil {
 		return nil, err

auth/credentials/idtoken/idtoken_test.go

@@ -27,6 +27,45 @@ import (
 	"cloud.google.com/go/auth/internal/credsfile"
 )
 
+func TestNewCredentials_Validate(t *testing.T) {
+	tests := []struct {
+		name    string
+		opts    *Options
+		wantErr error
+	}{
+		{
+			name:    "missing opts",
+			wantErr: errMissingOpts,
+		},
+		{
+			name:    "missing audience",
+			opts:    &Options{},
+			wantErr: errMissingAudience,
+		},
+		{
+			name: "both credentials",
+			opts: &Options{
+				Audience:        "aud",
+				CredentialsFile: "creds.json",
+				CredentialsJSON: []byte{0, 1},
+			},
+			wantErr: errBothFileAndJSON,
+		},
+	}
+	for _, tt := range tests {
+		name := tt.name
+		t.Run(name, func(t *testing.T) {
+			err := tt.opts.validate()
+			if err == nil {
+				t.Fatalf("error expected: %s", tt.wantErr)
+			}
+			if err != tt.wantErr {
+				t.Errorf("got %v, want %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
 func TestNewCredentials_ServiceAccount(t *testing.T) {
 	wantTok, _ := createRS256JWT(t)
 	b, err := os.ReadFile("../../internal/testdata/sa.json")

auth/credentials/impersonate/idtoken.go

@@ -47,13 +47,13 @@ type IDTokenOptions struct {
 	// chain. Optional.
 	Delegates []string
 
-	// Credentials used to fetch the ID token. If not provided, and a Client is
-	// also not provided, base credentials will try to be detected from the
-	// environment. Optional.
+	// Credentials used in generating the impersonated ID token. If empty, an
+	// attempt will be made to detect credentials from the environment (see
+	// [cloud.google.com/go/auth/credentials.DetectDefault]). Optional.
 	Credentials *auth.Credentials
 	// Client configures the underlying client used to make network requests
-	// when fetching tokens. If provided the client should provide it's own
-	// base credentials at call time. Optional.
+	// when fetching tokens. If provided this should be a fully-authenticated
+	// client. Optional.
 	Client *http.Client
 }
 
@@ -83,32 +83,27 @@ func NewIDTokenCredentials(opts *IDTokenOptions) (*auth.Credentials, error) {
 	if err := opts.validate(); err != nil {
 		return nil, err
 	}
-	var client *http.Client
-	var creds *auth.Credentials
-	if opts.Client == nil && opts.Credentials == nil {
+
+	client := opts.Client
+	creds := opts.Credentials
+	if client == nil {
 		var err error
-		// TODO: test not signed jwt more
-		creds, err = credentials.DetectDefault(&credentials.DetectOptions{
-			Scopes:           []string{defaultScope},
-			UseSelfSignedJWT: true,
-		})
-		if err != nil {
-			return nil, err
+		if creds == nil {
+			// TODO: test not signed jwt more
+			creds, err = credentials.DetectDefault(&credentials.DetectOptions{
+				Scopes:           []string{defaultScope},
+				UseSelfSignedJWT: true,
+			})
+			if err != nil {
+				return nil, err
+			}
 		}
 		client, err = httptransport.NewClient(&httptransport.Options{
 			Credentials: creds,
 		})
 		if err != nil {
 			return nil, err
 		}
-	} else if opts.Client == nil {
-		creds = opts.Credentials
-		client = internal.DefaultClient()
-		if err := httptransport.AddAuthorizationMiddleware(client, opts.Credentials); err != nil {
-			return nil, err
-		}
-	} else {
-		client = opts.Client
 	}
 
 	itp := impersonatedIDTokenProvider{

auth/credentials/impersonate/impersonate.go

@@ -38,7 +38,6 @@ var (
 	errMissingTargetPrincipal                   = errors.New("impersonate: target service account must be provided")
 	errMissingScopes                            = errors.New("impersonate: scopes must be provided")
 	errLifetimeOverMax                          = errors.New("impersonate: max lifetime is 12 hours")
-	errClientAndCredentials                     = errors.New("impersonate: client and credentials must not both be provided")
 	errUniverseNotSupportedDomainWideDelegation = errors.New("impersonate: service account user is configured for the credential. " +
 		"Domain-wide delegation is not supported in universes other than googleapis.com")
 )
@@ -64,20 +63,18 @@ func NewCredentials(opts *CredentialsOptions) (*auth.Credentials, error) {
 		isStaticToken = true
 	}
 
-	var client *http.Client
-	var creds *auth.Credentials
-	if opts.Client == nil {
+	client := opts.Client
+	creds := opts.Credentials
+	if client == nil {
 		var err error
-		if opts.Credentials == nil {
+		if creds == nil {
 			creds, err = credentials.DetectDefault(&credentials.DetectOptions{
 				Scopes:           []string{defaultScope},
 				UseSelfSignedJWT: true,
 			})
 			if err != nil {
 				return nil, err
 			}
-		} else {
-			creds = opts.Credentials
 		}
 		client, err = httptransport.NewClient(&httptransport.Options{
 			Credentials:    creds,
@@ -86,8 +83,6 @@ func NewCredentials(opts *CredentialsOptions) (*auth.Credentials, error) {
 		if err != nil {
 			return nil, err
 		}
-	} else {
-		client = opts.Client
 	}
 
 	universeDomainProvider := resolveUniverseDomainProvider(creds)
@@ -163,18 +158,17 @@ type CredentialsOptions struct {
 	// wide delegation. Optional.
 	Subject string
 
-	// Credentials is the provider of the credentials used to fetch the ID
-	// token. If not provided, and a Client is also not provided, credentials
-	// will try to be detected from the environment. Optional.
+	// Credentials used in generating the impersonated token. If empty, an
+	// attempt will be made to detect credentials from the environment (see
+	// [cloud.google.com/go/auth/credentials.DetectDefault]). Optional.
 	Credentials *auth.Credentials
 	// Client configures the underlying client used to make network requests
-	// when fetching tokens. If provided the client should provide its own
-	// credentials at call time. Optional.
+	// when fetching tokens. If provided this should be a fully-authenticated
+	// client. Optional.
 	Client *http.Client
 	// UniverseDomain is the default service domain for a given Cloud universe.
-	// The default value is "googleapis.com". This is the universe domain
-	// configured for the client, which will be compared to the universe domain
-	// that is separately configured for the credentials. Optional.
+	// This field has no default value, and only if provided will it be used to
+	// verify the universe domain from the credentials. Optional.
 	UniverseDomain string
 }
 
@@ -191,9 +185,6 @@ func (o *CredentialsOptions) validate() error {
 	if o.Lifetime.Hours() > 12 {
 		return errLifetimeOverMax
 	}
-	if o.Client != nil && o.Credentials != nil {
-		return errClientAndCredentials
-	}
 	return nil
 }
 

auth/credentials/impersonate/impersonate_test.go

@@ -57,16 +57,6 @@ func TestNewCredentials_serviceAccount(t *testing.T) {
 			},
 			wantErr: errLifetimeOverMax,
 		},
-		{
-			name: "credentials and client",
-			config: CredentialsOptions{
-				TargetPrincipal: "foo@project-id.iam.gserviceaccount.com",
-				Scopes:          []string{"scope"},
-				Client:          &http.Client{},
-				Credentials:     staticCredentials("googleapis.com"),
-			},
-			wantErr: errClientAndCredentials,
-		},
 		{
 			name: "works",
 			config: CredentialsOptions{