From f5130fe9cd247b227d10ddcca4d973e367e0f1e6 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 16 Aug 2023 11:30:47 -0400 Subject: [PATCH 1/8] fix(NODE-5548): ensure that tlsCertificateKeyFile maps to cert and key options --- src/mongo_client.ts | 13 +++++++++---- test/manual/tls_support.test.ts | 2 ++ test/unit/mongo_client.test.js | 1 + 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index b35d2811bf0..38c635281b1 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -438,7 +438,11 @@ export class MongoClient extends TypedEventEmitter { options.ca ??= await fs.readFile(options.tlsCAFile, { encoding: 'utf8' }); } if (typeof options.tlsCertificateKeyFile === 'string') { - options.key ??= await fs.readFile(options.tlsCertificateKeyFile, { encoding: 'utf8' }); + if (!options.key || !options.cert) { + const contents = await fs.readFile(options.tlsCertificateKeyFile, { encoding: 'utf8' }); + options.key ??= contents; + options.cert ??= contents; + } } } if (typeof options.srvHost === 'string') { @@ -787,6 +791,7 @@ export interface MongoOptions * |:----------------------|:----------------------------------------------|:-------------------| * | `ca` | `tlsCAFile` | `string` | * | `crl` | N/A | `string` | + * | `cert` | `tlsCertificateKeyFile` | `string` | * | `key` | `tlsCertificateKeyFile` | `string` | * | `passphrase` | `tlsCertificateKeyFilePassword` | `string` | * | `rejectUnauthorized` | `tlsAllowInvalidCertificates` | `boolean` | @@ -804,9 +809,9 @@ export interface MongoOptions * * The files specified by the paths passed in to the `tlsCAFile` and `tlsCertificateKeyFile` fields * are read lazily on the first call to `MongoClient.connect`. Once these files have been read and - * the `ca` and `key` fields are populated, they will not be read again on subsequent calls to - * `MongoClient.connect`. As a result, until the first call to `MongoClient.connect`, the `ca` - * and `key` fields will be undefined. + * the `ca`, `cert` and `key` fields are populated, they will not be read again on subsequent calls to + * `MongoClient.connect`. As a result, until the first call to `MongoClient.connect`, the `ca`, + * `cert` and `key` fields will be undefined. */ tls: boolean; diff --git a/test/manual/tls_support.test.ts b/test/manual/tls_support.test.ts index 179527b1c4b..f2f115b9d11 100644 --- a/test/manual/tls_support.test.ts +++ b/test/manual/tls_support.test.ts @@ -51,11 +51,13 @@ describe('TLS Support', function () { expect(client.options).property('tlsCertificateKeyFile', TLS_CERT_KEY_FILE); expect(client.options).not.have.property('ca'); expect(client.options).not.have.property('key'); + expect(client.options).not.have.property('cert'); await client.connect(); expect(client.options).property('ca').to.exist; expect(client.options).property('key').to.exist; + expect(client.options).property('cert').to.exist; }); context('when client has been opened and closed more than once', function () { diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 64ec92e2fef..c43a8a17ba0 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -58,6 +58,7 @@ describe('MongoOptions', function () { expect(options).to.not.have.property('tlsCertificateKeyFilePassword'); expect(options).to.not.have.property('key'); expect(options).to.not.have.property('ca'); + expect(options).to.not.have.property('cert'); expect(options).to.have.property('tlsCertificateKeyFile', filename); expect(options).to.have.property('tlsCAFile', filename); expect(options).has.property('passphrase', 'tlsCertificateKeyFilePassword'); From de598e865da7195740e915b9970b1d7c4369c194 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 16 Aug 2023 11:33:12 -0400 Subject: [PATCH 2/8] fix(NODE-5548): check for typeof string insteady of truthiness --- src/mongo_client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 38c635281b1..4dc56fe699e 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -438,7 +438,7 @@ export class MongoClient extends TypedEventEmitter { options.ca ??= await fs.readFile(options.tlsCAFile, { encoding: 'utf8' }); } if (typeof options.tlsCertificateKeyFile === 'string') { - if (!options.key || !options.cert) { + if (typeof options.key !== 'string' || typeof options.cert !== 'string') { const contents = await fs.readFile(options.tlsCertificateKeyFile, { encoding: 'utf8' }); options.key ??= contents; options.cert ??= contents; From d166fb657fa0e8361d7b480f8ba3731acb5b5891 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 16 Aug 2023 11:51:15 -0400 Subject: [PATCH 3/8] fix(NODE-5548): revert to truthiness check and drop encoding from readFile call --- src/mongo_client.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 4dc56fe699e..1e0971cb8c4 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -435,11 +435,11 @@ export class MongoClient extends TypedEventEmitter { if (options.tls) { if (typeof options.tlsCAFile === 'string') { - options.ca ??= await fs.readFile(options.tlsCAFile, { encoding: 'utf8' }); + options.ca ??= await fs.readFile(options.tlsCAFile); } if (typeof options.tlsCertificateKeyFile === 'string') { - if (typeof options.key !== 'string' || typeof options.cert !== 'string') { - const contents = await fs.readFile(options.tlsCertificateKeyFile, { encoding: 'utf8' }); + if (!options.key || !options.cert) { + const contents = await fs.readFile(options.tlsCertificateKeyFile); options.key ??= contents; options.cert ??= contents; } From 018d041b31a64be785f55418ac9baf8119802027 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Aug 2023 14:47:33 -0400 Subject: [PATCH 4/8] test(NODE-5548): add failure tests --- test/manual/tls_support.test.ts | 38 +++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/test/manual/tls_support.test.ts b/test/manual/tls_support.test.ts index f2f115b9d11..01b67993d6e 100644 --- a/test/manual/tls_support.test.ts +++ b/test/manual/tls_support.test.ts @@ -51,13 +51,11 @@ describe('TLS Support', function () { expect(client.options).property('tlsCertificateKeyFile', TLS_CERT_KEY_FILE); expect(client.options).not.have.property('ca'); expect(client.options).not.have.property('key'); - expect(client.options).not.have.property('cert'); await client.connect(); expect(client.options).property('ca').to.exist; expect(client.options).property('key').to.exist; - expect(client.options).property('cert').to.exist; }); context('when client has been opened and closed more than once', function () { @@ -108,6 +106,42 @@ describe('TLS Support', function () { }); }); }); + + context('when tlsCertificateKeyFile is provided, but tlsCAFile is missing', () => { + let client: MongoClient; + beforeEach(() => { + client = new MongoClient(CONNECTION_STRING, { + tls: true, + tlsCertificateKeyFile: TLS_CERT_KEY_FILE + }); + }); + afterEach(async () => { + if (client) await client.close(); + }); + + it('throws an error', async () => { + const err = await client.connect().catch(e => e); + expect(err).to.be.instanceOf(Error); + }); + }); + + context('when tlsCAFile is provided, but tlsCertificateKeyFile is missing', () => { + let client: MongoClient; + beforeEach(() => { + client = new MongoClient(CONNECTION_STRING, { + tls: true, + tlsCAFile: TLS_CA_FILE + }); + }); + afterEach(async () => { + if (client) await client.close(); + }); + + it('throws an error', async () => { + const err = await client.connect().catch(e => e); + expect(err).to.be.instanceOf(Error); + }); + }); }); function makeConnectionTest(connectionString: string, clientOptions?: MongoClientOptions) { From 247c4dc331c09397d4182923d45afbd114bf8f0d Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Aug 2023 14:52:21 -0400 Subject: [PATCH 5/8] test(NODE-5548): restore cert assertion --- test/manual/tls_support.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/manual/tls_support.test.ts b/test/manual/tls_support.test.ts index 01b67993d6e..2093a540325 100644 --- a/test/manual/tls_support.test.ts +++ b/test/manual/tls_support.test.ts @@ -51,11 +51,13 @@ describe('TLS Support', function () { expect(client.options).property('tlsCertificateKeyFile', TLS_CERT_KEY_FILE); expect(client.options).not.have.property('ca'); expect(client.options).not.have.property('key'); + expect(client.options).not.have.property('cert'); await client.connect(); expect(client.options).property('ca').to.exist; expect(client.options).property('key').to.exist; + expect(client.options).property('cert').to.exist; }); context('when client has been opened and closed more than once', function () { From bd6f2a0e73ca0d6e958d9937206c4f9a96d11325 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 18 Aug 2023 10:02:55 -0400 Subject: [PATCH 6/8] test(NODE-5548): fix tests --- test/manual/tls_support.test.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/manual/tls_support.test.ts b/test/manual/tls_support.test.ts index 2093a540325..c31bf968e49 100644 --- a/test/manual/tls_support.test.ts +++ b/test/manual/tls_support.test.ts @@ -114,7 +114,9 @@ describe('TLS Support', function () { beforeEach(() => { client = new MongoClient(CONNECTION_STRING, { tls: true, - tlsCertificateKeyFile: TLS_CERT_KEY_FILE + tlsCertificateKeyFile: TLS_CERT_KEY_FILE, + serverSelectionTimeoutMS: 5000, + connectTimeoutMS: 5000 }); }); afterEach(async () => { @@ -123,6 +125,7 @@ describe('TLS Support', function () { it('throws an error', async () => { const err = await client.connect().catch(e => e); + console.log(client); expect(err).to.be.instanceOf(Error); }); }); @@ -139,9 +142,9 @@ describe('TLS Support', function () { if (client) await client.close(); }); - it('throws an error', async () => { - const err = await client.connect().catch(e => e); - expect(err).to.be.instanceOf(Error); + it('connects without error', async () => { + const clientOrError = await client.connect().catch(e => e); + expect(clientOrError).to.be.instanceOf(MongoClient); }); }); }); From 35e916ca45bfcd373a133ff222ad25c64a97ad83 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 18 Aug 2023 10:03:32 -0400 Subject: [PATCH 7/8] test(NODE-5548): remove log statement --- test/manual/tls_support.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/manual/tls_support.test.ts b/test/manual/tls_support.test.ts index c31bf968e49..97d319077ec 100644 --- a/test/manual/tls_support.test.ts +++ b/test/manual/tls_support.test.ts @@ -125,7 +125,6 @@ describe('TLS Support', function () { it('throws an error', async () => { const err = await client.connect().catch(e => e); - console.log(client); expect(err).to.be.instanceOf(Error); }); }); From d713a4536f8bdeb81f6e721af8be7062d0307f6b Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 21 Aug 2023 09:54:01 -0400 Subject: [PATCH 8/8] test(NODE-5548): narrow error type --- test/manual/tls_support.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/manual/tls_support.test.ts b/test/manual/tls_support.test.ts index 97d319077ec..f50c43a48d2 100644 --- a/test/manual/tls_support.test.ts +++ b/test/manual/tls_support.test.ts @@ -1,7 +1,12 @@ import { expect } from 'chai'; import { promises as fs } from 'fs'; -import { LEGACY_HELLO_COMMAND, MongoClient, type MongoClientOptions } from '../mongodb'; +import { + LEGACY_HELLO_COMMAND, + MongoClient, + type MongoClientOptions, + MongoServerSelectionError +} from '../mongodb'; const REQUIRED_ENV = ['MONGODB_URI', 'SSL_KEY_FILE', 'SSL_CA_FILE']; @@ -123,9 +128,9 @@ describe('TLS Support', function () { if (client) await client.close(); }); - it('throws an error', async () => { + it('throws a MongoServerSelectionError', async () => { const err = await client.connect().catch(e => e); - expect(err).to.be.instanceOf(Error); + expect(err).to.be.instanceOf(MongoServerSelectionError); }); });