Skip to content

Commit f86c804

Browse files
Fix auth profiles misclassifying SPOG hosts as workspace configs (#4929)
## Summary - The SDK's `ConfigType()` classifies hosts by URL prefix (`accounts.*` → account, everything else → workspace). SPOG hosts don't match the `accounts.*` prefix, so they were misclassified as `WorkspaceConfig` and validated with `CurrentUser.Me`, which fails on account-scoped SPOG hosts. - Use the resolved `DiscoveryURL` from `/.well-known/databricks-config` to detect SPOG hosts with account-scoped OIDC (contains `/oidc/accounts/`), matching the routing logic in `auth.AuthArguments.ToOAuthArgument()` and the approach from #4853. - Add a fallback for legacy profiles with `experimental_is_unified_host` where `.well-known` is unreachable. ### Why not just check `account_id`? Since #4809, `runHostDiscovery` populates `account_id` on every workspace profile from the `.well-known` endpoint. A regular workspace profile now routinely carries `account_id`. The only reliable discriminator is the `oidc_endpoint` shape from `.well-known`, resolved at runtime (as established in #4853). ## Test plan - [x] Unit tests: `TestProfileLoadSPOGConfigType` — table-driven with mock HTTP servers covering SPOG account, SPOG workspace, SPOG with `workspace_id=none`, and classic workspace with discovery-populated `account_id`. - [x] Unit test: `TestProfileLoadUnifiedHostFallback` — `experimental_is_unified_host` profile with unreachable `.well-known` falls back to account validation. - [x] Unit test: `TestProfileLoadClassicAccountHost` — classic account-scoped OIDC host. - [x] Acceptance test: `cmd/auth/profiles/spog-account` — end-to-end: SPOG profile with `workspace_id=none` shows `valid:true`. - [x] `go test ./cmd/auth` and `go test ./acceptance -run TestAccept/cmd/auth/profiles` pass. --------- Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com>
1 parent 853e18f commit f86c804

File tree

6 files changed

+314
-1
lines changed

6 files changed

+314
-1
lines changed

acceptance/cmd/auth/profiles/spog-account/out.test.toml

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
=== SPOG account profile should be valid
3+
>>> [CLI] auth profiles --output json
4+
{
5+
"profiles": [
6+
{
7+
"name":"spog-account",
8+
"host":"[DATABRICKS_URL]",
9+
"account_id":"spog-acct-123",
10+
"workspace_id":"none",
11+
"cloud":"aws",
12+
"auth_type":"pat",
13+
"valid":true
14+
}
15+
]
16+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
sethome "./home"
2+
3+
# Create a SPOG account profile: non-accounts.* host with account_id, no workspace_id.
4+
# Before the fix, this was misclassified as WorkspaceConfig and validated with
5+
# CurrentUser.Me, which fails on account-scoped SPOG hosts.
6+
cat > "./home/.databrickscfg" <<EOF
7+
[spog-account]
8+
host = ${DATABRICKS_HOST}
9+
account_id = spog-acct-123
10+
workspace_id = none
11+
token = test-token
12+
EOF
13+
14+
title "SPOG account profile should be valid"
15+
trace $CLI auth profiles --output json
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
Ignore = [
2+
"home"
3+
]
4+
5+
# Override .well-known to return account-scoped OIDC, simulating a SPOG host.
6+
# The oidc_endpoint contains /oidc/accounts/ which signals account-scoped OIDC.
7+
[[Server]]
8+
Pattern = "GET /.well-known/databricks-config"
9+
Response.Body = '''
10+
{
11+
"account_id": "spog-acct-123",
12+
"oidc_endpoint": "localhost/oidc/accounts/spog-acct-123"
13+
}
14+
'''
15+
16+
# Provide a handler for the account workspaces list endpoint used by
17+
# auth profiles validation for account-type configs.
18+
[[Server]]
19+
Pattern = "GET /api/2.0/accounts/spog-acct-123/workspaces"
20+
Response.Body = '[]'

cmd/auth/profiles.go

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import (
55
"errors"
66
"fmt"
77
"io/fs"
8+
"strings"
89
"sync"
910
"time"
1011

12+
"github.com/databricks/cli/libs/auth"
1113
"github.com/databricks/cli/libs/cmdio"
1214
"github.com/databricks/cli/libs/databrickscfg"
1315
"github.com/databricks/cli/libs/databrickscfg/profile"
@@ -56,7 +58,43 @@ func (c *profileMetadata) Load(ctx context.Context, configFilePath string, skipV
5658
return
5759
}
5860

59-
switch cfg.ConfigType() {
61+
// ConfigType() classifies based on the host URL prefix (accounts.* →
62+
// AccountConfig, everything else → WorkspaceConfig). SPOG hosts don't
63+
// match the accounts.* prefix so they're misclassified as WorkspaceConfig.
64+
// Use the resolved DiscoveryURL (from .well-known/databricks-config) to
65+
// detect SPOG hosts with account-scoped OIDC, matching the routing logic
66+
// in auth.AuthArguments.ToOAuthArgument().
67+
configType := cfg.ConfigType()
68+
hasWorkspace := cfg.WorkspaceID != "" && cfg.WorkspaceID != auth.WorkspaceIDNone
69+
70+
isAccountScopedOIDC := cfg.DiscoveryURL != "" && strings.Contains(cfg.DiscoveryURL, "/oidc/accounts/")
71+
if configType != config.AccountConfig && cfg.AccountID != "" && isAccountScopedOIDC {
72+
if hasWorkspace {
73+
configType = config.WorkspaceConfig
74+
} else {
75+
configType = config.AccountConfig
76+
}
77+
}
78+
79+
// Legacy backward compat: SDK v0.126.0 removed the UnifiedHost case from
80+
// ConfigType(), so profiles with Experimental_IsUnifiedHost now get
81+
// InvalidConfig instead of being routed to account/workspace validation.
82+
// When .well-known is also unreachable (DiscoveryURL empty), the override
83+
// above can't help. Fall back to workspace_id to choose the validation
84+
// strategy, matching the IsUnifiedHost fallback in ToOAuthArgument().
85+
if configType == config.InvalidConfig && cfg.Experimental_IsUnifiedHost && cfg.AccountID != "" {
86+
if hasWorkspace {
87+
configType = config.WorkspaceConfig
88+
} else {
89+
configType = config.AccountConfig
90+
}
91+
}
92+
93+
if configType != cfg.ConfigType() {
94+
log.Debugf(ctx, "Profile %q: overrode config type from %s to %s (SPOG host)", c.Name, cfg.ConfigType(), configType)
95+
}
96+
97+
switch configType {
6098
case config.AccountConfig:
6199
a, err := databricks.NewAccountClient((*databricks.Config)(cfg))
62100
if err != nil {

cmd/auth/profiles_test.go

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package auth
22

33
import (
4+
"encoding/json"
5+
"net/http"
6+
"net/http/httptest"
7+
"os"
48
"path/filepath"
59
"runtime"
610
"testing"
@@ -74,3 +78,218 @@ func TestProfilesDefaultMarker(t *testing.T) {
7478
require.NoError(t, err)
7579
assert.Equal(t, "profile-a", defaultProfile)
7680
}
81+
82+
// newSPOGServer creates a mock SPOG server that returns account-scoped OIDC.
83+
// It serves both validation endpoints since SPOG workspace profiles (with a
84+
// real workspace_id) need CurrentUser.Me, while account profiles need
85+
// Workspaces.List. The workspace-only newWorkspaceServer omits the account
86+
// endpoint to prove routing correctness for non-SPOG hosts.
87+
func newSPOGServer(t *testing.T, accountID string) *httptest.Server {
88+
t.Helper()
89+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
90+
w.Header().Set("Content-Type", "application/json")
91+
switch r.URL.Path {
92+
case "/.well-known/databricks-config":
93+
_ = json.NewEncoder(w).Encode(map[string]any{
94+
"account_id": accountID,
95+
"oidc_endpoint": r.Host + "/oidc/accounts/" + accountID,
96+
})
97+
case "/api/2.0/accounts/" + accountID + "/workspaces":
98+
_ = json.NewEncoder(w).Encode([]map[string]any{})
99+
case "/api/2.0/preview/scim/v2/Me":
100+
// SPOG workspace profiles also need CurrentUser.Me to succeed.
101+
_ = json.NewEncoder(w).Encode(map[string]any{"userName": "test-user"})
102+
default:
103+
w.WriteHeader(http.StatusNotFound)
104+
}
105+
}))
106+
t.Cleanup(server.Close)
107+
return server
108+
}
109+
110+
// newWorkspaceServer creates a mock workspace server that returns workspace-scoped
111+
// OIDC and only serves the workspace validation endpoint. The account validation
112+
// endpoint returns 404 to prove the workspace path was taken.
113+
func newWorkspaceServer(t *testing.T, accountID string) *httptest.Server {
114+
t.Helper()
115+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
116+
w.Header().Set("Content-Type", "application/json")
117+
switch r.URL.Path {
118+
case "/.well-known/databricks-config":
119+
_ = json.NewEncoder(w).Encode(map[string]any{
120+
"account_id": accountID,
121+
"oidc_endpoint": r.Host + "/oidc",
122+
})
123+
case "/api/2.0/preview/scim/v2/Me":
124+
_ = json.NewEncoder(w).Encode(map[string]any{"userName": "test-user"})
125+
default:
126+
w.WriteHeader(http.StatusNotFound)
127+
}
128+
}))
129+
t.Cleanup(server.Close)
130+
return server
131+
}
132+
133+
func TestProfileLoadSPOGConfigType(t *testing.T) {
134+
spogServer := newSPOGServer(t, "spog-acct")
135+
wsServer := newWorkspaceServer(t, "ws-acct")
136+
137+
cases := []struct {
138+
name string
139+
host string
140+
accountID string
141+
workspaceID string
142+
wantValid bool
143+
}{
144+
{
145+
name: "SPOG account profile validated as account",
146+
host: spogServer.URL,
147+
accountID: "spog-acct",
148+
wantValid: true,
149+
},
150+
{
151+
name: "SPOG workspace profile validated as workspace",
152+
host: spogServer.URL,
153+
accountID: "spog-acct",
154+
workspaceID: "ws-123",
155+
wantValid: true,
156+
},
157+
{
158+
name: "SPOG profile with workspace_id=none validated as account",
159+
host: spogServer.URL,
160+
accountID: "spog-acct",
161+
workspaceID: "none",
162+
wantValid: true,
163+
},
164+
{
165+
name: "classic workspace with account_id from discovery stays workspace",
166+
host: wsServer.URL,
167+
accountID: "ws-acct",
168+
wantValid: true,
169+
},
170+
}
171+
172+
for _, tc := range cases {
173+
t.Run(tc.name, func(t *testing.T) {
174+
dir := t.TempDir()
175+
configFile := filepath.Join(dir, ".databrickscfg")
176+
t.Setenv("HOME", dir)
177+
if runtime.GOOS == "windows" {
178+
t.Setenv("USERPROFILE", dir)
179+
}
180+
181+
content := "[test-profile]\nhost = " + tc.host + "\ntoken = test-token\n"
182+
if tc.accountID != "" {
183+
content += "account_id = " + tc.accountID + "\n"
184+
}
185+
if tc.workspaceID != "" {
186+
content += "workspace_id = " + tc.workspaceID + "\n"
187+
}
188+
require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600))
189+
190+
p := &profileMetadata{
191+
Name: "test-profile",
192+
Host: tc.host,
193+
AccountID: tc.accountID,
194+
}
195+
p.Load(t.Context(), configFile, false)
196+
197+
assert.Equal(t, tc.wantValid, p.Valid, "Valid mismatch")
198+
assert.NotEmpty(t, p.Host, "Host should be set")
199+
assert.NotEmpty(t, p.AuthType, "AuthType should be set")
200+
})
201+
}
202+
}
203+
204+
func TestProfileLoadUnifiedHostFallback(t *testing.T) {
205+
// When Experimental_IsUnifiedHost is set but .well-known is unreachable,
206+
// ConfigType() returns InvalidConfig. The fallback should reclassify as
207+
// AccountConfig so the profile is still validated.
208+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
209+
w.Header().Set("Content-Type", "application/json")
210+
switch r.URL.Path {
211+
case "/.well-known/databricks-config":
212+
w.WriteHeader(http.StatusNotFound)
213+
case "/api/2.0/accounts/unified-acct/workspaces":
214+
_ = json.NewEncoder(w).Encode([]map[string]any{})
215+
default:
216+
w.WriteHeader(http.StatusNotFound)
217+
}
218+
}))
219+
t.Cleanup(server.Close)
220+
221+
dir := t.TempDir()
222+
configFile := filepath.Join(dir, ".databrickscfg")
223+
t.Setenv("HOME", dir)
224+
if runtime.GOOS == "windows" {
225+
t.Setenv("USERPROFILE", dir)
226+
}
227+
228+
content := "[unified-profile]\nhost = " + server.URL + "\ntoken = test-token\naccount_id = unified-acct\nexperimental_is_unified_host = true\n"
229+
require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600))
230+
231+
p := &profileMetadata{
232+
Name: "unified-profile",
233+
Host: server.URL,
234+
AccountID: "unified-acct",
235+
}
236+
p.Load(t.Context(), configFile, false)
237+
238+
assert.True(t, p.Valid, "unified host profile should be valid via fallback")
239+
assert.NotEmpty(t, p.Host)
240+
assert.NotEmpty(t, p.AuthType)
241+
}
242+
243+
func TestClassicAccountsHostConfigType(t *testing.T) {
244+
// Classic accounts.* hosts can't be tested through Load() because httptest
245+
// generates 127.0.0.1 URLs. Verify directly that ConfigType() classifies
246+
// them as AccountConfig, so the SPOG override is never needed.
247+
cfg := &config.Config{
248+
Host: "https://accounts.cloud.databricks.com",
249+
AccountID: "acct-123",
250+
}
251+
assert.Equal(t, config.AccountConfig, cfg.ConfigType())
252+
253+
// Even with SPOG-like discovery data, accounts.* stays AccountConfig.
254+
cfg.DiscoveryURL = "https://accounts.cloud.databricks.com/oidc/accounts/acct-123/.well-known/oauth-authorization-server"
255+
assert.Equal(t, config.AccountConfig, cfg.ConfigType())
256+
}
257+
258+
func TestProfileLoadNoDiscoveryStaysWorkspace(t *testing.T) {
259+
// When .well-known returns 404 and Experimental_IsUnifiedHost is false,
260+
// the SPOG override should NOT trigger even if account_id is set. The
261+
// profile should stay WorkspaceConfig and validate via CurrentUser.Me.
262+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
263+
w.Header().Set("Content-Type", "application/json")
264+
switch r.URL.Path {
265+
case "/.well-known/databricks-config":
266+
w.WriteHeader(http.StatusNotFound)
267+
case "/api/2.0/preview/scim/v2/Me":
268+
_ = json.NewEncoder(w).Encode(map[string]any{"userName": "test-user"})
269+
default:
270+
w.WriteHeader(http.StatusNotFound)
271+
}
272+
}))
273+
t.Cleanup(server.Close)
274+
275+
dir := t.TempDir()
276+
configFile := filepath.Join(dir, ".databrickscfg")
277+
t.Setenv("HOME", dir)
278+
if runtime.GOOS == "windows" {
279+
t.Setenv("USERPROFILE", dir)
280+
}
281+
282+
content := "[ws-profile]\nhost = " + server.URL + "\ntoken = test-token\naccount_id = some-acct\n"
283+
require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600))
284+
285+
p := &profileMetadata{
286+
Name: "ws-profile",
287+
Host: server.URL,
288+
AccountID: "some-acct",
289+
}
290+
p.Load(t.Context(), configFile, false)
291+
292+
assert.True(t, p.Valid, "should validate as workspace when discovery is unavailable")
293+
assert.NotEmpty(t, p.Host)
294+
assert.Equal(t, "pat", p.AuthType)
295+
}

0 commit comments

Comments
 (0)