feat(kernel): JWT private-key M2M auth on useKernel - #504
Conversation
Add JWT private-key client-assertion auth (RFC 7523) to the kernel backend. On `authType: 'databricks-oauth'`, supplying `oauthJwtKeyFile` selects the JWT flow: the kernel signs a short-lived assertion with the private key instead of sending a client secret and owns the token lifecycle (`authMode: 'OAuthM2mJwt'`). - KernelAuth: new JWT branch in buildKernelConnectionOptions (checked before the U2M/M2M-secret split; a private-key file is unambiguous JWT M2M intent), plus the OAuthM2mJwt native option shape. Requires oauthClientId + oauthJwtKid; optional oauthJwtPassphrase / oauthJwtAlgorithm / oauthScopes / tokenUrl. Mutually exclusive with oauthClientSecret. Also threads tokenUrl through the existing M2m branch. - IDBSQLClient: new oauthJwt* + tokenUrl fields on the databricks-oauth ConnectionOptions member. - DBSQLClient: on the useKernel path, do not build the connector's own OAuth provider (it eagerly starts the U2M browser flow / M2M exchange before the kernel is consulted); hand over a minimal PAT provider only when a token is present. Mirrors the Python connector. - tests: 9 unit tests for JWT routing / precedence / validation. Verified end-to-end: SELECT 1 via useKernel against an Azure Databricks warehouse, authenticated by Entra ID with a JWT private-key assertion (tokenUrl pointed at the Entra token endpoint). Requires a @databricks/databricks-sql-kernel build with JWT + tokenUrl support (kernel PRs #249 merged, #275 for tokenUrl). Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
There was a problem hiding this comment.
Verdict: 2 Low
Solid, well-documented addition; the JWT branch validation and precedence logic look correct and mirror the existing M2M/U2M handling. Two low-severity notes: telemetry authType misclassifies JWT M2M as external-browser (since it keys off oauthClientSecret), and the connect() auth-provider behavior change lacks direct unit coverage. Minor consistency gap worth noting: the PAT branch's ambiguity guard rejects token + oauthClientId/oauthClientSecret but not token + oauthJwtKeyFile, so a JWT key silently drops when authType: 'access-token' is used.
Addresses PR #504 review feedback: - Move oauthJwtKeyFile/oauthJwtKid/oauthJwtPassphrase/oauthJwtAlgorithm/ tokenUrl off the public `databricks-oauth` AuthOptions onto InternalConnectionOptions (kernel-only), mirroring `useKernel` and the TLS knobs. The Thrift backend has no JWT client-assertion path, so exposing them on the shared public type would let a Thrift caller set them and have them silently ignored (Eric's divergence concern). - Classify JWT M2M correctly in telemetry `mapAuthType` (`oauth-m2m-jwt`) instead of misreporting it as `external-browser` (bot F1). - Reject a PAT `token` supplied alongside `oauthJwtKeyFile` in the PAT-branch ambiguity guard, so a JWT key can't be silently dropped under authType 'access-token' (bot consistency note). - Add regression tests: connect() on the useKernel path installs no OAuth provider (no eager browser flow) / a PAT-only provider when a token is present (bot F2); plus the new PAT+JWT ambiguity guard. Co-authored-by: Isaac Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
|
@eric-wang-1990 what about the failing security scan jobs? They seem to be unrelated to this pr. Should I suppress them in this PR or there's a workaround? |
Addresses PR #504 review (peco-review-bot Low): the token_url parity fix threaded tokenUrl through the OAuthM2m (shared-secret) branch, but only the JWT path had a tokenUrl test. Add M2M cases asserting tokenUrl forwards when supplied and is absent otherwise, mirroring the JWT tests and guarding the conditional spread at KernelAuth.ts against a future refactor. Co-authored-by: Isaac Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
There was a problem hiding this comment.
Verdict: 2 Low
Looks good — the JWT M2M branch cleanly mirrors the existing M2M/U2M routing, its guards are consistent, and coverage is solid. Two low, narrow notes: telemetry mapAuthType can mislabel a Thrift-path connection as oauth-m2m-jwt, and the deprecated custom authProvider arg is silently dropped on the useKernel path.
|
@rahuls-db the security scan job is non-blocking |
…uthProvider Addresses PR #504 review (peco-review-bot, 2 Low): - mapAuthType keyed the `oauth-m2m-jwt` label purely off `oauthJwtKeyFile` and ran unconditionally. Since oauthJwtKeyFile is a kernel-only internal option, a Thrift-path connection that set it (and would actually run the U2M browser flow) got mislabeled. Gate the JWT label on `useKernel` so it reflects the backend that honors the field (bot F1). - On the useKernel path the deprecated custom `authProvider` arg was silently discarded (the kernel owns auth via the native binding, so a JS-side provider can't be plumbed through). Log a warning instead of dropping it silently so callers can diagnose it (bot F2). Adds mapAuthType tests (kernel vs Thrift JWT labeling) and a test asserting the authProvider-dropped warning. Co-authored-by: Isaac Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
There was a problem hiding this comment.
Verdict: 2 Low
Looks good — the JWT M2M branch is correctly ordered and thoroughly tested; the auth-provider gating on the kernel path is sound. Two low-severity ergonomics notes on the JWT branch around tokenUrl enforcement and the all-apis scope default, both of which would otherwise surface as opaque kernel-side failures.
| } | ||
| const jwt = { | ||
| ...base, | ||
| authMode: 'OAuthM2mJwt' as const, |
There was a problem hiding this comment.
🔵 Low — The JWT M2M branch validates oauthClientId and oauthJwtKid but treats tokenUrl as optional. Per this PR's own description, private_key_jwt cannot succeed against Databricks-native OIDC (it doesn't advertise the method), so tokenUrl (pointing at an external IdP like Entra) is effectively required for this flow to work at all. As written, omitting tokenUrl on the JWT branch passes all TS-side validation and then fails opaquely inside the kernel at connect time. Consider validating tokenUrl presence here with a clear, actionable error — matching the early-validation pattern already used for oauthClientId / oauthJwtKid a few lines above — so callers get the same crisp signal instead of a downstream kernel invalid_client.
| jwtKeyFile: oauth.oauthJwtKeyFile, | ||
| jwtKid: oauth.oauthJwtKid, | ||
| // Configurable (parity with pyo3); defaults to `['all-apis']` in the kernel. | ||
| oauthScopes: |
There was a problem hiding this comment.
🔵 Low — The JWT branch defaults oauthScopes to M2M_DEFAULT_SCOPES (['all-apis']), inherited verbatim from the shared-secret M2M path. But all-apis is a Databricks-native scope; the JWT private_key_jwt flow targets an external IdP (Entra), which rejects all-apis and expects <resource-id>/.default (as the usage example itself shows). Because the value is forwarded explicitly, the kernel's own default is never consulted — so a caller who omits oauthScopes on the JWT branch gets a scope that is essentially guaranteed to be wrong for the only IdP this flow supports. Consider either omitting oauthScopes when unset (letting the kernel default apply) or documenting that oauthScopes is de-facto required for the JWT flow.
What
Adds OAuth machine-to-machine auth with a JWT private-key client assertion (RFC 7523) on the kernel backend (
useKernel: true). The kernel signs a short-lived JWT with the service principal's private key instead of sending a client secret, and owns the token lifecycle; the workspace's OAuth IdP verifies it against the SP's registered public key.Companion to the kernel-side feature (databricks-sql-kernel #249; napi
token_urlin #275) and the parallel databricks-sql-python / databricks-sql-go changes.How
lib/kernel/KernelAuth.ts— new JWT branch inbuildKernelConnectionOptions(checked before the U2M/M2M-secret split; a private-key file is unambiguous JWT M2M intent) plus theOAuthM2mJwtnative-option shape. RequiresoauthClientId+oauthJwtKid; optionaloauthJwtPassphrase/oauthJwtAlgorithm/oauthScopes/tokenUrl. Mutually exclusive withoauthClientSecret. Also threadstokenUrlthrough the existing M2M branch.lib/contracts/IDBSQLClient.ts— newoauthJwt*+tokenUrlfields on thedatabricks-oauthConnectionOptionsmember.lib/DBSQLClient.ts— on theuseKernelpath, do not build the connector's own OAuth provider. It eagerly starts the U2M browser flow / M2M token exchange at connect time (a telemetry / feature-flag client callsauthProvider.authenticate()) before the kernel is consulted — which, for the no-secret JWT case, launched a spurious browser listener. Hand over a minimal PAT provider only when atokenis present. Mirrors the Python connector'suse_kernelhandling.Usage
Testing
tests/unit/kernel/auth-m2m-jwt.test.ts(routing, precedence, required-field validation, ambiguity guards); full kernel unit suite 318 passing. prettier + eslint clean.SELECT 1→[{"n":1}], with the client's backend asserted to beKernelBackend(kernel path, not Thrift).Requires a
@databricks/databricks-sql-kernelbuild with JWT +tokenUrlsupport.This pull request and its description were written by Isaac.