diff --git a/CONNECTION_PARAMETERS.md b/CONNECTION_PARAMETERS.md index ea0af4fb..6d30a651 100644 --- a/CONNECTION_PARAMETERS.md +++ b/CONNECTION_PARAMETERS.md @@ -65,7 +65,8 @@ column. | `oauthClientId` | `string` | ✅ | ✅ | `databricks-sql-connector` when absent | OAuth client id, used on **both** U2M and M2M. Forwarded verbatim on both backends when set. When absent it defaults to `databricks-sql-connector` — Thrift via `getClientId()` for both flows; kernel via `oauthClientId ?? DEFAULT_OAUTH_CLIENT_ID` on M2M, and by letting the napi binding apply its own (identical) default on U2M. **Parity:** `oauthClientId` + no secret routes to **U2M** with the id forwarded (flow selection keys off `oauthClientSecret` presence — see that row), so it does **not** throw an M2M "secret required" error. | | `oauthClientSecret` (M2M) | `string` | ✅ | ✅ | — | M2M client-credentials secret; its **presence** is the U2M-vs-M2M flow selector on both backends (`undefined` ⇒ U2M). Thrift → `DatabricksOAuth.clientSecret`. Kernel → native `oauthClientSecret` (workspace-OIDC M2M) or remapped to `azureClientSecret` (Entra-direct `AzureSpM2m`). A blank/reserved secret is forwarded verbatim and still selects M2M (Thrift parity) — except the Azure SP arm, which rejects it. | | `azureTenantId` / `useDatabricksOAuthInAzure` | `string` / `boolean` | ✅ | ⚠️ | — | **Honored on both.** By design the kernel routes **all U2M** (no secret, any cloud) to its cloud-blind in-house OAuth U2M flow — there is no Azure-specific U2M mode, so `useDatabricksOAuthInAzure` is inert on U2M and every Azure workspace (including `.databricks.azure.us` US-gov) is always supported, on any kernel build. `useDatabricksOAuthInAzure` selects only the **M2M** mechanism on an Azure host: absent/`false` → Entra-direct service-principal M2M (native `AzureSpM2m` mode, creds ride `oauthClientId`/`oauthClientSecret`, `azureTenantId` optional — kernel auto-discovers from the workspace `/aad/auth` redirect when omitted); `true` → workspace-OIDC M2M. (`lib/kernel/KernelAuth.ts` `buildKernelConnectionOptions`.) | -| `persistence` (custom OAuth token store) | `OAuthPersistence` | ✅ | ❌ | — | **Thrift-only.** Kernel throws; it auto-persists U2M tokens to `~/.config/databricks-sql-kernel/oauth/` and does not cache M2M. | +| `persistence` (custom OAuth token store) | `OAuthPersistence` | ✅ | ❌ | — | **Thrift-only.** Kernel throws (no custom-store hook). Its built-in U2M on-disk cache (`~/.config/databricks-sql-kernel/oauth/`) is **optional**, controlled by `tokenCacheEnabled` and **disabled by default** (`false`/omitted ⇒ in-memory only); it never caches M2M. See the `tokenCacheEnabled` row. | +| `tokenCacheEnabled` (U2M on-disk cache) | `boolean` | ❌ | ✅ | `false` (disabled by default) | **Kernel U2M-only.** Controls the kernel's built-in on-disk token cache for U2M OAuth flows. When `true`, the refresh token is persisted (AES-256 encrypted) to `~/.config/databricks-sql-kernel/oauth/`. When `false` or omitted (the default), tokens remain in-memory only — silent-no-persist parity. No effect on M2M or other auth types. Distinct from the Thrift `persistence` custom-store hook. TODO: publish only after PR #283 (`tokenCacheEnabled` napi support) ships in `@databricks/databricks-sql-kernel`. | | `enableTokenFederation` / `federationClientId` | `boolean` / `string` | ✅ | ⚠️ | `false` / — | On the kernel backend these options apply only to `static-token`. Federation is always enabled, so `enableTokenFederation` is ignored; an omitted or empty client ID selects account-wide WIF and a non-empty ID selects SP-wide WIF. Thrift honors the boolean and also supports these options for `token-provider` and `external-token`. | ## HTTP client, proxy, retries @@ -177,6 +178,9 @@ backend, so they are read regardless of `useKernel`. Defaults are sourced from `checkServerCertificate`, `customCaCert`, `clientCert`, `clientKey` — **are** public and honored on the Thrift backend, which verifies certificates by default via `checkServerCertificate ?? true`; see the TLS / SSL section.) +3. `tokenCacheEnabled` — enables the kernel's built-in on-disk token cache for + U2M OAuth flows only (defaults to disabled for silent-no-persist parity). Distinct + from the Thrift `persistence` custom-store hook. ### Behavioral divergences to watch diff --git a/KERNEL_REV b/KERNEL_REV index 322dd971..8d29e509 100644 --- a/KERNEL_REV +++ b/KERNEL_REV @@ -1 +1 @@ -ef1a6f2dbe72c66a9b6eefc4ca1ff31788f1efa9 +628abd6f5045897efcadb38ec77a1e9e0c23544e diff --git a/lib/contracts/IDBSQLClient.ts b/lib/contracts/IDBSQLClient.ts index 761ebafd..8a5bdef1 100644 --- a/lib/contracts/IDBSQLClient.ts +++ b/lib/contracts/IDBSQLClient.ts @@ -26,6 +26,13 @@ type AuthOptions = // U2M flow to `['sql', 'offline_access']` (parity with the Thrift driver's // `defaultOAuthScopes`), overriding the kernel's bare `all-apis offline_access`. oauthScopes?: Array; + // Enable the kernel's built-in on-disk OAuth token cache for U2M flows only. + // When `true`, the kernel persists the U2M refresh token (AES-256 encrypted) to + // `~/.config/databricks-sql-kernel/oauth/`. When `false` or omitted, tokens remain + // in-memory only (the default, for security and silent-no-persist behavior parity). + // Has no effect on M2M or other auth types. This option is distinct from the + // Thrift `persistence` custom-store hook, which is not yet supported on the kernel. + tokenCacheEnabled?: boolean; } | { authType: 'custom'; diff --git a/lib/kernel/KernelAuth.ts b/lib/kernel/KernelAuth.ts index f8c9529f..5c5eff6c 100644 --- a/lib/kernel/KernelAuth.ts +++ b/lib/kernel/KernelAuth.ts @@ -251,6 +251,7 @@ export type KernelNativeConnectionOptions = KernelSessionDefaults & oauthRedirectPort: number; oauthScopes?: Array; oauthClientId?: string; + tokenCacheEnabled?: boolean; } | { hostName: string; @@ -669,6 +670,7 @@ export function buildKernelConnectionOptions(options: ConnectionOptions): Kernel oauthScopes?: Array; azureTenantId?: string; useDatabricksOAuthInAzure?: boolean; + tokenCacheEnabled?: boolean; persistence?: unknown; }; @@ -820,8 +822,9 @@ export function buildKernelConnectionOptions(options: ConnectionOptions): Kernel throw new HiveDriverError( 'kernel backend: `persistence` (custom OAuth token store) is not yet wired through ' + 'to the kernel — requires `AuthConfig::External` plumbing. ' + - 'Today the kernel auto-persists U2M tokens to ' + - '`~/.config/databricks-sql-kernel/oauth/` which works for the standard flow; ' + + 'The kernel offers an optional built-in on-disk token cache at ' + + '`~/.config/databricks-sql-kernel/oauth/`, controlled by `tokenCacheEnabled` ' + + '(disabled by default); ' + "the JS-supplied hook (matching thrift's `OAuthPersistence` interface) lands " + 'when the kernel exposes it.', ); @@ -833,6 +836,9 @@ export function buildKernelConnectionOptions(options: ConnectionOptions): Kernel // Scopes default to Thrift parity (`sql offline_access`); overridable. oauthScopes: Array.isArray(oauth.oauthScopes) && oauth.oauthScopes.length > 0 ? oauth.oauthScopes : U2M_DEFAULT_SCOPES, + // Token cache is disabled by default for security (silent-no-persist parity); + // explicitly set to false unless the caller opts in. + tokenCacheEnabled: oauth.tokenCacheEnabled ?? false, }; // clientId: Thrift uses `oauthClientId ?? default`. Forward it verbatim // when set; when absent the napi applies the same default diff --git a/native/kernel/index.d.ts b/native/kernel/index.d.ts index 6fb596e2..e85eb2c6 100644 --- a/native/kernel/index.d.ts +++ b/native/kernel/index.d.ts @@ -679,6 +679,13 @@ export interface ConnectionOptions { * authority for [`AuthMode::OAuthM2m`] / [`AuthMode::OAuthM2mJwt`]. */ tokenUrl?: string + /** + * U2M on-disk token-cache control (kernel path). Omitted ⇒ the kernel + * default (cache enabled, encrypted under a machine-local key, survives + * restarts). `false` disables on-disk persistence (re-login each fresh + * process); `true` keeps it enabled. Applies to [`AuthMode::OAuthU2m`]. + */ + tokenCacheEnabled?: boolean /** * Path to the PEM private-key file. Required for * [`AuthMode::OAuthM2mJwt`]. diff --git a/tests/e2e/kernel/auth-u2m-e2e.test.ts b/tests/e2e/kernel/auth-u2m-e2e.test.ts index 00287c10..4bbc5577 100644 --- a/tests/e2e/kernel/auth-u2m-e2e.test.ts +++ b/tests/e2e/kernel/auth-u2m-e2e.test.ts @@ -13,61 +13,183 @@ // limitations under the License. import { expect } from 'chai'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as nodePath from 'path'; import { DBSQLClient } from '../../../lib'; import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient'; import { InternalConnectionOptions } from '../../../lib/contracts/InternalConnectionOptions'; /** - * kernel-auth M1 OAuth U2M end-to-end — **SKIPPED pending browser harness**. + * kernel-auth OAuth U2M end-to-end, focused on the `tokenCacheEnabled` + * control (PR #513 / kernel #283). * - * U2M is interactive: the kernel opens a system browser - * (`auth/oauth/u2m.rs:414`, via the `open` crate), binds a local - * listener on port 8030 (via the JS adapter's hardcoded override), and - * waits up to 120s for the user to authenticate. + * U2M is **interactive**: the kernel opens a system browser, binds a local + * listener on port 8030 (via the JS adapter's hardcoded override), and waits + * for the user to complete the workspace login + consent. Because a human has + * to click through the browser, this suite cannot run unattended — it is + * therefore gated behind an explicit opt-in env var (`DATABRICKS_KERNEL_U2M_INTERACTIVE`) + * on top of the workspace host/path, so CI (which sets neither) skips it and + * never flaps. * - * Driving this from CI requires Playwright/Puppeteer to navigate the - * browser through the workspace login + consent screens. That harness - * is tracked as `TBD-oauth_u2m_test_harness` in testing-agent's - * findings; until it exists, this test stays `it.skip` so the e2e - * suite carries a slot for whoever lands the harness work. + * What it proves end-to-end, through + * DBSQLClient.connect({ useKernel: true, authType: 'databricks-oauth' }) + * → KernelBackend → napi binding → live workspace U2M browser flow: * - * The intended assertion sequence (mirrors `auth-m2m-e2e.test.ts`): - * 1. `client.connect({ useKernel: true, authType: 'databricks-oauth' })` - * — NO `oauthClientSecret` → kernel picks the U2M flow. - * 2. `openSession()` — kernel opens browser, waits for callback on - * localhost:8030, exchanges the auth code, returns Bearer token, - * issues the create-session request to kernel. - * 3. `session.close()` then `client.close()`. + * 1. **Disabled by default.** With `tokenCacheEnabled` unset, the connector + * passes `tokenCacheEnabled: false` to the kernel, so NO token is written + * to the on-disk cache — matching the Thrift backend's in-memory posture. + * 2. **Opt-in enable.** With `tokenCacheEnabled: true`, the kernel persists + * the U2M refresh token (AES-256 encrypted, not plaintext) to its on-disk + * cache under `dirs::config_dir()/databricks-sql-kernel/oauth/`. * - * Required env (gated additionally via `it.skip` until the harness - * lands, so absent env is a no-op today): + * Required env (suite skips unless ALL are set): * - DATABRICKS_PECOTESTING_SERVER_HOSTNAME * - DATABRICKS_PECOTESTING_HTTP_PATH - * - (no client_id/secret — U2M uses kernel default `databricks-cli`) + * - DATABRICKS_KERNEL_U2M_INTERACTIVE (any non-empty value — the human opt-in) + * + * Each case opens a fresh browser login (the cache dir is emptied between + * cases so a prior case's file can't produce a cache-hit that skips the + * browser). A human must complete both logins within the suite timeout. + */ + +/** + * The kernel stores U2M cache files at `dirs::config_dir()/databricks-sql-kernel/oauth/` + * (see `src/auth/oauth/cache.rs`). Mirror the Rust `dirs` crate's per-platform + * `config_dir()` here so the assertion checks the same directory the kernel + * writes to — on macOS that is `~/Library/Application Support`, NOT `~/.config` + * (the Linux path the docstrings mention). */ -describe('kernel-auth e2e — OAuth U2M through DBSQLClient ↔ KernelBackend ↔ napi binding', function suite() { +function kernelOAuthCacheDir(): string { + const home = os.homedir(); + if (process.platform === 'darwin') { + return nodePath.join(home, 'Library', 'Application Support', 'databricks-sql-kernel', 'oauth'); + } + if (process.platform === 'win32') { + const appData = process.env.APPDATA || nodePath.join(home, 'AppData', 'Roaming'); + return nodePath.join(appData, 'databricks-sql-kernel', 'oauth'); + } + const xdg = process.env.XDG_CONFIG_HOME || nodePath.join(home, '.config'); + return nodePath.join(xdg, 'databricks-sql-kernel', 'oauth'); +} + +/** Cache files are named `{sha256}.json` (`CacheKey::to_filename`). */ +function listCacheFiles(dir: string): string[] { + if (!fs.existsSync(dir)) { + return []; + } + return fs.readdirSync(dir).filter((f) => f.endsWith('.json')); +} + +describe('kernel-auth e2e — OAuth U2M token cache (interactive)', function suite() { + const host = process.env.DATABRICKS_PECOTESTING_SERVER_HOSTNAME; + const httpPath = process.env.DATABRICKS_PECOTESTING_HTTP_PATH; + const interactive = process.env.DATABRICKS_KERNEL_U2M_INTERACTIVE; + + const cacheDir = kernelOAuthCacheDir(); + let backupDir: string | undefined; + // Only true once the gate has passed AND the backup has been taken. The + // `after` hook keys its destructive cleanup off this so a skipped run (gate + // env vars absent) never touches the developer's real on-disk cache. + let suiteActive = false; + + // Interactive browser login + live warehouse round-trip; give the human time. this.timeout(300_000); - it.skip('[pending TBD-oauth_u2m_test_harness] interactive U2M round-trip', async () => { - const host = process.env.DATABRICKS_PECOTESTING_SERVER_HOSTNAME as string; - const path = process.env.DATABRICKS_PECOTESTING_HTTP_PATH as string; + before(function gate() { + if (!host || !httpPath || !interactive) { + // eslint-disable-next-line no-invalid-this + this.skip(); + } + // Never destroy a real user's cached tokens: move any existing cache + // aside for the duration of the suite and restore it in `after`. + if (fs.existsSync(cacheDir)) { + backupDir = `${cacheDir}.e2e-backup-${process.pid}`; + fs.renameSync(cacheDir, backupDir); + } + suiteActive = true; + }); + + after(() => { + // A skipped run took no backup — leave the developer's cache untouched. + if (!suiteActive) { + return; + } + // Remove whatever the test wrote, then restore the user's originals. + if (fs.existsSync(cacheDir)) { + fs.rmSync(cacheDir, { recursive: true, force: true }); + } + if (backupDir && fs.existsSync(backupDir)) { + fs.renameSync(backupDir, cacheDir); + } + }); + beforeEach(() => { + // Start each case from an empty cache dir so a prior case's file cannot + // leak into this assertion — nor produce a cache-hit that skips the browser. + if (fs.existsSync(cacheDir)) { + fs.rmSync(cacheDir, { recursive: true, force: true }); + } + }); + + it('default (tokenCacheEnabled unset) persists NO token to disk', async () => { const client = new DBSQLClient(); + await client.connect({ + host: host as string, + path: httpPath as string, + authType: 'databricks-oauth', + useKernel: true, + } as ConnectionOptions & InternalConnectionOptions); - const connected = await client.connect({ - host, - path, + const session = await client.openSession(); + expect(session.id).to.be.a('string'); + + const operation = await session.executeStatement('SELECT 1 AS one'); + const rows = (await operation.fetchAll()) as Array>; + expect(Number(rows[0].one)).to.equal(1); + + await operation.close(); + await session.close(); + await client.close(); + + expect(listCacheFiles(cacheDir), 'no on-disk token cache when tokenCacheEnabled is unset').to.have.length(0); + }); + + it('tokenCacheEnabled:true persists an encrypted token to disk', async () => { + const client = new DBSQLClient(); + await client.connect({ + host: host as string, + path: httpPath as string, authType: 'databricks-oauth', useKernel: true, + tokenCacheEnabled: true, } as ConnectionOptions & InternalConnectionOptions); - expect(connected).to.equal(client); const session = await client.openSession(); expect(session.id).to.be.a('string'); - const status = await session.close(); - expect(status.isSuccess).to.equal(true); + const operation = await session.executeStatement('SELECT 1 AS one'); + const rows = (await operation.fetchAll()) as Array>; + expect(Number(rows[0].one)).to.equal(1); + await operation.close(); + await session.close(); await client.close(); + + const files = listCacheFiles(cacheDir); + expect(files, 'a token cache file is written when tokenCacheEnabled is true').to.have.length.greaterThan(0); + + // Encrypted at rest: the persisted file is AES-256 ciphertext, so it must + // NOT parse as the plaintext JSON token structure. + const raw = fs.readFileSync(nodePath.join(cacheDir, files[0])); + expect(raw.length, 'cache file is non-empty').to.be.greaterThan(0); + let parsedAsJson = false; + try { + JSON.parse(raw.toString('utf8')); + parsedAsJson = true; + } catch { + // expected — ciphertext is not valid UTF-8 JSON + } + expect(parsedAsJson, 'cache file must be encrypted, not plaintext JSON').to.equal(false); }); }); diff --git a/tests/unit/kernel/auth-u2m.test.ts b/tests/unit/kernel/auth-u2m.test.ts index 1943e099..75149a44 100644 --- a/tests/unit/kernel/auth-u2m.test.ts +++ b/tests/unit/kernel/auth-u2m.test.ts @@ -37,6 +37,7 @@ describe('KernelAuth + KernelBackend — OAuth U2M auth flow', () => { authMode: 'OAuthU2m', oauthRedirectPort: 8030, oauthScopes: ['sql', 'offline_access'], + tokenCacheEnabled: false, }); }); @@ -122,6 +123,7 @@ describe('KernelAuth + KernelBackend — OAuth U2M auth flow', () => { authMode: 'OAuthU2m', oauthRedirectPort: 8030, oauthScopes: ['sql', 'offline_access'], + tokenCacheEnabled: false, }); }); @@ -144,9 +146,35 @@ describe('KernelAuth + KernelBackend — OAuth U2M auth flow', () => { authMode: 'OAuthU2m', oauthRedirectPort: 8030, oauthScopes: ['sql', 'offline_access'], + tokenCacheEnabled: false, }); }); + it('disables tokenCacheEnabled by default (silent-no-persist parity)', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'databricks-oauth', + }; + + const native = buildKernelConnectionOptions(opts); + expect(native.authMode).to.equal('OAuthU2m'); + expect((native as { tokenCacheEnabled?: boolean }).tokenCacheEnabled).to.equal(false); + }); + + it('honors tokenCacheEnabled: true to enable the kernel on-disk token cache', () => { + const opts: ConnectionOptions = { + host: 'example.cloud.databricks.com', + path: '/sql/1.0/warehouses/abc', + authType: 'databricks-oauth', + tokenCacheEnabled: true, + } as ConnectionOptions; + + const native = buildKernelConnectionOptions(opts); + expect(native.authMode).to.equal('OAuthU2m'); + expect((native as { tokenCacheEnabled?: boolean }).tokenCacheEnabled).to.equal(true); + }); + it('rejects a `persistence` hook on U2M citing the AuthConfig::External kernel-plumbing gap', () => { const opts: ConnectionOptions = { host: 'example.cloud.databricks.com', @@ -187,6 +215,7 @@ describe('KernelAuth + KernelBackend — OAuth U2M auth flow', () => { authMode: 'OAuthU2m', oauthRedirectPort: 8030, oauthScopes: ['sql', 'offline_access'], + tokenCacheEnabled: false, }); await session.close();