auth: silently fall back to plaintext when keyring is unreachable on login

cmd/auth/login.go

@@ -147,7 +147,11 @@ a new profile is created.
 		ctx := cmd.Context()
 		profileName := cmd.Flag("profile").Value.String()
 
-		tokenCache, mode, err := storage.ResolveCache(ctx, "")
+		// Resolve the cache before the browser step so a missing/locked keyring
+		// surfaces here rather than after the user completes OAuth. When secure
+		// is selected but the keyring is unreachable, this silently falls back
+		// to plaintext and persists auth_storage = plaintext for next time.
+		tokenCache, mode, err := storage.ResolveCacheForLogin(ctx, "")
 		if err != nil {
 			return err
 		}

libs/auth/storage/cache.go

@@ -4,6 +4,9 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/databricks/cli/libs/databrickscfg"
+	"github.com/databricks/cli/libs/env"
+	"github.com/databricks/cli/libs/log"
 	"github.com/databricks/databricks-sdk-go/credentials/u2m"
 	"github.com/databricks/databricks-sdk-go/credentials/u2m/cache"
 )
@@ -12,15 +15,17 @@ import (
 // so unit tests can inject stubs without hitting the real OS keyring or
 // filesystem. Production code uses defaultCacheFactories().
 type cacheFactories struct {
-	newFile    func(context.Context) (cache.TokenCache, error)
-	newKeyring func() cache.TokenCache
+	newFile      func(context.Context) (cache.TokenCache, error)
+	newKeyring   func() cache.TokenCache
+	probeKeyring func() error
 }
 
 // defaultCacheFactories returns the production factory set.
 func defaultCacheFactories() cacheFactories {
 	return cacheFactories{
-		newFile:    func(ctx context.Context) (cache.TokenCache, error) { return NewFileTokenCache(ctx) },
-		newKeyring: NewKeyringCache,
+		newFile:      func(ctx context.Context) (cache.TokenCache, error) { return NewFileTokenCache(ctx) },
+		newKeyring:   NewKeyringCache,
+		probeKeyring: ProbeKeyring,
 	}
 }
 
@@ -38,6 +43,24 @@ func ResolveCache(ctx context.Context, override StorageMode) (cache.TokenCache,
 	return resolveCacheWith(ctx, override, defaultCacheFactories())
 }
 
+// ResolveCacheForLogin resolves the cache like ResolveCache with extra rules
+// for the auth login path:
+//
+//  1. When the resolved mode is secure and the user did not explicitly ask for
+//     it (no override flag, no env var, no config), and the OS keyring is
+//     unreachable, fall back silently to plaintext and persist
+//     auth_storage = plaintext so subsequent commands skip the probe.
+//  2. When the user explicitly asked for secure (override, env var, or config)
+//     but the keyring is unreachable, return an error. An explicit "I want
+//     secure" is honored strictly: never silently downgrade.
+//
+// Login-specific. Read paths (auth token, bundle commands) keep the original
+// keyring error so they don't silently mint plaintext copies of tokens that
+// were stored in the keyring on another machine.
+func ResolveCacheForLogin(ctx context.Context, override StorageMode) (cache.TokenCache, StorageMode, error) {
+	return resolveCacheForLoginWith(ctx, override, defaultCacheFactories())
+}
+
 // WrapForOAuthArgument wraps tokenCache so SDK-side writes (Challenge, refresh)
 // dual-write to the legacy host-based cache key when mode is plaintext. Other
 // modes return tokenCache unchanged: secure mode never writes a host-key entry,
@@ -73,3 +96,56 @@ func resolveCacheWith(ctx context.Context, override StorageMode, f cacheFactorie
 		return nil, "", fmt.Errorf("unsupported storage mode %q", string(mode))
 	}
 }
+
+// resolveCacheForLoginWith is the pure form of ResolveCacheForLogin. It takes
+// the factory set as a parameter so tests can inject stubs.
+func resolveCacheForLoginWith(ctx context.Context, override StorageMode, f cacheFactories) (cache.TokenCache, StorageMode, error) {
+	mode, explicit, err := ResolveStorageModeWithSource(ctx, override)
+	if err != nil {
+		return nil, "", err
+	}
+	return applyLoginFallback(ctx, mode, explicit, f)
+}
+
+// applyLoginFallback realizes the login-time fallback rules given an already-
+// resolved mode and whether the user explicitly asked for it. Split out so
+// tests can drive the (mode, explicit) input space directly without depending
+// on whatever the resolver's default mode happens to be at any point in time.
+func applyLoginFallback(ctx context.Context, mode StorageMode, explicit bool, f cacheFactories) (cache.TokenCache, StorageMode, error) {
+	if mode != StorageModeSecure {
+		switch mode {
+		case StorageModePlaintext:
+			c, err := f.newFile(ctx)
+			if err != nil {
+				return nil, "", fmt.Errorf("open file token cache: %w", err)
+			}
+			return c, mode, nil
+		default:
+			return nil, "", fmt.Errorf("unsupported storage mode %q", string(mode))
+		}
+	}
+	if probeErr := f.probeKeyring(); probeErr != nil {
+		if explicit {
+			return nil, "", fmt.Errorf("secure storage was requested but the OS keyring is not reachable: %w", probeErr)
+		}
+		log.Debugf(ctx, "secure storage unavailable (%v), falling back to plaintext", probeErr)
+		fileCache, fileErr := f.newFile(ctx)
+		if fileErr != nil {
+			return nil, "", fmt.Errorf("open file token cache: %w", fileErr)
+		}
+		if err := persistPlaintextFallback(ctx); err != nil {
+			log.Debugf(ctx, "persisting auth_storage fallback failed: %v", err)
+		}
+		return fileCache, StorageModePlaintext, nil
+	}
+	return f.newKeyring(), StorageModeSecure, nil
+}
+
+// persistPlaintextFallback writes auth_storage = plaintext to [__settings__]
+// in .databrickscfg. Only called when the user did not explicitly choose
+// secure, so overwriting any existing value would only happen if a previous
+// fallback already wrote the same value: a no-op write.
+func persistPlaintextFallback(ctx context.Context) error {
+	configPath := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
+	return databrickscfg.SetConfiguredAuthStorage(ctx, string(StorageModePlaintext), configPath)
+}

libs/auth/storage/cache_test.go

@@ -3,9 +3,11 @@ package storage
 import (
 	"context"
 	"errors"
+	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/databricks/cli/libs/databrickscfg"
 	"github.com/databricks/cli/libs/env"
 	"github.com/databricks/databricks-sdk-go/credentials/u2m"
 	"github.com/databricks/databricks-sdk-go/credentials/u2m/cache"
@@ -24,8 +26,9 @@ func (stubCache) Lookup(string) (*oauth2.Token, error) { return nil, cache.ErrNo
 func fakeFactories(t *testing.T) cacheFactories {
 	t.Helper()
 	return cacheFactories{
-		newFile:    func(context.Context) (cache.TokenCache, error) { return stubCache{source: "file"}, nil },
-		newKeyring: func() cache.TokenCache { return stubCache{source: "keyring"} },
+		newFile:      func(context.Context) (cache.TokenCache, error) { return stubCache{source: "file"}, nil },
+		newKeyring:   func() cache.TokenCache { return stubCache{source: "keyring"} },
+		probeKeyring: func() error { return nil },
 	}
 }
 
@@ -106,8 +109,9 @@ func TestResolveCache_FileFactoryErrorPropagates(t *testing.T) {
 	ctx := t.Context()
 	boom := errors.New("disk full")
 	factories := cacheFactories{
-		newFile:    func(context.Context) (cache.TokenCache, error) { return nil, boom },
-		newKeyring: func() cache.TokenCache { return stubCache{source: "keyring"} },
+		newFile:      func(context.Context) (cache.TokenCache, error) { return nil, boom },
+		newKeyring:   func() cache.TokenCache { return stubCache{source: "keyring"} },
+		probeKeyring: func() error { return nil },
 	}
 
 	_, _, err := resolveCacheWith(ctx, StorageModePlaintext, factories)
@@ -116,6 +120,118 @@ func TestResolveCache_FileFactoryErrorPropagates(t *testing.T) {
 	assert.ErrorIs(t, err, boom)
 }
 
+func TestResolveCacheForLogin_PlaintextSkipsProbe(t *testing.T) {
+	hermetic(t)
+	ctx := t.Context()
+	probed := false
+	f := fakeFactories(t)
+	f.probeKeyring = func() error {
+		probed = true
+		return nil
+	}
+
+	got, mode, err := resolveCacheForLoginWith(ctx, StorageModePlaintext, f)
+
+	require.NoError(t, err)
+	assert.Equal(t, StorageModePlaintext, mode)
+	assert.Equal(t, "file", got.(stubCache).source)
+	assert.False(t, probed, "probe must not run when mode is already plaintext")
+}
+
+func TestResolveCacheForLogin_SecureProbeOK(t *testing.T) {
+	hermetic(t)
+	ctx := env.Set(t.Context(), EnvVar, "secure")
+
+	got, mode, err := resolveCacheForLoginWith(ctx, "", fakeFactories(t))
+
+	require.NoError(t, err)
+	assert.Equal(t, StorageModeSecure, mode)
+	assert.Equal(t, "keyring", got.(stubCache).source)
+}
+
+func TestResolveCacheForLogin_ExplicitEnvSecure_ProbeFail_Errors(t *testing.T) {
+	hermetic(t)
+	ctx := env.Set(t.Context(), EnvVar, "secure")
+	configPath := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
+
+	f := fakeFactories(t)
+	f.probeKeyring = func() error { return errors.New("no keyring") }
+
+	_, _, err := resolveCacheForLoginWith(ctx, "", f)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "secure storage was requested")
+
+	persisted, gerr := databrickscfg.GetConfiguredAuthStorage(ctx, configPath)
+	require.NoError(t, gerr)
+	assert.Equal(t, "", persisted, "env-set secure must not be persisted as plaintext")
+}
+
+func TestResolveCacheForLogin_ExplicitConfigSecure_ProbeFail_Errors(t *testing.T) {
+	hermetic(t)
+	ctx := t.Context()
+	configPath := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
+	require.NoError(t, os.WriteFile(configPath, []byte("[__settings__]\nauth_storage = secure\n"), 0o600))
+
+	f := fakeFactories(t)
+	f.probeKeyring = func() error { return errors.New("no keyring") }
+
+	_, _, err := resolveCacheForLoginWith(ctx, "", f)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "secure storage was requested")
+
+	persisted, gerr := databrickscfg.GetConfiguredAuthStorage(ctx, configPath)
+	require.NoError(t, gerr)
+	assert.Equal(t, "secure", persisted, "config-set secure must not be silently rewritten")
+}
+
+func TestResolveCacheForLogin_ExplicitOverrideSecure_ProbeFail_Errors(t *testing.T) {
+	hermetic(t)
+	ctx := t.Context()
+
+	f := fakeFactories(t)
+	f.probeKeyring = func() error { return errors.New("no keyring") }
+
+	_, _, err := resolveCacheForLoginWith(ctx, StorageModeSecure, f)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "secure storage was requested")
+}
+
+func TestApplyLoginFallback_DefaultSecure_ProbeFail_FallsBackAndPersists(t *testing.T) {
+	hermetic(t)
+	ctx := t.Context()
+	configPath := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
+
+	f := fakeFactories(t)
+	f.probeKeyring = func() error { return errors.New("no keyring") }
+
+	got, mode, err := applyLoginFallback(ctx, StorageModeSecure, false, f)
+
+	require.NoError(t, err)
+	assert.Equal(t, StorageModePlaintext, mode)
+	assert.Equal(t, "file", got.(stubCache).source)
+
+	persisted, err := databrickscfg.GetConfiguredAuthStorage(ctx, configPath)
+	require.NoError(t, err)
+	assert.Equal(t, "plaintext", persisted, "default-mode fallback must persist auth_storage = plaintext")
+}
+
+func TestApplyLoginFallback_ExplicitSecure_ProbeFail_Errors(t *testing.T) {
+	hermetic(t)
+	ctx := t.Context()
+	configPath := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
+
+	f := fakeFactories(t)
+	f.probeKeyring = func() error { return errors.New("no keyring") }
+
+	_, _, err := applyLoginFallback(ctx, StorageModeSecure, true, f)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "secure storage was requested")
+
+	persisted, gerr := databrickscfg.GetConfiguredAuthStorage(ctx, configPath)
+	require.NoError(t, gerr)
+	assert.Equal(t, "", persisted, "explicit-secure error must not write config")
+}
+
 func TestWrapForOAuthArgument(t *testing.T) {
 	const (
 		host       = "https://example.com"

libs/auth/storage/keyring.go

@@ -17,6 +17,11 @@ import (
 // cache key the SDK passes through TokenCache.Store / Lookup.
 const keyringServiceName = "databricks-cli"
 
+// keyringProbeAccount is the account name ProbeKeyring writes and deletes
+// to verify the keyring is reachable. Distinct from any real cache key so a
+// concurrent probe cannot collide with an actual OAuth token entry.
+const keyringProbeAccount = "__probe__"
+
 // defaultKeyringTimeout is how long a single keyring operation is allowed
 // to run before the wrapper returns a TimeoutError. Matches the value used
 // by GitHub CLI.
@@ -79,6 +84,34 @@ func NewKeyringCache() cache.TokenCache {
 	}
 }
 
+// ProbeKeyring returns nil if the OS keyring is reachable and accepts a
+// write+delete cycle within the standard timeout. A non-nil error means the
+// keyring cannot be used in this environment (no backend, headless Linux
+// session waiting on a UI prompt, locked keychain refusing access, etc.).
+//
+// Used by databricks auth login to decide whether to silently fall back to
+// plaintext storage before opening the browser, so the user does not
+// complete an OAuth flow only to fail at the final Store call.
+func ProbeKeyring() error {
+	return probeWithBackend(zalandoBackend{}, defaultKeyringTimeout)
+}
+
+func probeWithBackend(backend keyringBackend, timeout time.Duration) error {
+	c := &keyringCache{
+		backend:        backend,
+		timeout:        timeout,
+		keyringSvcName: keyringServiceName,
+	}
+	tok := &oauth2.Token{AccessToken: "probe"}
+	if err := c.Store(keyringProbeAccount, tok); err != nil {
+		return fmt.Errorf("write: %w", err)
+	}
+	if err := c.Store(keyringProbeAccount, nil); err != nil {
+		return fmt.Errorf("delete: %w", err)
+	}
+	return nil
+}
+
 // Store stores t under key. Nil t deletes the entry; deleting a missing
 // entry is not an error.
 func (k *keyringCache) Store(key string, t *oauth2.Token) error {

libs/auth/storage/keyring_test.go

@@ -217,3 +217,43 @@ func TestKeyringCache_StoreNil_TimesOut(t *testing.T) {
 	var timeoutErr *TimeoutError
 	assert.ErrorAs(t, err, &timeoutErr, "expected TimeoutError, got %T: %v", err, err)
 }
+
+func TestProbeKeyring_SuccessLeavesNoEntry(t *testing.T) {
+	backend := newFakeBackend()
+
+	err := probeWithBackend(backend, 100*time.Millisecond)
+	require.NoError(t, err)
+
+	_, ok := backend.items[itemKey(keyringServiceName, keyringProbeAccount)]
+	assert.False(t, ok, "probe must clean up after itself")
+}
+
+func TestProbeKeyring_SetErrorPropagates(t *testing.T) {
+	boom := errors.New("no backend")
+	backend := newFakeBackend()
+	backend.setErr = boom
+
+	err := probeWithBackend(backend, 100*time.Millisecond)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, boom)
+}
+
+func TestProbeKeyring_SetTimesOut(t *testing.T) {
+	backend := newFakeBackend()
+	backend.setBlock = true
+
+	err := probeWithBackend(backend, 50*time.Millisecond)
+	require.Error(t, err)
+	var timeoutErr *TimeoutError
+	assert.ErrorAs(t, err, &timeoutErr)
+}
+
+func TestProbeKeyring_DeleteErrorPropagates(t *testing.T) {
+	boom := errors.New("delete failed")
+	backend := newFakeBackend()
+	backend.deleteErr = boom
+
+	err := probeWithBackend(backend, 100*time.Millisecond)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, boom)
+}