diff --git a/.nextchanges/cli/profile-fingerprint.md b/.nextchanges/cli/profile-fingerprint.md new file mode 100644 index 00000000000..80a36266df0 --- /dev/null +++ b/.nextchanges/cli/profile-fingerprint.md @@ -0,0 +1 @@ +* Require a new OAuth login before reusing cached credentials after the corresponding profile configuration changes. ([#6427](https://github.com/databricks/cli/pull/6427)) diff --git a/acceptance/cmd/auth/logout/stale-account-id-workspace-host/output.txt b/acceptance/cmd/auth/logout/stale-account-id-workspace-host/output.txt index 8c3218ad06e..b8774acbe4c 100644 --- a/acceptance/cmd/auth/logout/stale-account-id-workspace-host/output.txt +++ b/acceptance/cmd/auth/logout/stale-account-id-workspace-host/output.txt @@ -19,9 +19,9 @@ Logged out of profile "logfood". Use --delete to also remove it from the config [DEFAULT] [logfood] -host = [DATABRICKS_URL] +host = [DATABRICKS_URL] account_id = stale-account -auth_type = databricks-cli +auth_type = databricks-cli [__settings__] default_profile = logfood diff --git a/acceptance/cmd/auth/logout/stale-account-id-workspace-host/script b/acceptance/cmd/auth/logout/stale-account-id-workspace-host/script index bd477db2e29..69e0f11ef13 100644 --- a/acceptance/cmd/auth/logout/stale-account-id-workspace-host/script +++ b/acceptance/cmd/auth/logout/stale-account-id-workspace-host/script @@ -1,44 +1,40 @@ sethome "./home" +export BROWSER="browser.py" +export DATABRICKS_AUTH_STORAGE=plaintext +# Setup: create a normal workspace profile and log in. cat > "./home/.databrickscfg" < "./home/.databricks/token-cache.json" </dev/null 2>&1 +# Confirm that login created a valid profile and both cached token entries. title "Profiles before logout — logfood should be valid" trace $CLI auth profiles title "Token cache keys before logout\n" jq -S '.tokens | keys' "./home/.databricks/token-cache.json" +# Finish setup by adding the stale account routing that caused the regression. +sed -i.bak '/^host/a\ +account_id = stale-account +' "./home/.databrickscfg" +sed -i.bak '/workspace_id/d' "./home/.databrickscfg" + +# Test: logout must still find the tokens using the workspace host. title "Logout without --delete" trace $CLI auth logout --profile logfood --auto-approve +# Validation: logout preserves the profile and removes both cached entries. title "Config after logout — profile should still exist\n" cat "./home/.databrickscfg" diff --git a/acceptance/cmd/auth/profile-change/out.test.toml b/acceptance/cmd/auth/profile-change/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/cmd/auth/profile-change/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/cmd/auth/profile-change/output.txt b/acceptance/cmd/auth/profile-change/output.txt new file mode 100644 index 00000000000..346c2639b4f --- /dev/null +++ b/acceptance/cmd/auth/profile-change/output.txt @@ -0,0 +1,6 @@ + +>>> [CLI] auth login --host [DATABRICKS_URL] --profile fingerprint-test --scopes jobs +Profile fingerprint-test was successfully saved + +>>> musterr [CLI] clusters list --profile fingerprint-test +Error: error getting token: cache: profile "fingerprint-test" has changed since the last login; run `databricks auth login --profile "fingerprint-test"` to sign in again diff --git a/acceptance/cmd/auth/profile-change/script b/acceptance/cmd/auth/profile-change/script new file mode 100644 index 00000000000..b4486616f4f --- /dev/null +++ b/acceptance/cmd/auth/profile-change/script @@ -0,0 +1,13 @@ +# Ordinary workspace commands must reject a cached token after its profile changes. +sethome "./home" +export BROWSER="browser.py" +export DATABRICKS_AUTH_STORAGE=plaintext + +# Login creates a cached token bound to the saved profile. +trace $CLI auth login --host $DATABRICKS_HOST --profile fingerprint-test --scopes jobs + +# Simulate changing an authentication-related profile field by hand. +sed -i.bak 's/scopes = jobs/scopes = all-apis,sql/' "./home/.databrickscfg" + +# An ordinary workspace command must reject the token bound to the old profile. +trace musterr $CLI clusters list --profile fingerprint-test diff --git a/acceptance/cmd/auth/profile-change/test.toml b/acceptance/cmd/auth/profile-change/test.toml new file mode 100644 index 00000000000..77a03b9e28f --- /dev/null +++ b/acceptance/cmd/auth/profile-change/test.toml @@ -0,0 +1,5 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + "home", +] diff --git a/acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/script b/acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/script index b1a76cafb1e..3bb02f1df5c 100644 --- a/acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/script +++ b/acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/script @@ -1,4 +1,11 @@ +# Create the profile and a valid profile-bound token that can be refreshed. setup_test_profile setup_test_token_cache +# Replace the valid refresh token with one the fake OIDC server rejects. +jq '.tokens["test-profile"].refresh_token = "invalid-refresh-token"' \ + "./home/.databricks/token-cache.json" > "./token-cache.json" +mv "./token-cache.json" "./home/.databricks/token-cache.json" + +# The CLI must report that the user needs to log in again. musterr $CLI auth token --profile test-profile --force-refresh diff --git a/acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/test.toml b/acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/test.toml deleted file mode 100644 index 95067d76b72..00000000000 --- a/acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/test.toml +++ /dev/null @@ -1,4 +0,0 @@ -[[Server]] -Pattern = "POST /oidc/v1/token" -Response.StatusCode = 401 -Response.Body = '{"error": "invalid_request", "error_description": "Refresh token is invalid"}' diff --git a/acceptance/cmd/auth/token/force-refresh-success/script b/acceptance/cmd/auth/token/force-refresh-success/script index 6f4dda4b572..fc88c0a0e9b 100644 --- a/acceptance/cmd/auth/token/force-refresh-success/script +++ b/acceptance/cmd/auth/token/force-refresh-success/script @@ -1,3 +1,4 @@ +# Create the profile and a valid profile-bound token that can be refreshed. setup_test_profile setup_test_token_cache diff --git a/acceptance/cmd/auth/token/legacy-profile-fingerprint/out.test.toml b/acceptance/cmd/auth/token/legacy-profile-fingerprint/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/cmd/auth/token/legacy-profile-fingerprint/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/cmd/auth/token/legacy-profile-fingerprint/output.txt b/acceptance/cmd/auth/token/legacy-profile-fingerprint/output.txt new file mode 100644 index 00000000000..cc13249b015 --- /dev/null +++ b/acceptance/cmd/auth/token/legacy-profile-fingerprint/output.txt @@ -0,0 +1,3 @@ + +>>> musterr [CLI] auth token --profile test-profile +Error: cache: cached credentials for profile "test-profile" predate profile change detection; run `databricks auth login --profile "test-profile"` to sign in again diff --git a/acceptance/cmd/auth/token/legacy-profile-fingerprint/script b/acceptance/cmd/auth/token/legacy-profile-fingerprint/script new file mode 100644 index 00000000000..25e34728aba --- /dev/null +++ b/acceptance/cmd/auth/token/legacy-profile-fingerprint/script @@ -0,0 +1,10 @@ +# A token without fingerprint metadata must require a new login. +# Create the profile that the legacy cached token belongs to. +setup_test_profile + +# Install a cache written by an older CLI that omits profile_fingerprint. +mkdir -p "./home/.databricks" +cp "./token-cache.json" "./home/.databricks/token-cache.json" + +# The CLI must reject a cached token whose profile binding cannot be verified. +trace musterr $CLI auth token --profile test-profile diff --git a/acceptance/cmd/auth/token/legacy-profile-fingerprint/test.toml b/acceptance/cmd/auth/token/legacy-profile-fingerprint/test.toml new file mode 100644 index 00000000000..9609e1af299 --- /dev/null +++ b/acceptance/cmd/auth/token/legacy-profile-fingerprint/test.toml @@ -0,0 +1 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/cmd/auth/token/legacy-profile-fingerprint/token-cache.json b/acceptance/cmd/auth/token/legacy-profile-fingerprint/token-cache.json new file mode 100644 index 00000000000..c8cb0ec7a42 --- /dev/null +++ b/acceptance/cmd/auth/token/legacy-profile-fingerprint/token-cache.json @@ -0,0 +1,11 @@ +{ + "version": 1, + "tokens": { + "test-profile": { + "access_token": "cached-access-token", + "token_type": "Bearer", + "refresh_token": "test-refresh-token", + "expiry": "2099-01-01T00:00:00Z" + } + } +} diff --git a/acceptance/cmd/auth/token/profile-change/out.test.toml b/acceptance/cmd/auth/token/profile-change/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/cmd/auth/token/profile-change/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/cmd/auth/token/profile-change/output.txt b/acceptance/cmd/auth/token/profile-change/output.txt new file mode 100644 index 00000000000..5440be048e0 --- /dev/null +++ b/acceptance/cmd/auth/token/profile-change/output.txt @@ -0,0 +1,7 @@ + +>>> [CLI] auth login --host [DATABRICKS_URL] --profile fingerprint-test --scopes jobs +Profile fingerprint-test was successfully saved + +>>> musterr [CLI] auth token --profile fingerprint-test +Error: cache: profile "fingerprint-test" has changed since the last login; run `databricks auth login --profile "fingerprint-test"` to sign in again +Token cache unchanged diff --git a/acceptance/cmd/auth/token/profile-change/script b/acceptance/cmd/auth/token/profile-change/script new file mode 100644 index 00000000000..d06a36b9e11 --- /dev/null +++ b/acceptance/cmd/auth/token/profile-change/script @@ -0,0 +1,23 @@ +# A cached token must be rejected without mutation after its profile changes. +sethome "./home" +export BROWSER="browser.py" +export DATABRICKS_AUTH_STORAGE=plaintext + +# Login creates a cached token bound to the saved profile. +trace $CLI auth login --host $DATABRICKS_HOST --profile fingerprint-test --scopes jobs + +# Keep the original cache so we can verify that rejection does not modify it. +cp "./home/.databricks/token-cache.json" "./token-cache.before.json" + +# Simulate changing an authentication-related profile field by hand. +sed -i.bak 's/scopes = jobs/scopes = all-apis,sql/' "./home/.databrickscfg" + +# auth token must reject the token bound to the old profile. +trace musterr $CLI auth token --profile fingerprint-test + +# Rejecting the stale token must leave the cached credentials unchanged. +if cmp -s "./token-cache.before.json" "./home/.databricks/token-cache.json"; then + echo "Token cache unchanged" +else + echo "Token cache changed" +fi diff --git a/acceptance/cmd/auth/token/profile-change/test.toml b/acceptance/cmd/auth/token/profile-change/test.toml new file mode 100644 index 00000000000..5be07f22387 --- /dev/null +++ b/acceptance/cmd/auth/token/profile-change/test.toml @@ -0,0 +1,6 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + "home", + "token-cache.before.json", +] diff --git a/acceptance/cmd/auth/token/script.prepare b/acceptance/cmd/auth/token/script.prepare index afdf008034d..51c82053143 100644 --- a/acceptance/cmd/auth/token/script.prepare +++ b/acceptance/cmd/auth/token/script.prepare @@ -1,4 +1,5 @@ setup_test_profile() { + # Preserve the test server host before clearing ambient profile selection. export DATABRICKS_HOST_ORIG="$DATABRICKS_HOST" sethome "./home" @@ -6,6 +7,7 @@ setup_test_profile() { unset DATABRICKS_TOKEN unset DATABRICKS_CONFIG_PROFILE + # Write the minimal OAuth profile used by token-cache tests. cat > "./home/.databrickscfg" < "./home/.databricks/token-cache.json" </dev/null 2>&1 + + # Change only the token fields needed to control refresh behavior in each test. + jq '.tokens["test-profile"] += { + "access_token": "cached-access-token", + "refresh_token": "test-refresh-token", + "expiry": "2099-01-01T00:00:00Z" + }' "./home/.databricks/token-cache.json" > "./token-cache.json" + mv "./token-cache.json" "./home/.databricks/token-cache.json" } diff --git a/cmd/auth/in_memory_test.go b/cmd/auth/in_memory_test.go index 3c11a40e4fb..f9a67e9ef34 100644 --- a/cmd/auth/in_memory_test.go +++ b/cmd/auth/in_memory_test.go @@ -6,7 +6,8 @@ import ( ) type inMemoryStore struct { - Tokens map[string]*oauth2.Token + Tokens map[string]*oauth2.Token + Fingerprints map[string]string } // Lookup returns a copy to match real (file-backed) cache behavior, where @@ -19,7 +20,10 @@ func (i *inMemoryStore) Lookup(key string) (storage.Entry, error) { return storage.Entry{}, storage.ErrNotFound } cp := *token - return storage.Entry{Token: &cp}, nil + return storage.Entry{ + Token: &cp, + ProfileFingerprint: i.Fingerprints[key], + }, nil } // Put stores a copy to prevent callers from mutating cached entries after @@ -27,6 +31,13 @@ func (i *inMemoryStore) Lookup(key string) (storage.Entry, error) { func (i *inMemoryStore) Put(key string, e storage.Entry) error { cp := *e.Token i.Tokens[key] = &cp + + if i.Fingerprints == nil { + i.Fingerprints = make(map[string]string) + } + + i.Fingerprints[key] = e.ProfileFingerprint + return nil } @@ -34,6 +45,7 @@ func (i *inMemoryStore) Put(key string, e storage.Entry) error { // an error. func (i *inMemoryStore) Delete(key string) error { delete(i.Tokens, key) + delete(i.Fingerprints, key) return nil } diff --git a/cmd/auth/login.go b/cmd/auth/login.go index 5835297b335..186ca2d5b29 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -17,6 +17,7 @@ import ( "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/cli/libs/databrickscfg/cfgpickers" "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/databrickscfg/profilehash" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" @@ -88,6 +89,29 @@ func (d *defaultDiscoveryClient) IntrospectToken(ctx context.Context, host, acce return auth.IntrospectToken(ctx, host, accessToken, nil) } +// setTokenProfileFingerprint runs after profile saving because OAuth-dependent +// workspace and compute selection can change the final profile contents. +func setTokenProfileFingerprint(ctx context.Context, profiler profile.Profiler, tokenStore storage.Store, profileName string) error { + savedProfile, err := loadProfileByName(ctx, profileName, profiler) + if err != nil { + return fmt.Errorf("load saved profile %q: %w", profileName, err) + } + if savedProfile == nil { + return fmt.Errorf("saved profile %q not found", profileName) + } + + fingerprint, err := profilehash.Compute(*savedProfile) + if err != nil { + return fmt.Errorf("compute profile fingerprint: %w", err) + } + + if err := storage.SetProfileFingerprint(tokenStore, profileName, fingerprint); err != nil { + return fmt.Errorf("save profile fingerprint: %w", err) + } + + return nil +} + func newLoginCommand(authArguments *auth.AuthArguments) *cobra.Command { defaultConfigPath := "~/.databrickscfg" if runtime.GOOS == "windows" { @@ -398,6 +422,10 @@ a new profile is created. return err } + if err := setTokenProfileFingerprint(ctx, profile.DefaultProfiler, tokenStore, profileName); err != nil { + return err + } + cmdio.LogString(ctx, fmt.Sprintf("Profile %s was successfully saved", profileName)) } @@ -758,6 +786,10 @@ func discoveryLogin(ctx context.Context, in discoveryLoginInputs) error { return fmt.Errorf("saving profile %q: %w", in.profileName, err) } + if err := setTokenProfileFingerprint(ctx, profile.DefaultProfiler, in.tokenStore, in.profileName); err != nil { + return err + } + cmdio.LogString(ctx, fmt.Sprintf("Profile %s was successfully saved", in.profileName)) return nil } diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index cd9e20bc234..4fa2b269481 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -19,6 +19,7 @@ import ( "github.com/databricks/cli/libs/auth/u2m" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/databrickscfg/profilehash" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" @@ -30,7 +31,10 @@ import ( // newTestStore returns an in-memory token cache for tests so that // discoveryLogin and other login helpers don't touch ~/.databricks/token-cache.json. func newTestStore() storage.Store { - return &inMemoryStore{Tokens: map[string]*oauth2.Token{}} + // Prepopulate the entry because the fake Challenge does not perform the real OAuth cache write. + return &inMemoryStore{Tokens: map[string]*oauth2.Token{ + "DISCOVERY": {AccessToken: "test-token"}, + }} } // logBuffer is a thread-safe bytes.Buffer for capturing log output in tests. @@ -806,6 +810,29 @@ func TestDiscoveryLogin_IntrospectionFailureStillSavesProfile(t *testing.T) { assert.Empty(t, savedProfile.WorkspaceID) } +// TestSetTokenProfileFingerprint verifies that a cached token is bound to the +// profile saved by login. +func TestSetTokenProfileFingerprint(t *testing.T) { + savedProfile := profile.Profile{ + Name: "DISCOVERY", + Host: "https://workspace.example.test", + } + profiler := profile.InMemoryProfiler{ + Profiles: profile.Profiles{savedProfile}, + } + tokenStore := newTestStore() + + err := setTokenProfileFingerprint(t.Context(), profiler, tokenStore, savedProfile.Name) + require.NoError(t, err) + + want, err := profilehash.Compute(savedProfile) + require.NoError(t, err) + entry, err := tokenStore.Lookup(savedProfile.Name) + require.NoError(t, err) + + assert.Equal(t, want, entry.ProfileFingerprint) +} + func TestDiscoveryLogin_AccountIDMismatchWarning(t *testing.T) { tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, ".databrickscfg") diff --git a/cmd/auth/token.go b/cmd/auth/token.go index dc8f54ecfc3..6b64ea76951 100644 --- a/cmd/auth/token.go +++ b/cmd/auth/token.go @@ -18,6 +18,7 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/databrickscfg/profilehash" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/log" @@ -171,6 +172,9 @@ func loadToken(ctx context.Context, args loadTokenArgs) (*oauth2.Token, error) { if err != nil { return nil, err } + if args.profileName != "" && existingProfile == nil { + return nil, fmt.Errorf("%w: %q", errNoProfileFound, args.profileName) + } // When no explicit profile, host, or positional args are provided, attempt to // resolve the target through environment variables or interactive profile selection. @@ -264,13 +268,24 @@ func loadToken(ctx context.Context, args loadTokenArgs) (*oauth2.Token, error) { if err != nil { return nil, err } - allArgs := append([]u2m.PersistentAuthOption{u2m.WithTokenCache(storage.OAuthTokenCache(ctx, args.tokenStore, args.mode))}, args.persistentAuthOpts...) + + tokenStore := args.tokenStore + if existingProfile != nil { + fingerprint, err := profilehash.Compute(*existingProfile) + if err != nil { + return nil, fmt.Errorf("compute profile fingerprint: %w", err) + } + tokenStore = storage.NewProfileFingerprintStore(tokenStore, existingProfile.Name, fingerprint) + } + + allArgs := append([]u2m.PersistentAuthOption{u2m.WithTokenCache(storage.OAuthTokenCache(ctx, tokenStore, args.mode))}, args.persistentAuthOpts...) allArgs = append(allArgs, u2m.WithOAuthArgument(oauthArgument)) persistentAuth, err := u2m.NewPersistentAuth(ctx, allArgs...) if err != nil { helpMsg := helpfulError(ctx, args.profileName, oauthArgument) return nil, fmt.Errorf("%w. %s", err, helpMsg) } + var t *oauth2.Token if args.forceRefresh { t, err = persistentAuth.ForceRefreshToken() @@ -278,6 +293,11 @@ func loadToken(ctx context.Context, args loadTokenArgs) (*oauth2.Token, error) { t, err = persistentAuth.Token() } if err != nil { + // Fingerprint errors already include the exact login command needed to + // replace the stale grant, so the generic recovery suffix would duplicate it. + if errors.Is(err, storage.ErrProfileChanged) { + return nil, err + } if errors.Is(err, cache.ErrNotFound) { // The error returned by the SDK when the token cache doesn't exist or doesn't contain a token // for the given host changed in SDK v0.77.0: https://github.com/databricks/databricks-sdk-go/pull/1250. @@ -463,6 +483,10 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler, tokenStore s return "", nil, err } + if err := setTokenProfileFingerprint(ctx, profiler, tokenStore, profileName); err != nil { + return "", nil, err + } + cmdio.LogString(ctx, fmt.Sprintf("Profile %s was successfully saved", profileName)) p, err := loadProfileByName(ctx, profileName, profiler) diff --git a/cmd/auth/token_test.go b/cmd/auth/token_test.go index 11a5a937bd9..b99d4bb2f30 100644 --- a/cmd/auth/token_test.go +++ b/cmd/auth/token_test.go @@ -14,9 +14,11 @@ import ( "github.com/databricks/cli/libs/auth/u2m" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/databrickscfg/profilehash" "github.com/databricks/cli/libs/env" "github.com/databricks/databricks-sdk-go/httpclient/fixtures" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/oauth2" ) @@ -36,6 +38,117 @@ func (upgradeHintStore) Lookup(string) (storage.Entry, error) { ) } +func newProfileFingerprintTokenArgs(t *testing.T, loggedInProfile, currentProfile profile.Profile, forceRefresh bool) loadTokenArgs { + t.Helper() + + fingerprint, err := profilehash.Compute(loggedInProfile) + require.NoError(t, err) + profileName := currentProfile.Name + + tokenStore := &inMemoryStore{ + Tokens: map[string]*oauth2.Token{ + profileName: { + AccessToken: "jobs-token", + RefreshToken: "jobs-refresh-token", + Expiry: time.Now().Add(time.Hour), + }, + }, + Fingerprints: map[string]string{profileName: fingerprint}, + } + + return loadTokenArgs{ + authArguments: &auth.AuthArguments{}, + profileName: profileName, + tokenTimeout: time.Minute, + profiler: profile.InMemoryProfiler{Profiles: profile.Profiles{ + currentProfile, + }}, + tokenStore: tokenStore, + forceRefresh: forceRefresh, + } +} + +// TestLoadTokenAcceptsMatchingProfileFingerprint verifies that a cached token +// remains usable while its profile is unchanged. +func TestLoadTokenAcceptsMatchingProfileFingerprint(t *testing.T) { + loggedInProfile := profile.Profile{ + Name: "TEST", + Host: "https://workspace.example.test", + Scopes: "jobs", + AuthType: "databricks-cli", + } + currentProfile := loggedInProfile + args := newProfileFingerprintTokenArgs(t, loggedInProfile, currentProfile, false) + + got, err := loadToken(cmdio.MockDiscard(t.Context()), args) + require.NoError(t, err) + assert.Equal(t, "jobs-token", got.AccessToken) +} + +// TestLoadTokenRejectsChangedProfile verifies that cached credentials cannot +// be reused or refreshed after the profile changes. +func TestLoadTokenRejectsChangedProfile(t *testing.T) { + tests := []struct { + name string + forceRefresh bool + }{ + { + name: "reuse", + }, + { + name: "force refresh", + forceRefresh: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + loggedInProfile := profile.Profile{ + Name: "TEST", + Host: "https://workspace.example.test", + Scopes: "jobs", + AuthType: "databricks-cli", + } + currentProfile := profile.Profile{ + Name: "TEST", + Host: "https://workspace.example.test", + Scopes: "all-apis,sql", + AuthType: "databricks-cli", + } + args := newProfileFingerprintTokenArgs(t, loggedInProfile, currentProfile, tt.forceRefresh) + + _, err := loadToken(cmdio.MockDiscard(t.Context()), args) + + assert.ErrorIs(t, err, storage.ErrProfileChanged) + assert.ErrorContains(t, err, `profile "TEST" has changed since the last login`) + }) + } +} + +// TestLoadTokenRejectsMissingNamedProfile verifies that an orphaned cached +// token cannot be loaded after its profile is removed. +func TestLoadTokenRejectsMissingNamedProfile(t *testing.T) { + args := loadTokenArgs{ + authArguments: &auth.AuthArguments{ + Host: "https://workspace.example.test", + }, + profileName: "TEST", + tokenTimeout: time.Minute, + profiler: profile.InMemoryProfiler{}, + tokenStore: &inMemoryStore{Tokens: map[string]*oauth2.Token{ + "TEST": { + AccessToken: "orphaned-token", + Expiry: time.Now().Add(time.Hour), + }, + }}, + } + + _, err := loadToken(cmdio.MockDiscard(t.Context()), args) + + assert.ErrorIs(t, err, errNoProfileFound) + assert.ErrorContains(t, err, `"TEST"`) +} + var _ storage.Store = upgradeHintStore{} type failOnCallTransport struct{} diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index 5e5173ea151..375de74c91f 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -12,12 +12,16 @@ import ( "github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/auth/storage" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/databrickscfg/profilehash" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/oauth2" ) type roundTripperFunc func(*http.Request) (*http.Response, error) @@ -273,15 +277,29 @@ func TestMustWorkspaceClientRewritesInvalidRefreshTokenForPickedProfile(t *testi // Expired cached token (keyed by profile name) so the command triggers a // refresh, which the server rejects. - require.NoError(t, os.MkdirAll(filepath.Join(home, ".databricks"), 0o700)) - require.NoError(t, os.WriteFile(filepath.Join(home, ".databricks", "token-cache.json"), - []byte(`{"version":1,"tokens":{"only-workspace":{"access_token":"x","token_type":"Bearer","refresh_token":"rt","expiry":"2020-01-01T00:00:00Z"}}}`), 0o600)) + fingerprint, err := profilehash.Compute(profile.Profile{ + Name: "only-workspace", + Host: server.URL, + AuthType: "databricks-cli", + }) + require.NoError(t, err) + tokenStore, err := storage.NewFileStore(t.Context()) + require.NoError(t, err) + require.NoError(t, tokenStore.Put("only-workspace", storage.Entry{ + Token: &oauth2.Token{ + AccessToken: "x", + TokenType: "Bearer", + RefreshToken: "rt", + Expiry: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), + }, + ProfileFingerprint: fingerprint, + })) ctx, tt := cmdio.SetupTest(t.Context(), cmdio.TestOptions{PromptSupported: true}) t.Cleanup(tt.Done) cmd := New(ctx) - err := MustWorkspaceClient(cmd, []string{}) + err = MustWorkspaceClient(cmd, []string{}) require.Error(t, err) assert.Contains(t, err.Error(), "A new access token could not be retrieved because the refresh token is invalid") assert.Contains(t, err.Error(), "databricks auth login --profile only-workspace") diff --git a/libs/auth/arguments.go b/libs/auth/arguments.go index deac0b5b1cc..e87c0d9eeb8 100644 --- a/libs/auth/arguments.go +++ b/libs/auth/arguments.go @@ -2,12 +2,13 @@ package auth import ( "github.com/databricks/cli/libs/auth/u2m" + "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/databricks-sdk-go/config" ) // WorkspaceIDNone is a sentinel value persisted to .databrickscfg when the // user explicitly skips workspace selection for SPOG account-level access. -const WorkspaceIDNone = "none" +const WorkspaceIDNone = databrickscfg.WorkspaceIDNone // AuthArguments is a struct that contains the common arguments passed to // `databricks auth` commands. diff --git a/libs/auth/credentials.go b/libs/auth/credentials.go index 10736a19bd1..246e2ef9587 100644 --- a/libs/auth/credentials.go +++ b/libs/auth/credentials.go @@ -3,9 +3,12 @@ package auth import ( "context" "errors" + "fmt" "github.com/databricks/cli/libs/auth/storage" "github.com/databricks/cli/libs/auth/u2m" + "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/databrickscfg/profilehash" "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/config/credentials" "github.com/databricks/databricks-sdk-go/config/experimental/auth" @@ -107,6 +110,21 @@ func (c CLICredentials) Configure(ctx context.Context, cfg *config.Config) (cred if err != nil { return nil, err } + + // Fingerprints bind profile-keyed OAuth tokens to the profile that created + // them. Without a profile name, there is no saved profile to validate and no + // profile-keyed cache entry to wrap. Leave the store unchanged so existing + // profile-less databricks-cli authentication can still look up a legacy + // host-keyed token. PAT and M2M authentication do not reach this strategy; + // they are handled earlier in the credential chain. + if cfg.Profile != "" { + fingerprint, err := profilehash.Compute(profile.FromConfig(cfg)) + if err != nil { + return nil, fmt.Errorf("compute profile fingerprint: %w", err) + } + tokenStore = storage.NewProfileFingerprintStore(tokenStore, cfg.Profile, fingerprint) + } + ts, err := c.persistentAuth(ctx, u2m.WithOAuthArgument(oauthArg), u2m.WithTokenCache(storage.OAuthTokenCache(ctx, tokenStore, mode)), @@ -114,6 +132,7 @@ func (c CLICredentials) Configure(ctx context.Context, cfg *config.Config) (cred if err != nil { return nil, err } + cp := credentials.NewOAuthCredentialsProviderFromTokenSource( auth.NewCachedTokenSource(ts, auth.WithAsyncRefresh(!cfg.DisableOAuthRefreshToken)), ) diff --git a/libs/auth/credentials_test.go b/libs/auth/credentials_test.go index 1c6fceac2d7..712114311d9 100644 --- a/libs/auth/credentials_test.go +++ b/libs/auth/credentials_test.go @@ -205,6 +205,28 @@ func TestCLICredentialsConfigure(t *testing.T) { } } +// TestCLICredentialsConfigureUsesResolvedProfile verifies that fingerprinting +// does not try to load the profile from the configuration file again. +func TestCLICredentialsConfigureUsesResolvedProfile(t *testing.T) { + hermeticAuthStorage(t) + + c := CLICredentials{ + persistentAuthFn: func(_ context.Context, _ ...u2m.PersistentAuthOption) (auth.TokenSource, error) { + return auth.TokenSourceFn(func(_ context.Context) (*oauth2.Token, error) { + return &oauth2.Token{AccessToken: "token"}, nil + }), nil + }, + } + cfg := &config.Config{ + Profile: "TEST", + Host: "https://workspace.example.test", + AuthType: "databricks-cli", + } + + _, err := c.Configure(t.Context(), cfg) + require.NoError(t, err) +} + // TestCLICredentialsConfigure_ThreadsResolvedTokenCache guards against a // regression where Configure forgot to pass u2m.WithTokenCache. Without it, // the SDK's NewPersistentAuth silently defaulted to the file cache, so users diff --git a/libs/auth/storage/filestore.go b/libs/auth/storage/filestore.go index 328d7d466c6..f0df53327a2 100644 --- a/libs/auth/storage/filestore.go +++ b/libs/auth/storage/filestore.go @@ -38,7 +38,8 @@ const ( // "access_token": "", // "token_type": "", // "refresh_token": "", - // "expiry": "" + // "expiry": "", + // "profile_fingerprint": "" // } // } // } @@ -50,6 +51,7 @@ const ( // the historical bare-token format, leaving room for additive sibling fields. type fileEntry struct { *oauth2.Token + ProfileFingerprint string `json:"profile_fingerprint,omitempty"` } // tokenStoreFile is the format of the token store file. @@ -106,7 +108,10 @@ func (c *fileStore) Put(key string, e Entry) error { if f.Tokens == nil { f.Tokens = map[string]*fileEntry{} } - f.Tokens[key] = &fileEntry{Token: e.Token} + f.Tokens[key] = &fileEntry{ + Token: e.Token, + ProfileFingerprint: e.ProfileFingerprint, + } return c.write(f) } @@ -122,7 +127,10 @@ func (c *fileStore) Lookup(key string) (Entry, error) { if !ok { return Entry{}, ErrNotFound } - return Entry{Token: fe.Token}, nil + return Entry{ + Token: fe.Token, + ProfileFingerprint: fe.ProfileFingerprint, + }, nil } // Delete implements the Store interface. Removing a missing key is a no-op. diff --git a/libs/auth/storage/filestore_test.go b/libs/auth/storage/filestore_test.go index d905e304839..a22d2527dff 100644 --- a/libs/auth/storage/filestore_test.go +++ b/libs/auth/storage/filestore_test.go @@ -18,9 +18,12 @@ func setup(t *testing.T) string { func TestStoreAndLookup(t *testing.T) { c, err := NewFileStore(t.Context(), WithFileLocation(setup(t))) require.NoError(t, err) - err = c.Put("x", Entry{Token: &oauth2.Token{ - AccessToken: "abc", - }}) + err = c.Put("x", Entry{ + Token: &oauth2.Token{ + AccessToken: "abc", + }, + ProfileFingerprint: "fingerprint-x", + }) require.NoError(t, err) err = c.Put("y", Entry{Token: &oauth2.Token{ @@ -31,6 +34,7 @@ func TestStoreAndLookup(t *testing.T) { got, err := c.Lookup("x") require.NoError(t, err) assert.Equal(t, "abc", got.Token.AccessToken) + assert.Equal(t, "fingerprint-x", got.ProfileFingerprint) _, err = c.Lookup("z") assert.Equal(t, ErrNotFound, err) diff --git a/libs/auth/storage/keyring.go b/libs/auth/storage/keyring.go index ddc41648abc..b51c1c497ba 100644 --- a/libs/auth/storage/keyring.go +++ b/libs/auth/storage/keyring.go @@ -47,7 +47,8 @@ type keyringBackend interface { // checksum, store time, ...) without breaking older CLI versions that read // the same entry. type keyringEntry struct { - Token *oauth2.Token `json:"token"` + Token *oauth2.Token `json:"token"` + ProfileFingerprint string `json:"profile_fingerprint,omitempty"` } // zalandoBackend delegates to the process-wide zalando/go-keyring provider. diff --git a/libs/auth/storage/keyring_test.go b/libs/auth/storage/keyring_test.go index 9a2074931aa..2d547b9cf78 100644 --- a/libs/auth/storage/keyring_test.go +++ b/libs/auth/storage/keyring_test.go @@ -82,7 +82,10 @@ func TestKeyringStore_Store_WritesJSON(t *testing.T) { tok := &oauth2.Token{AccessToken: "abc", TokenType: "Bearer"} - require.NoError(t, c.Put("my-profile", Entry{Token: tok})) + require.NoError(t, c.Put("my-profile", Entry{ + Token: tok, + ProfileFingerprint: "profile-fingerprint", + })) stored, ok := backend.items[itemKey("databricks-cli", "my-profile")] require.True(t, ok, "token should be stored under service=databricks-cli, account=my-profile") @@ -92,6 +95,7 @@ func TestKeyringStore_Store_WritesJSON(t *testing.T) { require.NotNil(t, got.Token) assert.Equal(t, "abc", got.Token.AccessToken) assert.Equal(t, "Bearer", got.Token.TokenType) + assert.Equal(t, "profile-fingerprint", got.ProfileFingerprint) } func TestKeyringStore_Store_PropagatesBackendError(t *testing.T) { @@ -110,12 +114,16 @@ func TestKeyringStore_Lookup_ReturnsStoredToken(t *testing.T) { c := newTestStore(backend) want := &oauth2.Token{AccessToken: "abc", TokenType: "Bearer"} - require.NoError(t, c.Put("my-profile", Entry{Token: want})) + require.NoError(t, c.Put("my-profile", Entry{ + Token: want, + ProfileFingerprint: "profile-fingerprint", + })) got, err := c.Lookup("my-profile") require.NoError(t, err) assert.Equal(t, "abc", got.Token.AccessToken) assert.Equal(t, "Bearer", got.Token.TokenType) + assert.Equal(t, "profile-fingerprint", got.ProfileFingerprint) } func TestKeyringStore_Lookup_MissingReturnsCacheErrNotFound(t *testing.T) { diff --git a/libs/auth/storage/profile_fingerprint.go b/libs/auth/storage/profile_fingerprint.go new file mode 100644 index 00000000000..c01318d824e --- /dev/null +++ b/libs/auth/storage/profile_fingerprint.go @@ -0,0 +1,89 @@ +package storage + +import ( + "fmt" +) + +// ProfileFingerprintError tells the user why an otherwise valid cached token +// cannot be reused after the corresponding profile changed. +type ProfileFingerprintError struct { + Profile string + Missing bool +} + +func (e *ProfileFingerprintError) Error() string { + if e.Missing { + return fmt.Sprintf("cached credentials for profile %q predate profile change detection; run `databricks auth login --profile %q` to sign in again", e.Profile, e.Profile) + } + return fmt.Sprintf("profile %q has changed since the last login; run `databricks auth login --profile %q` to sign in again", e.Profile, e.Profile) +} + +func (e *ProfileFingerprintError) Unwrap() error { + return ErrProfileChanged +} + +// ProfileFingerprintStore stamps token writes and rejects reads whose metadata +// does not match the current profile. +type ProfileFingerprintStore struct { + inner Store + profile string + fingerprint string +} + +// NewProfileFingerprintStore binds token reads and refresh writes to fingerprint. +func NewProfileFingerprintStore(inner Store, profile, fingerprint string) *ProfileFingerprintStore { + return &ProfileFingerprintStore{ + inner: inner, + profile: profile, + fingerprint: fingerprint, + } +} + +func (s *ProfileFingerprintStore) Put(key string, entry Entry) error { + // The SDK replaces the entire token entry after a refresh, so stamp the + // binding again instead of losing it with the old access token. + entry.ProfileFingerprint = s.fingerprint + + return s.inner.Put(key, entry) +} + +func (s *ProfileFingerprintStore) Lookup(key string) (Entry, error) { + entry, err := s.inner.Lookup(key) + if err != nil { + return Entry{}, err + } + + if entry.ProfileFingerprint == "" { + return Entry{}, &ProfileFingerprintError{Profile: s.profile, Missing: true} + } + + if entry.ProfileFingerprint != s.fingerprint { + return Entry{}, &ProfileFingerprintError{Profile: s.profile} + } + + return entry, nil +} + +func (s *ProfileFingerprintStore) Delete(key string) error { + return s.inner.Delete(key) +} + +// SetProfileFingerprint binds an existing profile-keyed token to the profile +// left on disk after login. Legacy host-key copies are intentionally excluded +// because one host can be shared by multiple profiles. +func SetProfileFingerprint(store Store, profile, fingerprint string) error { + entry, err := store.Lookup(profile) + if err != nil { + return fmt.Errorf("load token %q: %w", profile, err) + } + + entry.ProfileFingerprint = fingerprint + + if err := store.Put(profile, entry); err != nil { + return fmt.Errorf("update token %q: %w", profile, err) + } + + return nil +} + +var _ Store = (*ProfileFingerprintStore)(nil) diff --git a/libs/auth/storage/profile_fingerprint_test.go b/libs/auth/storage/profile_fingerprint_test.go new file mode 100644 index 00000000000..0dda19fdfac --- /dev/null +++ b/libs/auth/storage/profile_fingerprint_test.go @@ -0,0 +1,89 @@ +package storage + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" +) + +// TestProfileFingerprintStoreLookup accepts an entry with the current fingerprint. +func TestProfileFingerprintStoreLookup(t *testing.T) { + inner := newMemStore() + store := NewProfileFingerprintStore(inner, "TEST", "current") + require.NoError(t, inner.Put("TEST", Entry{ + Token: &oauth2.Token{AccessToken: "token"}, + ProfileFingerprint: "current", + })) + + got, err := store.Lookup("TEST") + require.NoError(t, err) + assert.Equal(t, "token", got.Token.AccessToken) +} + +// TestProfileFingerprintStoreRejectsInvalidFingerprint verifies the distinct +// errors returned for changed and legacy cache entries. +func TestProfileFingerprintStoreRejectsInvalidFingerprint(t *testing.T) { + currentFingerprint := "current" + + tests := []struct { + name string + storedFingerprint string + wantMissing bool + }{ + { + name: "changed fingerprint", + storedFingerprint: "old", + }, + { + name: "missing legacy fingerprint", + wantMissing: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inner := newMemStore() + store := NewProfileFingerprintStore(inner, "TEST", currentFingerprint) + require.NoError(t, inner.Put("TEST", Entry{ + Token: &oauth2.Token{AccessToken: "token"}, + ProfileFingerprint: tt.storedFingerprint, + })) + + _, err := store.Lookup("TEST") + + assert.ErrorIs(t, err, ErrProfileChanged) + changedErr, ok := errors.AsType[*ProfileFingerprintError](err) + require.True(t, ok) + assert.Equal(t, tt.wantMissing, changedErr.Missing) + }) + } +} + +// TestProfileFingerprintStoreStampsWrites verifies that replacement token writes retain +// the fingerprint binding, as required when OAuth refresh replaces a cache entry. +func TestProfileFingerprintStoreStampsWrites(t *testing.T) { + inner := newMemStore() + store := NewProfileFingerprintStore(inner, "TEST", "current") + + require.NoError(t, store.Put("TEST", Entry{Token: &oauth2.Token{AccessToken: "token"}})) + assert.Equal(t, "current", inner.entries["TEST"].ProfileFingerprint) +} + +// TestSetProfileFingerprintOnlyUpdatesProfileKey verifies that login binds the +// profile-keyed token without binding the shared legacy host-keyed copy. +func TestSetProfileFingerprintOnlyUpdatesProfileKey(t *testing.T) { + inner := newMemStore() + + require.NoError(t, inner.Put("TEST", Entry{Token: &oauth2.Token{AccessToken: "token"}})) + require.NoError(t, inner.Put("https://workspace.example.com", Entry{Token: &oauth2.Token{AccessToken: "token"}})) + require.NoError(t, SetProfileFingerprint(inner, "TEST", "current")) + + assert.Equal(t, "current", inner.entries["TEST"].ProfileFingerprint) + + // A host can be shared by multiple profiles, so its compatibility copy is + // not bound to any one profile. + assert.Empty(t, inner.entries["https://workspace.example.com"].ProfileFingerprint) +} diff --git a/libs/auth/storage/storage.go b/libs/auth/storage/storage.go index dd208a3819e..a04c3686307 100644 --- a/libs/auth/storage/storage.go +++ b/libs/auth/storage/storage.go @@ -20,6 +20,10 @@ import ( // ToU2MTokenCache translates between the storage and OAuth layers. var ErrNotFound = errors.New("token not found") +// ErrProfileChanged identifies cached credentials that no longer correspond +// to the profile that selected them. +var ErrProfileChanged = errors.New("profile configuration changed") + // Entry is the value held in the CLI token store. It wraps the credential so // the schema can grow additive metadata (e.g. a config fingerprint, scopes) // without changing the Store interface. Backends persist it verbatim and never @@ -27,6 +31,10 @@ var ErrNotFound = errors.New("token not found") type Entry struct { // Token is the cached OAuth token. Always set for stored entries. Token *oauth2.Token + + // ProfileFingerprint binds the OAuth grant to the simplified profile that + // was present when the token was minted. + ProfileFingerprint string } // Store is the CLI's token-storage abstraction: a key/value store with no diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 732dcf9024f..dc982d5706d 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -94,6 +94,10 @@ func findMatchingProfile(configFile *config.File, matcher func(*ini.Section) boo // hostAttr is the SDK config attribute name for the workspace/account host. const hostAttr = "host" +// WorkspaceIDNone is a legacy sentinel for profiles that intentionally do not +// select a workspace. +const WorkspaceIDNone = "none" + // envAlwaysSkipAttrs lists env attributes envLoader must never read when a // profile is explicitly selected, in either pass: routing (workspace_id, // account_id) and auth-steering fields tagged auth:"-" (auth_type, discovery_url, diff --git a/libs/databrickscfg/profile/file_test.go b/libs/databrickscfg/profile/file_test.go index 8f6c5ad790c..424c1db0460 100644 --- a/libs/databrickscfg/profile/file_test.go +++ b/libs/databrickscfg/profile/file_test.go @@ -5,10 +5,40 @@ import ( "testing" "github.com/databricks/cli/libs/env" + "github.com/databricks/databricks-sdk-go/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// TestFromConfig verifies that fingerprint-relevant resolved configuration is +// represented by the simplified profile. +func TestFromConfig(t *testing.T) { + cfg := &config.Config{ + Profile: "TEST", + Host: "https://workspace.example.test", + AccountID: "account-id", + WorkspaceID: "workspace-id", + ClusterID: "cluster-id", + ServerlessComputeID: "serverless-compute-id", + ClientID: "client-id", + ClientSecret: "client-secret", + Scopes: []string{"all-apis", "sql"}, + AuthType: "databricks-cli", + } + + assert.Equal(t, Profile{ + Name: "TEST", + Host: "https://workspace.example.test", + AccountID: "account-id", + WorkspaceID: "workspace-id", + ClusterID: "cluster-id", + ServerlessComputeID: "serverless-compute-id", + HasClientCredentials: true, + Scopes: "all-apis,sql", + AuthType: "databricks-cli", + }, FromConfig(cfg)) +} + func TestProfileCloud(t *testing.T) { assert.Equal(t, "AWS", Profile{Host: "https://dbc-XXXXXXXX-YYYY.cloud.databricks.com"}.Cloud()) assert.Equal(t, "Azure", Profile{Host: "https://adb-xxx.y.azuredatabricks.net/"}.Cloud()) diff --git a/libs/databrickscfg/profile/profile.go b/libs/databrickscfg/profile/profile.go index efd358cd4e5..692658f977a 100644 --- a/libs/databrickscfg/profile/profile.go +++ b/libs/databrickscfg/profile/profile.go @@ -6,9 +6,11 @@ import ( "github.com/databricks/databricks-sdk-go/config" ) -// Profile holds a subset of the keys in a databrickscfg profile. -// It should only be used for prompting and filtering. -// Use its name to construct a config.Config. +// Profile is the simplified representation of a databrickscfg profile used for +// prompting, filtering, and cached-token fingerprints. Every field contributes +// to the fingerprint, so fields should only be added when changing them must +// invalidate cached credentials. Use the profile name to construct a complete +// config.Config. type Profile struct { Name string Host string @@ -21,6 +23,21 @@ type Profile struct { AuthType string } +// FromConfig returns the simplified profile represented by a resolved config. +func FromConfig(cfg *config.Config) Profile { + return Profile{ + Name: cfg.Profile, + Host: cfg.Host, + AccountID: cfg.AccountID, + WorkspaceID: cfg.WorkspaceID, + ClusterID: cfg.ClusterID, + ServerlessComputeID: cfg.ServerlessComputeID, + HasClientCredentials: cfg.ClientID != "" && cfg.ClientSecret != "", + Scopes: strings.Join(cfg.Scopes, ","), + AuthType: cfg.AuthType, + } +} + func (p Profile) Cloud() string { cfg := config.Config{Host: p.Host} switch { diff --git a/libs/databrickscfg/profile/profiler.go b/libs/databrickscfg/profile/profiler.go index 56cfdb5f522..aa38047e1ea 100644 --- a/libs/databrickscfg/profile/profiler.go +++ b/libs/databrickscfg/profile/profiler.go @@ -3,7 +3,7 @@ package profile import ( "context" - "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/databricks-sdk-go/config" ) @@ -13,7 +13,7 @@ func MatchWorkspaceProfiles(p Profile) bool { // Workspace profile: has workspace_id (covers both classic and SPOG profiles), // or is a regular workspace host (no account_id). // workspace_id = "none" is a sentinel for "skip workspace", so it does NOT count. - return (p.WorkspaceID != "" && p.WorkspaceID != auth.WorkspaceIDNone) || p.AccountID == "" + return (p.WorkspaceID != "" && p.WorkspaceID != databrickscfg.WorkspaceIDNone) || p.AccountID == "" } func MatchAccountProfiles(p Profile) bool { @@ -21,7 +21,7 @@ func MatchAccountProfiles(p Profile) bool { // workspace_id = "none" is a sentinel for account-level access, treated as empty. // This covers classic accounts.* profiles, legacy unified-host account profiles, // and new SPOG account profiles. - return p.Host != "" && p.AccountID != "" && (p.WorkspaceID == "" || p.WorkspaceID == auth.WorkspaceIDNone) + return p.Host != "" && p.AccountID != "" && (p.WorkspaceID == "" || p.WorkspaceID == databrickscfg.WorkspaceIDNone) } func MatchAllProfiles(p Profile) bool { diff --git a/libs/databrickscfg/profilehash/profilehash.go b/libs/databrickscfg/profilehash/profilehash.go new file mode 100644 index 00000000000..116da220712 --- /dev/null +++ b/libs/databrickscfg/profilehash/profilehash.go @@ -0,0 +1,55 @@ +package profilehash + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "slices" + "strings" + + "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/databricks-sdk-go/config" +) + +// Compute hashes every field in the simplified profile representation. +func Compute(p profile.Profile) (string, error) { + normalized := p + normalized.Host = normalizeHost(normalized.Host) + normalized.Scopes = normalizeScopes(normalized.Scopes) + + // Marshal the whole simplified profile so newly added profile fields are + // included automatically. Only the code constructing Profile decides which + // configuration fields belong in the fingerprint. + serialized, err := json.Marshal(normalized) + if err != nil { + return "", err + } + + sum := sha256.Sum256(serialized) + + return hex.EncodeToString(sum[:]), nil +} + +// A stored profile can contain a host without a scheme, while resolving a +// configuration adds the default HTTPS scheme. Normalize the stored value in +// the same way so both forms produce the same fingerprint. +func normalizeHost(value string) string { + return (&config.Config{Host: value}).CanonicalHostName() +} + +// A stored profile preserves the order in which its scopes were written, while +// resolving a configuration sorts and removes duplicate scopes. Normalize the +// stored value in the same way so both forms produce the same fingerprint. +func normalizeScopes(value string) string { + scopes := strings.Split(value, ",") + for i := range scopes { + scopes[i] = strings.TrimSpace(scopes[i]) + } + scopes = slices.DeleteFunc(scopes, func(scope string) bool { + return scope == "" + }) + slices.Sort(scopes) + scopes = slices.Compact(scopes) + + return strings.Join(scopes, ",") +} diff --git a/libs/databrickscfg/profilehash/profilehash_test.go b/libs/databrickscfg/profilehash/profilehash_test.go new file mode 100644 index 00000000000..ada68dc013c --- /dev/null +++ b/libs/databrickscfg/profilehash/profilehash_test.go @@ -0,0 +1,164 @@ +package profilehash + +import ( + "testing" + + "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestComputeIncludesAllProfileFields verifies that every field in the +// simplified profile contributes to its fingerprint. +func TestComputeIncludesAllProfileFields(t *testing.T) { + base := profile.Profile{ + Name: "TEST", + Host: "https://workspace.example.test", + AccountID: "account-id", + WorkspaceID: "workspace-id", + ClusterID: "cluster-id", + ServerlessComputeID: "serverless-compute-id", + HasClientCredentials: true, + Scopes: "all-apis", + AuthType: "databricks-cli", + } + + want, err := Compute(base) + require.NoError(t, err) + + tests := []struct { + name string + change func(*profile.Profile) + }{ + { + name: "name", + change: func(p *profile.Profile) { + p.Name = "OTHER" + }, + }, + { + name: "host", + change: func(p *profile.Profile) { + p.Host = "https://other.example.test" + }, + }, + { + name: "account ID", + change: func(p *profile.Profile) { + p.AccountID = "other-account" + }, + }, + { + name: "workspace ID", + change: func(p *profile.Profile) { + p.WorkspaceID = "other-workspace" + }, + }, + { + name: "cluster ID", + change: func(p *profile.Profile) { + p.ClusterID = "other-cluster" + }, + }, + { + name: "serverless compute ID", + change: func(p *profile.Profile) { + p.ServerlessComputeID = "other-serverless" + }, + }, + { + name: "client credentials", + change: func(p *profile.Profile) { + p.HasClientCredentials = false + }, + }, + { + name: "scopes", + change: func(p *profile.Profile) { + p.Scopes = "jobs" + }, + }, + { + name: "auth type", + change: func(p *profile.Profile) { + p.AuthType = "pat" + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + changed := base + tt.change(&changed) + + got, err := Compute(changed) + require.NoError(t, err) + + assert.NotEqual(t, want, got) + }) + } +} + +// TestComputeCanonicalizesHost verifies that equivalent host representations +// produce the same profile fingerprint. +func TestComputeCanonicalizesHost(t *testing.T) { + want, err := Compute(profile.Profile{Host: "https://workspace.example.test"}) + require.NoError(t, err) + + tests := []struct { + name string + host string + }{ + { + name: "without scheme", + host: "workspace.example.test", + }, + { + name: "with trailing slash", + host: "https://workspace.example.test/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Compute(profile.Profile{Host: tt.host}) + require.NoError(t, err) + + assert.Equal(t, want, got) + }) + } +} + +// TestComputeCanonicalizesScopes verifies that semantically equivalent scope +// lists produce the same profile fingerprint. +func TestComputeCanonicalizesScopes(t *testing.T) { + want, err := Compute(profile.Profile{Scopes: "all-apis,sql"}) + require.NoError(t, err) + + tests := []struct { + name string + scopes string + }{ + { + name: "different order", + scopes: "sql,all-apis", + }, + { + name: "surrounding whitespace", + scopes: " sql , all-apis ", + }, + { + name: "duplicates and empty values", + scopes: "sql,,all-apis,sql,", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Compute(profile.Profile{Scopes: tt.scopes}) + require.NoError(t, err) + + assert.Equal(t, want, got) + }) + } +} diff --git a/libs/testserver/fake_oidc.go b/libs/testserver/fake_oidc.go index db6a682c9bd..6487f95f304 100644 --- a/libs/testserver/fake_oidc.go +++ b/libs/testserver/fake_oidc.go @@ -49,6 +49,24 @@ func (s *FakeOidc) OidcAuthorize(req Request) Response { } func (s *FakeOidc) OidcToken(req Request) Response { + parameters, err := url.ParseQuery(string(req.Body)) + if err != nil { + return Response{ + StatusCode: http.StatusBadRequest, + Body: err.Error(), + } + } + + if parameters.Get("refresh_token") == "invalid-refresh-token" { + return Response{ + StatusCode: http.StatusUnauthorized, + Body: map[string]string{ + "error": "invalid_grant", + "error_description": "Refresh token is invalid", + }, + } + } + return Response{ Body: map[string]string{ "access_token": "oauth-token",