diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 409a875ad1..2e0b512efe 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -4,6 +4,8 @@ ### CLI +* `databricks auth describe` now reports where U2M (`databricks-cli`) tokens are stored: `plaintext` (`~/.databricks/token-cache.json`) or `secure` (OS keyring), and the source of the choice (env var, config setting, or default). + ### Bundles * Fixed `--force-pull` on `bundle summary` and `bundle open` so the flag bypasses the local state cache and reads state from the workspace. diff --git a/acceptance/cmd/auth/describe/u2m-json-output/out.test.toml b/acceptance/cmd/auth/describe/u2m-json-output/out.test.toml new file mode 100644 index 0000000000..f784a18325 --- /dev/null +++ b/acceptance/cmd/auth/describe/u2m-json-output/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/auth/describe/u2m-json-output/output.txt b/acceptance/cmd/auth/describe/u2m-json-output/output.txt new file mode 100644 index 0000000000..7e2ac070cb --- /dev/null +++ b/acceptance/cmd/auth/describe/u2m-json-output/output.txt @@ -0,0 +1,8 @@ + +>>> [CLI] auth describe --profile u2m-profile --output json +Warn: [hostmetadata] failed to fetch host metadata for https://u2m-profile.databricks.test, will skip for 1m0s +{ + "mode": "plaintext", + "location": "~/.databricks/token-cache.json", + "source": "default" +} diff --git a/acceptance/cmd/auth/describe/u2m-json-output/script b/acceptance/cmd/auth/describe/u2m-json-output/script new file mode 100644 index 0000000000..668d237449 --- /dev/null +++ b/acceptance/cmd/auth/describe/u2m-json-output/script @@ -0,0 +1,16 @@ +sethome "./home" + +unset DATABRICKS_HOST +unset DATABRICKS_TOKEN +unset DATABRICKS_CONFIG_PROFILE +unset DATABRICKS_AUTH_STORAGE + +cat > "./home/.databrickscfg" <>> [CLI] auth describe --profile u2m-profile +Warn: [hostmetadata] failed to fetch host metadata for https://u2m-profile.databricks.test, will skip for 1m0s +Unable to authenticate: error getting token: cache: token not found +Token storage: plaintext, ~/.databricks/token-cache.json (from auth_storage in [__settings__] section of home/.databrickscfg) +----- +Current configuration: + ✓ host: https://u2m-profile.databricks.test (from ./home/.databrickscfg config file) + ✓ profile: u2m-profile (from --profile flag) + ✓ databricks_cli_path: [CLI] + ✓ auth_type: databricks-cli (from ./home/.databrickscfg config file) + ✓ rate_limit: [NUMID] (from DATABRICKS_RATE_LIMIT environment variable) diff --git a/acceptance/cmd/auth/describe/u2m-plaintext-config/script b/acceptance/cmd/auth/describe/u2m-plaintext-config/script new file mode 100644 index 0000000000..1cf6d3267d --- /dev/null +++ b/acceptance/cmd/auth/describe/u2m-plaintext-config/script @@ -0,0 +1,17 @@ +sethome "./home" + +unset DATABRICKS_HOST +unset DATABRICKS_TOKEN +unset DATABRICKS_CONFIG_PROFILE +unset DATABRICKS_AUTH_STORAGE + +cat > "./home/.databrickscfg" <>> [CLI] auth describe --profile u2m-profile +Warn: [hostmetadata] failed to fetch host metadata for https://u2m-profile.databricks.test, will skip for 1m0s +Unable to authenticate: error getting token: cache: token not found +Token storage: plaintext, ~/.databricks/token-cache.json (from default) +----- +Current configuration: + ✓ host: https://u2m-profile.databricks.test (from ./home/.databrickscfg config file) + ✓ profile: u2m-profile (from --profile flag) + ✓ databricks_cli_path: [CLI] + ✓ auth_type: databricks-cli (from ./home/.databrickscfg config file) + ✓ rate_limit: [NUMID] (from DATABRICKS_RATE_LIMIT environment variable) diff --git a/acceptance/cmd/auth/describe/u2m-plaintext-default/script b/acceptance/cmd/auth/describe/u2m-plaintext-default/script new file mode 100644 index 0000000000..d0b1ce4002 --- /dev/null +++ b/acceptance/cmd/auth/describe/u2m-plaintext-default/script @@ -0,0 +1,14 @@ +sethome "./home" + +unset DATABRICKS_HOST +unset DATABRICKS_TOKEN +unset DATABRICKS_CONFIG_PROFILE +unset DATABRICKS_AUTH_STORAGE + +cat > "./home/.databrickscfg" <>> [CLI] auth describe --profile u2m-profile +Warn: [hostmetadata] failed to fetch host metadata for https://u2m-profile.databricks.test, will skip for 1m0s +Unable to authenticate: error getting token: cache: token not found +Token storage: plaintext, ~/.databricks/token-cache.json (from DATABRICKS_AUTH_STORAGE environment variable) +----- +Current configuration: + ✓ host: https://u2m-profile.databricks.test (from ./home/.databrickscfg config file) + ✓ profile: u2m-profile (from --profile flag) + ✓ databricks_cli_path: [CLI] + ✓ auth_type: databricks-cli (from ./home/.databrickscfg config file) + ✓ rate_limit: [NUMID] (from DATABRICKS_RATE_LIMIT environment variable) diff --git a/acceptance/cmd/auth/describe/u2m-plaintext-env/script b/acceptance/cmd/auth/describe/u2m-plaintext-env/script new file mode 100644 index 0000000000..21bfdf231f --- /dev/null +++ b/acceptance/cmd/auth/describe/u2m-plaintext-env/script @@ -0,0 +1,15 @@ +sethome "./home" + +unset DATABRICKS_HOST +unset DATABRICKS_TOKEN +unset DATABRICKS_CONFIG_PROFILE + +export DATABRICKS_AUTH_STORAGE=plaintext + +cat > "./home/.databrickscfg" </.databrickscfg) so the output matches +// the SDK's config.Source style ("from config file") rather than +// hardcoding ".databrickscfg" when a custom path is in use. +func storageSourceLabel(ctx context.Context, source storage.StorageSource) string { + if source != storage.StorageSourceConfig { + return source.String() + } + return "auth_storage in [__settings__] section of " + resolvedConfigPath(ctx) +} + +// resolvedConfigPath returns the path the storage-mode resolver loaded from +// for [__settings__].auth_storage: DATABRICKS_CONFIG_FILE if set, otherwise +// /.databrickscfg. Falls back to "~/.databrickscfg" only when the home +// directory cannot be determined (rare; describe should not crash on this +// secondary metadata path). +func resolvedConfigPath(ctx context.Context) string { + if path := env.Get(ctx, "DATABRICKS_CONFIG_FILE"); path != "" { + return path + } + home, err := env.UserHomeDir(ctx) + if err != nil { + log.Debugf(ctx, "auth describe: resolve home dir: %v", err) + return "~/.databrickscfg" + } + return filepath.ToSlash(filepath.Join(home, ".databrickscfg")) } func getAuthDetails(cmd *cobra.Command, cfg *config.Config, showSensitive bool) config.AuthDetails { diff --git a/cmd/auth/describe_test.go b/cmd/auth/describe_test.go index ef654aae3d..528decf102 100644 --- a/cmd/auth/describe_test.go +++ b/cmd/auth/describe_test.go @@ -2,13 +2,16 @@ package auth import ( "errors" + "path/filepath" "testing" + "github.com/databricks/cli/libs/auth/storage" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -223,3 +226,141 @@ func TestGetAccountAuthStatus(t *testing.T) { require.Equal(t, "--profile flag", status.Details.Configuration["profile"].Source.String()) require.False(t, status.Details.Configuration["profile"].AuthTypeMismatch) } + +func TestResolveTokenStorageInfo(t *testing.T) { + cases := []struct { + name string + authType string + envValue string + want *tokenStorageInfo + }{ + { + name: "non-databricks-cli auth has no token storage", + authType: "pat", + want: nil, + }, + { + name: "databricks-cli with default plaintext", + authType: authTypeDatabricksCLI, + want: &tokenStorageInfo{ + Mode: "plaintext", + Location: plaintextLocation, + Source: "default", + }, + }, + { + name: "databricks-cli with secure from env", + authType: authTypeDatabricksCLI, + envValue: "secure", + want: &tokenStorageInfo{ + Mode: "secure", + Location: secureLocation, + Source: "DATABRICKS_AUTH_STORAGE environment variable", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(storage.EnvVar, tc.envValue) + t.Setenv("DATABRICKS_CONFIG_FILE", t.TempDir()+"/.databrickscfg") + + got := resolveTokenStorageInfo(t.Context(), tc.authType) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestStorageSourceLabel_ConfigUsesResolvedPath(t *testing.T) { + ctx := t.Context() + t.Setenv("DATABRICKS_CONFIG_FILE", "/custom/path/.databrickscfg") + got := storageSourceLabel(ctx, storage.StorageSourceConfig) + assert.Equal(t, "auth_storage in [__settings__] section of /custom/path/.databrickscfg", got) +} + +func TestStorageSourceLabel_ConfigDefaultsToHome(t *testing.T) { + ctx := t.Context() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("DATABRICKS_CONFIG_FILE", "") + got := storageSourceLabel(ctx, storage.StorageSourceConfig) + expected := "auth_storage in [__settings__] section of " + filepath.ToSlash(filepath.Join(home, ".databrickscfg")) + assert.Equal(t, expected, got) +} + +func TestStorageSourceLabel_NonConfigDelegatesToSource(t *testing.T) { + ctx := t.Context() + t.Setenv("DATABRICKS_CONFIG_FILE", "/should/not/appear") + assert.Equal(t, "default", storageSourceLabel(ctx, storage.StorageSourceDefault)) + assert.Equal(t, "DATABRICKS_AUTH_STORAGE environment variable", storageSourceLabel(ctx, storage.StorageSourceEnvVar)) + assert.Equal(t, "command-line override", storageSourceLabel(ctx, storage.StorageSourceOverride)) +} + +func TestGetWorkspaceAuthStatus_U2M_PopulatesTokenStorage(t *testing.T) { + ctx := t.Context() + m := mocks.NewMockWorkspaceClient(t) + ctx = cmdctx.SetWorkspaceClient(ctx, m.WorkspaceClient) + + cmd := &cobra.Command{} + cmd.SetContext(ctx) + + currentUserApi := m.GetMockCurrentUserAPI() + currentUserApi.EXPECT().Me(mock.Anything).Return(&iam.User{UserName: "u2m-user"}, nil) + + cmd.Flags().String("host", "", "") + cmd.Flags().String("profile", "", "") + require.NoError(t, cmd.Flag("profile").Value.Set("u2m-profile")) + cmd.Flag("profile").Changed = true + + cfg := &config.Config{Profile: "u2m-profile"} + m.WorkspaceClient.Config = cfg + t.Setenv(storage.EnvVar, "secure") + t.Setenv("DATABRICKS_CONFIG_FILE", t.TempDir()+"/.databrickscfg") + require.NoError(t, config.ConfigAttributes.Configure(cfg)) + + status, err := getAuthStatus(cmd, []string{}, false, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) { + require.NoError(t, config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ + "host": "https://test.test", + "auth_type": authTypeDatabricksCLI, + })) + return cfg, false, nil + }) + require.NoError(t, err) + require.NotNil(t, status) + require.NotNil(t, status.TokenStorage) + assert.Equal(t, "secure", status.TokenStorage.Mode) + assert.Equal(t, secureLocation, status.TokenStorage.Location) + assert.Equal(t, "DATABRICKS_AUTH_STORAGE environment variable", status.TokenStorage.Source) +} + +func TestGetWorkspaceAuthStatus_NonU2M_OmitsTokenStorage(t *testing.T) { + ctx := t.Context() + m := mocks.NewMockWorkspaceClient(t) + ctx = cmdctx.SetWorkspaceClient(ctx, m.WorkspaceClient) + + cmd := &cobra.Command{} + cmd.SetContext(ctx) + + currentUserApi := m.GetMockCurrentUserAPI() + currentUserApi.EXPECT().Me(mock.Anything).Return(&iam.User{UserName: "pat-user"}, nil) + + cmd.Flags().String("host", "", "") + cmd.Flags().String("profile", "", "") + + cfg := &config.Config{} + m.WorkspaceClient.Config = cfg + require.NoError(t, config.ConfigAttributes.Configure(cfg)) + + status, err := getAuthStatus(cmd, []string{}, false, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) { + require.NoError(t, config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ + "host": "https://test.test", + "token": "pat-token", + "auth_type": "pat", + })) + return cfg, false, nil + }) + require.NoError(t, err) + require.NotNil(t, status) + assert.Nil(t, status.TokenStorage) +} diff --git a/libs/auth/storage/cache.go b/libs/auth/storage/cache.go index 8e8b779afe..cfd204fa32 100644 --- a/libs/auth/storage/cache.go +++ b/libs/auth/storage/cache.go @@ -106,11 +106,11 @@ func resolveCacheWith(ctx context.Context, override StorageMode, f cacheFactorie // 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) + mode, source, err := ResolveStorageModeWithSource(ctx, override) if err != nil { return nil, "", err } - return applyLoginFallback(ctx, mode, explicit, f) + return applyLoginFallback(ctx, mode, source.Explicit(), f) } // applyLoginFallback realizes the login-time fallback rules given an already- diff --git a/libs/auth/storage/mode.go b/libs/auth/storage/mode.go index 2caace171f..6dd3c6e5df 100644 --- a/libs/auth/storage/mode.go +++ b/libs/auth/storage/mode.go @@ -38,6 +38,50 @@ const ( // EnvVar is the environment variable that selects the storage mode. const EnvVar = "DATABRICKS_AUTH_STORAGE" +// StorageSource identifies which precedence level produced the resolved +// storage mode. Callers use it both to decide whether the user explicitly +// asked for a mode (everything except StorageSourceDefault) and to surface +// where the choice came from in user-facing output. +type StorageSource int + +const ( + // StorageSourceDefault is the zero value: no override, env, or config + // was set, so the resolver fell through to the built-in default. + StorageSourceDefault StorageSource = iota + StorageSourceOverride + StorageSourceEnvVar + StorageSourceConfig +) + +// Explicit reports whether the source came from a user-supplied input +// (override flag, env var, or config) rather than the built-in default. +func (s StorageSource) Explicit() bool { + return s != StorageSourceDefault +} + +// String returns a human-readable label for the source, matching the style +// used by the SDK's config.Source.String() (e.g. "DATABRICKS_HOST environment +// variable"). +// +// The label for StorageSourceConfig intentionally does not name a specific +// config file: callers that know the resolved path (e.g. auth describe) +// should append it themselves to match the SDK's "from config file" +// convention. The label for StorageSourceOverride is generic because no +// CLI command currently exposes a storage-mode flag; if one is added in +// the future, that command can replace the label at the call site. +func (s StorageSource) String() string { + switch s { + case StorageSourceOverride: + return "command-line override" + case StorageSourceEnvVar: + return EnvVar + " environment variable" + case StorageSourceConfig: + return "auth_storage in [__settings__] section of config file" + default: + return "default" + } +} + // ParseMode parses raw as a StorageMode. Whitespace is trimmed and matching // is case-insensitive. Empty or unrecognized input returns StorageModeUnknown; // callers decide whether that is an error (user-supplied value) or a @@ -70,32 +114,31 @@ func ResolveStorageMode(ctx context.Context, override StorageMode) (StorageMode, } // ResolveStorageModeWithSource is like ResolveStorageMode but also reports -// whether the resolved mode came from an explicit user choice (override flag, -// env var, or config) versus the built-in default. Callers use this to honor -// "I want secure" strictly: when the user explicitly asked for secure storage -// but it cannot be provided, the right move is to error out, not to silently -// downgrade. -func ResolveStorageModeWithSource(ctx context.Context, override StorageMode) (StorageMode, bool, error) { +// which precedence level produced the resolved mode. Callers use the source +// both to honor "I want secure" strictly (when source.Explicit() is true and +// secure cannot be provided, error out instead of silently downgrading) and +// to surface where the choice came from in user-facing output. +func ResolveStorageModeWithSource(ctx context.Context, override StorageMode) (StorageMode, StorageSource, error) { if override != StorageModeUnknown { - return override, true, nil + return override, StorageSourceOverride, nil } if raw := env.Get(ctx, EnvVar); raw != "" { mode, err := parseFromSource(raw, EnvVar) - return mode, true, err + return mode, StorageSourceEnvVar, err } configPath := env.Get(ctx, "DATABRICKS_CONFIG_FILE") raw, err := databrickscfg.GetConfiguredAuthStorage(ctx, configPath) if err != nil { - return "", false, fmt.Errorf("read auth_storage setting: %w", err) + return "", StorageSourceDefault, fmt.Errorf("read auth_storage setting: %w", err) } if raw != "" { mode, err := parseFromSource(raw, "auth_storage") - return mode, true, err + return mode, StorageSourceConfig, err } - return StorageModePlaintext, false, nil + return StorageModePlaintext, StorageSourceDefault, nil } func parseFromSource(raw, source string) (StorageMode, error) { diff --git a/libs/auth/storage/mode_test.go b/libs/auth/storage/mode_test.go index bec3e571eb..4c6cf0e161 100644 --- a/libs/auth/storage/mode_test.go +++ b/libs/auth/storage/mode_test.go @@ -131,36 +131,36 @@ func TestResolveStorageMode_SkipsConfigReadWhenOverrideOrEnvSet(t *testing.T) { func TestResolveStorageModeWithSource(t *testing.T) { cases := []struct { - name string - override StorageMode - envValue string - configBody string - wantMode StorageMode - wantExplicit bool - wantErrSub string + name string + override StorageMode + envValue string + configBody string + wantMode StorageMode + wantSource StorageSource + wantErrSub string }{ { - name: "default is not explicit", - wantMode: StorageModePlaintext, - wantExplicit: false, + name: "default", + wantMode: StorageModePlaintext, + wantSource: StorageSourceDefault, }, { - name: "override is explicit", - override: StorageModeSecure, - wantMode: StorageModeSecure, - wantExplicit: true, + name: "override", + override: StorageModeSecure, + wantMode: StorageModeSecure, + wantSource: StorageSourceOverride, }, { - name: "env is explicit", - envValue: "secure", - wantMode: StorageModeSecure, - wantExplicit: true, + name: "env", + envValue: "secure", + wantMode: StorageModeSecure, + wantSource: StorageSourceEnvVar, }, { - name: "config is explicit", - configBody: "[__settings__]\nauth_storage = secure\n", - wantMode: StorageModeSecure, - wantExplicit: true, + name: "config", + configBody: "[__settings__]\nauth_storage = secure\n", + wantMode: StorageModeSecure, + wantSource: StorageSourceConfig, }, { name: "invalid env is rejected", @@ -183,7 +183,7 @@ func TestResolveStorageModeWithSource(t *testing.T) { t.Setenv("DATABRICKS_CONFIG_FILE", cfgPath) t.Setenv(EnvVar, tc.envValue) - mode, explicit, err := ResolveStorageModeWithSource(t.Context(), tc.override) + mode, source, err := ResolveStorageModeWithSource(t.Context(), tc.override) if tc.wantErrSub != "" { require.Error(t, err) assert.Contains(t, err.Error(), tc.wantErrSub) @@ -191,7 +191,21 @@ func TestResolveStorageModeWithSource(t *testing.T) { } require.NoError(t, err) assert.Equal(t, tc.wantMode, mode) - assert.Equal(t, tc.wantExplicit, explicit) + assert.Equal(t, tc.wantSource, source) }) } } + +func TestStorageSource_Explicit(t *testing.T) { + assert.False(t, StorageSourceDefault.Explicit()) + assert.True(t, StorageSourceOverride.Explicit()) + assert.True(t, StorageSourceEnvVar.Explicit()) + assert.True(t, StorageSourceConfig.Explicit()) +} + +func TestStorageSource_String(t *testing.T) { + assert.Equal(t, "default", StorageSourceDefault.String()) + assert.Equal(t, "command-line override", StorageSourceOverride.String()) + assert.Equal(t, "DATABRICKS_AUTH_STORAGE environment variable", StorageSourceEnvVar.String()) + assert.Equal(t, "auth_storage in [__settings__] section of config file", StorageSourceConfig.String()) +}