From 3232bf29224feaa13da3975b37affcb575a80257 Mon Sep 17 00:00:00 2001 From: Daria Pardue <81593090+dariakp@users.noreply.github.com> Date: Mon, 13 Sep 2021 15:00:29 -0400 Subject: [PATCH] fix(NODE-3591): tlsCertificateKeyFile option does not default cert (#2979) --- src/connection_string.ts | 27 +++---- src/mongo_client.ts | 2 +- test/unit/core/connection_string.test.js | 2 +- test/unit/mongo_client_options.test.js | 91 +++++++++++++++++++++++- 4 files changed, 106 insertions(+), 16 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 0ed6cbd3de2..c188e2df85d 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -176,7 +176,7 @@ function getBoolean(name: string, value: unknown): boolean { const valueString = String(value).toLowerCase(); if (TRUTHS.has(valueString)) return true; if (FALSEHOODS.has(valueString)) return false; - throw new MongoParseError(`For ${name} Expected stringified boolean value, got: ${value}`); + throw new MongoParseError(`Expected ${name} to be stringified boolean value, got: ${value}`); } function getInt(name: string, value: unknown): number { @@ -335,6 +335,19 @@ export function parseOptions( allOptions.set(key, values); } + if (allOptions.has('tlsCertificateKeyFile') && !allOptions.has('tlsCertificateFile')) { + allOptions.set('tlsCertificateFile', allOptions.get('tlsCertificateKeyFile')); + } + + if (allOptions.has('tls') || allOptions.has('ssl')) { + const tlsAndSslOpts = (allOptions.get('tls') || []) + .concat(allOptions.get('ssl') || []) + .map(getBoolean.bind(null, 'tls/ssl')); + if (new Set(tlsAndSslOpts).size !== 1) { + throw new MongoParseError('All values of tls/ssl must be the same.'); + } + } + const unsupportedOptions = setDifference( allKeys, Array.from(Object.keys(OPTIONS)).map(s => s.toLowerCase()) @@ -384,18 +397,6 @@ export function parseOptions( mongoOptions.dbName = 'test'; } - if (allOptions.has('tls')) { - if (new Set(allOptions.get('tls')?.map(getBoolean)).size !== 1) { - throw new MongoParseError('All values of tls must be the same.'); - } - } - - if (allOptions.has('ssl')) { - if (new Set(allOptions.get('ssl')?.map(getBoolean)).size !== 1) { - throw new MongoParseError('All values of ssl must be the same.'); - } - } - checkTLSOptions(mongoOptions); if (options.promiseLibrary) PromiseProvider.set(options.promiseLibrary); diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 19192aa0208..b57563f51dd 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -112,7 +112,7 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC ssl?: boolean; /** Specifies the location of a local TLS Certificate */ tlsCertificateFile?: string; - /** Specifies the location of a local .pem file that contains either the client’s TLS/SSL certificate or the client’s TLS/SSL certificate and key. */ + /** Specifies the location of a local .pem file that contains either the client's TLS/SSL certificate and key or only the client's TLS/SSL key when tlsCertificateFile is used to provide the certificate. */ tlsCertificateKeyFile?: string; /** Specifies the password to de-crypt the tlsCertificateKeyFile. */ tlsCertificateKeyFilePassword?: string; diff --git a/test/unit/core/connection_string.test.js b/test/unit/core/connection_string.test.js index 41f0f227ba0..fd8bc17344f 100644 --- a/test/unit/core/connection_string.test.js +++ b/test/unit/core/connection_string.test.js @@ -136,7 +136,7 @@ describe('Connection String', function () { it('should validate non-equal tls values', function () { expect(() => parseOptions('mongodb://localhost/?tls=true&tls=false')).to.throw( MongoParseError, - 'All values of tls must be the same.' + 'All values of tls/ssl must be the same.' ); }); }); diff --git a/test/unit/mongo_client_options.test.js b/test/unit/mongo_client_options.test.js index 083908f2913..47a7785c95a 100644 --- a/test/unit/mongo_client_options.test.js +++ b/test/unit/mongo_client_options.test.js @@ -128,7 +128,7 @@ describe('MongoOptions', function () { ssl: true, sslPass: 'pass', sslValidate: true, - tls: false, + tls: true, tlsAllowInvalidCertificates: true, tlsAllowInvalidHostnames: true, tlsCertificateKeyFilePassword: 'tls-pass', @@ -245,6 +245,18 @@ describe('MongoOptions', function () { expect(options).has.property('tls', false); }); + it('ssl= can be used to set tls=true', function () { + const options = parseOptions('mongodb+srv://server.example.com/?ssl=true'); + expect(options).has.property('srvHost', 'server.example.com'); + expect(options).has.property('tls', true); + }); + + it('tls= can be used to set tls=true', function () { + const options = parseOptions('mongodb+srv://server.example.com/?tls=true'); + expect(options).has.property('srvHost', 'server.example.com'); + expect(options).has.property('tls', true); + }); + it('supports ReadPreference option in url', function () { const options = parseOptions('mongodb://localhost/?readPreference=nearest'); expect(options.readPreference).to.be.an.instanceof(ReadPreference); @@ -366,6 +378,83 @@ describe('MongoOptions', function () { expect(optionsUndefined.checkServerIdentity).to.equal(undefined); }); + describe('[tls certificate handling]', () => { + before(() => { + fs.writeFileSync('testCertKey.pem', 'cert key'); + fs.writeFileSync('testKey.pem', 'test key'); + fs.writeFileSync('testCert.pem', 'test cert'); + }); + + after(() => { + fs.unlinkSync('testCertKey.pem'); + fs.unlinkSync('testKey.pem'); + fs.unlinkSync('testCert.pem'); + }); + + it('correctly sets the cert and key if only tlsCertificateKeyFile is provided', function () { + const optsFromObject = parseOptions('mongodb://localhost/', { + tlsCertificateKeyFile: 'testCertKey.pem' + }); + expect(optsFromObject).to.have.property('cert', 'cert key'); + expect(optsFromObject).to.have.property('key', 'cert key'); + + const optsFromUri = parseOptions('mongodb://localhost?tlsCertificateKeyFile=testCertKey.pem'); + expect(optsFromUri).to.have.property('cert', 'cert key'); + expect(optsFromUri).to.have.property('key', 'cert key'); + }); + + it('correctly sets the cert and key if both tlsCertificateKeyFile and tlsCertificateFile is provided', function () { + const optsFromObject = parseOptions('mongodb://localhost/', { + tlsCertificateKeyFile: 'testKey.pem', + tlsCertificateFile: 'testCert.pem' + }); + expect(optsFromObject).to.have.property('cert', 'test cert'); + expect(optsFromObject).to.have.property('key', 'test key'); + + const optsFromUri = parseOptions( + 'mongodb://localhost?tlsCertificateKeyFile=testKey.pem&tlsCertificateFile=testCert.pem' + ); + expect(optsFromUri).to.have.property('cert', 'test cert'); + expect(optsFromUri).to.have.property('key', 'test key'); + }); + }); + + it('throws an error if multiple tls parameters are not all set to the same value', () => { + expect(() => parseOptions('mongodb://localhost?tls=true&tls=false')).to.throw( + 'All values of tls/ssl must be the same.' + ); + }); + + it('throws an error if multiple ssl parameters are not all set to the same value', () => { + expect(() => parseOptions('mongodb://localhost?ssl=true&ssl=false')).to.throw( + 'All values of tls/ssl must be the same.' + ); + }); + + it('throws an error if tls and ssl parameters are not all set to the same value', () => { + expect(() => parseOptions('mongodb://localhost?tls=true&ssl=false')).to.throw( + 'All values of tls/ssl must be the same.' + ); + expect(() => parseOptions('mongodb://localhost?tls=false&ssl=true')).to.throw( + 'All values of tls/ssl must be the same.' + ); + }); + + it('correctly sets tls if multiple tls parameters are all set to the same value', () => { + expect(parseOptions('mongodb://localhost?tls=true&tls=true')).to.have.property('tls', true); + expect(parseOptions('mongodb://localhost?tls=false&tls=false')).to.have.property('tls', false); + }); + + it('correctly sets tls if multiple ssl parameters are all set to the same value', () => { + expect(parseOptions('mongodb://localhost?ssl=true&ssl=true')).to.have.property('tls', true); + expect(parseOptions('mongodb://localhost?ssl=false&ssl=false')).to.have.property('tls', false); + }); + + it('correctly sets tls if tls and ssl parameters are all set to the same value', () => { + expect(parseOptions('mongodb://localhost?ssl=true&tls=true')).to.have.property('tls', true); + expect(parseOptions('mongodb://localhost?ssl=false&tls=false')).to.have.property('tls', false); + }); + it('transforms tlsInsecure correctly', function () { const optionsTrue = parseOptions('mongodb://localhost/', { tlsInsecure: true