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

cmd/auth/login.go

@@ -146,7 +146,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,30 @@ 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 to [__settings__] so subsequent commands
+//     skip the (slow/blocking) probe and route directly to the file cache.
+//  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.
+//
+// Both rules are dormant today: the resolver default is plaintext, so
+// (mode=Secure, explicit=false) is unreachable. They activate when the
+// default flips to secure (MS4 / cli-ga). Wiring lands now so MS4 is a
+// single-line default flip plus pin-on-success additions.
+//
+// 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 +102,66 @@ 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.
+//
+// Pin-on-success across modes (locking in the first working behavior to
+// insulate users from keyring flakiness) is intentionally not implemented
+// here. It lands with MS4 alongside the default flip; pinning before the
+// flip would freeze every default user into plaintext and make the flip a
+// no-op for them.
+func applyLoginFallback(ctx context.Context, mode StorageMode, explicit bool, f cacheFactories) (cache.TokenCache, StorageMode, error) {
+	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
+	case StorageModeSecure:
+		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(), mode, nil
+	default:
+		return nil, "", fmt.Errorf("unsupported storage mode %q", string(mode))
+	}
+}
+
+// persistPlaintextFallback writes auth_storage = plaintext to [__settings__]
+// in .databrickscfg so subsequent commands skip the (slow/blocking) keyring
+// probe and route straight to the file cache.
+//
+// Only called on the (mode=Secure, explicit=false) probe-failure branch. That
+// branch is unreachable today (resolver default is plaintext), so this is
+// dormant infrastructure: it activates when the default flips to secure
+// (MS4) and lets default-on-broken-keyring users avoid a 3s probe on every
+// command.
+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

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/databricks/databricks-sdk-go/credentials/u2m/cache"
+	"github.com/google/uuid"
 	"github.com/zalando/go-keyring"
 	"golang.org/x/oauth2"
 )
@@ -17,6 +18,14 @@ import (
 // cache key the SDK passes through TokenCache.Store / Lookup.
 const keyringServiceName = "databricks-cli"
 
+// keyringProbeAccountPrefix is prefixed onto a per-call random suffix to form
+// the account name ProbeKeyring writes and deletes. A fixed name like
+// "__probe__" could collide with a user profile of the same name (which is
+// what keyringCache uses as the account field), so the probe would clobber
+// and delete that user's stored token. Per-call randomness also means
+// concurrent probes don't step on each other.
+const keyringProbeAccountPrefix = "__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 +88,35 @@ 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,
+	}
+	account := keyringProbeAccountPrefix + uuid.NewString()
+	tok := &oauth2.Token{AccessToken: "probe"}
+	if err := c.Store(account, tok); err != nil {
+		return fmt.Errorf("write: %w", err)
+	}
+	if err := c.Store(account, 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 {