Skip to content

Commit 10c3bf9

Browse files
authored
Add host discovery to login.databricks.com flow (#4829)
## Why The discovery login flow via `login.databricks.com` (PR #4702) is a separate code path from the regular `--host` login. After #4809 added SPOG host detection via `.well-known/databricks-config` to the regular login path, the discovery flow was missing this behavior. Profiles created via `login.databricks.com` for SPOG hosts had no `account_id` and no discovery metadata, which breaks re-authentication (because `ToOAuthArgument()` needs `account_id` to route to unified OAuth). ## Changes **Before:** `discoveryLogin()` only used token introspection for `workspace_id` and deliberately skipped saving `account_id`. **Now:** After getting the host from `login.databricks.com`, `discoveryLogin()` calls `runHostDiscovery()` on the discovered host to populate `account_id`, `workspace_id`, and `DiscoveryURL` from `.well-known/databricks-config`. Token introspection is kept as a fallback for hosts where discovery is unavailable (e.g. classic workspace hosts). `account_id` is now saved to the profile. **Note:** This is not the full SPOG story for discovery login. Most SPOG workspaces will need additional handling during the login.databricks.com flow (e.g. workspace selection after discovery detects a multi-workspace account). A follow-up PR will address this. ## Test plan - New test: discovery login with SPOG host (mock `.well-known/databricks-config`) verifies `account_id` and `workspace_id` come from discovery - New test: discovery login where host discovery fails verifies fallback to introspection - Updated existing tests to assert `account_id` is now saved - All `go test ./cmd/auth/... ./libs/auth/...` pass - `make checks` passes This pull request was AI-assisted by Isaac.
1 parent 42bd421 commit 10c3bf9

File tree

4 files changed

+107
-14
lines changed

4 files changed

+107
-14
lines changed

acceptance/cmd/auth/login/discovery/out.databrickscfg

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33

44
[discovery-test]
55
host = [DATABRICKS_URL]
6-
workspace_id = 12345
6+
account_id = test-account-123
7+
workspace_id = [NUMID]
78
auth_type = databricks-cli
89

910
[__settings__]

acceptance/cmd/auth/login/discovery/test.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ Ignore = [
44
RecordRequests = true
55

66
# Override the introspection endpoint so we can verify it gets called.
7+
# Host discovery (via .well-known/databricks-config) provides workspace_id.
8+
# Introspection provides account_id as a fallback since the default
9+
# discovery handler doesn't return it.
710
[[Server]]
811
Pattern = "GET /api/2.0/tokens/introspect"
912
Response.Body = '''

cmd/auth/login.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -613,24 +613,33 @@ func discoveryLogin(ctx context.Context, dc discoveryClient, profileName string,
613613
return discoveryErr("login succeeded but no workspace host was discovered", nil)
614614
}
615615

616-
// Get the token for introspection
616+
// Run host metadata discovery on the discovered host to detect SPOG hosts
617+
// and populate account_id/workspace_id. This ensures profiles created via
618+
// login.databricks.com have the same metadata as profiles created via the
619+
// regular --host login path.
620+
hostArgs := &auth.AuthArguments{Host: discoveredHost}
621+
runHostDiscovery(ctx, hostArgs)
622+
accountID := hostArgs.AccountID
623+
workspaceID := hostArgs.WorkspaceID
624+
625+
// Best-effort introspection as a fallback for workspace_id when host
626+
// metadata discovery didn't return it (e.g. classic workspace hosts).
617627
tok, err := persistentAuth.Token()
618628
if err != nil {
619629
return fmt.Errorf("retrieving token after login: %w", err)
620630
}
621631

622-
// Best-effort introspection for metadata.
623-
var workspaceID string
624632
introspection, err := dc.IntrospectToken(ctx, discoveredHost, tok.AccessToken)
625633
if err != nil {
626634
log.Debugf(ctx, "token introspection failed (non-fatal): %v", err)
627635
} else {
628-
// TODO: Save introspection.AccountID once the SDKs are ready to use
629-
// account_id as part of the profile/cache key. Adding it now would break
630-
// existing auth flows that don't expect account_id on workspace profiles.
631-
workspaceID = introspection.WorkspaceID
636+
if workspaceID == "" {
637+
workspaceID = introspection.WorkspaceID
638+
}
639+
if accountID == "" {
640+
accountID = introspection.AccountID
641+
}
632642

633-
// Warn if the detected account_id differs from what's already saved in the profile.
634643
if existingProfile != nil && existingProfile.AccountID != "" && introspection.AccountID != "" &&
635644
existingProfile.AccountID != introspection.AccountID {
636645
log.Warnf(ctx, "detected account ID %q differs from existing profile account ID %q",
@@ -641,10 +650,10 @@ func discoveryLogin(ctx context.Context, dc discoveryClient, profileName string,
641650
configFile := env.Get(ctx, "DATABRICKS_CONFIG_FILE")
642651
clearKeys := oauthLoginClearKeys()
643652
// Discovery login always produces a workspace-level profile pointing at the
644-
// discovered host. Any previous routing metadata (account_id, workspace_id,
645-
// is_unified_host, cluster_id, serverless_compute_id) from a prior login to
646-
// a different host type must be cleared so they don't leak into the new
647-
// profile. workspace_id is re-added only when introspection succeeds.
653+
// discovered host. Any previous routing metadata (is_unified_host,
654+
// cluster_id, serverless_compute_id) from a prior login to a different host
655+
// type must be cleared so they don't leak into the new profile. account_id
656+
// and workspace_id are re-added from discovery/introspection results.
648657
clearKeys = append(clearKeys,
649658
"account_id",
650659
"workspace_id",
@@ -656,6 +665,7 @@ func discoveryLogin(ctx context.Context, dc discoveryClient, profileName string,
656665
Profile: profileName,
657666
Host: discoveredHost,
658667
AuthType: authTypeDatabricksCLI,
668+
AccountID: accountID,
659669
WorkspaceID: workspaceID,
660670
Scopes: scopesList,
661671
ConfigFile: configFile,

cmd/auth/login_test.go

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,11 +678,12 @@ func TestDiscoveryLogin_AccountIDMismatchWarning(t *testing.T) {
678678
assert.Contains(t, logBuf.String(), "new-account-id")
679679
assert.Contains(t, logBuf.String(), "old-account-id")
680680

681-
// Verify the profile was saved without account_id (not overwritten).
681+
// Account ID from introspection is now saved to the profile.
682682
savedProfile, err := loadProfileByName(ctx, "DISCOVERY", profile.DefaultProfiler)
683683
require.NoError(t, err)
684684
require.NotNil(t, savedProfile)
685685
assert.Equal(t, "https://workspace.example.com", savedProfile.Host)
686+
assert.Equal(t, "new-account-id", savedProfile.AccountID)
686687
assert.Equal(t, "12345", savedProfile.WorkspaceID)
687688
}
688689

@@ -816,6 +817,83 @@ func TestDiscoveryLogin_ExplicitScopesOverrideExistingProfile(t *testing.T) {
816817
assert.Equal(t, "all-apis", savedProfile.Scopes)
817818
}
818819

820+
func TestDiscoveryLogin_SPOGHostPopulatesAccountIDFromDiscovery(t *testing.T) {
821+
// Start a mock server that returns SPOG discovery metadata.
822+
server := newDiscoveryServer(t, map[string]any{
823+
"account_id": "discovered-account",
824+
"workspace_id": "discovered-ws",
825+
"oidc_endpoint": "https://spog.example.com/oidc/accounts/discovered-account",
826+
})
827+
828+
tmpDir := t.TempDir()
829+
configPath := filepath.Join(tmpDir, ".databrickscfg")
830+
err := os.WriteFile(configPath, []byte(""), 0o600)
831+
require.NoError(t, err)
832+
t.Setenv("DATABRICKS_CONFIG_FILE", configPath)
833+
834+
oauthArg, err := u2m.NewBasicDiscoveryOAuthArgument("DISCOVERY")
835+
require.NoError(t, err)
836+
oauthArg.SetDiscoveredHost(server.URL)
837+
838+
dc := &fakeDiscoveryClient{
839+
oauthArg: oauthArg,
840+
persistentAuth: &fakeDiscoveryPersistentAuth{
841+
token: &oauth2.Token{AccessToken: "test-token"},
842+
},
843+
// Introspection returns different values to verify discovery takes precedence.
844+
introspection: &auth.IntrospectionResult{
845+
AccountID: "introspection-account",
846+
WorkspaceID: "introspection-ws",
847+
},
848+
}
849+
850+
ctx, _ := cmdio.NewTestContextWithStdout(t.Context())
851+
err = discoveryLogin(ctx, dc, "DISCOVERY", time.Second, "", nil, func(string) error { return nil })
852+
require.NoError(t, err)
853+
854+
savedProfile, err := loadProfileByName(ctx, "DISCOVERY", profile.DefaultProfiler)
855+
require.NoError(t, err)
856+
require.NotNil(t, savedProfile)
857+
assert.Equal(t, server.URL, savedProfile.Host)
858+
assert.Equal(t, "discovered-account", savedProfile.AccountID, "account_id should come from host discovery")
859+
assert.Equal(t, "discovered-ws", savedProfile.WorkspaceID, "workspace_id should come from host discovery")
860+
}
861+
862+
func TestDiscoveryLogin_IntrospectionFallsBackWhenDiscoveryFails(t *testing.T) {
863+
tmpDir := t.TempDir()
864+
configPath := filepath.Join(tmpDir, ".databrickscfg")
865+
err := os.WriteFile(configPath, []byte(""), 0o600)
866+
require.NoError(t, err)
867+
t.Setenv("DATABRICKS_CONFIG_FILE", configPath)
868+
869+
// Use a host that won't respond to .well-known/databricks-config.
870+
oauthArg, err := u2m.NewBasicDiscoveryOAuthArgument("DISCOVERY")
871+
require.NoError(t, err)
872+
oauthArg.SetDiscoveredHost("https://workspace.example.com")
873+
874+
dc := &fakeDiscoveryClient{
875+
oauthArg: oauthArg,
876+
persistentAuth: &fakeDiscoveryPersistentAuth{
877+
token: &oauth2.Token{AccessToken: "test-token"},
878+
},
879+
introspection: &auth.IntrospectionResult{
880+
AccountID: "introspection-account",
881+
WorkspaceID: "introspection-ws",
882+
},
883+
}
884+
885+
ctx, _ := cmdio.NewTestContextWithStdout(t.Context())
886+
err = discoveryLogin(ctx, dc, "DISCOVERY", time.Second, "", nil, func(string) error { return nil })
887+
require.NoError(t, err)
888+
889+
savedProfile, err := loadProfileByName(ctx, "DISCOVERY", profile.DefaultProfiler)
890+
require.NoError(t, err)
891+
require.NotNil(t, savedProfile)
892+
assert.Equal(t, "https://workspace.example.com", savedProfile.Host)
893+
assert.Equal(t, "introspection-account", savedProfile.AccountID, "account_id should fall back to introspection")
894+
assert.Equal(t, "introspection-ws", savedProfile.WorkspaceID, "workspace_id should fall back to introspection")
895+
}
896+
819897
func TestDiscoveryLogin_ClearsStaleRoutingFieldsFromUnifiedProfile(t *testing.T) {
820898
tmpDir := t.TempDir()
821899
configPath := filepath.Join(tmpDir, ".databrickscfg")
@@ -911,5 +989,6 @@ auth_type = databricks-cli
911989
require.NoError(t, err)
912990
require.NotNil(t, savedProfile)
913991
assert.Equal(t, "https://new-workspace.example.com", savedProfile.Host)
992+
assert.Equal(t, "fresh-account", savedProfile.AccountID, "account_id should be saved from introspection")
914993
assert.Equal(t, "222222", savedProfile.WorkspaceID, "workspace_id should be updated to fresh introspection value")
915994
}

0 commit comments

Comments
 (0)