auth profiles: always validate SPOG profiles as account#5284
auth profiles: always validate SPOG profiles as account#5284mihaimitrea-db wants to merge 3 commits into
Conversation
ResolveConfigType used to route SPOG profiles with a real workspace_id to WorkspaceConfig, so auth profiles validated them with CurrentUser.Me. SPOG OAuth is account-scoped, so every token's audience is the account, and the workspace API rejects those tokens with `400 "Unable to load OAuth Config"` — flagging otherwise-functional profiles as invalid. Always classify SPOG as AccountConfig so validation goes through Workspaces.List, which the account-audience token can actually authenticate. The SPOG mock in newSPOGServer now returns 500 on /scim/v2/Me so any future regression that reintroduces the workspace branch fails the test. Co-authored-by: Isaac
mihaimitrea-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
mihaimitrea-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Approval status: pending
|
|
One thing I’m wondering about: does this make any non-OAuth SPOG workspace profile worse? My understanding is that So I think this is clearly the right direction for the OAuth case, but should the Not entirely sure this credential shape matters in practice, but it seems like the main strict-worse case to rule out. |
|
One broader thought: I wonder if we can avoid having It would be great if this command could move toward treating profiles as unified profiles by default, i.e. not first deciding "this is account" vs "this is workspace" based on host shape. The tricky part is the validation probe. Maybe the rule could be: try the account-level probe when That would make the SPOG OAuth case work because I’m not entirely sure this is the best probing strategy, but I think the direction of removing the SPOG-specific classification would be nice if we can make the semantics clear. |
I think this is a good approach. However my main concern is how we would handle having to do double the number of API calls? Do we make them async and accept the extra calls or do we make them sequentially and accept that validation might take double the time? I would lean towards async. We also need to account for the host somehow. If I remember correctly if you follow this sequence of actions:
You end up with a profile which has a stale account ID for a workspace profile. So we need the host to also be a signal for which endpoint to try. |
Replace ResolveConfigType's "either account or workspace" routing with a permissive validator: probe whichever API surfaces the profile has a signal for (host shape or field presence), and mark the profile valid if any probe succeeds. This addresses review feedback on the previous fix — it now also handles SPOG workspace-scoped credentials (e.g. a PAT), which the strict "SPOG always validates as account" rule would have falsely flagged as invalid. Probes run sequentially against the shared cfg because the SDK's lazy Authenticate() chain writes cfg.Host (via fixHostIfNeeded) while the other client's construction reads cfg.Host unlocked — go test -race flags the parallel version. Profile-level parallelism in newProfilesCommand is unchanged, so overall auth profiles wall-clock is still bounded by the slowest profile. ResolveConfigType is removed; IsSPOGHost and IsClassicWorkspaceHost are added in libs/auth as named wrappers so the host-shape check in profiles.go reads as a flat block of named booleans. Tests: - TestProfileLoadSPOGWorkspaceCredential covers PAT-on-SPOG explicitly: workspace probe succeeds, account probe 403s, OR yields Valid=true. - TestIsSPOGHost and TestIsClassicWorkspaceHost cover the three-way host classification, including the accounts-dod.* variant. Co-authored-by: Isaac
mihaimitrea-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
mihaimitrea-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
The previous validator probed the account API whenever `account_id` was set in cfg, on top of host signals. That over-fires in practice: `account_id` is back-filled by discovery onto every workspace profile, and can linger from a prior account login on the same profile name. The acceptance test `cmd/auth/login/discovery` failed because the new account probe hit an endpoint the test's mock server didn't declare, even though the underlying validation outcome was still correct (workspace probe carried the profile to Valid=true). Gate the account probe on host signals only (classic accounts.* or SPOG). `hasRealWorkspaceID` still adds the workspace probe on account hosts so the SPOG-PAT case keeps working. Co-authored-by: Isaac
mihaimitrea-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
mihaimitrea-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
2 participants
Summary
auth profileswas reportingValid: NOfor SPOG profiles with a realworkspace_id, even thoughauth tokenworked. Root cause:ResolveConfigTyperouted those profiles toWorkspaceConfig, so validation went throughCurrentUser.Me— but SPOG OAuth tokens are account-audience and the workspace API rejects them with400 "Unable to load OAuth Config".Replace the either/or routing with a host-shape-aware probe: probe the account API for classic
accounts.*and SPOG hosts, probe the workspace API for everything else, and additionally probe workspace if the profile carries an explicitworkspace_id(covers SPOG-with-PAT). A profile is valid if any applicable probe succeeds.Probes run sequentially against the shared cfg — parallel probes race on
cfg.Host(SDK's lazyAuthenticate()writes it while the other probe's client construction reads it). Profile-level parallelism is unchanged.Test plan
go test -race ./libs/auth/... ./cmd/auth/...— green.db-deco-test.gcp.databricks.com):Valid: NO->YES.TestProfileLoadSPOGWorkspaceCredentialcovers SPOG-with-PAT.TestIsSPOGHost/TestIsClassicWorkspaceHostlock in the three-way host classification includingaccounts-dod.*.This pull request and its description were written by Isaac.