From 1195d3b9d2fcd4e52a390da227d1980fc7e0d379 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 14 Aug 2023 15:33:33 -0400 Subject: [PATCH 1/5] fix(NODE-5536): remove credentials from ConnectionPoolCreatedEvent options --- src/cmap/connection_pool_events.ts | 9 ++++- .../connection_monitoring_and_pooling.test.ts | 37 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/cmap/connection_pool_events.ts b/src/cmap/connection_pool_events.ts index 0f56bf6bbb..92df40b7fd 100644 --- a/src/cmap/connection_pool_events.ts +++ b/src/cmap/connection_pool_events.ts @@ -28,12 +28,17 @@ export class ConnectionPoolMonitoringEvent { */ export class ConnectionPoolCreatedEvent extends ConnectionPoolMonitoringEvent { /** The options used to create this connection pool */ - options?: ConnectionPoolOptions; + options: Omit & { credentials?: Record }; /** @internal */ constructor(pool: ConnectionPool) { super(pool); - this.options = pool.options; + if (pool.options.credentials != null) { + // Intentionally remove credentials: NODE-5460 + this.options = { ...pool.options, credentials: {} }; + } else { + this.options = pool.options; + } } } diff --git a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts index b76cd66a50..980701ad7d 100644 --- a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts @@ -1,3 +1,7 @@ +import { expect } from 'chai'; +import { once } from 'events'; + +import { MongoClient } from '../../../src'; import { loadSpecTests } from '../../spec'; import { CmapTest, runCmapTestSuite } from '../../tools/cmap_spec_runner'; @@ -16,4 +20,37 @@ describe('Connection Monitoring and Pooling (Node Driver)', function () { } ] }); + + describe('ConnectionPoolCreatedEvent', () => { + let client: MongoClient; + beforeEach(async function () { + client = this.configuration.newClient(); + }); + + afterEach(async function () { + await client.close(); + }); + + describe('constructor()', () => { + it('when auth is enabled redacts credentials from options', { + metadata: { requires: { auth: 'enabled' } }, + async test() { + const poolCreated = once(client, 'connectionPoolCreated'); + await client.connect(); + const [event] = await poolCreated; + expect(event).to.have.deep.nested.property('options.credentials', {}); + } + }); + + it('when auth is disabled does not add a credentials property to options', { + metadata: { requires: { auth: 'disabled' } }, + async test() { + const poolCreated = once(client, 'connectionPoolCreated'); + await client.connect(); + const [event] = await poolCreated; + expect(event).to.not.have.nested.property('options.credentials'); + } + }); + }); + }); }); From 3f5b37346ebfb386acd9b396f81e38e9be9393f0 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 15 Aug 2023 15:03:40 -0400 Subject: [PATCH 2/5] test: pool options still have credentials set --- .../connection_monitoring_and_pooling.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts index 980701ad7d..458764c224 100644 --- a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { once } from 'events'; -import { MongoClient } from '../../../src'; +import { MongoClient, MongoCredentials } from '../../../src'; import { loadSpecTests } from '../../spec'; import { CmapTest, runCmapTestSuite } from '../../tools/cmap_spec_runner'; @@ -39,6 +39,18 @@ describe('Connection Monitoring and Pooling (Node Driver)', function () { await client.connect(); const [event] = await poolCreated; expect(event).to.have.deep.nested.property('options.credentials', {}); + + const poolOptions = Array.from(client.topology?.s.servers.values() ?? []).map( + s => s.s.pool.options + ); + expect(poolOptions).to.have.length.of.at.least(1); + + for (const { credentials = {} } of poolOptions) { + expect( + Object.keys(credentials), + 'pool.options.credentials must exist and have keys' + ).to.not.equal(0); + } } }); From 0163fe534a1fea2b1b7c0367a67d8710b00dcfd3 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 15 Aug 2023 16:25:07 -0400 Subject: [PATCH 3/5] fix: lint --- .../connection_monitoring_and_pooling.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts index 458764c224..41a157b65e 100644 --- a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { once } from 'events'; -import { MongoClient, MongoCredentials } from '../../../src'; +import { MongoClient } from '../../../src'; import { loadSpecTests } from '../../spec'; import { CmapTest, runCmapTestSuite } from '../../tools/cmap_spec_runner'; From 9fe9d72ebf90fe4e9828e23c497e59aef800b023 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 16 Aug 2023 10:41:08 -0400 Subject: [PATCH 4/5] fix: lint by actually changing the src :/ --- src/operations/operation.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 573a896b1a..ff936594db 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -19,10 +19,6 @@ export const Aspect = { /** @public */ export type Hint = string | Document; -export interface OperationConstructor extends Function { - aspects?: Set; -} - /** @public */ export interface OperationOptions extends BSONSerializeOptions { /** Specify ClientSession for this command */ @@ -122,7 +118,7 @@ export abstract class AbstractOperation { } export function defineAspects( - operation: OperationConstructor, + operation: { new (...args: any[]): any }, aspects: symbol | symbol[] | Set ): Set { if (!Array.isArray(aspects) && !(aspects instanceof Set)) { From 862dde75e660808c2791f8541735dd90887397c1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 16 Aug 2023 11:08:25 -0400 Subject: [PATCH 5/5] fix: cast --- src/operations/operation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/operations/operation.ts b/src/operations/operation.ts index ff936594db..5bdd41477b 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -92,7 +92,7 @@ export abstract class AbstractOperation { ): void; hasAspect(aspect: symbol): boolean { - const ctor = this.constructor as OperationConstructor; + const ctor = this.constructor as { aspects?: Set }; if (ctor.aspects == null) { return false; }