From 269773494b482ef93e517ffc8d756d397672a010 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 12 Jan 2022 21:31:38 +0100 Subject: [PATCH 1/5] fix: persist custom appname MONGOSH-1015 --- packages/cli-repl/src/arg-mapper.ts | 20 ++++++++++++++++++ packages/cli-repl/src/cli-repl.ts | 15 ++++++++++++++ packages/cli-repl/src/run.ts | 3 +-- packages/cli-repl/test/e2e.spec.ts | 32 +++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/cli-repl/src/arg-mapper.ts b/packages/cli-repl/src/arg-mapper.ts index 0d5af1e652..1b404aabea 100644 --- a/packages/cli-repl/src/arg-mapper.ts +++ b/packages/cli-repl/src/arg-mapper.ts @@ -70,6 +70,7 @@ function mapCliToDriver(options: CliOptions): MongoClientOptions { } } applyTlsCertificateSelector(options.tlsCertificateSelector, nodeOptions); + applyDriverInfo(nodeOptions); return nodeOptions; } @@ -103,6 +104,25 @@ export function applyTlsCertificateSelector( } } +/** + * Applies driverInfo to track usage of mongosh, + * because appName can be overwritten by a user in their connection string. + * + * @param {MongoClientOptions} nodeOptions - The MongoClient options. + * + * @returns {DriverInfo} The driverInfo metadata. + */ +export function applyDriverInfo( + nodeOptions: MongoClientOptions +): void { + const { version } = require('../package.json'); + nodeOptions.driverInfo = { + name: 'mongosh', + version, + platform: process.platform + }; +} + function getCertificateExporter(): TlsCertificateExporter | undefined { if (process.env.TEST_OS_EXPORT_CERTIFICATE_AND_KEY_PATH) { return require(process.env.TEST_OS_EXPORT_CERTIFICATE_AND_KEY_PATH); diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index af126532fd..8386c6e004 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -163,6 +163,10 @@ class CliRepl { cs.password = await this.requirePassword(); driverUri = cs.href; } + if (this.isAppNameMissingURI(cs)) { + cs.searchParams.set('appName', `mongosh ${version}`); + driverUri = cs.href; + } } this.ensurePasswordFieldIsPresentInAuth(driverOptions); } @@ -454,6 +458,17 @@ class CliRepl { ); } + /** + * Is the appName missing from the connection string? + * + * @param {ConnectionString} cs - The existing connection string. + * + * @returns {boolean} If the appName is missing. + */ + isAppNameMissingURI(cs: ConnectionString): boolean { + return !cs.searchParams.get('appName'); + } + /** * Sets the auth.password field to undefined in the driverOptions if the auth * object is present with a truthy username. This is required by the driver, e.g. diff --git a/packages/cli-repl/src/run.ts b/packages/cli-repl/src/run.ts index 340a20da98..f72303606a 100644 --- a/packages/cli-repl/src/run.ts +++ b/packages/cli-repl/src/run.ts @@ -86,7 +86,6 @@ import stream from 'stream'; process.title = title; setTerminalWindowTitle(title); - const appName = `mongosh ${version}`; const shellHomePaths = getStoragePaths(); repl = new CliRepl({ shellCliOptions: { @@ -98,7 +97,7 @@ import stream from 'stream'; onExit: process.exit, shellHomePaths: shellHomePaths }); - await repl.start(driverUri, { appName, ...driverOptions }); + await repl.start(driverUri, driverOptions); } } catch (e) { console.error(`${e.name}: ${e.message}`); diff --git a/packages/cli-repl/test/e2e.spec.ts b/packages/cli-repl/test/e2e.spec.ts index dc99a87447..f3b61cf0b4 100644 --- a/packages/cli-repl/test/e2e.spec.ts +++ b/packages/cli-repl/test/e2e.spec.ts @@ -203,6 +203,38 @@ describe('e2e', function() { } }); + describe('set appName', () => { + context('with default appName', () => { + let shell; + beforeEach(async() => { + shell = TestShell.start({ args: [`mongodb://${await testServer.hostport()}/`] }); + await shell.waitForPrompt(); + shell.assertNoErrors(); + }); + it('appName set correctly', async() => { + const currentOp = await shell.executeLine('db.currentOp()'); + expect(currentOp).to.include("appName: 'mongosh 0.0.0-dev.0'"); + expect(currentOp).to.include("name: 'nodejs|mongosh'"); + shell.assertNoErrors(); + }); + }); + + context('with custom appName', () => { + let shell; + beforeEach(async() => { + shell = TestShell.start({ args: [`mongodb://${await testServer.hostport()}/?appName=Felicia`] }); + await shell.waitForPrompt(); + shell.assertNoErrors(); + }); + it('appName set correctly', async() => { + const currentOp = await shell.executeLine('db.currentOp()'); + expect(currentOp).to.include("appName: 'Felicia'"); + expect(currentOp).to.include("name: 'nodejs|mongosh'"); + shell.assertNoErrors(); + }); + }); + }); + describe('with connection string', () => { let db; let client; From 6d2c92dd326b6e01ffa10f50d9ad5b50eb591aad Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 13 Jan 2022 01:19:45 +0100 Subject: [PATCH 2/5] test: mapCliToDriver result always contains driverInfo --- packages/cli-repl/src/arg-mapper.spec.ts | 152 +++++++++++++++++++++-- 1 file changed, 139 insertions(+), 13 deletions(-) diff --git a/packages/cli-repl/src/arg-mapper.spec.ts b/packages/cli-repl/src/arg-mapper.spec.ts index dca2fa95cf..7b4fc91eaf 100644 --- a/packages/cli-repl/src/arg-mapper.spec.ts +++ b/packages/cli-repl/src/arg-mapper.spec.ts @@ -12,7 +12,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps to authSource', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - authSource: 'authDb' + authSource: 'authDb', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -22,7 +27,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps to authMechanism', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - authMechanism: 'SCRAM-SHA-1' + authMechanism: 'SCRAM-SHA-1', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -32,7 +42,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps to loggerLevel', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - loggerLevel: 'error' + loggerLevel: 'error', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -42,7 +57,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps to loggerLevel', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - loggerLevel: 'debug' + loggerLevel: 'debug', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -54,6 +74,11 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ auth: { username: 'richard' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -66,6 +91,11 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ auth: { password: 'aphextwin' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -79,6 +109,11 @@ describe('arg-mapper.mapCliToDriver', () => { auth: { username: 'richard', password: 'aphextwin' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -89,7 +124,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps the same argument', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - retryWrites: true + retryWrites: true, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -99,7 +139,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps the same argument', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - tls: true + tls: true, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -109,7 +154,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps the same argument', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - tlsAllowInvalidCertificates: true + tlsAllowInvalidCertificates: true, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -119,7 +169,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps the same argument', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - tlsAllowInvalidHostnames: true + tlsAllowInvalidHostnames: true, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -129,7 +184,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps the same argument', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - tlsCAFile: 'ca' + tlsCAFile: 'ca', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -139,7 +199,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps to sslCRL', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - sslCRL: 'key' + sslCRL: 'key', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -149,7 +214,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps the same argument', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - tlsCertificateKeyFile: 'key' + tlsCertificateKeyFile: 'key', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -159,7 +229,12 @@ describe('arg-mapper.mapCliToDriver', () => { it('maps the same argument', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ - tlsCertificateKeyFilePassword: 'pw' + tlsCertificateKeyFilePassword: 'pw', + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } }); }); }); @@ -175,6 +250,11 @@ describe('arg-mapper.mapCliToDriver', () => { accessKeyId: 'awskey' } } + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -191,6 +271,11 @@ describe('arg-mapper.mapCliToDriver', () => { secretAccessKey: 'secretkey' } } + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -203,6 +288,11 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ authMechanismProperties: { AWS_SESSION_TOKEN: 'token' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -215,6 +305,11 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ authMechanismProperties: { SERVICE_NAME: 'alternate' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -227,6 +322,11 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ authMechanismProperties: { SERVICE_REALM: 'REALM.COM' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -237,7 +337,13 @@ describe('arg-mapper.mapCliToDriver', () => { const cliOptions: CliOptions = { sspiHostnameCanonicalization: 'none' }; it('is not mapped to authMechanismProperties', () => { - expect(mapCliToDriver(cliOptions)).to.deep.equal({}); + expect(mapCliToDriver(cliOptions)).to.deep.equal({ + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' + } + }); }); }); @@ -248,6 +354,11 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ authMechanismProperties: { gssapiCanonicalizeHostName: 'true' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -275,6 +386,11 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ autoEncryption: { keyVaultNamespace: 'db.datakeys' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -297,6 +413,11 @@ describe('arg-mapper.mapCliToDriver', () => { secretAccessKey: 'secretkey' } } + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); @@ -315,6 +436,11 @@ describe('arg-mapper.mapCliToDriver', () => { strict: true, deprecationErrors: true, version: '1' + }, + driverInfo: { + name: 'mongosh', + platform: 'darwin', + version: '0.0.0-dev.0' } }); }); From f0d0f331b96ae3f78f66015f912bb2105cdb435e Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 13 Jan 2022 12:56:57 +0100 Subject: [PATCH 3/5] refactor: address pr comments --- packages/cli-repl/src/arg-mapper.spec.ts | 102 ++++++++--------------- packages/cli-repl/src/arg-mapper.ts | 41 ++++----- packages/cli-repl/src/cli-repl.ts | 29 +++---- 3 files changed, 62 insertions(+), 110 deletions(-) diff --git a/packages/cli-repl/src/arg-mapper.spec.ts b/packages/cli-repl/src/arg-mapper.spec.ts index 7b4fc91eaf..3c21165770 100644 --- a/packages/cli-repl/src/arg-mapper.spec.ts +++ b/packages/cli-repl/src/arg-mapper.spec.ts @@ -3,9 +3,11 @@ import chai, { expect } from 'chai'; import path from 'path'; import sinonChai from 'sinon-chai'; import sinon from 'ts-sinon'; -import mapCliToDriver, { applyTlsCertificateSelector } from './arg-mapper'; +import mapCliToDriver, { getTlsCertificateSelector } from './arg-mapper'; chai.use(sinonChai); +const packageJSON = require('../package.json'); + describe('arg-mapper.mapCliToDriver', () => { context('when cli args have authenticationDatabase', () => { const cliOptions: CliOptions = { authenticationDatabase: 'authDb' }; @@ -15,8 +17,7 @@ describe('arg-mapper.mapCliToDriver', () => { authSource: 'authDb', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -30,8 +31,7 @@ describe('arg-mapper.mapCliToDriver', () => { authMechanism: 'SCRAM-SHA-1', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -45,8 +45,7 @@ describe('arg-mapper.mapCliToDriver', () => { loggerLevel: 'error', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -60,8 +59,7 @@ describe('arg-mapper.mapCliToDriver', () => { loggerLevel: 'debug', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -77,8 +75,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -94,8 +91,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -112,8 +108,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -127,8 +122,7 @@ describe('arg-mapper.mapCliToDriver', () => { retryWrites: true, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -142,8 +136,7 @@ describe('arg-mapper.mapCliToDriver', () => { tls: true, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -157,8 +150,7 @@ describe('arg-mapper.mapCliToDriver', () => { tlsAllowInvalidCertificates: true, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -172,8 +164,7 @@ describe('arg-mapper.mapCliToDriver', () => { tlsAllowInvalidHostnames: true, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -187,8 +178,7 @@ describe('arg-mapper.mapCliToDriver', () => { tlsCAFile: 'ca', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -202,8 +192,7 @@ describe('arg-mapper.mapCliToDriver', () => { sslCRL: 'key', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -217,8 +206,7 @@ describe('arg-mapper.mapCliToDriver', () => { tlsCertificateKeyFile: 'key', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -232,8 +220,7 @@ describe('arg-mapper.mapCliToDriver', () => { tlsCertificateKeyFilePassword: 'pw', driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -253,8 +240,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -274,8 +260,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -291,8 +276,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -308,8 +292,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -325,8 +308,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -340,8 +322,7 @@ describe('arg-mapper.mapCliToDriver', () => { expect(mapCliToDriver(cliOptions)).to.deep.equal({ driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -357,8 +338,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -389,8 +369,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -416,8 +395,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -439,8 +417,7 @@ describe('arg-mapper.mapCliToDriver', () => { }, driverInfo: { name: 'mongosh', - platform: 'darwin', - version: '0.0.0-dev.0' + version: packageJSON.version } }); }); @@ -462,16 +439,13 @@ describe('arg-mapper.applyTlsCertificateSelector', () => { }); it('leaves node options unchanged when no selector is given', () => { - const options = {}; - applyTlsCertificateSelector(undefined, options); - expect(options).to.deep.equal({}); + const applyTlsCertificateSelector = getTlsCertificateSelector(undefined); + expect(applyTlsCertificateSelector).to.not.exist; }); it('throws when the selector has an odd format', () => { - const options = {}; - expect(() => applyTlsCertificateSelector('foo=bar', options)) + expect(() => getTlsCertificateSelector('foo=bar')) .to.throw(/tlsCertificateSelector needs to include subject or thumbprint/); - expect(options).to.deep.equal({}); }); it('returns passphrase and pfx as given by the (fake) OS', () => { @@ -480,9 +454,8 @@ describe('arg-mapper.applyTlsCertificateSelector', () => { exportCertificateAndPrivateKey.returns({ passphrase, pfx }); - const options = {}; - applyTlsCertificateSelector('subject=Foo Bar', options); - expect(options).to.deep.equal({ + const applyTlsCertificateSelector = getTlsCertificateSelector('subject=Foo Bar'); + expect(applyTlsCertificateSelector).to.deep.equal({ passphrase, pfx }); }); @@ -493,8 +466,7 @@ describe('arg-mapper.applyTlsCertificateSelector', () => { if (process.platform === 'win32' || process.platform === 'darwin') { return this.skip(); } - const options = {}; - expect(() => applyTlsCertificateSelector('subject=Foo Bar', options)) + expect(() => getTlsCertificateSelector('subject=Foo Bar')) .to.throw(/tlsCertificateSelector is not supported on this platform/); }); @@ -502,8 +474,7 @@ describe('arg-mapper.applyTlsCertificateSelector', () => { if (process.platform !== 'win32') { return this.skip(); } - const options = {}; - expect(() => applyTlsCertificateSelector('subject=Foo Bar', options)) + expect(() => getTlsCertificateSelector('subject=Foo Bar')) .to.throw(/Could not resolve certificate specification/); }); @@ -511,8 +482,7 @@ describe('arg-mapper.applyTlsCertificateSelector', () => { if (process.platform !== 'darwin') { return this.skip(); } - const options = {}; - expect(() => applyTlsCertificateSelector('subject=Foo Bar', options)) + expect(() => getTlsCertificateSelector('subject=Foo Bar')) .to.throw(/Could not find a matching certificate/); }); }); diff --git a/packages/cli-repl/src/arg-mapper.ts b/packages/cli-repl/src/arg-mapper.ts index 1b404aabea..9913cc6748 100644 --- a/packages/cli-repl/src/arg-mapper.ts +++ b/packages/cli-repl/src/arg-mapper.ts @@ -69,16 +69,23 @@ function mapCliToDriver(options: CliOptions): MongoClientOptions { } } } - applyTlsCertificateSelector(options.tlsCertificateSelector, nodeOptions); - applyDriverInfo(nodeOptions); + + const tlsCertificateSelector = getTlsCertificateSelector(options.tlsCertificateSelector); + if (tlsCertificateSelector) { + nodeOptions.passphrase = tlsCertificateSelector.passphrase; + nodeOptions.pfx = tlsCertificateSelector.pfx; + } + + const { version } = require('../package.json'); + nodeOptions.driverInfo = { name: 'mongosh', version }; + return nodeOptions; } type TlsCertificateExporter = (search: { subject: string } | { thumbprint: Buffer }) => { passphrase: string, pfx: Buffer }; -export function applyTlsCertificateSelector( - selector: string | undefined, - nodeOptions: MongoClientOptions -): void { +export function getTlsCertificateSelector( + selector: string | undefined +): { passphrase: string, pfx: Buffer }|undefined { if (!selector) { return; } @@ -97,32 +104,12 @@ export function applyTlsCertificateSelector( try { const { passphrase, pfx } = exportCertificateAndPrivateKey(search); - nodeOptions.passphrase = passphrase; - nodeOptions.pfx = pfx; + return { passphrase, pfx }; } catch (err) { throw new MongoshInvalidInputError(`Could not resolve certificate specification '${selector}': ${err.message}`); } } -/** - * Applies driverInfo to track usage of mongosh, - * because appName can be overwritten by a user in their connection string. - * - * @param {MongoClientOptions} nodeOptions - The MongoClient options. - * - * @returns {DriverInfo} The driverInfo metadata. - */ -export function applyDriverInfo( - nodeOptions: MongoClientOptions -): void { - const { version } = require('../package.json'); - nodeOptions.driverInfo = { - name: 'mongosh', - version, - platform: process.platform - }; -} - function getCertificateExporter(): TlsCertificateExporter | undefined { if (process.env.TEST_OS_EXPORT_CERTIFICATE_AND_KEY_PATH) { return require(process.env.TEST_OS_EXPORT_CERTIFICATE_AND_KEY_PATH); diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index 8386c6e004..a123e8650c 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -155,19 +155,25 @@ class CliRepl { await this.verifyNodeVersion(); if (!this.cliOptions.nodb) { + let cs; + if (driverUri !== '') { + cs = new ConnectionString(driverUri); + + if (!cs.searchParams.get('appName')) { + cs.searchParams.set('appName', `mongosh ${version}`); + driverUri = cs.href; + } + } + if (this.isPasswordMissingOptions(driverOptions)) { (driverOptions.auth as any).password = await this.requirePassword(); - } else if (driverUri !== '') { - const cs = new ConnectionString(driverUri); + } else if (cs) { if (this.isPasswordMissingURI(cs)) { cs.password = await this.requirePassword(); driverUri = cs.href; } - if (this.isAppNameMissingURI(cs)) { - cs.searchParams.set('appName', `mongosh ${version}`); - driverUri = cs.href; - } } + this.ensurePasswordFieldIsPresentInAuth(driverOptions); } @@ -458,17 +464,6 @@ class CliRepl { ); } - /** - * Is the appName missing from the connection string? - * - * @param {ConnectionString} cs - The existing connection string. - * - * @returns {boolean} If the appName is missing. - */ - isAppNameMissingURI(cs: ConnectionString): boolean { - return !cs.searchParams.get('appName'); - } - /** * Sets the auth.password field to undefined in the driverOptions if the auth * object is present with a truthy username. This is required by the driver, e.g. From f23e3910734ec1ea9150275a4d5918882b6b92d3 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 13 Jan 2022 14:57:06 +0100 Subject: [PATCH 4/5] refactor: remove extra if for driverUri --- packages/cli-repl/src/arg-mapper.ts | 14 +++++--------- packages/cli-repl/src/cli-repl.ts | 21 +++++++-------------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/packages/cli-repl/src/arg-mapper.ts b/packages/cli-repl/src/arg-mapper.ts index 9913cc6748..9ee24d85bc 100644 --- a/packages/cli-repl/src/arg-mapper.ts +++ b/packages/cli-repl/src/arg-mapper.ts @@ -70,16 +70,12 @@ function mapCliToDriver(options: CliOptions): MongoClientOptions { } } - const tlsCertificateSelector = getTlsCertificateSelector(options.tlsCertificateSelector); - if (tlsCertificateSelector) { - nodeOptions.passphrase = tlsCertificateSelector.passphrase; - nodeOptions.pfx = tlsCertificateSelector.pfx; - } - const { version } = require('../package.json'); - nodeOptions.driverInfo = { name: 'mongosh', version }; - - return nodeOptions; + return { + ...nodeOptions, + ...getTlsCertificateSelector(options.tlsCertificateSelector), + driverInfo: { name: 'mongosh', version } + }; } type TlsCertificateExporter = (search: { subject: string } | { thumbprint: Buffer }) => { passphrase: string, pfx: Buffer }; diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index a123e8650c..5397f1d7d3 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -155,26 +155,19 @@ class CliRepl { await this.verifyNodeVersion(); if (!this.cliOptions.nodb) { - let cs; - if (driverUri !== '') { - cs = new ConnectionString(driverUri); - - if (!cs.searchParams.get('appName')) { - cs.searchParams.set('appName', `mongosh ${version}`); - driverUri = cs.href; - } + const cs = new ConnectionString(driverUri); + if (!cs.searchParams.get('appName')) { + cs.searchParams.set('appName', `mongosh ${version}`); } if (this.isPasswordMissingOptions(driverOptions)) { (driverOptions.auth as any).password = await this.requirePassword(); - } else if (cs) { - if (this.isPasswordMissingURI(cs)) { - cs.password = await this.requirePassword(); - driverUri = cs.href; - } + } else if (this.isPasswordMissingURI(cs)) { + cs.password = await this.requirePassword(); } - this.ensurePasswordFieldIsPresentInAuth(driverOptions); + + driverUri = cs.href; } try { From 58499db3eabdc0ecfe41451e51a43d779a0d7c9f Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 13 Jan 2022 16:10:42 +0100 Subject: [PATCH 5/5] test: skip set appname tests for api strict --- packages/cli-repl/test/e2e.spec.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cli-repl/test/e2e.spec.ts b/packages/cli-repl/test/e2e.spec.ts index f3b61cf0b4..05111e4385 100644 --- a/packages/cli-repl/test/e2e.spec.ts +++ b/packages/cli-repl/test/e2e.spec.ts @@ -211,7 +211,10 @@ describe('e2e', function() { await shell.waitForPrompt(); shell.assertNoErrors(); }); - it('appName set correctly', async() => { + it('appName set correctly', async function() { + if (process.env.MONGOSH_TEST_FORCE_API_STRICT) { + return this.skip(); // $currentOp is unversioned + } const currentOp = await shell.executeLine('db.currentOp()'); expect(currentOp).to.include("appName: 'mongosh 0.0.0-dev.0'"); expect(currentOp).to.include("name: 'nodejs|mongosh'"); @@ -226,7 +229,10 @@ describe('e2e', function() { await shell.waitForPrompt(); shell.assertNoErrors(); }); - it('appName set correctly', async() => { + it('appName set correctly', async function() { + if (process.env.MONGOSH_TEST_FORCE_API_STRICT) { + return this.skip(); // $currentOp is unversioned + } const currentOp = await shell.executeLine('db.currentOp()'); expect(currentOp).to.include("appName: 'Felicia'"); expect(currentOp).to.include("name: 'nodejs|mongosh'");