feat(kernel): route Azure Entra auth (azure-sp-m2m + azure-oauth) through the auth bridge - #919
Conversation
Sorry, something went wrong.
…h bridge The kernel auth bridge rejected azure-oauth and had no azure-sp-m2m path. Route both Azure auth types onto the kernel's generic OAuth flows (the kernel needs no Azure-specific code; PR databricks/databricks-sql-kernel#263 added the token_url/scope override plumbing this relies on): - azure-oauth (Azure AD U2M) -> oauth-u2m with the Azure app client id (96eecda7-...), redirect port 8030, and the {app_id}/user_impersonation offline_access delegated scope (via AzureOAuthEndpointCollection, honoring DATABRICKS_AZURE_TENANT_ID). The kernel discovers endpoints via the workspace /oidc redirector. (PECOBLR-4120) - azure-sp-m2m (Azure service principal) -> oauth-m2m with the Azure creds, an Entra v2.0 token_url, and the {effective_app_id}/.default scope. Requires an explicit azure_tenant_id (the kernel path does not auto-discover it). The management-token header / azure_workspace_resource_id are not applied on the kernel path -- no SQL connector uses them, matching Go and Node. (PECOBLR-4141) kernel_auth_kwargs now takes hostname (for the effective Azure app id); the client passes self._server_hostname. TDD: replaced the azure-oauth NotSupportedError test with routing tests and added a TestKernelAzureSpM2M suite (routing, required tenant/creds, federation client id). 50 bridge tests pass; black clean. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the use_kernel=True auth bridge to support Azure Entra (Azure AD) OAuth by routing the connector’s Azure auth types (azure-oauth U2M and azure-sp-m2m M2M) onto the kernel’s generic OAuth flows with Azure-specific overrides.
Changes:
- Route auth_type="azure-oauth" to kernel oauth-u2m using the Azure OAuth app bundle and Azure scope mapping.
- Route auth_type="azure-sp-m2m" to kernel oauth-m2m using Entra v2.0 token URL and {effective_app_id}/.default scope; require azure_tenant_id.
- Thread hostname into kernel_auth_kwargs (via KernelDatabricksClient.open_session) and add/adjust unit tests + changelog entry.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/databricks/sql/backend/kernel/auth_bridge.py | Adds Azure auth-type routing and related override plumbing for kernel session kwargs. |
| src/databricks/sql/backend/kernel/client.py | Passes the server hostname into kernel_auth_kwargs during session open. |
| tests/unit/test_kernel_auth_bridge.py | Updates U2M tests for azure-oauth routing and adds azure-sp-m2m routing/validation tests. |
| CHANGELOG.md | Documents Azure Entra OAuth support on the kernel backend. |
src/databricks/sql/backend/kernel/auth_bridge.py:262
- auth_type='azure-sp-m2m' uses azure_client_id/azure_client_secret, but because this branch returns before the generic ambiguity guards, a caller can also supply oauth_client_id/oauth_client_secret and have them silently ignored. This makes misconfigurations hard to diagnose.
if auth_type == "azure-sp-m2m":
azure_client_id = opts.get("azure_client_id")
azure_client_secret = opts.get("azure_client_secret")
azure_tenant_id = opts.get("azure_tenant_id")
if not (azure_client_id and azure_client_secret):
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
There was a problem hiding this comment.
Verdict: 1 Medium
Routing logic is clean and well-tested overall. One medium concern: the new Azure branches return before the ambiguity guards, so azure-oauth + oauth_client_secret (or + credentials_provider) is silently accepted as U2M whereas the parallel databricks-oauth case raises NotSupportedError. Nit (not filed inline): the step-3 comment # Only databricks-oauth reaches here (azure-oauth rejected up front) in the U2M block is now stale — azure-oauth is routed up front, not rejected.
Sorry, something went wrong.
The auth table marked the azure_* fields as Kernel-unsupported and claimed azure-oauth 'still works on the kernel' (it was actually rejected). Reflect the new routing: azure-sp-m2m + azure-oauth now work on the kernel path; azure_tenant_id is required there; the management token / azure_workspace_resource_id are not applied (matching Go/Node). Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — clean, well-documented routing with strong test coverage for both new Azure flows (routing, required tenant/creds, federation client id). One low-severity consistency note: the azure-oauth branch returns before the ambiguity guards, so oauth_client_secret / credentials_provider are silently ignored there, unlike the databricks-oauth U2M path which rejects them.
Sorry, something went wrong.
The kernel is the auth core now: for azure-oauth the bridge forwards only auth_type='azure-oauth' (+ optional client_id/redirect_port passthrough), and the kernel pins the workspace v2.0 authorize/token endpoints, the Azure app client id, port 8030, and the user_impersonation scope. Drops the connector-side endpoint/scope construction (and the AzureOAuthEndpointCollection / PYSQL_OAUTH_AZURE_* imports) from the kernel path. Live-verified end-to-end against an Azure workspace. azure-sp-m2m still routes to oauth-m2m here pending the kernel's dedicated azure-sp-m2m variant. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Medium · 2 Low
Overall solid — the azure-sp-m2m v2.0 .default scope + v2.0 token endpoint correctly mirror the Thrift path's v1.0 resource= form, and the empty-hostname fallback is safe. Main issue is a stale module docstring (F1, medium) that claims the azure-oauth bridge synthesises the client id/port/scope via AzureOAuthEndpointCollection when the code actually just forwards the azure-oauth selector and lets the kernel own resolution — the inline comment already contradicts it. Plus two low doc/consistency nits.
Sorry, something went wrong.
…optional mgmt token
Make the `azure-sp-m2m` bridge thin, matching the kernel becoming the
Azure-aware auth core. The connector now forwards
`auth_type='azure-sp-m2m'` + `azure_client_id` / `azure_client_secret`
(and optional `azure_tenant_id` / `azure_workspace_resource_id`) straight
to the kernel Session, instead of constructing the Entra token endpoint
and `{app_id}/.default` scope itself.
Behavior changes on the kernel path (Thrift parity):
- `azure_tenant_id` is now OPTIONAL — the kernel auto-discovers the
tenant from the workspace's `/aad/auth` redirect when omitted, exactly
as the Thrift backend does. (Previously the kernel path required it.)
- `azure_workspace_resource_id` is now honored as an optional add-on:
forward it and the kernel fetches an Azure-management token and sends
the `X-Databricks-Azure-SP-Management-Token` +
`X-Databricks-Azure-Workspace-Resource-Id` pair, so an SP with only an
Azure RBAC role (not a workspace member) can authenticate. (Previously
it was dropped with a warning.)
Also thread the `azure_*` connection kwargs into `kernel_auth_options`
in session.py — without this the bridge never received them and
`azure-sp-m2m` failed at session-open with "requires azure_client_id".
Adds a regression test for that threading, and rewrites the bridge tests
for thin forwarding (tenant optional, resource id forwarded). Drops the
now-unused `get_effective_azure_login_app_id` import and
`_AZURE_AAD_LOGIN_HOST` constant.
Co-authored-by: Isaac
Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Medium · 1 Low
Looks good overall — the Azure routing logic is thin, well-tested, and correct. Two cleanup items: an unused hostname parameter now threaded through client.py for no effect (medium), and a dead test import (low). Nit: the comment at auth_bridge.py:318 ("azure-oauth rejected up front") is now stale — azure-oauth is routed up front, not rejected.
Sorry, something went wrong.
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — clean, thin forwarding of both Azure Entra flows with thorough unit coverage (routing, ambiguity guards extended to azure-oauth, required-creds, optional passthroughs, session threading). One low-severity consistency note: the azure-sp-m2m early return bypasses the module's fail-loud ambiguity guards, so conflicting cross-namespace signals are silently ignored rather than rejected.
Sorry, something went wrong.
Addresses: - #3828326882 at src/databricks/sql/backend/kernel/auth_bridge.py:200 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — clean, thin binding that correctly forwards the Azure selector + creds and lets the kernel own resolution, with thorough unit coverage (bridge + session threading + ambiguity guards parametrized over both U2M types). One low-severity note about the intentional ambiguity-guard exemption on the azure-sp-m2m branch diverging from the module's loud-failure philosophy.
Sorry, something went wrong.
Addresses: - #3828367785 at src/databricks/sql/backend/kernel/auth_bridge.py:265 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 Low · 1 Nit
Looks good — a clean, thin binding that forwards the Azure selector + credentials to the kernel, with the azure-sp-m2m branch correctly placed before the ambiguity guards and session.py threading the azure_* kwargs only into the kernel path. Test coverage is solid; just one low (untested silent-ignore asymmetry for conflicting signals on azure-sp-m2m) and one nit (stale docstring count).
Other findings
- ⚪ Nit — Module docstring says "Three auth shapes are supported on the kernel path" but the bullet list now enumerates four top-level categories (PAT, OAuth M2M, OAuth U2M, and Azure Entra) plus JWT private-key M2M. The count was already slightly stale before this PR (JWT M2M), and this PR's new Azure Entra section makes it more so. Consider dropping the fixed count (e.g. "The following auth shapes are supported on the kernel path:") so it doesn't need updating each time a flow is added.
Sorry, something went wrong.
Addresses: - #3828404026 at src/databricks/sql/backend/kernel/auth_bridge.py:279 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — clean, well-tested addition routing azure-sp-m2m and azure-oauth through the kernel auth bridge, with matching session threading and thorough unit coverage (branch ordering, ambiguity guards, and the intentional azure-sp-m2m asymmetry are all exercised). One low note on log visibility for the silently-ignored conflicting-credential path.
Sorry, something went wrong.
Addresses: - #3828445075 at src/databricks/sql/backend/kernel/auth_bridge.py:285 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
…urface Point kernel-e2e's KERNEL_REV at the kernel commit that adds the pyo3 azure-sp-m2m surface (databricks-sql-kernel#263), so the connector's kernel-e2e builds a kernel wheel that can accept auth_type='azure-sp-m2m'. Temporary pin to the unmerged #263 branch tip; re-point to a kernel main SHA once #263 merges and a release is cut. Customer-facing pin (pyproject databricks-sql-kernel ^0.2.0) still needs a release bump to a published kernel with #263. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — the Azure Entra routing (azure-sp-m2m up-front handling, azure-oauth folded into the shared U2M branch, both ambiguity guards extended) is correct, and session.py threads the azure_* kwargs only on the kernel path. Tests cover every new branch. Only one minor docstring-count inaccuracy noted inline.
Sorry, something went wrong.
Addresses: - #3828603835 at src/databricks/sql/backend/kernel/auth_bridge.py:18 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Two quality-check regressions from the azure-sp-m2m addition: - mypy [no-redef]: the new azure-sp-m2m branch's `kwargs` assignment now precedes the JWT branch's annotated `kwargs: Dict[str, Any]`. Move the annotation onto the first (azure-sp-m2m) assignment so the later ones are plain reassignments to the same annotated name. - black: split the over-long `ignored_signals` tuple. Verified locally: `black --check src` clean, `mypy src` reports no issues. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
What
Teaches the use_kernel=True auth bridge to route both Azure Entra auth flows to the kernel (databricks/databricks-sql-kernel#263):
How
src/databricks/sql/backend/kernel/auth_bridge.py:
The binding is thin — it does not construct endpoints or scopes; the kernel does.
Tests
Bridge + session + auth unit tests pass; black clean. (test_kernel_client.py skips without the kernel wheel, as before.)
E2E verification (live Azure workspace)
Ran the full connector path (sql.connect(use_kernel=True, ...)) against a live Azure Databricks workspace, kernel wheel built from databricks/databricks-sql-kernel#263. All three Azure kernel-path flows round-trip SELECT 1:
Negative controls confirm the pipeline is real, not a no-op: a mismatched SP secret surfaces Entra AADSTS7000215, and a service principal with no workspace access returns HTTP 403 User not authorized (a real control-plane authz decision, not a bridge error).
Note: this required a kernel wheel with the azure-sp-m2m pyo3 surface — the currently-published databricks-sql-kernel 0.2.0 wheel lacks it (Session.__new__() got an unexpected keyword argument 'azure_client_id'), so merging this depends on a kernel release that includes #263. (Azure U2M works on the old wheel — oauth-u2m predates it.) Not yet covered: the RBAC-only authorization case (a non-member SP authorized purely by an Azure role via the management token), which needs an Azure role assignment still pending.
Related
This pull request and its description were written by Isaac.