auth: keep keyring backend on probe timeout during login

NEXT_CHANGELOG.md

@@ -4,6 +4,8 @@
 
 ### CLI
 
+* `auth login` no longer falls back to plaintext when the OS keyring is reachable but locked. The unlock prompt shown by the probe now runs in parallel with the OAuth flow, and the token is stored in the keyring once the user has typed their password.
+
 ### Bundles
 
 * Fixed `--force-pull` on `bundle summary` and `bundle open` so the flag bypasses the local state cache and reads state from the workspace.

cmd/auth/login.go

@@ -146,10 +146,10 @@ a new profile is created.
 		ctx := cmd.Context()
 		profileName := cmd.Flag("profile").Value.String()
 
-		// 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.
+		// Resolve the cache before the browser step so an unavailable
+		// keyring surfaces here rather than after OAuth. The probe also
+		// triggers the OS unlock prompt, which the user can answer during
+		// OAuth.
 		tokenCache, mode, err := storage.ResolveCacheForLogin(ctx, "")
 		if err != nil {
 			return err

libs/auth/storage/cache.go

@@ -2,6 +2,7 @@ package storage
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/databricks/cli/libs/databrickscfg"
@@ -54,8 +55,11 @@ func ResolveCache(ctx context.Context, override StorageMode) (cache.TokenCache,
 //  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.
+//  3. When the probe times out, stay on keyring regardless of explicit.
+//     The timeout is ambiguous (locked vs hung); a misdiagnosis fails
+//     the final Store rather than silently downgrading to plaintext.
 //
-// Both rules are dormant today: the resolver default is plaintext, so
+// Rules 1 and 2 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.
@@ -133,6 +137,15 @@ func applyLoginFallback(ctx context.Context, mode StorageMode, explicit bool, f
 		return c, mode, nil
 	case StorageModeSecure:
 		if probeErr := f.probeKeyring(); probeErr != nil {
+			// Stay on keyring on timeout: a locked keyring being unlocked
+			// during OAuth is the common case, and a misdiagnosed hang
+			// fails the final Store anyway, which is better than a
+			// silent plaintext downgrade.
+			var timeoutErr *TimeoutError
+			if errors.As(probeErr, &timeoutErr) {
+				log.Debugf(ctx, "keyring probe timed out (%v); staying on keyring", probeErr)
+				return f.newKeyring(), mode, nil
+			}
 			if explicit {
 				return nil, "", fmt.Errorf("secure storage was requested but the OS keyring is not reachable: %w", probeErr)
 			}

libs/auth/storage/cache_test.go

@@ -3,6 +3,7 @@ package storage
 import (
 	"context"
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -232,6 +233,45 @@ func TestApplyLoginFallback_ExplicitSecure_ProbeFail_Errors(t *testing.T) {
 	assert.Equal(t, "", persisted, "explicit-secure error must not write config")
 }
 
+// A locked keyring with a slow user surfaces as TimeoutError. We want login
+// to stay on the keyring so the final Store lands there once the user has
+// finished unlocking, regardless of whether secure was explicit. Cover both
+// the bare TimeoutError (in case probe wraps thinner in the future) and the
+// real wrapped form returned by probeWithBackend.
+func TestApplyLoginFallback_ProbeTimeout_StaysOnKeyring(t *testing.T) {
+	cases := []struct {
+		name     string
+		explicit bool
+		probeErr error
+	}{
+		{"default-secure, bare TimeoutError", false, &TimeoutError{Op: "set"}},
+		{"default-secure, wrapped TimeoutError", false, fmt.Errorf("write: %w", &TimeoutError{Op: "set"})},
+		{"explicit-secure, bare TimeoutError", true, &TimeoutError{Op: "set"}},
+		{"explicit-secure, wrapped TimeoutError", true, fmt.Errorf("write: %w", &TimeoutError{Op: "set"})},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			hermetic(t)
+			ctx := t.Context()
+			configPath := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
+
+			f := fakeFactories(t)
+			f.probeKeyring = func() error { return tc.probeErr }
+
+			got, mode, err := applyLoginFallback(ctx, StorageModeSecure, tc.explicit, f)
+
+			require.NoError(t, err)
+			assert.Equal(t, StorageModeSecure, mode)
+			assert.Equal(t, "keyring", got.(stubCache).source)
+
+			persisted, gerr := databrickscfg.GetConfiguredAuthStorage(ctx, configPath)
+			require.NoError(t, gerr)
+			assert.Equal(t, "", persisted, "probe timeout must not persist plaintext fallback")
+		})
+	}
+}
+
 func TestWrapForOAuthArgument(t *testing.T) {
 	const (
 		host       = "https://example.com"

libs/auth/storage/keyring.go

@@ -88,14 +88,10 @@ 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.
+// ProbeKeyring returns nil if the OS keyring accepted a write+delete
+// cycle within the standard timeout. *TimeoutError means the keyring
+// was unresponsive (locked or hung, indistinguishable here); other
+// errors are definitive failures.
 func ProbeKeyring() error {
 	return probeWithBackend(zalandoBackend{}, defaultKeyringTimeout)
 }