[vuanhphung]

Problem

On the Thrift backend, Azure service-principal M2M (authType: 'databricks-oauth' with oauthClientSecret on an Azure host) fails when azureTenantId is omitted, even though the option is optional. AzureOAuthManager.getOIDCConfigUrl falls back to the multi-tenant /organizations authority (OAuthManager.ts:292-296), and Entra rejects the client-credentials grant there. The kernel backend (useKernel: true) and the Python connector discover the tenant from the workspace's /aad/auth redirect, so the same credentials work on SEA but not on Thrift.

Found by databricks-driver-test AUTH-012 (Azure SP M2M with the tenant auto-discovered), which now runs on Node.js Thrift as an xfail tripwire.

Change

For M2M without azureTenantId, AzureOAuthManager now reads the tenant from the first {host}/aad/auth redirect (login.microsoftonline.com/{tenant}/...) before OIDC discovery, through the connection's agent so proxy and TLS options apply. The redirect host must be a Microsoft login authority (commercial, US Gov or China), matched by suffix, as in the kernel's discover_tenant. A discovery failure is surfaced with a hint to pass azureTenantId. An explicit azureTenantId skips discovery, and U2M keeps /organizations.

Verification

• New unit tests for the redirect parsing (including look-alike hosts and connection failures), socket reuse on a keep-alive agent, and the AzureOAuthManager wiring (including the agent passed to discovery). npm test (1764 passing), npm run lint, npm run prettier.
• Integration, databricks-driver-test#2072 live on Azure:
  • On main, AUTH-012 Thrift fails with AADSTS50059: No tenant-identifying information found (run).
  • On aff1d42 (this PR's first commit, the full Node suite), the AUTH-012 Thrift tripwire throws XPASS: tenant-omitted M2M connects, opens one session, runs SELECT 1 and sends no Azure management headers. AUTH-013 (explicit tenant) and the rest of the suite pass on Thrift and SEA (run). The later commits only change the error raised when the discovery request fails and drain the redirect body.

This pull request and its description were written by Isaac.

[peco-review-bot]

Verdict: 1 Low

Looks good, with one low concern: network failures during tenant discovery reach the caller without the azureTenantId hint. Follow-up (this is not caused by the PR): discovery accepts the US Gov and China login hosts, but getOIDCConfigUrl always builds the URL on login.microsoftonline.com and drops the discovered authority host. Sovereign-cloud M2M therefore still doesn't work, and the same is true with an explicit azureTenantId.

[vuanhphung]

On the sovereign-cloud follow-up in the review summary: agreed, and it's left out of this PR on purpose. getOIDCConfigUrl builds the commercial login.microsoftonline.com authority for an explicit azureTenantId too, so US Gov and China M2M fail on main already. The kernel takes the same approach: it accepts the sovereign login hosts during discovery and lets a sovereign workspace fail at the commercial token exchange (src/auth/oauth/azure.rs, tenant_from_login_url). The driver-test spec AUTH-012 also puts sovereign clouds out of scope. Supporting them means carrying the discovered authority host into the OIDC URL, which is a separate change.

[peco-review-bot]

Verdict: 1 Low · 1 Nit

Looks good, with 2 low concerns: the redirect body is never drained on the keep-alive agent, and the tests don't check that the agent is passed to discovery. The discovery logic, host-suffix validation, credential-safe error messages (only code/type, as driver-pitfalls.md requires) and caching are correct. Follow-up, not caused by this PR: getOIDCConfigUrl always uses login.microsoftonline.com, so a tenant discovered from a US Gov or China redirect is still looked up on the public authority. An explicit azureTenantId has the same limitation today. Nit: the existing test 'falls back to /organizations/ when azureTenantId is not set' now only describes getOIDCConfigUrl before discovery runs, not what M2M actually does.

Other findings

• ⚪ Nit — The PR says discovery goes "through the connection's agent so proxy and TLS options apply". The wiring test only checks discoverStub.firstCall.args[0] (the host), not that args[1] is the agent from connectionProvider.getAgent('https:'). If someone later drops the agent argument, the request would quietly skip proxy/TLS config and every unit test would still pass. Asserting the second argument against the stub provider's https agent would lock in that contract.

[vuanhphung]

Review-body nits, both addressed in c4a9c21:

• Agent contract: the wiring test now stubs the connection provider's getAgent and asserts that discovery receives the https: agent (discoverStub.firstCall.args[1]), so dropping the argument fails the test.
• Test names: the two getOIDCConfigUrl cases now say they describe the state before discovery (uses /organizations/ when azureTenantId is not set and no tenant was discovered yet, and the empty/whitespace one). M2M behavior after discovery is covered by AzureOAuthManager tenant discovery.

The sovereign-cloud follow-up is out of scope, as in my earlier comment.

[peco-review-bot]

✅ No issues identified by the review bot.