Skip to content

Commit cb8f5cf

Browse files
vuanhphungIsaac
andcommitted
fix: send the token federation exchange through the connection agent
FederationProvider posted the `/oidc/v1/token` exchange with node-fetch and no agent, so `proxy`, `checkServerCertificate`, `customCaCert` and `clientCert`/`clientKey` did not apply to it. Behind a required proxy the exchange failed and the driver silently fell back to the original token. Pass the connection's agent for the exchange URL, as OAuth already does. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent 93a9112 commit cb8f5cf

5 files changed

Lines changed: 57 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Unreleased
44

5+
- The Thrift token federation exchange (`enableTokenFederation`) now goes through the connection's agent, so `proxy`, `checkServerCertificate`, `customCaCert` and `clientCert`/`clientKey` apply to it. Previously it connected directly with Node's default agent, so behind a required proxy the exchange failed and the driver fell back to the original token.
56
- Thrift CloudFetch downloads are now bounded end to end by `retriesTimeout` (default 15 minutes), including every attempt and retry backoff. Previously a stalled download, where the response headers or body never arrived, blocked the fetch forever. A download still running when the deadline expires is aborted and fails with `RetryError` (`TIMEOUT_EXCEEDED`, message `CloudFetch result download failed: Retry timeout exceeded`). (#520)
67
- Thrift connections made through `proxy` now apply `checkServerCertificate`, `customCaCert` and `clientCert`/`clientKey`. Previously the proxied path ignored them and verified against Node's process-wide trust store, so a certificate outside the configured trust set could be accepted. **Compatibility:** proxied connections now verify unless `checkServerCertificate: false` is set; as on direct connections, `NODE_TLS_REJECT_UNAUTHORIZED=0` no longer turns verification off. For an `https` proxy these options also apply to the proxy's own certificate. (#485)
78
- Honor the client `useCloudFetch: false` default on the kernel backend so complete results can be fetched without cloud-storage downloads. (#574)

‎lib/DBSQLClient.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,8 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
365365
if (enableFederation) {
366366
wrapped = new FederationProvider(wrapped, host, {
367367
clientId: federationClientId,
368+
// The connection's agent carries `proxy` and the TLS options.
369+
getAgent: async (url) => (await this.getConnectionProvider()).getAgent(url),
368370
});
369371
}
370372

‎lib/connection/auth/tokenProvider/FederationProvider.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import http from 'http';
12
import nodeFetch from 'node-fetch';
23
import ITokenProvider from './ITokenProvider';
34
import Token from './Token';
@@ -79,26 +80,31 @@ export default class FederationProvider implements ITokenProvider {
7980

8081
private readonly returnOriginalTokenOnFailure: boolean;
8182

83+
private readonly getAgent?: (url: string) => Promise<http.Agent | undefined>;
84+
8285
/**
8386
* Creates a new FederationProvider.
8487
* @param baseProvider - The underlying token provider
8588
* @param databricksHost - The Databricks workspace host URL
8689
* @param options - Optional configuration
8790
* @param options.clientId - Client ID for M2M/service principal federation
8891
* @param options.returnOriginalTokenOnFailure - Return original token if exchange fails (default: true)
92+
* @param options.getAgent - Agent for the exchange request, so the connection's proxy and TLS options apply
8993
*/
9094
constructor(
9195
baseProvider: ITokenProvider,
9296
databricksHost: string,
9397
options?: {
9498
clientId?: string;
9599
returnOriginalTokenOnFailure?: boolean;
100+
getAgent?: (url: string) => Promise<http.Agent | undefined>;
96101
},
97102
) {
98103
this.baseProvider = baseProvider;
99104
this.databricksHost = databricksHost;
100105
this.clientId = options?.clientId;
101106
this.returnOriginalTokenOnFailure = options?.returnOriginalTokenOnFailure ?? true;
107+
this.getAgent = options?.getAgent;
102108
}
103109

104110
async getToken(): Promise<Token> {
@@ -162,12 +168,14 @@ export default class FederationProvider implements ITokenProvider {
162168
*/
163169
private async attemptTokenExchange(body: string): Promise<Token> {
164170
const url = this.buildExchangeUrl();
171+
const agent = await this.getAgent?.(url);
165172
const controller = new AbortController();
166173
const timeoutId = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS);
167174

168175
try {
169176
const response = await fetchImpl(url, {
170177
method: 'POST',
178+
agent,
171179
headers: {
172180
'Content-Type': 'application/x-www-form-urlencoded',
173181
},

‎tests/unit/DBSQLClient.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { expect, AssertionError } from 'chai';
22
import sinon from 'sinon';
33
import fs from 'fs';
4+
import http from 'http';
45
import { Response } from 'node-fetch';
56
import DBSQLClient, { ThriftLibrary } from '../../lib/DBSQLClient';
67
import DBSQLSession from '../../lib/DBSQLSession';
@@ -9,6 +10,7 @@ import KernelBackend from '../../lib/kernel/KernelBackend';
910
import * as KernelNativeLoader from '../../lib/kernel/KernelNativeLoader';
1011

1112
import PlainHttpAuthentication from '../../lib/connection/auth/PlainHttpAuthentication';
13+
import FederationProvider from '../../lib/connection/auth/tokenProvider/FederationProvider';
1214
import DatabricksOAuth from '../../lib/connection/auth/DatabricksOAuth';
1315
import { DatabricksOAuthManager, AzureOAuthManager } from '../../lib/connection/auth/DatabricksOAuth/OAuthManager';
1416

@@ -674,6 +676,28 @@ describe('DBSQLClient[Symbol.asyncDispose]', () => {
674676
});
675677

676678
describe('DBSQLClient.createAuthProvider', () => {
679+
it('should send the token federation exchange through the connection agent', async () => {
680+
const client = new DBSQLClient();
681+
const connectionProvider = new ConnectionProviderStub();
682+
const agent = new http.Agent();
683+
const getAgent = sinon.stub(connectionProvider, 'getAgent').resolves(agent);
684+
client['connectionProvider'] = connectionProvider;
685+
686+
const provider = client['createAuthProvider']({
687+
...connectOptions,
688+
authType: 'static-token',
689+
staticToken: 'token',
690+
enableTokenFederation: true,
691+
});
692+
693+
const federation = (provider as unknown as { tokenProvider: FederationProvider }).tokenProvider;
694+
expect(federation).to.be.instanceOf(FederationProvider);
695+
const url = 'https://dbc-XXXXXXXX-XXXX.cloud.databricks.com/oidc/v1/token';
696+
expect(await federation['getAgent']!(url)).to.equal(agent);
697+
expect(getAgent.calledOnce).to.be.true;
698+
expect((getAgent.firstCall.args as unknown[])[0]).to.equal(url);
699+
});
700+
677701
it('should use access token auth method', () => {
678702
const client = new DBSQLClient();
679703

‎tests/unit/connection/auth/tokenProvider/FederationProvider.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { expect } from 'chai';
2+
import http from 'http';
23
import sinon from 'sinon';
34
import type nodeFetch from 'node-fetch';
45
import FederationProvider, {
@@ -132,6 +133,27 @@ describe('FederationProvider', () => {
132133
expect(passedSignal.aborted).to.be.false;
133134
});
134135

136+
it('should send the exchange through the agent from getAgent', async () => {
137+
const foreignJwt = createJWT({ iss: 'https://idp.example.com' });
138+
const agent = new http.Agent();
139+
const getAgent = sinon.stub().resolves(agent);
140+
const federationProvider = new FederationProvider(
141+
new MockTokenProvider(foreignJwt),
142+
'my-workspace.cloud.databricks.com',
143+
{ getAgent },
144+
);
145+
146+
const fetchStub = sinon
147+
.stub<Parameters<typeof nodeFetch>, ReturnType<typeof nodeFetch>>()
148+
.resolves(buildFakeResponse({ ok: true, body: { access_token: 'exchanged-databricks-token' } }));
149+
setFederationFetchForTest(fetchStub as unknown as typeof nodeFetch);
150+
151+
await federationProvider.getToken();
152+
153+
expect(getAgent.calledOnceWithExactly('https://my-workspace.cloud.databricks.com/oidc/v1/token')).to.be.true;
154+
expect(fetchStub.firstCall.args[1]!.agent).to.equal(agent);
155+
});
156+
135157
it('should propagate abort from the controller to the signal observed by fetch', async () => {
136158
const foreignJwt = createJWT({ iss: 'https://idp.example.com' });
137159
const baseProvider = new MockTokenProvider(foreignJwt);

0 commit comments

Comments
 (0)