feat(kernel): JWT private-key M2M auth via WithJWTPrivateKeyM2M - #447
feat(kernel): JWT private-key M2M auth via WithJWTPrivateKeyM2M#447rahuls-db wants to merge 4 commits into
Conversation
Add OAuth M2M auth with a JWT private-key client assertion (RFC 7523) on
the kernel backend. The kernel signs a short-lived assertion with the
private key instead of sending a client secret and owns the token
lifecycle. Kernel-backend only (the pure-Go Thrift path can't sign the
assertion).
- auth/oauth/jwtm2m: new authenticator satisfying the (internal)
JWTM2MCredentialsProvider interface; Authenticate returns a clear
kernel-only error on the Thrift path.
- internal/backend/kernel: AuthJWTM2M mode + Auth JWT fields +
JWTM2MCredentialsProvider interface; setAuth maps it to the
kernel_session_config_set_auth_m2m_jwt C setter (8 args incl. token_url).
- kernel_config: resolveKernelAuth resolves the JWT authenticator to the
descriptor (scopes + token_url forwarded, unlike the scopes-less
shared-secret M2M setter).
- connector: WithJWTPrivateKeyM2M(JWTPrivateKeyM2MConfig{...}) — a struct
option (not positional args) so the many fields can't be transposed.
- Bump KERNEL_REV to a build with the JWT C-ABI setter; update the
TestSetRetry max<min case to the kernel's new correct-and-warn behavior.
- Tests: jwtm2m unit tests, resolveKernelAuth JWT cases, a cgo
set_auth_m2m_jwt marshalling case, and an env-gated live E2E
(TestKernelE2EJWTM2MSelect1).
Verified end-to-end: SELECT 1 via WithUseKernel(true) +
WithJWTPrivateKeyM2M against an Azure Databricks warehouse, authenticated
by Entra ID with a JWT private-key assertion.
DSN-string config (authType=...) for JWT M2M is deferred to a follow-up.
Signed-off-by: Rahul Singhal <rahul.singhal@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: 2 Low
Looks good — clean addition that faithfully mirrors the existing M2M/U2M structural-interface + kernel-setter pattern, with matching unit/cgo/e2e coverage. Two low-severity doc/consistency nits: a stray blank line splits the godoc option list, and the default-case rejection message + docstrings still enumerate only PAT/M2M/U2M as supported. The 8-arg set_auth_m2m_jwt marshalling (newCStr for required, newCStrOrNull for optional), scope forwarding, and free() handling all look correct and consistent with the U2M path.
| - WithMaxDownloadThreads (<num_threads> int). Sets up the max number of concurrent workers for cloud fetch. Default is 10. Optional | ||
| - WithAuthenticator (<authenticator> auth.Authenticator). Sets up authentication. Required if neither access token or client credentials are provided. | ||
| - WithClientCredentials(<clientID> string, <clientSecret> string). Sets up Oauth M2M authentication. | ||
| - WithJWTPrivateKeyM2M(<config> JWTPrivateKeyM2MConfig). Sets up OAuth M2M authentication with a JWT private-key client assertion (RFC 7523) instead of a client secret. Kernel backend only (requires WithUseKernel(true)); the kernel signs the assertion. See the kernel-backend section. Optional |
There was a problem hiding this comment.
🔵 Low — The new WithJWTPrivateKeyM2M bullet is followed by a blank line before the WithUseKernel bullet. In godoc's comment parser a blank line between list items terminates the list, so this splits the single "Supported functional options" bullet list into two separate lists — WithUseKernel / WithWarehouseID render as a detached second list rather than continuing the same one. Every other bullet in this block is contiguous. Remove the blank line to keep it one list.
| // an opaque Unauthenticated. | ||
| func resolveKernelAuth(cfg *config.Config) (kernel.Auth, error) { | ||
| switch a := cfg.Authenticator.(type) { | ||
| case kernel.JWTM2MCredentialsProvider: |
There was a problem hiding this comment.
🔵 Low — Now that JWT private-key M2M is a supported kernel auth form (this new case), the sibling enumerations that list what the kernel supports are stale: (1) the default: case's rejection message still reads "PAT (WithAccessToken) and OAuth M2M/U2M (WithClientCredentials / authType) are supported" — it omits JWT private-key M2M / WithJWTPrivateKeyM2M; and (2) the resolveKernelAuth doc comment's bullet list and the validateKernelConfig docstring ("PAT, or OAuth M2M/U2M") likewise don't mention the JWT mode. A caller who lands in the default rejection will see an incomplete list of supported options. Consider updating both the message and the docstrings to include JWT private-key M2M.
Address golangci-lint failures on #447: - gofmt doc.go after the WithJWTPrivateKeyM2M option-list addition. - //nolint:gosec G101 on the fakeJWTM2MAuth test literal (a fake passphrase in a unit test, not a real credential) — same convention the existing proxy tests use. Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
There was a problem hiding this comment.
Verdict: 2 Low
Solid, well-tested addition — kernel-only JWT private-key M2M auth is wired cleanly through the connector, auth descriptor, and cgo setter with matching unit/e2e coverage. Two low-severity notes: the option silently no-ops on a missing required field (misleading downstream PAT error), and JWT M2M kernel connections still run telemetry through an authenticator whose Authenticate always errors (best-effort, non-blocking, but a silent observability gap vs. the PAT/M2M paths).
| // this authenticator fails at authenticate time with a clear kernel-only error. | ||
| func WithJWTPrivateKeyM2M(cfg JWTPrivateKeyM2MConfig) ConnOption { | ||
| return func(c *config.Config) { | ||
| if cfg.ClientID != "" && cfg.KeyFile != "" && cfg.Kid != "" { |
There was a problem hiding this comment.
🔵 Low — WithJWTPrivateKeyM2M silently does nothing if any of the three required fields (ClientID, KeyFile, Kid) is empty — c.Authenticator is left untouched. This mirrors the existing WithClientCredentials guard, but here there are three required string fields (easy to forget Kid, or leave one blank from an unset env var), and the failure mode is confusing: with no other authenticator set, the kernel path falls through resolveKernelAuth to the PAT branch and reports "the kernel backend requires a personal access token" — which names the wrong problem. Worse, if WithAccessToken was also supplied, the connection silently authenticates as PAT while the caller believes they configured JWT M2M.
Consider not silently dropping a partially-specified JWT config — e.g. still install the authenticator (letting the kernel report the missing field) or surface the misconfiguration, so a typo/blank field doesn't degrade to a misleading PAT error. Not a blocker given the established convention, but the three-field surface makes this materially easier to hit than the two-field M2M option.
| "time" | ||
|
|
||
| "github.com/databricks/databricks-sql-go/auth" | ||
| "github.com/databricks/databricks-sql-go/auth/oauth/jwtm2m" |
There was a problem hiding this comment.
🔵 Low — On the kernel backend, telemetry is only skipped for U2M (interactiveU2MAuthenticator check here). A JWT M2M connection goes through normal telemetry init, whose feature-flag and export HTTP calls use the authenticated c.client. That client's Transport.RoundTrip calls t.Authr.Authenticate(req2) (client.go:549), and the jwtm2m authenticator's Authenticate unconditionally returns the kernel-only error — so every telemetry/feature-flag request fails auth. The connection still succeeds (telemetry is best-effort), but telemetry is effectively always disabled for JWT M2M kernel connections and each attempt burns a failing (potentially retried) HTTP round-trip at connect time. This is inconsistent with the PAT/M2M kernel paths, where Authenticate actually works. Worth considering extending the telemetry-skip (or providing a token source) for the JWT M2M kernel case.
(Anchored to the nearest changed line — see the description for the exact location.)
| AuthPAT AuthMode = iota // personal access token | ||
| AuthM2M // OAuth client-credentials (client id + secret) | ||
| AuthU2M // OAuth user-to-machine (browser/PKCE; kernel-owned flow) | ||
| AuthJWTM2M // OAuth client-credentials via a JWT private-key client assertion |
There was a problem hiding this comment.
Same here, we do not need it in Go if it does not support it now.
Addresses PR #447 review feedback: - Gate the public JWT private-key M2M surface behind `cgo && databricks_kernel`. Move WithJWTPrivateKeyM2M and JWTPrivateKeyM2MConfig out of connector.go into a new tagged connector_kernel_jwt.go, and tag the jwtm2m package itself. The pure-Go Thrift path has no JWT-signing implementation, so a default (Thrift-only) build no longer exposes an auth option it cannot honor (Eric's divergence concern). The internal JWTM2MCredentialsProvider interface stays untagged so resolveKernelAuth still compiles in the default build. - Skip telemetry on the kernel JWT M2M path: its Authenticate always returns a kernel-only error, so telemetry/feature-flag HTTP calls would fail every request and burn round-trips at connect. Extend the existing U2M telemetry-skip to cover it (bot F2). - Fix the godoc option list: restore the contiguous bullet style (a blank line between items terminates the godoc list) and add the JWT bullet inline (bot F1). - Update the stale supported-auth enumerations: resolveKernelAuth doc, validateKernelConfig docstring, and the default-case rejection message now include JWT private-key M2M (bot F2, first review). Co-authored-by: Isaac Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good — a clean, well-tested addition of kernel-only JWT private-key M2M auth (RFC 7523). Build-tag gating, interface pattern, and unit/cgo/e2e coverage all match existing M2M/U2M conventions; one low note on silent partial-config drop. Nit: the TestKernelE2EJWTM2MSelect1 env-var doc block (kernel_jwt_e2e_test.go) lists DATABRICKS_JWT_TOKEN_URL/DATABRICKS_JWT_SCOPES as optional but omits DATABRICKS_JWT_PASSPHRASE and DATABRICKS_JWT_ALGORITHM, which the test body also reads — worth listing for completeness."
| func WithJWTPrivateKeyM2M(cfg JWTPrivateKeyM2MConfig) ConnOption { | ||
| return func(c *config.Config) { | ||
| if cfg.ClientID != "" && cfg.KeyFile != "" && cfg.Kid != "" { | ||
| c.Authenticator = jwtm2m.NewAuthenticator( |
There was a problem hiding this comment.
🔵 Low — WithJWTPrivateKeyM2M silently does nothing when any of the three required fields (ClientID, KeyFile, Kid) is empty — c.Authenticator is left untouched. This mirrors the established WithClientCredentials/WithAccessToken pattern, so it's consistent, but it is a footgun: a caller who supplies ClientID + KeyFile but forgets Kid gets no authenticator set, and the failure surfaces later as a confusing "unsupported authenticator" / unauthenticated error at connect rather than at option-construction time. Since a struct option already avoids field transposition (the stated design goal), consider at least logging a debug/warn when a partial JWT config is dropped, so the misconfiguration isn't wholly invisible. Not blocking — flagging for awareness given it's the intended pattern.
… vars Addresses PR #447 review (peco-review-bot, 1 Low + a doc nit): - WithJWTPrivateKeyM2M now warns when given a PARTIAL config (some of the required ClientID/KeyFile/Kid set, others blank — e.g. a typo or unset env var). A fully-empty config stays a silent no-op (consistent with WithClientCredentials/WithAccessToken), but a partial one would otherwise install no authenticator and later surface as a misleading "requires a personal access token" error, so the misconfiguration is now visible. - Document DATABRICKS_JWT_PASSPHRASE and DATABRICKS_JWT_ALGORITHM in the TestKernelE2EJWTM2MSelect1 env-var block (the test body already reads them). Adds a kernel-tagged unit test for full/empty/partial WithJWTPrivateKeyM2M. Co-authored-by: Isaac Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
2 participants
What
Adds OAuth machine-to-machine auth with a JWT private-key client assertion (RFC 7523) on the kernel backend (
WithUseKernel(true)), via a newWithJWTPrivateKeyM2M(JWTPrivateKeyM2MConfig{...})connector option. The kernel signs a short-lived assertion with the service principal's private key instead of sending a client secret and owns the token lifecycle.Companion to the kernel-side feature (databricks-sql-kernel #249; napi
token_urlin #275) and the parallel databricks-sql-python / databricks-sql-nodejs changes.How
auth/oauth/jwtm2m— new authenticator satisfying the internalJWTM2MCredentialsProviderinterface (same structural-interface pattern as M2M/U2M).Authenticatereturns a clear kernel-only error on the pure-Go Thrift path (JWT signing is the kernel's job).internal/backend/kernel—AuthJWTM2Mmode +AuthJWT fields +JWTM2MCredentialsProvider;setAuthmaps it to thekernel_session_config_set_auth_m2m_jwtC setter (8 args incl.token_url).kernel_config.go—resolveKernelAuthresolves the JWT authenticator to the descriptor (scopes +token_urlforwarded, unlike the scopes-less shared-secret M2M setter).connector.go—WithJWTPrivateKeyM2M(JWTPrivateKeyM2MConfig{...}), a struct option (not positional args) so the many string fields can't be transposed.KERNEL_REVbumped to a build including the JWT C-ABI setter; theTestSetRetrymax<min case updated to the kernel's newer correct-and-warn behavior.Usage
Testing
jwtm2mauthenticator tests;resolveKernelAuthJWT cases inkernel_config_test.go.set_auth_m2m_jwtmarshalling case inTestSetAuthByMode(tagged build).TestKernelE2EJWTM2MSelect1(tagged build). Verified end-to-end against an Azure Databricks warehouse:SELECT 1= 1 via the kernel backend, authenticated by Entra ID with a JWT private-key assertion.Not in scope
DSN-string configuration (
authType=oauthm2m-jwt+ key-file/passphrase/tokenUrl query params) is deferred to a follow-up — the programmaticWithJWTPrivateKeyM2Mis the primary surface (passing a key-file path + passphrase through a DSN query string is awkward, and there is no cross-driver DSN precedent for JWT). Use the option.This pull request and its description were written by Isaac.