diff --git a/packages/devtools-connect/src/connect.spec.ts b/packages/devtools-connect/src/connect.spec.ts index 5c346acd..7af6a2d8 100644 --- a/packages/devtools-connect/src/connect.spec.ts +++ b/packages/devtools-connect/src/connect.spec.ts @@ -6,6 +6,7 @@ import { MongoClient } from 'mongodb'; import sinon, { stubConstructor } from 'ts-sinon'; import chai, { expect } from 'chai'; import sinonChai from 'sinon-chai'; +import { Agent as HTTPSAgent } from 'https'; chai.use(sinonChai); @@ -375,7 +376,7 @@ describe('devtools connect', function () { }); describe('retryable TLS errors', function () { - it('retries TLS errors without system CA integration enabled', async function () { + it('retries TLS errors without system CA integration enabled -- MongoClient error', async function () { const uri = 'localhost:27017'; const mClient = new FakeMongoClient(); const mClientType = sinon.stub().returns(mClient); @@ -429,6 +430,67 @@ describe('devtools connect', function () { expect(earlyFailures).to.equal(2); }); + + it('retries TLS errors without system CA integration enabled -- OIDC error', async function () { + const uri = 'localhost:27017'; + const mClient = new FakeMongoClient(); + let opts: DevtoolsConnectOptions; + + const proxyConnectEvents: any[] = []; + bus.on('proxy:connect', (ev) => { + proxyConnectEvents.push({ ...ev, opts: { ...ev.opts } }); + ev.opts.ca = 'abcdef'; // will always fail + ev.agent.httpsAgent = new HTTPSAgent({ ca: 'abcdef' }); + }); + const mClientType = sinon.stub().callsFake((url, clientOpts) => { + opts = clientOpts; + return mClient; + }); + + mClient.connect = sinon.stub().callsFake(async () => { + return await opts.authMechanismProperties?.OIDC_HUMAN_CALLBACK?.({ + version: 1, + timeoutContext: new AbortController().signal, + idpInfo: { + issuer: 'https://mongodb.com/', + clientId: 'meow', + }, + }); + }); + mClient.topology = { + description: { + servers: new Map([['localhost:27017', {}]]), + }, + }; + + try { + await connectMongoClient( + uri, + { + ...defaultOpts, + authMechanism: 'MONGODB-OIDC', + }, + bus, + mClientType as any, + ); + expect.fail('missed exception'); + } catch (err: any) { + expect(err.message).to.include('Unable to fetch issuer metadata'); + } + + expect(mClientType.getCalls()).to.have.lengthOf(2); + expect((mClient as any).connect.getCalls()).to.have.lengthOf(2); + expect(proxyConnectEvents).to.have.lengthOf(2); + expect(proxyConnectEvents[0].agent).to.not.equal( + proxyConnectEvents[1].agent, + ); + expect(proxyConnectEvents[0].opts.ca).to.not.equal( + proxyConnectEvents[1].opts.ca, + ); + expect(proxyConnectEvents[0].agent.proxyOptions.ca).to.not.equal( + proxyConnectEvents[1].agent.proxyOptions.ca, + ); + }); }); }); diff --git a/packages/devtools-connect/src/connect.ts b/packages/devtools-connect/src/connect.ts index 3699ae2c..085f32a6 100644 --- a/packages/devtools-connect/src/connect.ts +++ b/packages/devtools-connect/src/connect.ts @@ -32,6 +32,7 @@ import { Tunnel, systemCA, createFetch, + isExistingAgentInstance, } from '@mongodb-js/devtools-proxy-support'; export type { DevtoolsProxyOptions, AgentWithInitialize }; @@ -490,6 +491,17 @@ async function connectMongoClientImpl({ ca = caWithSystemCerts; } + const addCAToProxyOptions = ( + opts: AgentWithInitialize | DevtoolsProxyOptions | undefined, + ): AgentWithInitialize | DevtoolsProxyOptions => { + if (opts && isExistingAgentInstance(opts)) return opts; + return { + caExcludeSystemCerts: !useSystemCA, + ...opts, + ...(ca ? { ca } : {}), + }; + }; + // Create a proxy agent, if requested. `useOrCreateAgent()` takes a target argument // that can be used to select a proxy for a specific procotol or host; // here we specify 'mongodb://' if we only intend to use the proxy for database @@ -497,12 +509,7 @@ async function connectMongoClientImpl({ const proxyAgent = clientOptions.proxy && useOrCreateAgent( - 'createConnection' in clientOptions.proxy - ? clientOptions.proxy - : { - ...(clientOptions.proxy as DevtoolsProxyOptions), - ...(ca ? { ca } : {}), - }, + addCAToProxyOptions(clientOptions.proxy), clientOptions.applyProxyToOIDC === true ? undefined : 'mongodb://', ); cleanupOnClientClose.push(() => proxyAgent?.destroy()); @@ -516,17 +523,16 @@ async function connectMongoClientImpl({ } else if (clientOptions.applyProxyToOIDC) { oidcProxyOptions = clientOptions.applyProxyToOIDC; } - if (!oidcProxyOptions && ca) { - oidcProxyOptions = { ca }; - } - if (oidcProxyOptions) { - clientOptions.oidc = { - customFetch: createFetch( - oidcProxyOptions, - ) as unknown as MongoDBOIDCPluginOptions['customFetch'], + + const oidcFetch = createFetch(addCAToProxyOptions(oidcProxyOptions)); + clientOptions = { + ...clientOptions, + oidc: { + customFetch: + oidcFetch as unknown as MongoDBOIDCPluginOptions['customFetch'], ...clientOptions.oidc, - }; - } + }, + }; let tunnel: Tunnel | undefined; if (proxyAgent && !hasProxyHostOption(uri, clientOptions)) { @@ -537,7 +543,11 @@ async function connectMongoClientImpl({ ); cleanupOnClientClose.push(() => tunnel?.close()); } - for (const proxyLogger of new Set([tunnel?.logger, proxyAgent?.logger])) { + for (const proxyLogger of new Set([ + tunnel?.logger, + proxyAgent?.logger, + oidcFetch?.agent?.logger, + ])) { if (proxyLogger) { copyEventEmitterEvents(proxyLogger, logger); } diff --git a/packages/devtools-connect/src/fast-failure-connect.ts b/packages/devtools-connect/src/fast-failure-connect.ts index b0fbc32d..e937af3b 100644 --- a/packages/devtools-connect/src/fast-failure-connect.ts +++ b/packages/devtools-connect/src/fast-failure-connect.ts @@ -71,7 +71,8 @@ function handleNestedErrors( ): (err: unknown) => boolean { const checker = (err: unknown): boolean => { if ( - Object.prototype.toString.call(err).toLowerCase() !== '[object error]' + // e.g. node-fetch defines its FetchError class to be a `[object FetchError]` + !Object.prototype.toString.call(err).match(/\[object .*error\]/i) ) { return checker(new Error(String(err))); } diff --git a/packages/devtools-proxy-support/src/agent.ts b/packages/devtools-proxy-support/src/agent.ts index 1ab2daa8..f321b6d7 100644 --- a/packages/devtools-proxy-support/src/agent.ts +++ b/packages/devtools-proxy-support/src/agent.ts @@ -101,6 +101,7 @@ class DevtoolsProxyAgent extends ProxyAgent implements AgentWithInitialize { } this._req = req; this._reqLock = new Promise((resolve) => (this._reqLockResolve = resolve)); + this.logger.emit('proxy:connect', { agent: this, req, opts }); const agent = await super.connect(req, opts); // Work around https://github.com/TooTallNate/proxy-agents/pull/330 if ('addRequest' in agent && typeof agent.addRequest === 'function') { @@ -131,7 +132,10 @@ class DevtoolsProxyAgentWithSystemCA extends AgentBase { super(); this.proxyOptions = proxyOptions; this.agent = (async () => { - const { ca } = await systemCA({ ca: proxyOptions.ca }); + const { ca } = await systemCA({ + ca: proxyOptions.ca, + excludeSystemCerts: proxyOptions.caExcludeSystemCerts, + }); return new DevtoolsProxyAgent( { ...proxyOptions, ca, allowPartialTrustChain: true }, this.logger, @@ -167,8 +171,8 @@ export function useOrCreateAgent( target?: string, useTargetRegardlessOfExistingAgent = false, ): AgentWithInitialize | undefined { - if ('createConnection' in proxyOptions) { - const agent = proxyOptions as AgentWithInitialize; + if (isExistingAgentInstance(proxyOptions)) { + const agent = proxyOptions; if ( useTargetRegardlessOfExistingAgent && target !== undefined && @@ -179,12 +183,15 @@ export function useOrCreateAgent( } return agent; } else { - if ( - target !== undefined && - !proxyForUrl(proxyOptions as DevtoolsProxyOptions, target) - ) { + if (target !== undefined && !proxyForUrl(proxyOptions, target)) { return undefined; } - return createAgent(proxyOptions as DevtoolsProxyOptions); + return createAgent(proxyOptions); } } + +export function isExistingAgentInstance( + options: DevtoolsProxyOptions | AgentWithInitialize, +): options is AgentWithInitialize { + return 'createConnection' in options; +} diff --git a/packages/devtools-proxy-support/src/index.ts b/packages/devtools-proxy-support/src/index.ts index 6377b951..1a89ba92 100644 --- a/packages/devtools-proxy-support/src/index.ts +++ b/packages/devtools-proxy-support/src/index.ts @@ -1,6 +1,11 @@ export * from './proxy-options-public'; export { Tunnel, TunnelOptions, createSocks5Tunnel } from './socks5'; -export { createAgent, useOrCreateAgent, AgentWithInitialize } from './agent'; +export { + createAgent, + useOrCreateAgent, + isExistingAgentInstance, + AgentWithInitialize, +} from './agent'; export { createFetch, Request, diff --git a/packages/devtools-proxy-support/src/logging.ts b/packages/devtools-proxy-support/src/logging.ts index e30edcd3..75868c5e 100644 --- a/packages/devtools-proxy-support/src/logging.ts +++ b/packages/devtools-proxy-support/src/logging.ts @@ -3,6 +3,11 @@ // values used here; in particular, // https://github.com/mongodb-js/compass/tree/55a5a608713d7316d158dc66febeb6b114d8b40d/packages/ssh-tunnel/src +import type { AgentConnectOpts } from 'agent-base'; +import type { ClientRequest } from 'http'; +import type { SecureContextOptions } from 'tls'; +import type { AgentWithInitialize } from './agent'; + interface BaseSocks5RequestMetadata { srcAddr: string; srcPort: number; @@ -45,6 +50,11 @@ export interface ProxyEventMap { retryableError: boolean; retriesLeft: number; }) => void; + 'proxy:connect': (ev: { + agent: AgentWithInitialize; + req: ClientRequest; + opts: AgentConnectOpts & Partial; + }) => void; } export type ProxyEventArgs = diff --git a/packages/devtools-proxy-support/src/proxy-options.ts b/packages/devtools-proxy-support/src/proxy-options.ts index f636d0ac..f5be30b5 100644 --- a/packages/devtools-proxy-support/src/proxy-options.ts +++ b/packages/devtools-proxy-support/src/proxy-options.ts @@ -21,8 +21,11 @@ export interface DevtoolsProxyOptions { }; // Not being honored by the translate-to-electron functionality - // This will be merged with the system CA list and the Node.js built-in CA list + // This will be merged with the system CA list and the Node.js built-in CA list, + // unless `caExcludeSystemCerts` is set ca?: ConnectionOptions['ca']; + // Exclude system certificates from the full certificate list. + caExcludeSystemCerts?: boolean; // Mostly intended for testing, defaults to `process.env`. // This should usually not be stored, and secrets will not be diff --git a/packages/devtools-proxy-support/src/system-ca.ts b/packages/devtools-proxy-support/src/system-ca.ts index 60d00a60..842310fe 100644 --- a/packages/devtools-proxy-support/src/system-ca.ts +++ b/packages/devtools-proxy-support/src/system-ca.ts @@ -58,6 +58,7 @@ export async function systemCA( existingOptions: { ca?: NodeJSCAOption; tlsCAFile?: string | null | undefined; + excludeSystemCerts?: boolean; } = {}, ): Promise<{ ca: string; @@ -81,9 +82,11 @@ export async function systemCA( const messages: string[] = []; try { - const systemCertsResult = await systemCertsCached(); - asyncFallbackError = systemCertsResult.asyncFallbackError; - systemCerts = systemCertsResult.certs; + if (!existingOptions.excludeSystemCerts) { + const systemCertsResult = await systemCertsCached(); + asyncFallbackError = systemCertsResult.asyncFallbackError; + systemCerts = systemCertsResult.certs; + } } catch (err: any) { systemCertsError = err; }