Fix `auth profiles` misclassifying SPOG hosts as workspace configs by mihaimitrea-db · Pull Request #4929 · databricks/cli

mihaimitrea-db · 2026-04-10T13:20:20Z

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 Fix auth logout failing to clear token for workspace profiles with account ID #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

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.
Unit test: TestProfileLoadUnifiedHostFallback — experimental_is_unified_host profile with unreachable .well-known falls back to account validation.
Unit test: TestProfileLoadClassicAccountHost — classic account-scoped OIDC host.
Acceptance test: cmd/auth/profiles/spog-account — end-to-end: SPOG profile with workspace_id=none shows valid:true.
go test ./cmd/auth and go test ./acceptance -run TestAccept/cmd/auth/profiles pass.

SPOG hosts (e.g. db-deco-test.gcp.databricks.com) don't match the accounts.* URL prefix, so ConfigType() classifies them as WorkspaceConfig. This causes `auth profiles` to validate with CurrentUser.Me instead of Workspaces.List, which fails for account-scoped SPOG profiles. Use the resolved DiscoveryURL from .well-known/databricks-config to detect SPOG hosts with account-scoped OIDC, matching the routing logic in auth.AuthArguments.ToOAuthArgument(). Also add a fallback for legacy profiles with Experimental_IsUnifiedHost where .well-known is unreachable.

github-actions · 2026-04-10T13:26:03Z

Approval status: pending

`/cmd/auth/` - needs approval

Files: cmd/auth/profiles.go, cmd/auth/profiles_test.go
Suggested: @tanmay-db
Also eligible: @simonfaltum, @tejaskochar-db, @renaudhartert-db, @hectorcast-db, @parthban-db, @Divyansh-db, @chrisst, @rauchy

General files (require maintainer)

Files: acceptance/cmd/auth/profiles/spog-account/out.test.toml, acceptance/cmd/auth/profiles/spog-account/output.txt, acceptance/cmd/auth/profiles/spog-account/script, acceptance/cmd/auth/profiles/spog-account/test.toml
Based on git history:

@simonfaltum -- recent work in cmd/auth/

_{Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db) can approve all areas.

See OWNERS for ownership rules.}

simonfaltum

Review (multi-agent swarm: Isaac + Cursor, cross-reviewed)

Overall the fix is correct and well-targeted. The routing logic properly mirrors ToOAuthArgument(), and the tests are thorough. A few suggestions below, nothing blocking.

Summary of findings

#	Severity	Finding	File
1	suggestion	Legacy fallback ignores `workspace_id`	`profiles.go:77-82`
2	suggestion	Logic duplication with `arguments.go`	`profiles.go:61-75`
3	suggestion	Misleading test name	`profiles_test.go:224`
4	nitpick	Tests don't prove which validation branch ran	`profiles_test.go:85`
5	nitpick	Unused `wantConfigCloud` test field	`profiles_test.go:123`
6	suggestion	Missing negative test case	`profiles_test.go`
7	pass	Go code structure checklist	all files

See inline comments for details.

simonfaltum · 2026-04-13T10:12:51Z

cmd/auth/profiles.go

+	// Legacy backward compat: profiles with Experimental_IsUnifiedHost where
+	// .well-known is unreachable (so DiscoveryURL is empty). Matches the
+	// fallback in auth.AuthArguments.ToOAuthArgument().
+	if configType == config.InvalidConfig && cfg.Experimental_IsUnifiedHost && cfg.AccountID != "" {
+		configType = config.AccountConfig
+	}


suggestion: This legacy fallback always forces AccountConfig regardless of workspace_id. In arguments.go:80-81, ignoring workspace_id is intentional because unified OAuth routing is always account-scoped. But here you're choosing a validation strategy, not an OAuth flow. MatchWorkspaceProfiles in profiler.go treats any non-none workspace_id as workspace-scoped.

If a legacy experimental_is_unified_host profile has a real workspace_id and .well-known is unreachable, this would validate it with Workspaces.List instead of CurrentUser.Me. That said, the combination of Experimental_IsUnifiedHost + real workspace_id is unlikely in practice since workspace-scoped SPOG came after the legacy flag era.

Consider either:

Adding the same workspace_id branching here as in the discovery block above, or

Adding a comment explaining why it's intentionally omitted

simonfaltum · 2026-04-13T10:12:51Z

cmd/auth/profiles.go

+	configType := cfg.ConfigType()
+	isAccountScopedOIDC := cfg.DiscoveryURL != "" && strings.Contains(cfg.DiscoveryURL, "/oidc/accounts/")
+	if configType != config.AccountConfig && cfg.AccountID != "" && isAccountScopedOIDC {
+		if cfg.WorkspaceID != "" && cfg.WorkspaceID != auth.WorkspaceIDNone {
+			configType = config.WorkspaceConfig
+		} else {
+			configType = config.AccountConfig
+		}
+	}


suggestion (follow-up): The SPOG detection heuristic (DiscoveryURL contains /oidc/accounts/ + IsUnifiedHost fallback) is now duplicated between here and libs/auth/arguments.go:69-82. The cross-referencing comments help, but if the routing logic changes in one place the other can drift.

Consider extracting a shared helper in libs/auth/ in a follow-up, e.g.:

func ResolveConfigType(cfg *config.Config) config.ConfigType { ... }

Not blocking for this PR since the logic is simple and well-documented.

This would be good to have. Bonus points that it moves some of the logic form the SDK to the CLI.

simonfaltum · 2026-04-13T10:12:51Z

cmd/auth/profiles_test.go

+}
+
+func TestProfileLoadClassicAccountHost(t *testing.T) {
+	// Verify that a host with account-scoped OIDC from discovery is validated


suggestion: This test name says "classic account host" but the httptest.Server URL (e.g. http://127.0.0.1:PORT) doesn't match the accounts.* prefix, so ConfigType() won't return AccountConfig for the classic-host reason. This actually exercises the SPOG discovery override path (same as TestProfileLoadSPOGConfigType).

Consider renaming to something like TestProfileLoadSPOGAccountWithDiscovery, or updating the comment to acknowledge that classic accounts.* behavior can't easily be tested with httptest and this serves as a supplementary SPOG case.

cmd/auth/profiles_test.go

simonfaltum · 2026-04-13T10:12:51Z

cmd/auth/profiles_test.go

+// newProfileTestServer creates a mock server for profile validation tests.
+// It serves /.well-known/databricks-config with the given OIDC shape and
+// responds to the workspace/account validation API endpoints.
+func newProfileTestServer(t *testing.T, accountScoped bool, accountID string) *httptest.Server {


nitpick: Both the workspace and account validation endpoints (/api/2.0/preview/scim/v2/Me and /api/2.0/accounts/{id}/workspaces) return success, so Valid == true doesn't prove which branch was taken. Consider making the "wrong" endpoint return an error for each test case to prove routing correctness.

suggestion: Also consider adding a negative test case where .well-known returns 404, account_id is set, Experimental_IsUnifiedHost is false, to verify the profile stays as WorkspaceConfig (proving the override doesn't trigger accidentally).

- Add workspace_id branching to legacy IsUnifiedHost fallback - Rename TestProfileLoadClassicAccountHost to TestProfileLoadSPOGAccountWithDiscovery - Extract hasWorkspace variable to deduplicate condition - Add negative test: no discovery + account_id stays WorkspaceConfig - Fix misleading mock server comments

mihaimitrea-db added 2 commits April 10, 2026 13:13

Add acceptance test for SPOG account profile validation

7b8d122

mihaimitrea-db marked this pull request as ready for review April 10, 2026 13:25

mihaimitrea-db requested review from andrewnester, anton-107, denik, pietern, shreyas-goenka and simonfaltum as code owners April 10, 2026 13:25

Use tagged switch to fix staticcheck QF1002 lint errors

456280f

simonfaltum removed request for andrewnester, anton-107, denik, pietern and shreyas-goenka April 10, 2026 13:27

Clean up test: remove unused field, fix misleading comment

a7198ac

simonfaltum reviewed Apr 13, 2026

View reviewed changes

mihaimitrea-db and others added 2 commits April 13, 2026 10:51

Merge branch 'main' into mihaimitrea-db/fix-profiles-spog-classification

05c4f39

mihaimitrea-db mentioned this pull request Apr 13, 2026

Extract IsSPOG and ResolveConfigType into libs/auth #4939

Open

3 tasks

mihaimitrea-db requested a review from simonfaltum April 13, 2026 11:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `auth profiles` misclassifying SPOG hosts as workspace configs#4929

Fix `auth profiles` misclassifying SPOG hosts as workspace configs#4929
mihaimitrea-db wants to merge 6 commits intomainfrom
mihaimitrea-db/fix-profiles-spog-classification

mihaimitrea-db commented Apr 10, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 10, 2026 •

edited

Loading

Uh oh!

simonfaltum left a comment

Uh oh!

simonfaltum Apr 13, 2026

Uh oh!

simonfaltum Apr 13, 2026

Uh oh!

mihaimitrea-db Apr 13, 2026

Uh oh!

simonfaltum Apr 13, 2026

Uh oh!

Uh oh!

simonfaltum Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mihaimitrea-db commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why not just check account_id?

Test plan

Uh oh!

github-actions bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approval status: pending

/cmd/auth/ - needs approval

General files (require maintainer)

Uh oh!

simonfaltum left a comment

Choose a reason for hiding this comment

Review (multi-agent swarm: Isaac + Cursor, cross-reviewed)

Summary of findings

Uh oh!

simonfaltum Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

simonfaltum Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

mihaimitrea-db Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

simonfaltum Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

simonfaltum Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mihaimitrea-db commented Apr 10, 2026 •

edited

Loading

Why not just check `account_id`?

github-actions bot commented Apr 10, 2026 •

edited

Loading

`/cmd/auth/` - needs approval