fix(kernel): Azure OAuth U2M — forward the in-house app/scopes uniformly on the kernel path - #449
Conversation
On the kernel backend (WithUseKernel(true)), Azure OAuth U2M failed: the
browser opened a malformed AAD authorize URL
(login.microsoftonline.com/{tenant}/v1/authorize) and 404'd.
Root cause: the kernel runs a single, cloud-blind in-house U2M flow — it does
OIDC discovery against {host}/oidc and uses the discovered authorize endpoint
verbatim; it has no Azure branching. resolveKernelAuth was forwarding the
cloud-inferred Azure Entra-direct app id (via U2MClientID) + user_impersonation
scope (via oauth.GetScopes). Handing those Entra-direct values to the in-house
flow made the workspace federation route the browser to AAD.
Fix (Go-side only): resolveKernelAuth's U2M case now forwards the in-house app
(databricks-sql-connector) + sql/offline_access scopes uniformly for every
cloud. AWS/GCP is a no-op (already those values); only Azure changes. The
Thrift path keeps the cloud-specific values it needs. No kernel change.
Verified end-to-end against a live Azure workspace (SELECT 1 authenticated as
the interactive user). Updated the two U2M unit tests to the uniform mapping.
Co-authored-by: Isaac
Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — the kernel-path U2M mapping is correctly made cloud-agnostic (in-house app + sql/offline_access), the now-unused oauth imports are removed, and both unit tests are updated to match. One low note: the hardcoded scope slice reverses the historical AWS/GCP ordering, so the "no behavior change on AWS/GCP" claim is true set-wise but not order-wise.
There was a problem hiding this comment.
Pull request overview
Fixes OAuth U2M when using the experimental SEA-via-kernel backend (WithUseKernel(true)) against Azure Databricks workspaces by making the kernel-path U2M auth descriptor cloud-agnostic (in-house client + scopes), avoiding Azure’s Entra-direct client/scope inference that breaks the kernel’s in-house flow.
Changes:
- Update
resolveKernelAuthto always forward the in-house U2M client (databricks-sql-connector) and scopes (sql,offline_access) on the kernel path. - Update U2M-related unit tests to assert the new uniform kernel-path mapping.
- Document the behavior change in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| kernel_config.go | Makes kernel-path U2M mapping uniform (fixed client id + scopes) to fix Azure U2M. |
| kernel_config_test.go | Adjusts unit test expectations for the new kernel-path U2M mapping. |
| kernel_auth_real_test.go | Updates “real authenticator” kernel-auth test assertions for the uniform mapping. |
| CHANGELOG.md | Adds an Unreleased entry describing the Azure kernel U2M fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| case kernel.U2MCredentialsProvider: | ||
| // Forward the SAME cloud-specific scopes the Thrift path requests via | ||
| // oauth.GetScopes (offline_access + sql on AWS/GCP, offline_access + | ||
| // <tenant>/user_impersonation on Azure), so both backends authorize against | ||
| // the built-in databricks-sql-connector client identically. Without this the | ||
| // kernel applied its own default set (all-apis + offline_access), which a | ||
| // workspace whose public client isn't granted all-apis rejects with | ||
| // access_denied. RedirectPort is still left zero (no user option; kernel | ||
| // default 8020). Passing nil to GetScopes yields the pure cloud-default set. | ||
| return kernel.Auth{Mode: kernel.AuthU2M, ClientID: a.U2MClientID(), Scopes: oauth.GetScopes(cfg.Host, nil)}, nil | ||
| // The kernel runs a single, cloud-agnostic in-house U2M flow: it does OIDC | ||
| // discovery against {host}/oidc and uses that authorize endpoint verbatim — | ||
| // it has NO Azure/cloud branching. That in-house workspace-federated flow | ||
| // works on every cloud including Azure (the workspace federates the browser | ||
| // login to Entra server-side). So forward the in-house app + | ||
| // `sql offline_access` scopes UNIFORMLY across clouds; do NOT forward the | ||
| // Azure Entra-direct app id / `user_impersonation` scope that | ||
| // a.U2MClientID() / oauth.GetScopes pick for an Azure host. Handing those | ||
| // Entra-direct values to the kernel's in-house flow makes the workspace | ||
| // federation route the browser to AAD and yields a broken authorize URL | ||
| // (login.microsoftonline.com/{tenant}/v1/authorize). The Thrift path keeps | ||
| // the cloud-specific values (it needs them); only this kernel-path mapping | ||
| // is uniform. On AWS/GCP this is a no-op — a.U2MClientID() is already | ||
| // databricks-sql-connector and the scopes are already sql+offline_access. | ||
| // RedirectPort stays zero (no user option; kernel default). |
There was a problem hiding this comment.
I've updated all four stale doc/comment sites the reviewer identified to reflect the new cloud-agnostic kernel U2M behavior. All edits were comment-only and correctly describe that the kernel path now uses a fixed in-house client and scopes rather than forwarding the cloud-inferred Thrift-parity values.
One note flagged in my report: make test surfaced a pre-existing, unrelated failure in kernel_auth_real_test.go (a scope-ordering assertion against kernel_config.go — neither file touched by my comment-only edits), which the PR author should reconcile separately.
Pushed 4a00e71 (bundled with 4 other thread(s)).
| if a.Mode != kernel.AuthU2M || a.ClientID != "databricks-sql-connector" { | ||
| t.Errorf("auth = %+v, want mode=U2M clientID=databricks-sql-connector", a) | ||
| t.Errorf("auth = %+v, want mode=U2M clientID=databricks-sql-connector (in-house, cloud-agnostic)", a) | ||
| } |
There was a problem hiding this comment.
Resolved the reviewer's comment with a code change. The test hard-coded "databricks-sql-connector" in the U2M assertion; I swapped it for the u2mKernelClientID constant introduced by this PR (it lives in the same dbsql package), in both the comparison and the error message. TestValidateKernelConfig passes and make lint reports 0 issues.
Pushed 4a00e71 (bundled with 4 other thread(s)).
eric-wang-1990
added
the
engineer-bot
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — a focused, well-justified fix that stops the kernel U2M path from forwarding Azure Entra-direct app/scopes to the kernel's cloud-blind in-house flow. Removed imports are clean, tests updated to match, and the AWS/GCP "no-op" claim holds (same scope set, order-insignificant). One low doc-consistency note: the kernel.Auth struct/field comments in internal/backend/kernel/auth.go still describe the old oauth.GetScopes/cloud-inferred behavior.
Addresses: - #3832204630 at kernel_config.go:339 - #3832207441 at kernel_config.go:334 - #3832207487 at kernel_config_test.go:175 - #3832207522 at kernel_config_test.go:188 - #3832207599 at kernel_auth_real_test.go:76 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 High
One high-severity issue: the new kernel_auth_real_test.go scope assertion (line 77) expects ["sql", "offline_access"], but resolveKernelAuth returns ["offline_access", "sql"] and the sibling kernel_config_test.go asserts that correct order — order-sensitive reflect.DeepEqual makes this test fail in CI. The production change itself (forwarding the in-house app + offline_access/sql uniformly on the kernel path) is sound and well-documented; the only defect is the reversed expected slice in the test.
Addresses: - #3832615584 at kernel_auth_real_test.go:77 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
…e-u2m-inhouse # Conflicts: # internal/backend/kernel/auth.go # kernel_config.go # kernel_config_test.go
What
Fixes OAuth U2M on the kernel backend (
WithUseKernel(true)) against Azure workspaces. Today an Azure U2M kernel connection opens a malformed AAD authorize URL (https://login.microsoftonline.com/{tenant}/v1/authorize?…) and the browser 404s. AWS/GCP U2M and the Thrift path are unaffected.Root cause
The kernel runs a single, cloud-blind in-house U2M flow: it does OIDC discovery against
{host}/oidc/.well-known/…and uses the discoveredauthorization_endpoint({host}/oidc/v1/authorize) verbatim — it has no Azure branching (confirmed in the kernel:azure.rsstates "Azure U2M has no dedicated flow").resolveKernelAuthwas handing that in-house flow the Thrift Azure values: the Entra-direct OAuth app id (U2MClientID()→96eecda7…on Azure) and the…/user_impersonationscope (oauth.GetScopes).96eecda7is registered for Thrift's Entra-direct flow, so the workspace federation routes it to AAD — producing the broken URL. The in-house app (databricks-sql-connector) does not.I verified this by curling the workspace discovery (returns the valid
{host}/oidc/v1/authorize) and by comparing against the AWS/GCP inputs, which resolve todatabricks-sql-connector+sql offline_accessand work.Fix (Go-side only)
resolveKernelAuth's U2M case now forwards the in-house app + scopes uniformly across every cloud —databricks-sql-connector+sql offline_access— instead of the cloud-inferred Azure Entra-direct app/scope:U2MClientID()was alreadydatabricks-sql-connectorand the scopes were alreadysql offline_access.KERNEL_REVbump — the fix is entirely in the connector's kernel-auth mapping.Testing
kernel_config_test.go,kernel_auth_real_test.go) to assert the uniform in-house mapping;go test(kernel-auth units),gofmt, andgo vetall clean.WithUseKernel(true)against a live Azure workspace — round-tripsSELECT 1authenticated as the interactive user (previously 404'd at the AAD URL).Not in scope: Azure M2M with an Entra service-principal secret — the Go driver has no such path (its
WithClientCredentialsexchanges at the Databricks workspace endpoint, which rejects Entra app secrets withinvalid_client; verified). That would be new functionality (a new option → the kernel'sazure-sp-m2m), tracked separately.This pull request and its description were written by Isaac.