-
Notifications
You must be signed in to change notification settings - Fork 51
feat(kernel): JWT private-key M2M auth on useKernel #504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -721,14 +721,31 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I | |
| // hit endpoints that don't carry the workspace in their URL path. | ||
| this.config.customHeaders = this.buildCustomHeaders(options.path, options.customHeaders); | ||
|
|
||
| this.authProvider = this.createAuthProvider(options, authProvider); | ||
|
|
||
| this.connectionProvider = this.createConnectionProvider(options); | ||
|
|
||
| // M0: `useKernel` is consumed via a non-exported internal-options cast so it | ||
| // doesn't ship in the public `.d.ts`. Mirrors Python's `kwargs.get("use_kernel")` | ||
| // pattern (see databricks-sql-python/src/databricks/sql/session.py). | ||
| const internalOptions = options as ConnectionOptions & InternalConnectionOptions; | ||
|
|
||
| // On the kernel path the kernel owns the full auth lifecycle (it resolves | ||
| // M2M / U2M / JWT purely from the raw options via `buildKernelConnectionOptions`). | ||
| // We must NOT build the connector's own OAuth provider here: for OAuth it | ||
| // eagerly runs the U2M browser flow / M2M token exchange at connect() time | ||
| // (a telemetry / feature-flag client calls `authProvider.authenticate()`), | ||
| // racing — and conflicting with — the kernel's auth. So for `useKernel` we | ||
| // hand over only a minimal PAT provider when a `token` is present, and | ||
| // `undefined` otherwise. Mirrors Python's use_kernel auth-provider handling. | ||
| if (internalOptions.useKernel) { | ||
| const { token } = options as { token?: string }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 Low — This is a behavior change to |
||
| this.authProvider = | ||
| typeof token === 'string' && token.length > 0 | ||
| ? new PlainHttpAuthentication({ username: 'token', password: token, context: this }) | ||
| : undefined; | ||
| } else { | ||
| this.authProvider = this.createAuthProvider(options, authProvider); | ||
| } | ||
|
|
||
| this.connectionProvider = this.createConnectionProvider(options); | ||
|
|
||
| const backend = internalOptions.useKernel | ||
| ? new KernelBackend({ context: this }) | ||
| : new ThriftBackend({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| // Copyright (c) 2026 Databricks, Inc. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| import { expect } from 'chai'; | ||
| import { buildKernelConnectionOptions } from '../../../lib/kernel/KernelAuth'; | ||
| import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient'; | ||
| import HiveDriverError from '../../../lib/errors/HiveDriverError'; | ||
| import AuthenticationError from '../../../lib/errors/AuthenticationError'; | ||
|
|
||
| // A private-key file selects JWT client-assertion M2M (RFC 7523); the kernel | ||
| // signs a short-lived assertion with the key instead of sending a secret. | ||
| const baseJwt = { | ||
| host: 'example.azuredatabricks.net', | ||
| path: '/sql/1.0/warehouses/abc', | ||
| authType: 'databricks-oauth' as const, | ||
| oauthClientId: 'sp-uuid', | ||
| oauthJwtKeyFile: '/keys/jwt.pem', | ||
| oauthJwtKid: 'kid-1', | ||
| }; | ||
|
|
||
| describe('KernelAuth — OAuth M2M JWT private-key auth flow', () => { | ||
| it('routes oauthJwtKeyFile to authMode OAuthM2mJwt with the required fields', () => { | ||
| const native = buildKernelConnectionOptions(baseJwt as ConnectionOptions); | ||
| expect(native.authMode).to.equal('OAuthM2mJwt'); | ||
| const jwt = native as { | ||
| oauthClientId?: string; | ||
| jwtKeyFile?: string; | ||
| jwtKid?: string; | ||
| oauthScopes?: string[]; | ||
| }; | ||
| expect(jwt.oauthClientId).to.equal('sp-uuid'); | ||
| expect(jwt.jwtKeyFile).to.equal('/keys/jwt.pem'); | ||
| expect(jwt.jwtKid).to.equal('kid-1'); | ||
| // Defaults to the M2M scope (parity with pyo3 / the secret M2M path). | ||
| expect(jwt.oauthScopes).to.deep.equal(['all-apis']); | ||
| }); | ||
|
|
||
| it('forwards optional passphrase / algorithm / tokenUrl / scopes when present', () => { | ||
| const native = buildKernelConnectionOptions({ | ||
| ...baseJwt, | ||
| oauthJwtPassphrase: 'pw', | ||
| oauthJwtAlgorithm: 'ES256', | ||
| tokenUrl: 'https://login.microsoftonline.com/tenant/oauth2/v2.0/token', | ||
| oauthScopes: ['2ff814a6-.../.default'], | ||
| } as ConnectionOptions); | ||
| const jwt = native as { | ||
| jwtPassphrase?: string; | ||
| jwtAlgorithm?: string; | ||
| tokenUrl?: string; | ||
| oauthScopes?: string[]; | ||
| }; | ||
| expect(jwt.jwtPassphrase).to.equal('pw'); | ||
| expect(jwt.jwtAlgorithm).to.equal('ES256'); | ||
| expect(jwt.tokenUrl).to.equal('https://login.microsoftonline.com/tenant/oauth2/v2.0/token'); | ||
| expect(jwt.oauthScopes).to.deep.equal(['2ff814a6-.../.default']); | ||
| }); | ||
|
|
||
| it('omits optional fields when not supplied', () => { | ||
| const native = buildKernelConnectionOptions(baseJwt as ConnectionOptions); | ||
| expect(native).to.not.have.property('jwtPassphrase'); | ||
| expect(native).to.not.have.property('jwtAlgorithm'); | ||
| expect(native).to.not.have.property('tokenUrl'); | ||
| }); | ||
|
|
||
| it('takes precedence over the shared-secret M2M / U2M split', () => { | ||
| // A private key present makes this JWT M2M regardless of anything else | ||
| // (no secret ⇒ would otherwise be U2M). | ||
| const native = buildKernelConnectionOptions(baseJwt as ConnectionOptions); | ||
| expect(native.authMode).to.equal('OAuthM2mJwt'); | ||
| }); | ||
|
|
||
| it('rejects oauthJwtKeyFile together with oauthClientSecret (ambiguous)', () => { | ||
| expect(() => | ||
| buildKernelConnectionOptions({ | ||
| ...baseJwt, | ||
| oauthClientSecret: 'shh', | ||
| } as ConnectionOptions), | ||
| ).to.throw(HiveDriverError, /both `oauthJwtKeyFile`.*`oauthClientSecret`/); | ||
| }); | ||
|
|
||
| it('requires oauthClientId', () => { | ||
| const { oauthClientId, ...noClientId } = baseJwt; | ||
| expect(() => buildKernelConnectionOptions(noClientId as ConnectionOptions)).to.throw( | ||
| AuthenticationError, | ||
| /requires `oauthClientId`/, | ||
| ); | ||
| }); | ||
|
|
||
| it('requires oauthJwtKid', () => { | ||
| const { oauthJwtKid, ...noKid } = baseJwt; | ||
| expect(() => buildKernelConnectionOptions(noKid as ConnectionOptions)).to.throw( | ||
| AuthenticationError, | ||
| /requires `oauthJwtKid`/, | ||
| ); | ||
| }); | ||
|
|
||
| it('rejects persistence on the JWT M2M path', () => { | ||
| expect(() => | ||
| buildKernelConnectionOptions({ | ||
| ...baseJwt, | ||
| persistence: {} as never, | ||
| } as ConnectionOptions), | ||
| ).to.throw(HiveDriverError, /persistence/); | ||
| }); | ||
|
|
||
| it('prepends `/` to the path on the JWT branch too', () => { | ||
| const native = buildKernelConnectionOptions({ | ||
| ...baseJwt, | ||
| path: 'sql/1.0/warehouses/abc', | ||
| } as ConnectionOptions); | ||
| expect((native as { httpPath: string }).httpPath).to.equal('/sql/1.0/warehouses/abc'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 Low — The new JWT private-key M2M flow is not reflected in telemetry
authType.mapAuthType(same file, called unconditionally at the top ofconnect()) keys offoauthClientSecret === undefined ? 'external-browser' : 'oauth-m2m'. For the JWT pathoauthClientSecretis (and must be) undefined, so every JWT M2M kernel connection is reported to telemetry asexternal-browser(i.e. U2M browser flow) — the opposite of its actual machine-to-machine nature. Consider distinguishing the JWT case (e.g. presence ofoauthJwtKeyFile) so telemetry attribution is accurate.