Return auth error when auth type is set

NEXT_CHANGELOG.md

@@ -6,6 +6,9 @@
 
 ### Bug Fixes
 
+- Stop swallowing authentication errors when a non-default auth type is 
+  explicitly set ([#1223](https://github.com/databricks/databricks-sdk-go/pull/1223))
+
 ### Documentation
 
 ### Internal Changes

config/auth_azure_msi_test.go

@@ -47,6 +47,7 @@ func TestMsiHappyFlow(t *testing.T) {
 	assertHeaders(t, &Config{
 		AzureUseMSI:     true,
 		AzureResourceID: "/a/b/c",
+		AuthType:        "azure-msi",
 		HTTPTransport: fixtures.MappingTransport{
 			"GET /metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fmanagement.azure.com%2F": {
 				ExpectedHeaders: map[string]string{
@@ -86,6 +87,7 @@ func TestMsiFailsOnResolveWorkspace(t *testing.T) {
 	_, err := authenticateRequest(&Config{
 		AzureUseMSI:     true,
 		AzureResourceID: "/a/b/c",
+		AuthType:        "azure-msi",
 		HTTPTransport: fixtures.MappingTransport{
 			"GET /metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fmanagement.azure.com%2F": {
 				Response: someValidToken("bcd"),
@@ -108,6 +110,7 @@ func TestMsiTokenNotFound(t *testing.T) {
 		AzureUseMSI:     true,
 		AzureClientID:   "abc",
 		AzureResourceID: "/a/b/c",
+		AuthType:        "azure-msi",
 		HTTPTransport: fixtures.MappingTransport{
 			"GET /metadata/identity/oauth2/token?api-version=2018-02-01&client_id=abc&resource=https%3A%2F%2Fmanagement.azure.com%2F": {
 				Status:   404,
@@ -122,6 +125,7 @@ func TestMsiInvalidTokenExpiry(t *testing.T) {
 	_, err := authenticateRequest(&Config{
 		AzureUseMSI:     true,
 		AzureResourceID: "/a/b/c",
+		AuthType:        "azure-msi",
 		HTTPTransport: fixtures.MappingTransport{
 			"GET /metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fmanagement.azure.com%2F": {
 				Response: map[string]any{

config/auth_default.go

@@ -2,125 +2,123 @@ package config
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"github.com/databricks/databricks-sdk-go/config/credentials"
 	"github.com/databricks/databricks-sdk-go/config/experimental/auth/oidc"
 	"github.com/databricks/databricks-sdk-go/logger"
 )
 
-// Constructs all Databricks OIDC Credentials Strategies
-func buildOidcTokenCredentialStrategies(cfg *Config) []CredentialsStrategy {
-	type namedIdTokenSource struct {
-		name        string
-		tokenSource oidc.IDTokenSource
-	}
-	idTokenSources := []namedIdTokenSource{
-		{
-			name: "env-oidc",
-			// If the OIDCTokenEnv is not set, use DATABRICKS_OIDC_TOKEN as
-			// default value.
-			tokenSource: func() oidc.IDTokenSource {
-				v := cfg.OIDCTokenEnv
-				if v == "" {
-					v = "DATABRICKS_OIDC_TOKEN"
-				}
-				return oidc.NewEnvIDTokenSource(v)
-			}(),
-		},
-		{
-			name:        "file-oidc",
-			tokenSource: oidc.NewFileTokenSource(cfg.OIDCTokenFilepath),
-		},
-		{
-			name: "github-oidc",
-			tokenSource: oidc.NewGithubIDTokenSource(
-				cfg.refreshClient,
-				cfg.ActionsIDTokenRequestURL,
-				cfg.ActionsIDTokenRequestToken,
-			),
-		},
-		// Add new providers at the end of the list
-	}
+const authDocURL = "https://docs.databricks.com/en/dev-tools/auth.html#databricks-client-unified-authentication"
 
-	strategies := []CredentialsStrategy{}
-	for _, idTokenSource := range idTokenSources {
-		oidcConfig := oidc.DatabricksOIDCTokenSourceConfig{
-			ClientID:              cfg.ClientID,
-			Host:                  cfg.CanonicalHostName(),
-			TokenEndpointProvider: cfg.getOidcEndpoints,
-			Audience:              cfg.TokenAudience,
-			IDTokenSource:         idTokenSource.tokenSource,
-		}
-		if cfg.IsAccountClient() {
-			oidcConfig.AccountID = cfg.AccountID
-		}
-		tokenSource := oidc.NewDatabricksOIDCTokenSource(oidcConfig)
-		strategies = append(strategies, NewTokenSourceStrategy(idTokenSource.name, tokenSource))
+type DefaultCredentials struct {
+	name string
+}
+
+func (c *DefaultCredentials) Name() string {
+	if c.name == "" {
+		return "default"
 	}
-	return strategies
+	return c.name
 }
 
-func buildDefaultStrategies(cfg *Config) []CredentialsStrategy {
-	strategies := []CredentialsStrategy{}
-	strategies = append(strategies,
+func (c *DefaultCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {
+	err := cfg.EnsureResolved()
+	if err != nil {
+		return nil, err
+	}
+
+	// Order in which strategies are tested. Iteration proceeds from most
+	// specific to most generic, and the first strategy to return a non-nil
+	// credentials provider is selected.
+	//
+	// Modifying this order could break authentication for users whose
+	// environments are compatible with multiple strategies and who rely on the
+	// current priority for tie-breaking. While arguably an anti-pattern, this
+	// order is maintained for backward compatibility.
+	strategies := []CredentialsStrategy{
 		PatCredentials{},
 		BasicCredentials{},
 		M2mCredentials{},
-		DatabricksCliCredentials,
-		MetadataServiceCredentials{})
-	strategies = append(strategies, buildOidcTokenCredentialStrategies(cfg)...)
-	strategies = append(strategies,
-		// Attempt to configure auth from most specific to most generic (the Azure CLI).
+		u2mCredentials{},
+		MetadataServiceCredentials{},
+		// OIDC Strategies.
+		githubOIDC(cfg),
+		envOIDC(cfg),
+		fileOIDC(cfg),
+		// Azure strategies.
 		AzureGithubOIDCCredentials{},
 		AzureMsiCredentials{},
 		AzureClientSecretCredentials{},
 		AzureCliCredentials{},
-		// Attempt to configure auth from most specific to most generic (Google Application Default Credentials).
+		// Google strategies.
 		GoogleCredentials{},
-		GoogleDefaultCredentials{})
-	return strategies
+		GoogleDefaultCredentials{},
+	}
+
+	// If an auth type is specified, try to configure the credentials for that
+	// specific auth type. If an error is encountered, return it.
+	if cfg.AuthType != "" {
+		for _, s := range strategies {
+			if s.Name() == cfg.AuthType {
+				logger.Tracef(ctx, "Attempting to configure auth: %q", s.Name())
+				c.name = s.Name()
+				return s.Configure(ctx, cfg)
+			}
+		}
+		return nil, fmt.Errorf("auth type %q not found, please check %s for a list of supported auth types", cfg.AuthType, authDocURL)
+	}
+
+	// If no auth type is specified, try the strategies in order. If a strategy
+	// succeeds, returns the credentials provider. If a strategy fails, swallow
+	// the error and try the next strategy.
+	for _, s := range strategies {
+		logger.Tracef(ctx, "Attempting to configure auth: %q", s.Name())
+		cp, err := s.Configure(ctx, cfg)
+		if err != nil || cp == nil {
+			logger.Debugf(ctx, "Failed to configure auth: %q", s.Name())
+			continue
+		}
+		c.name = s.Name()
+		return cp, nil
+	}
+
+	return nil, fmt.Errorf("cannot configure default credentials, please check %s to configure credentials for your preferred authentication method", authDocURL)
 }
 
-type DefaultCredentials struct {
-	name string
+func githubOIDC(cfg *Config) CredentialsStrategy {
+	return oidcStrategy(cfg, "github-oidc", oidc.NewGithubIDTokenSource(
+		cfg.refreshClient,
+		cfg.ActionsIDTokenRequestURL,
+		cfg.ActionsIDTokenRequestToken,
+	))
 }
 
-func (c *DefaultCredentials) Name() string {
-	if c.name == "" {
-		return "default"
+func envOIDC(cfg *Config) CredentialsStrategy {
+	v := cfg.OIDCTokenEnv
+	if v == "" {
+		v = "DATABRICKS_OIDC_TOKEN"
 	}
-	return c.name
+	return oidcStrategy(cfg, "env-oidc", oidc.NewEnvIDTokenSource(v))
 }
 
-var authFlowUrl = "https://docs.databricks.com/en/dev-tools/auth.html#databricks-client-unified-authentication"
-var errorMessage = fmt.Sprintf("cannot configure default credentials, please check %s to configure credentials for your preferred authentication method", authFlowUrl)
-
-// ErrCannotConfigureAuth (experimental) is returned when no auth is configured
-var ErrCannotConfigureAuth = errors.New(errorMessage)
+func fileOIDC(cfg *Config) CredentialsStrategy {
+	return oidcStrategy(cfg, "file-oidc", oidc.NewFileTokenSource(cfg.OIDCTokenFilepath))
+}
 
-func (c *DefaultCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {
-	err := cfg.EnsureResolved()
-	if err != nil {
-		return nil, err
+// oidcStrategy returns a new CredentialsStrategy to authenticate with
+// Databricks using the given OIDC IDTokenSource.
+func oidcStrategy(cfg *Config, name string, ts oidc.IDTokenSource) CredentialsStrategy {
+	oidcConfig := oidc.DatabricksOIDCTokenSourceConfig{
+		ClientID:              cfg.ClientID,
+		Host:                  cfg.CanonicalHostName(),
+		TokenEndpointProvider: cfg.getOidcEndpoints,
+		Audience:              cfg.TokenAudience,
+		IDTokenSource:         ts,
 	}
-	for _, p := range buildDefaultStrategies(cfg) {
-		if cfg.AuthType != "" && p.Name() != cfg.AuthType {
-			// ignore other auth types if one is explicitly enforced
-			logger.Infof(ctx, "Ignoring %s auth, because %s is preferred", p.Name(), cfg.AuthType)
-			continue
-		}
-		logger.Tracef(ctx, "Attempting to configure auth: %s", p.Name())
-		credentialsProvider, err := p.Configure(ctx, cfg)
-		if err != nil {
-			return nil, fmt.Errorf("%s: %w", p.Name(), err)
-		}
-		if credentialsProvider == nil {
-			continue
-		}
-		c.name = p.Name()
-		return credentialsProvider, nil
+	if cfg.IsAccountClient() {
+		oidcConfig.AccountID = cfg.AccountID
 	}
-	return nil, ErrCannotConfigureAuth
+	tokenSource := oidc.NewDatabricksOIDCTokenSource(oidcConfig)
+	return NewTokenSourceStrategy(name, tokenSource)
 }

config/auth_default_test.go

@@ -1,19 +1,49 @@
-package config_test
+package config
 
 import (
 	"context"
-	"errors"
+	"strings"
 	"testing"
-
-	"github.com/databricks/databricks-sdk-go"
-	"github.com/databricks/databricks-sdk-go/config"
-	"github.com/databricks/databricks-sdk-go/internal/env"
-	"github.com/stretchr/testify/assert"
 )
 
-func TestErrCannotConfigureAuth(t *testing.T) {
-	env.CleanupEnvironment(t)
-	w := databricks.Must(databricks.NewWorkspaceClient())
-	_, err := w.CurrentUser.Me(context.Background())
-	assert.True(t, errors.Is(err, config.ErrCannotConfigureAuth))
+func TestDefaultCredentials_Configure(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		authType string
+		wantErr  string
+	}{
+		{
+			desc:     "unknown auth type",
+			authType: "unknown-auth-type-1337",
+			wantErr:  "auth type \"unknown-auth-type-1337\" not found",
+		},
+		{
+			desc:     "not valid auth",
+			authType: "",
+			wantErr:  "cannot configure default credentials",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx := context.Background()
+			cfg := &Config{
+				AuthType: tc.authType,
+				resolved: true, // avoid calling EnsureResolved
+			}
+
+			dc := DefaultCredentials{}
+			got, gotErr := dc.Configure(ctx, cfg)
+
+			if got != nil {
+				t.Errorf("DefaultCredentials.Configure: got %v, want nil", got)
+			}
+			if gotErr == nil {
+				t.Errorf("DefaultCredentials.Configure: got error %v, want non-nil", gotErr)
+			}
+			if !strings.Contains(gotErr.Error(), tc.wantErr) {
+				t.Errorf("DefaultCredentials.Configure: got error %v, want error containing %q", gotErr, tc.wantErr)
+			}
+		})
+	}
 }

config/auth_m2m_test.go

@@ -74,6 +74,7 @@ func TestM2mNotSupported(t *testing.T) {
 		Host:         "a",
 		ClientID:     "b",
 		ClientSecret: "c",
+		AuthType:     "oauth-m2m",
 		HTTPTransport: fixtures.MappingTransport{
 			"GET /oidc/.well-known/oauth-authorization-server": {
 				Status:   404,

config/auth_metadata_service.go

@@ -66,15 +66,15 @@ func (c MetadataServiceCredentials) Configure(ctx context.Context, cfg *Config)
 		metadataServiceURL: parsedMetadataServiceURL,
 		config:             cfg,
 	}
-	response, err := ms.Get()
+	response, err := ms.Get(ctx)
 	if err != nil {
 		return nil, err
 	}
 	if response == nil {
 		return nil, nil
 	}
-	visitor := refreshableVisitor(&ms)
-	return credentials.NewCredentialsProvider(visitor), nil
+
+	return credentials.NewOAuthCredentialsProviderFromTokenSource(ms), nil
 }
 
 type metadataService struct {
@@ -83,8 +83,8 @@ type metadataService struct {
 }
 
 // performs a request to the metadata service and returns the token
-func (s metadataService) Get() (*oauth2.Token, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), metadataServiceTimeout)
+func (s metadataService) Get(ctx context.Context) (*oauth2.Token, error) {
+	ctx, cancel := context.WithTimeout(ctx, metadataServiceTimeout)
 	defer cancel()
 	var inner msiToken
 	err := s.config.refreshClient.Do(ctx, http.MethodGet,
@@ -99,15 +99,15 @@ func (s metadataService) Get() (*oauth2.Token, error) {
 	return inner.Token()
 }
 
-func (t metadataService) Token() (*oauth2.Token, error) {
-	token, err := t.Get()
+func (t metadataService) Token(ctx context.Context) (*oauth2.Token, error) {
+	token, err := t.Get(ctx)
 	if err != nil {
 		return nil, err
 	}
 	if token == nil {
 		return nil, fmt.Errorf("no token returned from metadata service")
 	}
-	logger.Debugf(context.Background(),
+	logger.Debugf(ctx,
 		"Refreshed access token from local metadata service, which expires on %s",
 		token.Expiry.Format(time.RFC3339))
 	return token, nil