Skip to content

Commit 19e0d0e

Browse files
committed
fix(kernel): move JWT M2M options to internal type; address review
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>
1 parent 651159e commit 19e0d0e

6 files changed

Lines changed: 134 additions & 19 deletions

File tree

‎lib/DBSQLClient.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,8 +497,17 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
497497
*/
498498
private mapAuthType(options: ConnectionOptions): string {
499499
switch (options.authType) {
500-
case 'databricks-oauth':
500+
case 'databricks-oauth': {
501+
// JWT private-key M2M (kernel-only) presents no `oauthClientSecret`,
502+
// so without this check it would misreport as `external-browser`
503+
// (U2M) — the opposite of its machine-to-machine nature. The field
504+
// lives on the internal options surface (see InternalConnectionOptions).
505+
const { oauthJwtKeyFile } = options as ConnectionOptions & InternalConnectionOptions;
506+
if (oauthJwtKeyFile !== undefined) {
507+
return 'oauth-m2m-jwt';
508+
}
501509
return options.oauthClientSecret === undefined ? 'external-browser' : 'oauth-m2m';
510+
}
502511
case 'custom':
503512
return 'custom';
504513
case 'token-provider':

‎lib/contracts/IDBSQLClient.ts‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,6 @@ type AuthOptions =
2626
// U2M flow to `['sql', 'offline_access']` (parity with the Thrift driver's
2727
// `defaultOAuthScopes`), overriding the kernel's bare `all-apis offline_access`.
2828
oauthScopes?: Array<string>;
29-
// JWT private-key M2M (RFC 7523 client assertion) — KERNEL BACKEND ONLY
30-
// (`useKernel: true`). Supplying `oauthJwtKeyFile` selects the JWT
31-
// client-assertion flow: the kernel signs a short-lived assertion with the
32-
// private key instead of sending a client secret. Requires `oauthClientId`
33-
// and `oauthJwtKid`. Optional `oauthJwtPassphrase` (encrypted PKCS#8 key),
34-
// `oauthJwtAlgorithm` (default `RS256`), `oauthScopes`, and `tokenUrl` (the
35-
// IdP token endpoint — required when auth is against an external IdP such as
36-
// Entra ID, which is where `private_key_jwt` is supported). Mutually
37-
// exclusive with `oauthClientSecret`.
38-
oauthJwtKeyFile?: string;
39-
oauthJwtKid?: string;
40-
oauthJwtPassphrase?: string;
41-
oauthJwtAlgorithm?: string;
42-
// OAuth token endpoint override (kernel backend). Points the M2M /
43-
// JWT client-assertion grant at the workspace's IdP token endpoint.
44-
tokenUrl?: string;
4529
}
4630
| {
4731
authType: 'custom';

‎lib/contracts/InternalConnectionOptions.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,55 @@ export interface InternalConnectionOptions {
7474
* @internal kernel path only.
7575
*/
7676
clientKeyPem?: Buffer | string;
77+
78+
/**
79+
* kernel-only: JWT private-key M2M (RFC 7523 client assertion). Supplying
80+
* `oauthJwtKeyFile` (alongside `authType: 'databricks-oauth'`) selects the
81+
* JWT client-assertion flow: the kernel signs a short-lived assertion with
82+
* the private key instead of sending a client secret. Requires
83+
* `oauthClientId` (the assertion issuer/subject) and `oauthJwtKid` (the key
84+
* id written into the JWT header). Mutually exclusive with
85+
* `oauthClientSecret`.
86+
*
87+
* These live on the internal options surface — NOT the public
88+
* `databricks-oauth` `AuthOptions` — because the Thrift backend has no
89+
* JWT client-assertion path; exposing them publicly would let a Thrift
90+
* caller set them and have them silently ignored. The kernel path reads
91+
* them via the `InternalConnectionOptions` cast, exactly like `useKernel`
92+
* and the TLS knobs above.
93+
* @internal kernel path only.
94+
*/
95+
oauthJwtKeyFile?: string;
96+
97+
/**
98+
* kernel-only: key id written into the JWT assertion header so the IdP can
99+
* select the registered public key. Required when `oauthJwtKeyFile` is set.
100+
* @internal kernel path only.
101+
*/
102+
oauthJwtKid?: string;
103+
104+
/**
105+
* kernel-only: passphrase for an encrypted PKCS#8 private key
106+
* (`oauthJwtKeyFile`). Omit for an unencrypted key.
107+
* @internal kernel path only.
108+
*/
109+
oauthJwtPassphrase?: string;
110+
111+
/**
112+
* kernel-only: JWT signing algorithm for the client assertion. Defaults to
113+
* `RS256` in the kernel when omitted.
114+
* @internal kernel path only.
115+
*/
116+
oauthJwtAlgorithm?: string;
117+
118+
/**
119+
* kernel-only: OAuth token-endpoint override. Points the M2M /
120+
* JWT client-assertion grant at the workspace's IdP token endpoint —
121+
* required when auth is against an external IdP such as Entra ID, which is
122+
* where `private_key_jwt` is supported. Applies to both shared-secret M2M
123+
* and JWT M2M (auth-method-agnostic, matching JDBC's
124+
* `OAuth2ConnAuthTokenEndpoint`).
125+
* @internal kernel path only.
126+
*/
127+
tokenUrl?: string;
77128
}

‎lib/kernel/KernelAuth.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,9 +629,13 @@ export function buildKernelConnectionOptions(options: ConnectionOptions): Kernel
629629
"kernel backend: a non-empty PAT must be supplied via `token` when using `authType: 'access-token'`.",
630630
);
631631
}
632-
if (oauth.oauthClientId !== undefined || oauth.oauthClientSecret !== undefined) {
632+
if (
633+
oauth.oauthClientId !== undefined ||
634+
oauth.oauthClientSecret !== undefined ||
635+
oauth.oauthJwtKeyFile !== undefined
636+
) {
633637
throw new HiveDriverError(
634-
'kernel backend: cannot supply both `token` and `oauthClientId`/`oauthClientSecret` ' +
638+
'kernel backend: cannot supply both `token` and `oauthClientId`/`oauthClientSecret`/`oauthJwtKeyFile` ' +
635639
"on the same connection. Pick one: 'access-token' (PAT) uses `token`; " +
636640
"'databricks-oauth' uses the OAuth fields.",
637641
);

‎tests/unit/DBSQLClient.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,57 @@ describe('DBSQLClient.connect', () => {
284284
}
285285
});
286286

287+
it('useKernel: true with an OAuth flow installs NO auth provider (kernel owns auth; no eager browser flow)', async () => {
288+
const client = new DBSQLClient();
289+
290+
// `useKernel` + `databricks-oauth` (U2M: no secret, no token). The kernel
291+
// owns the full auth lifecycle here, so `connect()` must NOT build the
292+
// connector's own OAuth provider (which would eagerly open a browser /
293+
// run the token exchange at connect() time via a telemetry client). The
294+
// authProvider is assigned before the backend connects, so it is set even
295+
// though the subsequent KernelBackend connect() rejects (absent native
296+
// binding in CI / no live workspace).
297+
const kernelOAuthOptions = {
298+
...connectOptions,
299+
token: undefined,
300+
authType: 'databricks-oauth',
301+
useKernel: true,
302+
} as any;
303+
304+
try {
305+
await client.connect(kernelOAuthOptions);
306+
} catch (error) {
307+
if (error instanceof AssertionError || !(error instanceof Error)) {
308+
throw error;
309+
}
310+
// Expected: KernelBackend connect() rejects (native binding absent / no
311+
// live workspace). The contract under test is the authProvider decision,
312+
// which happened before the throw.
313+
}
314+
315+
expect(client['authProvider']).to.be.undefined;
316+
});
317+
318+
it('useKernel: true with a token installs a PAT-only PlainHttpAuthentication provider', async () => {
319+
const client = new DBSQLClient();
320+
321+
// `useKernel` + a PAT: the connector hands the kernel a minimal PAT
322+
// provider (for the telemetry / feature-flag clients) rather than
323+
// undefined, and still must NOT build an OAuth provider.
324+
const kernelPatOptions = { ...connectOptions, token: 'dapiXXXX', useKernel: true } as any;
325+
326+
try {
327+
await client.connect(kernelPatOptions);
328+
} catch (error) {
329+
if (error instanceof AssertionError || !(error instanceof Error)) {
330+
throw error;
331+
}
332+
// Expected: KernelBackend connect() rejects (native binding absent).
333+
}
334+
335+
expect(client['authProvider']).to.be.instanceOf(PlainHttpAuthentication);
336+
});
337+
287338
it('populates config.customHeaders with org-id parsed from ?o= (SPOG)', async () => {
288339
const client = new DBSQLClient();
289340
await client.connect({ ...connectOptions, path: '/sql/1.0/warehouses/abc?o=12345678901234' });

‎tests/unit/kernel/auth-m2m-jwt.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,22 @@ describe('KernelAuth — OAuth M2M JWT private-key auth flow', () => {
105105
);
106106
});
107107

108+
it('rejects a PAT `token` supplied alongside `oauthJwtKeyFile` (ambiguous)', () => {
109+
// A JWT key under the PAT path (authType access-token) would otherwise be
110+
// silently dropped; the PAT-branch ambiguity guard must reject it, just as
111+
// it does for oauthClientId / oauthClientSecret.
112+
expect(() =>
113+
buildKernelConnectionOptions({
114+
host: 'example.azuredatabricks.net',
115+
path: '/sql/1.0/warehouses/abc',
116+
authType: 'access-token',
117+
token: 'dapiXXXX',
118+
oauthJwtKeyFile: '/keys/jwt.pem',
119+
oauthJwtKid: 'kid-1',
120+
} as ConnectionOptions),
121+
).to.throw(HiveDriverError, /both `token` and .*`oauthJwtKeyFile`/);
122+
});
123+
108124
it('rejects persistence on the JWT M2M path', () => {
109125
expect(() =>
110126
buildKernelConnectionOptions({

0 commit comments

Comments
 (0)