From bfaa2cc3fc6c10d0d5e11f0f166c2da5dbe8266e Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Fri, 15 May 2026 01:39:43 +0000 Subject: [PATCH 1/2] =?UTF-8?q?sea-auth:=20PAT=20auth=20flow=20through=20S?= =?UTF-8?q?eaBackend=20=E2=86=92=20napi=20binding?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SeaBackend.connect() now wires PAT options to the napi binding's openSession(). Non-PAT modes rejected with clear M0-scope error (OAuth/Azure/Federation land in M1). E2E test against pecotesting confirms PAT round-trips: connect → openSession → close all clean. No new dependencies. SeaAuth helper is ~30 LOC. Signed-off-by: Madhavendra Rathore --- lib/sea/SeaAuth.ts | 83 +++++++ lib/sea/SeaBackend.ts | 166 ++++++++++++- tests/integration/.mocharc.js | 11 + tests/integration/sea/auth-pat-e2e.test.ts | 75 ++++++ tests/unit/sea/SeaBackend.test.ts | 39 --- tests/unit/sea/auth-pat.test.ts | 263 +++++++++++++++++++++ 6 files changed, 588 insertions(+), 49 deletions(-) create mode 100644 lib/sea/SeaAuth.ts create mode 100644 tests/integration/.mocharc.js create mode 100644 tests/integration/sea/auth-pat-e2e.test.ts delete mode 100644 tests/unit/sea/SeaBackend.test.ts create mode 100644 tests/unit/sea/auth-pat.test.ts diff --git a/lib/sea/SeaAuth.ts b/lib/sea/SeaAuth.ts new file mode 100644 index 00000000..cf16c80f --- /dev/null +++ b/lib/sea/SeaAuth.ts @@ -0,0 +1,83 @@ +// 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 { ConnectionOptions } from '../contracts/IDBSQLClient'; +import AuthenticationError from '../errors/AuthenticationError'; +import HiveDriverError from '../errors/HiveDriverError'; + +/** + * Shape consumed by the napi-binding's `openSession()` (see + * `native/sea/index.d.ts`). M0 supports PAT only — `token` is required. + * + * Mirrors `ConnectionOptions` in the binding's `.d.ts`; declared locally + * to avoid coupling the JS-side adapter to the auto-generated TS file. + */ +export interface SeaNativeConnectionOptions { + hostName: string; + httpPath: string; + token: string; +} + +function prependSlash(str: string): string { + if (str.length > 0 && str.charAt(0) !== '/') { + return `/${str}`; + } + return str; +} + +/** + * Validate that the user-supplied `ConnectionOptions` describe a PAT auth + * configuration and build the napi-binding's connection-options shape. + * + * M0 SCOPE: PAT only. + * - Accepts `authType: 'access-token'` and the undefined-authType default + * (which already means PAT throughout the existing driver — see + * `DBSQLClient.createAuthProvider`). + * - Rejects every other `authType` discriminant with a clear + * "M0 supports only PAT" message so callers know OAuth / Federation / + * custom providers land in M1. + * + * Throws: + * - `AuthenticationError` when the auth mode is PAT but `token` is missing + * or empty. + * - `HiveDriverError` when the auth mode is anything other than PAT. + */ +export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNativeConnectionOptions { + const { authType } = options as { authType?: string }; + + if (authType !== undefined && authType !== 'access-token') { + throw new HiveDriverError( + `SEA backend (M0) supports only PAT auth (authType: 'access-token'); ` + + `got authType: '${authType}'. Other auth modes (databricks-oauth, ` + + `token-provider, external-token, static-token, custom) will land in M1.`, + ); + } + + // PAT path — at this point `options` is structurally the access-token branch + // of `AuthOptions`, which guarantees a `token` field at the type level. We + // still defensively re-check because the public ConnectionOptions type + // permits `authType: undefined` with no token at runtime. + const { token } = options as { token?: string }; + if (typeof token !== 'string' || token.length === 0) { + throw new AuthenticationError( + 'SEA backend: a non-empty PAT must be supplied via `token` when using `authType: \'access-token\'`.', + ); + } + + return { + hostName: options.host, + httpPath: prependSlash(options.path), + token, + }; +} diff --git a/lib/sea/SeaBackend.ts b/lib/sea/SeaBackend.ts index 43958679..ee20a1ba 100644 --- a/lib/sea/SeaBackend.ts +++ b/lib/sea/SeaBackend.ts @@ -1,23 +1,169 @@ +// 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 IBackend from '../contracts/IBackend'; import ISessionBackend from '../contracts/ISessionBackend'; +import IOperationBackend from '../contracts/IOperationBackend'; import { ConnectionOptions, OpenSessionRequest } from '../contracts/IDBSQLClient'; +import { + ExecuteStatementOptions, + TypeInfoRequest, + CatalogsRequest, + SchemasRequest, + TablesRequest, + TableTypesRequest, + ColumnsRequest, + FunctionsRequest, + PrimaryKeysRequest, + CrossReferenceRequest, +} from '../contracts/IDBSQLSession'; +import Status from '../dto/Status'; +import InfoValue from '../dto/InfoValue'; import HiveDriverError from '../errors/HiveDriverError'; +import { getSeaNative, SeaNativeBinding } from './SeaNativeLoader'; +import { buildSeaConnectionOptions, SeaNativeConnectionOptions } from './SeaAuth'; + +const NOT_IMPLEMENTED_SESSION = + 'SEA session backend: method not implemented in sea-auth (M0); lands in sea-execution/sea-operation.'; + +/** + * Opaque handle to the napi binding's `Connection` class. The exact + * shape lives in `native/sea/index.d.ts` (auto-generated). We type it as + * a structural minimum here so the loader's pass-through typing doesn't + * leak into every call site. + */ +interface NativeConnection { + close(): Promise; +} + +/** + * Minimal `ISessionBackend` that wraps the napi-binding's `Connection`. + * + * For M0 (sea-auth) only `id` and `close()` are functional — they're the + * subset required to round-trip a connect-open-close cycle. Every other + * method throws a clear "not implemented in M0" `HiveDriverError`. + * + * The `id` field is currently a synthetic counter-based string; the kernel + * exposes a real session-id through a follow-on getter that + * `sea-execution` will wire through. + */ +export class SeaSessionBackend implements ISessionBackend { + private static seq = 0; + + public readonly id: string; + + private readonly connection: NativeConnection; + + constructor(connection: NativeConnection) { + this.connection = connection; + SeaSessionBackend.seq += 1; + this.id = `sea-session-${SeaSessionBackend.seq}`; + } + + /* eslint-disable @typescript-eslint/no-unused-vars */ + public async getInfo(_infoType: number): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async executeStatement( + _statement: string, + _options: ExecuteStatementOptions, + ): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async getTypeInfo(_request: TypeInfoRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async getCatalogs(_request: CatalogsRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async getSchemas(_request: SchemasRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } -const NOT_IMPLEMENTED = 'SEA backend not implemented yet — wired in sea-napi-binding feature'; + public async getTables(_request: TablesRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async getTableTypes(_request: TableTypesRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async getColumns(_request: ColumnsRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + public async getFunctions(_request: FunctionsRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async getPrimaryKeys(_request: PrimaryKeysRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + + public async getCrossReference(_request: CrossReferenceRequest): Promise { + throw new HiveDriverError(NOT_IMPLEMENTED_SESSION); + } + /* eslint-enable @typescript-eslint/no-unused-vars */ + + public async close(): Promise { + await this.connection.close(); + return Status.success(); + } +} + +/** + * M0 SeaBackend — wires PAT auth + napi `openSession` end-to-end. + * + * Connect is a no-op at this layer (the napi binding has no notion of a + * standalone "connect"; a session is opened directly). We capture the + * validated PAT options and hand them to `openSession()` on demand. + * + * Subsequent milestones (`sea-execution`, `sea-operation`) replace the + * stubbed `ISessionBackend` / `IOperationBackend` methods with real + * napi-binding calls. + */ export default class SeaBackend implements IBackend { - // eslint-disable-next-line @typescript-eslint/no-unused-vars, class-methods-use-this + private nativeOptions?: SeaNativeConnectionOptions; + + private readonly native: SeaNativeBinding; + + constructor(native: SeaNativeBinding = getSeaNative()) { + this.native = native; + } + public async connect(options: ConnectionOptions): Promise { - throw new HiveDriverError(NOT_IMPLEMENTED); + // Validate PAT auth + capture the napi-binding option shape. + // Any non-PAT mode (or a missing token) throws here, before we ever + // touch the native binding. + this.nativeOptions = buildSeaConnectionOptions(options); } - // eslint-disable-next-line @typescript-eslint/no-unused-vars, class-methods-use-this - public async openSession(request: OpenSessionRequest): Promise { - throw new HiveDriverError(NOT_IMPLEMENTED); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + public async openSession(_request: OpenSessionRequest): Promise { + if (!this.nativeOptions) { + throw new HiveDriverError('SeaBackend: connect() must be called before openSession().'); + } + const connection = (await this.native.openSession(this.nativeOptions)) as NativeConnection; + return new SeaSessionBackend(connection); } - // No-op so DBSQLClient.close() can finish its state-clearing block after a - // failed useSEA: true connect. Real teardown lands with the M1 SEA impl. - // eslint-disable-next-line @typescript-eslint/no-empty-function, class-methods-use-this - public async close(): Promise {} + public async close(): Promise { + // Connection-level resources are owned by the session wrapper. No-op here. + this.nativeOptions = undefined; + } } diff --git a/tests/integration/.mocharc.js b/tests/integration/.mocharc.js new file mode 100644 index 00000000..f7113140 --- /dev/null +++ b/tests/integration/.mocharc.js @@ -0,0 +1,11 @@ +'use strict'; + +const allSpecs = 'tests/integration/**/*.test.ts'; + +const argvSpecs = process.argv.slice(4); + +module.exports = { + spec: argvSpecs.length > 0 ? argvSpecs : allSpecs, + timeout: '300000', + require: ['ts-node/register'], +}; diff --git a/tests/integration/sea/auth-pat-e2e.test.ts b/tests/integration/sea/auth-pat-e2e.test.ts new file mode 100644 index 00000000..8bff9748 --- /dev/null +++ b/tests/integration/sea/auth-pat-e2e.test.ts @@ -0,0 +1,75 @@ +// 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 { DBSQLClient } from '../../../lib'; + +/** + * sea-auth M0 end-to-end: + * 1. Construct a DBSQLClient. + * 2. `connect({ useSEA: true, token })` against pecotesting. + * 3. `openSession()` — round-trips through the napi binding. + * 4. Close the session, then the client. + * + * No query is executed here — execution is the responsibility of the + * sea-execution feature's own e2e. This test exists solely to confirm + * the PAT round-trips end-to-end and the napi binding's `openSession` + * surface is reachable from `DBSQLClient`. + * + * Required env (exported by `~/.zshrc` on the developer machine): + * - DATABRICKS_PECOTESTING_SERVER_HOSTNAME + * - DATABRICKS_PECOTESTING_HTTP_PATH + * - DATABRICKS_PECOTESTING_TOKEN_PERSONAL (preferred — personal PAT) + * - DATABRICKS_PECOTESTING_TOKEN (fallback — shared PAT) + * + * If any of the three required env vars is missing, the suite is skipped + * so CI machines without secrets don't fail-flap. + */ +describe('sea-auth e2e — PAT through DBSQLClient ↔ SeaBackend ↔ napi binding', function suite() { + const host = process.env.DATABRICKS_PECOTESTING_SERVER_HOSTNAME; + const path = process.env.DATABRICKS_PECOTESTING_HTTP_PATH; + const token = + process.env.DATABRICKS_PECOTESTING_TOKEN_PERSONAL || process.env.DATABRICKS_PECOTESTING_TOKEN; + + this.timeout(120_000); + + before(function gate() { + if (!host || !path || !token) { + // eslint-disable-next-line no-invalid-this + this.skip(); + } + }); + + it('connects, opens a session, closes the session, closes the client', async () => { + const client = new DBSQLClient(); + + const connected = await client.connect({ + host: host as string, + path: path as string, + token: token as string, + useSEA: true, + }); + expect(connected).to.equal(client); + + const session = await client.openSession(); + expect(session).to.exist; + expect(session.id).to.be.a('string'); + expect(session.id.length).to.be.greaterThan(0); + + const status = await session.close(); + expect(status.isSuccess).to.equal(true); + + await client.close(); + }); +}); diff --git a/tests/unit/sea/SeaBackend.test.ts b/tests/unit/sea/SeaBackend.test.ts deleted file mode 100644 index ff9e45c9..00000000 --- a/tests/unit/sea/SeaBackend.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { expect, AssertionError } from 'chai'; -import SeaBackend from '../../../lib/sea/SeaBackend'; -import HiveDriverError from '../../../lib/errors/HiveDriverError'; -import { ConnectionOptions, OpenSessionRequest } from '../../../lib/contracts/IDBSQLClient'; - -describe('SeaBackend stub', () => { - it('connect() rejects with HiveDriverError until M1 wires the binding', async () => { - const backend = new SeaBackend(); - try { - await backend.connect({ host: '', path: '', token: '' } as ConnectionOptions); - expect.fail('It should throw an error'); - } catch (error) { - if (error instanceof AssertionError || !(error instanceof Error)) { - throw error; - } - expect(error).to.be.instanceOf(HiveDriverError); - expect(error.message).to.contain('not implemented'); - } - }); - - it('openSession() rejects with HiveDriverError until M1 wires the binding', async () => { - const backend = new SeaBackend(); - try { - await backend.openSession({} as OpenSessionRequest); - expect.fail('It should throw an error'); - } catch (error) { - if (error instanceof AssertionError || !(error instanceof Error)) { - throw error; - } - expect(error).to.be.instanceOf(HiveDriverError); - expect(error.message).to.contain('not implemented'); - } - }); - - it('close() is a no-op so DBSQLClient.close() can finish state-clearing after a failed connect', async () => { - const backend = new SeaBackend(); - await backend.close(); - }); -}); diff --git a/tests/unit/sea/auth-pat.test.ts b/tests/unit/sea/auth-pat.test.ts new file mode 100644 index 00000000..5476d722 --- /dev/null +++ b/tests/unit/sea/auth-pat.test.ts @@ -0,0 +1,263 @@ +// 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 SeaBackend from '../../../lib/sea/SeaBackend'; +import { buildSeaConnectionOptions } from '../../../lib/sea/SeaAuth'; +import { SeaNativeBinding } from '../../../lib/sea/SeaNativeLoader'; +import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient'; +import AuthenticationError from '../../../lib/errors/AuthenticationError'; +import HiveDriverError from '../../../lib/errors/HiveDriverError'; + +/** + * Fake napi binding that records the option object handed to `openSession` + * and returns a fake `Connection` whose `close()` we can observe. No real + * native code runs in this suite. + */ +function makeFakeBinding() { + const calls: Array<{ method: string; args: unknown[] }> = []; + + const fakeConnection = { + async executeStatement() { + throw new Error('not used in this test'); + }, + async close() { + calls.push({ method: 'connection.close', args: [] }); + }, + }; + + const binding: SeaNativeBinding = { + version() { + return 'fake-binding'; + }, + async openSession(opts: { hostName: string; httpPath: string; token: string }) { + calls.push({ method: 'openSession', args: [opts] }); + return fakeConnection as unknown; + }, + Connection: function FakeConnection() {} as unknown as Function, + Statement: function FakeStatement() {} as unknown as Function, + }; + + return { binding, calls }; +} + +describe('SeaAuth + SeaBackend — PAT auth flow', () => { + describe('buildSeaConnectionOptions', () => { + it('accepts a bare access-token PAT (undefined authType)', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + token: 'dapi-fake-pat', + }; + + const native = buildSeaConnectionOptions(opts); + expect(native).to.deep.equal({ + hostName: 'example.cloud.databricks.com', + httpPath: '/sql/1.0/warehouses/abc', + token: 'dapi-fake-pat', + }); + }); + + it('accepts an explicit access-token PAT', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'access-token', + token: 'dapi-fake-pat', + }; + + const native = buildSeaConnectionOptions(opts); + expect(native.token).to.equal('dapi-fake-pat'); + }); + + it('prepends `/` to a path missing the leading slash', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: 'sql/1.0/warehouses/abc', + token: 'dapi-fake-pat', + }; + + const native = buildSeaConnectionOptions(opts); + expect(native.httpPath).to.equal('/sql/1.0/warehouses/abc'); + }); + + it('throws AuthenticationError when token is missing', () => { + const opts = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'access-token', + // no token + } as unknown as ConnectionOptions; + + expect(() => buildSeaConnectionOptions(opts)).to.throw(AuthenticationError, /non-empty PAT/); + }); + + it('throws AuthenticationError when token is an empty string', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + token: '', + }; + + expect(() => buildSeaConnectionOptions(opts)).to.throw(AuthenticationError, /non-empty PAT/); + }); + + it('rejects OAuth with a clear M0-scope error', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'databricks-oauth', + }; + + expect(() => buildSeaConnectionOptions(opts)).to.throw( + HiveDriverError, + /M0\) supports only PAT.*databricks-oauth.*M1/, + ); + }); + + it('rejects token-provider with a clear M0-scope error', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'token-provider', + tokenProvider: { getToken: async () => 'tok' } as unknown as ConnectionOptions extends infer T + ? // eslint-disable-next-line @typescript-eslint/no-explicit-any + any + : never, + }; + + expect(() => buildSeaConnectionOptions(opts)).to.throw(HiveDriverError, /token-provider.*M1/); + }); + + it('rejects external-token, static-token, and custom auth modes', () => { + const authTypes = ['external-token', 'static-token', 'custom'] as const; + for (const authType of authTypes) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const opts = { + host: 'h', + path: '/p', + authType, + } as any; + expect(() => buildSeaConnectionOptions(opts)).to.throw(HiveDriverError, /M0\) supports only PAT/); + } + }); + }); + + describe('SeaBackend.connect + openSession', () => { + it('resolves on a valid PAT options object and round-trips through the napi binding', async () => { + const { binding, calls } = makeFakeBinding(); + const backend = new SeaBackend(binding); + + await backend.connect({ + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + token: 'dapi-fake-pat', + }); + + const session = await backend.openSession({}); + expect(session).to.exist; + expect(session.id).to.match(/^sea-session-\d+$/); + + expect(calls).to.have.lengthOf(1); + expect(calls[0].method).to.equal('openSession'); + expect(calls[0].args[0]).to.deep.equal({ + hostName: 'example.cloud.databricks.com', + httpPath: '/sql/1.0/warehouses/abc', + token: 'dapi-fake-pat', + }); + + // Round-trip close. + const status = await session.close(); + expect(status.isSuccess).to.equal(true); + expect(calls[1].method).to.equal('connection.close'); + + await backend.close(); + }); + + it('rejects connect() when token is missing with AuthenticationError', async () => { + const { binding, calls } = makeFakeBinding(); + const backend = new SeaBackend(binding); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const opts = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'access-token', + } as any; + + let caught: unknown; + try { + await backend.connect(opts); + } catch (e) { + caught = e; + } + expect(caught).to.be.instanceOf(AuthenticationError); + expect(calls).to.have.lengthOf(0); + }); + + it('rejects connect() for OAuth with the M0-scope error', async () => { + const { binding, calls } = makeFakeBinding(); + const backend = new SeaBackend(binding); + + let caught: unknown; + try { + await backend.connect({ + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'databricks-oauth', + }); + } catch (e) { + caught = e; + } + expect(caught).to.be.instanceOf(HiveDriverError); + expect((caught as Error).message).to.match(/M0\) supports only PAT/); + expect(calls).to.have.lengthOf(0); + }); + + it('throws when openSession() is called before connect()', async () => { + const { binding } = makeFakeBinding(); + const backend = new SeaBackend(binding); + + let caught: unknown; + try { + await backend.openSession({}); + } catch (e) { + caught = e; + } + expect(caught).to.be.instanceOf(HiveDriverError); + expect((caught as Error).message).to.match(/connect\(\) must be called/); + }); + + it('stubbed session methods reject with a clear M0-scope error', async () => { + const { binding } = makeFakeBinding(); + const backend = new SeaBackend(binding); + + await backend.connect({ + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + token: 'dapi-fake-pat', + }); + const session = await backend.openSession({}); + + let caught: unknown; + try { + await session.executeStatement('SELECT 1', {}); + } catch (e) { + caught = e; + } + expect(caught).to.be.instanceOf(HiveDriverError); + expect((caught as Error).message).to.match(/not implemented in sea-auth \(M0\)/); + }); + }); +}); From 50f436e93dff525767d373d8829d5d9c514d157e Mon Sep 17 00:00:00 2001 From: Madhavendra Rathore Date: Sun, 31 May 2026 00:14:31 +0000 Subject: [PATCH 2/2] =?UTF-8?q?sea-auth:=20address=20PR=20#379=20review=20?= =?UTF-8?q?(F1=E2=80=93F10)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1: fix the fake-binding casts in auth-pat.test.ts so the unit suite type-checks (nyc registers ts-node/register with full type-checking — the bad casts were failing the whole unit-test CI job). Cast through the binding's own member types. - F2: the PAT e2e test couldn't compile (useSEA excess-property) and was orphaned under tests/integration (run by no CI script, outside the lint glob). Cast useSEA as ConnectionOptions & InternalConnectionOptions and move it to tests/e2e/sea/ (wired by `npm run e2e` + linted); drop the orphaned tests/integration/.mocharc.js. - F3: surface the real server-issued session id from the kernel Connection.sessionId getter instead of a process-local synthetic counter, so DBSQLSession's logged id correlates with kernel/server logs. - F4: only build the Thrift auth/connection providers on the Thrift path; the SEA path never reads them, so this stops validating the PAT twice and constructing a throwaway OAuth provider for an OAuth+useSEA call. - F5: derive SeaNativeConnectionOptions via Pick instead of re-declaring it, so a kernel field rename fails to compile rather than silently drifting. - F6: reject whitespace/control chars in the PAT (parity with the Python driver; the kernel HeaderValue already blocks CR/LF/NUL). - F7: extract the duplicated prependSlash into lib/utils/prependSlash.ts. - F8: lazy-load the native binding (resolve on first use, not in the constructor) so constructing SeaBackend never throws on a platform without the optional .node — connect()'s clearer validation runs first. - F9: thread the client logger into SeaBackend and log backend selection + session open/close (token excluded). - F10: document that SEA connect() does no network round-trip. Verified: tsc clean (0 errors, was 4), 13/13 unit tests, lib lint clean. Co-authored-by: Isaac --- lib/DBSQLClient.ts | 34 +++++------ lib/sea/SeaAuth.ts | 36 ++++++------ lib/sea/SeaBackend.ts | 57 ++++++++++++++----- lib/utils/prependSlash.ts | 25 ++++++++ .../sea/auth-pat-e2e.test.ts | 7 ++- tests/integration/.mocharc.js | 11 ---- tests/unit/sea/auth-pat.test.ts | 19 +++++-- 7 files changed, 124 insertions(+), 65 deletions(-) create mode 100644 lib/utils/prependSlash.ts rename tests/{integration => e2e}/sea/auth-pat-e2e.test.ts (85%) delete mode 100644 tests/integration/.mocharc.js diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index 7c6430bc..76dff592 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -32,13 +32,7 @@ import IDBSQLLogger, { LogLevel } from './contracts/IDBSQLLogger'; import DBSQLLogger from './DBSQLLogger'; import CloseableCollection from './utils/CloseableCollection'; import IConnectionProvider from './connection/contracts/IConnectionProvider'; - -function prependSlash(str: string): string { - if (str.length > 0 && str.charAt(0) !== '/') { - return `/${str}`; - } - return str; -} +import prependSlash from './utils/prependSlash'; export type ThriftLibrary = Pick; @@ -234,20 +228,26 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I this.config.userAgentEntry = options.userAgentEntry; } - this.authProvider = this.createAuthProvider(options, authProvider); - - this.connectionProvider = this.createConnectionProvider(options); - // M0: `useSEA` 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_sea")` // pattern (see databricks-sql-python/src/databricks/sql/session.py). const internalOptions = options as ConnectionOptions & InternalConnectionOptions; - this.backend = internalOptions.useSEA - ? new SeaBackend() - : new ThriftBackend({ - context: this, - onConnectionEvent: (event, payload) => this.forwardConnectionEvent(event, payload), - }); + + if (internalOptions.useSEA) { + // The SEA backend authenticates inside the native binding; the + // Thrift auth/connection providers are never read on this path, so + // we don't build them (avoids validating the PAT twice and + // constructing a throwaway OAuth provider for an OAuth+useSEA call). + this.logger.log(LogLevel.info, 'Connecting via the SEA (native) backend'); + this.backend = new SeaBackend(undefined, this.logger); + } else { + this.authProvider = this.createAuthProvider(options, authProvider); + this.connectionProvider = this.createConnectionProvider(options); + this.backend = new ThriftBackend({ + context: this, + onConnectionEvent: (event, payload) => this.forwardConnectionEvent(event, payload), + }); + } await this.backend.connect(options); diff --git a/lib/sea/SeaAuth.ts b/lib/sea/SeaAuth.ts index cf16c80f..3a7838da 100644 --- a/lib/sea/SeaAuth.ts +++ b/lib/sea/SeaAuth.ts @@ -15,26 +15,17 @@ import { ConnectionOptions } from '../contracts/IDBSQLClient'; import AuthenticationError from '../errors/AuthenticationError'; import HiveDriverError from '../errors/HiveDriverError'; +import prependSlash from '../utils/prependSlash'; +import { SeaConnectionOptions } from './SeaNativeLoader'; /** - * Shape consumed by the napi-binding's `openSession()` (see - * `native/sea/index.d.ts`). M0 supports PAT only — `token` is required. - * - * Mirrors `ConnectionOptions` in the binding's `.d.ts`; declared locally - * to avoid coupling the JS-side adapter to the auto-generated TS file. + * Shape consumed by the napi-binding's `openSession()`. M0 sends only the + * PAT triple, so we `Pick` those fields off the binding's generated + * `ConnectionOptions` (re-exported as `SeaConnectionOptions`) rather than + * re-declaring them — if the kernel renames `hostName`/`httpPath`/`token` + * this stops compiling instead of silently drifting. */ -export interface SeaNativeConnectionOptions { - hostName: string; - httpPath: string; - token: string; -} - -function prependSlash(str: string): string { - if (str.length > 0 && str.charAt(0) !== '/') { - return `/${str}`; - } - return str; -} +export type SeaNativeConnectionOptions = Pick; /** * Validate that the user-supplied `ConnectionOptions` describe a PAT auth @@ -74,6 +65,17 @@ export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNative 'SEA backend: a non-empty PAT must be supplied via `token` when using `authType: \'access-token\'`.', ); } + // Reject whitespace / control characters in the PAT. The kernel's + // reqwest `HeaderValue` already hard-rejects CR/LF/NUL at build time so + // this isn't a header-injection fix — it's parity with the Python + // driver (auth_bridge.py rejects `[\x00-\x20\x7f]`) and catches + // copy-paste whitespace before a confusing downstream failure. + // eslint-disable-next-line no-control-regex + if (/[\x00-\x20\x7f]/.test(token)) { + throw new AuthenticationError( + 'SEA backend: the PAT supplied via `token` must not contain whitespace or control characters.', + ); + } return { hostName: options.host, diff --git a/lib/sea/SeaBackend.ts b/lib/sea/SeaBackend.ts index ee20a1ba..79ae607a 100644 --- a/lib/sea/SeaBackend.ts +++ b/lib/sea/SeaBackend.ts @@ -31,6 +31,7 @@ import { import Status from '../dto/Status'; import InfoValue from '../dto/InfoValue'; import HiveDriverError from '../errors/HiveDriverError'; +import IDBSQLLogger, { LogLevel } from '../contracts/IDBSQLLogger'; import { getSeaNative, SeaNativeBinding } from './SeaNativeLoader'; import { buildSeaConnectionOptions, SeaNativeConnectionOptions } from './SeaAuth'; @@ -44,6 +45,8 @@ const NOT_IMPLEMENTED_SESSION = * leak into every call site. */ interface NativeConnection { + /** Server-issued session id (kernel `Connection.sessionId` getter). */ + readonly sessionId: string; close(): Promise; } @@ -54,21 +57,22 @@ interface NativeConnection { * subset required to round-trip a connect-open-close cycle. Every other * method throws a clear "not implemented in M0" `HiveDriverError`. * - * The `id` field is currently a synthetic counter-based string; the kernel - * exposes a real session-id through a follow-on getter that - * `sea-execution` will wire through. + * `id` is the server-issued session id read straight off the kernel + * `Connection` (its `sessionId` getter, readable even after close()), so + * the value logged by `DBSQLSession` correlates with kernel / server logs + * rather than being a process-local synthetic counter. */ export class SeaSessionBackend implements ISessionBackend { - private static seq = 0; - public readonly id: string; private readonly connection: NativeConnection; - constructor(connection: NativeConnection) { + private readonly logger?: IDBSQLLogger; + + constructor(connection: NativeConnection, logger?: IDBSQLLogger) { this.connection = connection; - SeaSessionBackend.seq += 1; - this.id = `sea-session-${SeaSessionBackend.seq}`; + this.logger = logger; + this.id = connection.sessionId; } /* eslint-disable @typescript-eslint/no-unused-vars */ @@ -121,6 +125,7 @@ export class SeaSessionBackend implements ISessionBackend { /* eslint-enable @typescript-eslint/no-unused-vars */ public async close(): Promise { + this.logger?.log(LogLevel.debug, `SEA session closing with id: ${this.id}`); await this.connection.close(); return Status.success(); } @@ -140,16 +145,36 @@ export class SeaSessionBackend implements ISessionBackend { export default class SeaBackend implements IBackend { private nativeOptions?: SeaNativeConnectionOptions; - private readonly native: SeaNativeBinding; + private readonly injectedNative?: SeaNativeBinding; + + private cachedNative?: SeaNativeBinding; + + private readonly logger?: IDBSQLLogger; - constructor(native: SeaNativeBinding = getSeaNative()) { - this.native = native; + // `native` is injectable (tests pass a fake); production leaves it + // undefined and the binding is resolved lazily on first use so that + // constructing a SeaBackend never throws on a platform without the + // optional `.node` — the clearer auth/option validation in connect() + // runs first. + constructor(native?: SeaNativeBinding, logger?: IDBSQLLogger) { + this.injectedNative = native; + this.logger = logger; + } + + private get native(): SeaNativeBinding { + if (!this.cachedNative) { + this.cachedNative = this.injectedNative ?? getSeaNative(); + } + return this.cachedNative; } public async connect(options: ConnectionOptions): Promise { - // Validate PAT auth + capture the napi-binding option shape. - // Any non-PAT mode (or a missing token) throws here, before we ever - // touch the native binding. + // Validate PAT auth + capture the napi-binding option shape. Any + // non-PAT mode (or a missing token) throws here, before we ever touch + // the native binding. NOTE: unlike Thrift, this performs no network + // round-trip — the session is opened lazily in openSession(), so a + // resolved connect() does not by itself prove the endpoint is + // reachable or the credential is valid. this.nativeOptions = buildSeaConnectionOptions(options); } @@ -159,7 +184,9 @@ export default class SeaBackend implements IBackend { throw new HiveDriverError('SeaBackend: connect() must be called before openSession().'); } const connection = (await this.native.openSession(this.nativeOptions)) as NativeConnection; - return new SeaSessionBackend(connection); + const session = new SeaSessionBackend(connection, this.logger); + this.logger?.log(LogLevel.info, `SEA session opened with id: ${session.id}`); + return session; } public async close(): Promise { diff --git a/lib/utils/prependSlash.ts b/lib/utils/prependSlash.ts new file mode 100644 index 00000000..a3ed7d92 --- /dev/null +++ b/lib/utils/prependSlash.ts @@ -0,0 +1,25 @@ +// 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. + +/** + * Normalise an HTTP path to a leading-slash form. Empty strings are left + * untouched. Shared by the Thrift connect path (`DBSQLClient`) and the + * SEA auth adapter (`SeaAuth`) so the two can't drift. + */ +export default function prependSlash(str: string): string { + if (str.length > 0 && str.charAt(0) !== '/') { + return `/${str}`; + } + return str; +} diff --git a/tests/integration/sea/auth-pat-e2e.test.ts b/tests/e2e/sea/auth-pat-e2e.test.ts similarity index 85% rename from tests/integration/sea/auth-pat-e2e.test.ts rename to tests/e2e/sea/auth-pat-e2e.test.ts index 8bff9748..335b60e5 100644 --- a/tests/integration/sea/auth-pat-e2e.test.ts +++ b/tests/e2e/sea/auth-pat-e2e.test.ts @@ -14,6 +14,8 @@ import { expect } from 'chai'; import { DBSQLClient } from '../../../lib'; +import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient'; +import { InternalConnectionOptions } from '../../../lib/contracts/InternalConnectionOptions'; /** * sea-auth M0 end-to-end: @@ -58,8 +60,11 @@ describe('sea-auth e2e — PAT through DBSQLClient ↔ SeaBackend ↔ napi bindi host: host as string, path: path as string, token: token as string, + // `useSEA` is an internal opt-in (InternalConnectionOptions), not a + // public ConnectionOptions field — cast exactly as DBSQLClient.connect + // does internally so the literal passes excess-property checking. useSEA: true, - }); + } as ConnectionOptions & InternalConnectionOptions); expect(connected).to.equal(client); const session = await client.openSession(); diff --git a/tests/integration/.mocharc.js b/tests/integration/.mocharc.js deleted file mode 100644 index f7113140..00000000 --- a/tests/integration/.mocharc.js +++ /dev/null @@ -1,11 +0,0 @@ -'use strict'; - -const allSpecs = 'tests/integration/**/*.test.ts'; - -const argvSpecs = process.argv.slice(4); - -module.exports = { - spec: argvSpecs.length > 0 ? argvSpecs : allSpecs, - timeout: '300000', - require: ['ts-node/register'], -}; diff --git a/tests/unit/sea/auth-pat.test.ts b/tests/unit/sea/auth-pat.test.ts index 5476d722..f06126a7 100644 --- a/tests/unit/sea/auth-pat.test.ts +++ b/tests/unit/sea/auth-pat.test.ts @@ -29,6 +29,9 @@ function makeFakeBinding() { const calls: Array<{ method: string; args: unknown[] }> = []; const fakeConnection = { + // Mirrors the kernel `Connection.sessionId` getter; SeaSessionBackend + // surfaces this as its `id`. + sessionId: '01ef-fake-session-id', async executeStatement() { throw new Error('not used in this test'); }, @@ -43,10 +46,17 @@ function makeFakeBinding() { }, async openSession(opts: { hostName: string; httpPath: string; token: string }) { calls.push({ method: 'openSession', args: [opts] }); - return fakeConnection as unknown; + // Cast through the binding's own member types: `SeaNativeBinding` is + // `typeof import('../../native/sea')`, so `openSession`'s resolved + // return type is the napi `Connection`. A bare `as unknown` stops + // short of that and fails to satisfy the annotation. + return fakeConnection as unknown as Awaited>; }, - Connection: function FakeConnection() {} as unknown as Function, - Statement: function FakeStatement() {} as unknown as Function, + // `Connection`/`Statement` are exported as type aliases in + // SeaNativeLoader, so `typeof Connection` is illegal (TS2693); index + // the binding type instead to get the napi class constructor type. + Connection: function FakeConnection() {} as unknown as SeaNativeBinding['Connection'], + Statement: function FakeStatement() {} as unknown as SeaNativeBinding['Statement'], }; return { binding, calls }; @@ -167,7 +177,8 @@ describe('SeaAuth + SeaBackend — PAT auth flow', () => { const session = await backend.openSession({}); expect(session).to.exist; - expect(session.id).to.match(/^sea-session-\d+$/); + // id is the real server-issued session id (kernel `sessionId`). + expect(session.id).to.equal('01ef-fake-session-id'); expect(calls).to.have.lengthOf(1); expect(calls[0].method).to.equal('openSession');