From 12a547bd825fee49942c45a5ae061427fc1b6a92 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 25 May 2023 17:09:47 +0200 Subject: [PATCH] feat(data-service): share OIDC state with embedded mongosh COMPASS-6802 (#4419) This commit is sufficient to make OIDC connectivity work when using the connection string to set OIDC as the auth mechanism. Co-authored-by: Rhys --- configs/webpack-config-compass/.depcheckrc | 2 + configs/webpack-config-compass/src/index.ts | 5 + package-lock.json | 108 +++++++++-- packages/compass-e2e-tests/fixtures/curl.js | 13 ++ packages/compass-e2e-tests/package.json | 5 + packages/compass-e2e-tests/tests/oidc.test.ts | 176 ++++++++++++++++++ .../compass-e2e-tests/tests/shell.test.ts | 1 + packages/data-service/package.json | 2 +- .../src/connect-mongo-client.spec.ts | 34 ++-- .../data-service/src/connect-mongo-client.ts | 87 +++++++-- packages/data-service/src/data-service.ts | 26 ++- 11 files changed, 410 insertions(+), 49 deletions(-) create mode 100755 packages/compass-e2e-tests/fixtures/curl.js create mode 100644 packages/compass-e2e-tests/tests/oidc.test.ts diff --git a/configs/webpack-config-compass/.depcheckrc b/configs/webpack-config-compass/.depcheckrc index 24f58e91ad5..3944bbe8d5a 100644 --- a/configs/webpack-config-compass/.depcheckrc +++ b/configs/webpack-config-compass/.depcheckrc @@ -16,3 +16,5 @@ ignores: - 'babel-plugin-istanbul' # recursive - 'mongodb-compass' + # used as a resolve alias + - 'jose' diff --git a/configs/webpack-config-compass/src/index.ts b/configs/webpack-config-compass/src/index.ts index 649a28d8225..d4e4bf1cf70 100644 --- a/configs/webpack-config-compass/src/index.ts +++ b/configs/webpack-config-compass/src/index.ts @@ -67,6 +67,11 @@ const sharedResolveOptions = ( // Additionally `ampersand-sync` brings into the bundle a number of other dependencies // that are outdated and having known vulnerabilities. 'ampersand-sync': false, + // `jose` provides a browser export that uses webcrypto APIs and returns + // webcrypto objects to represent keys, but openid-client requires + // KeyObject instances from the Node.js crypto API (https://tinyurl.com/2rrtu2hy). + // Manually resolve `jose` to use the Node.js export here. + jose: require.resolve('jose'), }, }; }; diff --git a/package-lock.json b/package-lock.json index 20a891de020..a51c6e2bf85 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9604,10 +9604,16 @@ "resolved": "packages/redux-common", "link": true }, + "node_modules/@mongodb-js/oidc-mock-provider": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.3.0.tgz", + "integrity": "sha512-tW5JSMzJkx0qmzkrXaKOXF62RnsWLj4kozYtGeC7W29aNdMeaAdhA28KPHH3gfN95qdFqQ04pb4NE8uqa62+rQ==", + "dev": true + }, "node_modules/@mongodb-js/oidc-plugin": { - "version": "0.1.4", - "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-plugin/-/oidc-plugin-0.1.4.tgz", - "integrity": "sha512-FLJVhP66dXgfwKYKNCDk/Rc40+YGxsQjJyo4abotilP0/mlsOq3CKXglRWGM3ivqBsOl7hMFE/TlMlzM55nOTQ==", + "version": "0.1.5", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-plugin/-/oidc-plugin-0.1.5.tgz", + "integrity": "sha512-N05x/JKjw5tbdUfym64c2Bw704OVzuNsoxJC2kcDVynWGCMrx/z0r4rEplEpUHj/0poisx4dLK/+aUc6keSt8Q==", "dependencies": { "abort-controller": "^3.0.0", "express": "^4.18.2", @@ -14731,6 +14737,25 @@ "integrity": "sha512-ipixuVrh2OdNmauvtT51o3d8z12p6LtFW9in7U79der/kwejjdNchQC5UMn5u/KxNoM7VHHOs/l8KS8uHxhODQ==", "dev": true }, + "node_modules/@types/tar": { + "version": "6.1.5", + "resolved": "https://registry.npmjs.org/@types/tar/-/tar-6.1.5.tgz", + "integrity": "sha512-qm2I/RlZij5RofuY7vohTpYNaYcrSQlN2MyjucQc7ZweDwaEWkdN/EeNh6e9zjK6uEm6PwjdMXkcj05BxZdX1Q==", + "dev": true, + "dependencies": { + "@types/node": "*", + "minipass": "^4.0.0" + } + }, + "node_modules/@types/tar/node_modules/minipass": { + "version": "4.2.8", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-4.2.8.tgz", + "integrity": "sha512-fNzuVyifolSLFL4NzpF+wEF4qrgqaaKX0haXPQEdQ7NKAN+WecoKMHV09YcuL/DHxrUsYQOK3MiuDf7Ip2OXfQ==", + "dev": true, + "engines": { + "node": ">=8" + } + }, "node_modules/@types/temp": { "version": "0.9.1", "resolved": "https://registry.npmjs.org/@types/temp/-/temp-0.9.1.tgz", @@ -50538,19 +50563,19 @@ } }, "node_modules/tar": { - "version": "6.1.11", - "resolved": "https://registry.npmjs.org/tar/-/tar-6.1.11.tgz", - "integrity": "sha512-an/KZQzQUkZCkuoAA64hM92X0Urb6VpRhAFllDzz44U2mcD5scmT3zBc4VgVpkugF580+DQn8eAFSyoQt0tznA==", + "version": "6.1.15", + "resolved": "https://registry.npmjs.org/tar/-/tar-6.1.15.tgz", + "integrity": "sha512-/zKt9UyngnxIT/EAGYuxaMYgOIJiP81ab9ZfkILq4oNLPFX50qyYmu7jRj9qeXoxmJHjGlbH0+cm2uy1WCs10A==", "dependencies": { "chownr": "^2.0.0", "fs-minipass": "^2.0.0", - "minipass": "^3.0.0", + "minipass": "^5.0.0", "minizlib": "^2.1.1", "mkdirp": "^1.0.3", "yallist": "^4.0.0" }, "engines": { - "node": ">= 10" + "node": ">=10" } }, "node_modules/tar-fs": { @@ -50722,6 +50747,14 @@ "safe-buffer": "~5.1.0" } }, + "node_modules/tar/node_modules/minipass": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-5.0.0.tgz", + "integrity": "sha512-3FnjYuehv9k6ovOEbyOswadCDPX1piCfhV8ncmYtHOjuPwylVWsghTLo7rabjC3Rx5xD4HDx8Wm1xnMF7S5qFQ==", + "engines": { + "node": ">=8" + } + }, "node_modules/temp": { "version": "0.8.4", "resolved": "https://registry.npmjs.org/temp/-/temp-0.8.4.tgz", @@ -56936,11 +56969,13 @@ "devDependencies": { "@electron/rebuild": "^3.2.13", "@mongodb-js/eslint-config-compass": "^1.0.5", + "@mongodb-js/oidc-mock-provider": "^0.3.0", "@mongodb-js/prettier-config-compass": "^1.0.0", "@mongodb-js/tsconfig-compass": "^1.0.2", "@types/chai-as-promised": "^7.1.4", "@types/cross-spawn": "^6.0.2", "@types/puppeteer": "^5.4.4", + "@types/tar": "^6.1.5", "@wdio/types": "^7.16.13", "bson": "^5.2.0", "chai": "^4.3.4", @@ -56960,13 +56995,16 @@ "mocha": "^10.2.0", "mongodb": "^5.5.0", "mongodb-connection-string-url": "^2.6.0", + "mongodb-download-url": "^1.3.0", "mongodb-log-writer": "^1.2.0", + "node-fetch": "^2.6.1", "nyc": "^15.1.0", "prettier": "^2.7.1", "ps-list": "^8.1.0", "puppeteer": "^15.4.0", "resolve-mongodb-srv": "^1.1.2", "semver": "^7.5.0", + "tar": "^6.1.15", "ts-node": "^10.9.1", "webdriverio": "^7.16.13", "xvfb-maybe": "^0.2.1" @@ -61334,7 +61372,7 @@ "@mongodb-js/compass-logging": "^1.1.5", "@mongodb-js/compass-utils": "^0.3.0", "@mongodb-js/devtools-connect": "^2.1.0", - "@mongodb-js/oidc-plugin": "^0.1.4", + "@mongodb-js/oidc-plugin": "^0.1.5", "@mongodb-js/ssh-tunnel": "^2.0.5", "lodash": "^4.17.21", "mongodb-build-info": "^1.5.0", @@ -77983,10 +78021,16 @@ } } }, + "@mongodb-js/oidc-mock-provider": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.3.0.tgz", + "integrity": "sha512-tW5JSMzJkx0qmzkrXaKOXF62RnsWLj4kozYtGeC7W29aNdMeaAdhA28KPHH3gfN95qdFqQ04pb4NE8uqa62+rQ==", + "dev": true + }, "@mongodb-js/oidc-plugin": { - "version": "0.1.4", - "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-plugin/-/oidc-plugin-0.1.4.tgz", - "integrity": "sha512-FLJVhP66dXgfwKYKNCDk/Rc40+YGxsQjJyo4abotilP0/mlsOq3CKXglRWGM3ivqBsOl7hMFE/TlMlzM55nOTQ==", + "version": "0.1.5", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-plugin/-/oidc-plugin-0.1.5.tgz", + "integrity": "sha512-N05x/JKjw5tbdUfym64c2Bw704OVzuNsoxJC2kcDVynWGCMrx/z0r4rEplEpUHj/0poisx4dLK/+aUc6keSt8Q==", "requires": { "abort-controller": "^3.0.0", "express": "^4.18.2", @@ -82913,6 +82957,24 @@ "integrity": "sha512-ipixuVrh2OdNmauvtT51o3d8z12p6LtFW9in7U79der/kwejjdNchQC5UMn5u/KxNoM7VHHOs/l8KS8uHxhODQ==", "dev": true }, + "@types/tar": { + "version": "6.1.5", + "resolved": "https://registry.npmjs.org/@types/tar/-/tar-6.1.5.tgz", + "integrity": "sha512-qm2I/RlZij5RofuY7vohTpYNaYcrSQlN2MyjucQc7ZweDwaEWkdN/EeNh6e9zjK6uEm6PwjdMXkcj05BxZdX1Q==", + "dev": true, + "requires": { + "@types/node": "*", + "minipass": "^4.0.0" + }, + "dependencies": { + "minipass": { + "version": "4.2.8", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-4.2.8.tgz", + "integrity": "sha512-fNzuVyifolSLFL4NzpF+wEF4qrgqaaKX0haXPQEdQ7NKAN+WecoKMHV09YcuL/DHxrUsYQOK3MiuDf7Ip2OXfQ==", + "dev": true + } + } + }, "@types/temp": { "version": "0.9.1", "resolved": "https://registry.npmjs.org/@types/temp/-/temp-0.9.1.tgz", @@ -88837,11 +88899,13 @@ "requires": { "@electron/rebuild": "^3.2.13", "@mongodb-js/eslint-config-compass": "^1.0.5", + "@mongodb-js/oidc-mock-provider": "^0.3.0", "@mongodb-js/prettier-config-compass": "^1.0.0", "@mongodb-js/tsconfig-compass": "^1.0.2", "@types/chai-as-promised": "^7.1.4", "@types/cross-spawn": "^6.0.2", "@types/puppeteer": "^5.4.4", + "@types/tar": "^6.1.5", "@wdio/types": "^7.16.13", "bson": "^5.2.0", "chai": "^4.3.4", @@ -88861,13 +88925,16 @@ "mocha": "^10.2.0", "mongodb": "^5.5.0", "mongodb-connection-string-url": "^2.6.0", + "mongodb-download-url": "^1.3.0", "mongodb-log-writer": "^1.2.0", + "node-fetch": "^2.6.1", "nyc": "^15.1.0", "prettier": "^2.7.1", "ps-list": "^8.1.0", "puppeteer": "^15.4.0", "resolve-mongodb-srv": "^1.1.2", "semver": "^7.5.0", + "tar": "^6.1.15", "ts-node": "^10.9.1", "webdriverio": "^7.16.13", "xvfb-maybe": "^0.2.1" @@ -107238,7 +107305,7 @@ "@mongodb-js/devtools-docker-test-envs": "^1.2.4", "@mongodb-js/eslint-config-compass": "^1.0.5", "@mongodb-js/mocha-config-compass": "^1.1.1", - "@mongodb-js/oidc-plugin": "^0.1.4", + "@mongodb-js/oidc-plugin": "^0.1.5", "@mongodb-js/prettier-config-compass": "^1.0.0", "@mongodb-js/ssh-tunnel": "^2.0.5", "@mongodb-js/tsconfig-compass": "^1.0.2", @@ -115370,16 +115437,23 @@ "dev": true }, "tar": { - "version": "6.1.11", - "resolved": "https://registry.npmjs.org/tar/-/tar-6.1.11.tgz", - "integrity": "sha512-an/KZQzQUkZCkuoAA64hM92X0Urb6VpRhAFllDzz44U2mcD5scmT3zBc4VgVpkugF580+DQn8eAFSyoQt0tznA==", + "version": "6.1.15", + "resolved": "https://registry.npmjs.org/tar/-/tar-6.1.15.tgz", + "integrity": "sha512-/zKt9UyngnxIT/EAGYuxaMYgOIJiP81ab9ZfkILq4oNLPFX50qyYmu7jRj9qeXoxmJHjGlbH0+cm2uy1WCs10A==", "requires": { "chownr": "^2.0.0", "fs-minipass": "^2.0.0", - "minipass": "^3.0.0", + "minipass": "^5.0.0", "minizlib": "^2.1.1", "mkdirp": "^1.0.3", "yallist": "^4.0.0" + }, + "dependencies": { + "minipass": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-5.0.0.tgz", + "integrity": "sha512-3FnjYuehv9k6ovOEbyOswadCDPX1piCfhV8ncmYtHOjuPwylVWsghTLo7rabjC3Rx5xD4HDx8Wm1xnMF7S5qFQ==" + } } }, "tar-fs": { diff --git a/packages/compass-e2e-tests/fixtures/curl.js b/packages/compass-e2e-tests/fixtures/curl.js new file mode 100755 index 00000000000..d7108c327d9 --- /dev/null +++ b/packages/compass-e2e-tests/fixtures/curl.js @@ -0,0 +1,13 @@ +#!/usr/bin/env node +/* eslint-disable */ +'use strict'; +const fetch = require('node-fetch'); + +// fetch() an URL and ignore the response body +(async function () { + (await fetch(process.argv[2])).body?.resume(); +})().catch((err) => { + process.nextTick(() => { + throw err; + }); +}); diff --git a/packages/compass-e2e-tests/package.json b/packages/compass-e2e-tests/package.json index 3086a4c8ee5..ec9d1dddf2b 100644 --- a/packages/compass-e2e-tests/package.json +++ b/packages/compass-e2e-tests/package.json @@ -31,9 +31,11 @@ "@mongodb-js/eslint-config-compass": "^1.0.5", "@mongodb-js/prettier-config-compass": "^1.0.0", "@mongodb-js/tsconfig-compass": "^1.0.2", + "@mongodb-js/oidc-mock-provider": "^0.3.0", "@types/chai-as-promised": "^7.1.4", "@types/cross-spawn": "^6.0.2", "@types/puppeteer": "^5.4.4", + "@types/tar": "^6.1.5", "@wdio/types": "^7.16.13", "bson": "^5.2.0", "chai": "^4.3.4", @@ -53,13 +55,16 @@ "mocha": "^10.2.0", "mongodb": "^5.5.0", "mongodb-connection-string-url": "^2.6.0", + "mongodb-download-url": "^1.3.0", "mongodb-log-writer": "^1.2.0", + "node-fetch": "^2.6.1", "nyc": "^15.1.0", "prettier": "^2.7.1", "ps-list": "^8.1.0", "puppeteer": "^15.4.0", "resolve-mongodb-srv": "^1.1.2", "semver": "^7.5.0", + "tar": "^6.1.15", "ts-node": "^10.9.1", "webdriverio": "^7.16.13", "xvfb-maybe": "^0.2.1" diff --git a/packages/compass-e2e-tests/tests/oidc.test.ts b/packages/compass-e2e-tests/tests/oidc.test.ts new file mode 100644 index 00000000000..6a686829dc9 --- /dev/null +++ b/packages/compass-e2e-tests/tests/oidc.test.ts @@ -0,0 +1,176 @@ +import type { CompassBrowser } from '../helpers/compass-browser'; +import { beforeTests, afterTests, afterTest } from '../helpers/compass'; +import type { Compass } from '../helpers/compass'; +import type { OIDCMockProviderConfig } from '@mongodb-js/oidc-mock-provider'; +import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider'; +import type { ChildProcess } from 'child_process'; +import { spawn } from 'child_process'; +import path from 'path'; +import os from 'os'; +import { promises as fs } from 'fs'; +import { getDownloadURL } from 'mongodb-download-url'; +import tar from 'tar'; +import { promisify } from 'util'; +import type { Readable } from 'stream'; +import { pipeline, PassThrough } from 'stream'; +import { createInterface as readline } from 'readline'; +import https from 'https'; +import { once } from 'events'; +import { expect } from 'chai'; + +describe('OIDC integration', function () { + let compass: Compass; + let browser: CompassBrowser; + + let getTokenPayload: typeof oidcMockProviderConfig.getTokenPayload; + let oidcMockProviderConfig: OIDCMockProviderConfig; + let oidcMockProvider: OIDCMockProvider; + + let i = 0; + let tmpdir: string; + let server: ChildProcess; + let serverExit: Promise; + let connectionString: string; + + before(async function () { + // TODO(MONGOSH-1306): Get rid of all the setup code to download mongod here... :( + if (process.platform !== 'linux') { + // OIDC is only supported on Linux in the 7.0+ enterprise server. + return this.skip(); + } + + { + oidcMockProviderConfig = { + getTokenPayload(metadata: Parameters[0]) { + return getTokenPayload(metadata); + }, + }; + oidcMockProvider = await OIDCMockProvider.create(oidcMockProviderConfig); + } + + { + tmpdir = path.join( + os.tmpdir(), + `compass-oidc-${Date.now().toString(32)}-${++i}` + ); + await fs.mkdir(path.join(tmpdir, 'db'), { recursive: true }); + } + + { + const { url } = await getDownloadURL({ + version: '>= 7.0.0-rc0', + enterprise: true, + }); + + await promisify(pipeline)( + await new Promise((resolve) => https.get(url, resolve).end()), + tar.x({ cwd: tmpdir, strip: 1 }) + ); + } + + { + const serverOidcConfig = { + issuer: oidcMockProvider.issuer, + clientId: 'testServer', + requestScopes: ['mongodbGroups'], + authorizationClaim: 'groups', + audience: 'resource-server-audience-value', + authNamePrefix: 'dev', + }; + + server = spawn( + path.join(tmpdir, 'bin', 'mongod'), + [ + '--setParameter', + 'authenticationMechanisms=SCRAM-SHA-256,MONGODB-OIDC', + // enableTestCommands allows using http:// issuers such as http://localhost + '--setParameter', + 'enableTestCommands=true', + '--setParameter', + `oidcIdentityProviders=${JSON.stringify([serverOidcConfig])}`, + '--dbpath', + path.join(tmpdir, 'db'), + '--port', + '0', + ], + { + cwd: tmpdir, + stdio: ['inherit', 'pipe', 'inherit'], + } + ); + + serverExit = once(server, 'exit'); + + const port = await Promise.race([ + serverExit.then((code) => { + throw new Error(`mongod exited with code ${code}`); + }), + (async () => { + // Parse the log output written by mongod to stdout until we know + // which port it chose. + const pt = new PassThrough(); + server.stdout?.pipe(pt); + if (process.env.CI) server.stdout?.pipe(process.stderr); + for await (const l of readline({ input: pt })) { + const line = JSON.parse(l); + if (line.id === 23016 /* Waiting for connections */) { + server.stdout?.unpipe(pt); // Ignore all further output + return line.attr.port; + } + } + })(), + ]); + + connectionString = `mongodb://127.0.0.1:${port}/?authMechanism=MONGODB-OIDC`; + } + + process.env.COMPASS_TEST_OIDC_BROWSER_DUMMY = `${ + process.execPath + } ${path.resolve(__dirname, '..', 'fixtures', 'curl.js')}`; + }); + + beforeEach(async function () { + compass = await beforeTests(); + browser = compass.browser; + }); + + afterEach(async function () { + await afterTest(compass, this.currentTest); + }); + + after(async function () { + delete process.env.COMPASS_TEST_OIDC_BROWSER_DUMMY; + await afterTests(compass, this.currentTest); + server?.kill(); + await serverExit; + await oidcMockProvider?.close(); + if (tmpdir) await fs.rmdir(tmpdir, { recursive: true }); + }); + + it('can successfully connect', async function () { + let tokenFetchCalls = 0; + getTokenPayload = () => { + tokenFetchCalls++; + return { + expires_in: 3600, + payload: { + // Define the user information stored inside the access tokens + groups: ['testgroup'], + sub: 'testuser', + aud: 'resource-server-audience-value', + }, + }; + }; + await browser.connectWithConnectionString(connectionString); + const result: any = await browser.shellEval( + 'db.runCommand({ connectionStatus: 1 })', + true + ); + + expect(tokenFetchCalls).to.equal(1); // No separate request from the shell + expect(result.authInfo).to.deep.equal({ + authenticatedUsers: [{ user: 'dev/testuser', db: '$external' }], + authenticatedUserRoles: [{ role: 'dev/testgroup', db: 'admin' }], + }); + }); +}); diff --git a/packages/compass-e2e-tests/tests/shell.test.ts b/packages/compass-e2e-tests/tests/shell.test.ts index f234001ab42..1b3e7538fbf 100644 --- a/packages/compass-e2e-tests/tests/shell.test.ts +++ b/packages/compass-e2e-tests/tests/shell.test.ts @@ -23,6 +23,7 @@ describe('Shell', function () { }); afterEach(async function () { + await browser.setFeature('enableShell', true); await afterTest(compass, this.currentTest); }); diff --git a/packages/data-service/package.json b/packages/data-service/package.json index fe418efa19d..3c4a864f44f 100644 --- a/packages/data-service/package.json +++ b/packages/data-service/package.json @@ -58,7 +58,7 @@ "@mongodb-js/compass-logging": "^1.1.5", "@mongodb-js/compass-utils": "^0.3.0", "@mongodb-js/devtools-connect": "^2.1.0", - "@mongodb-js/oidc-plugin": "^0.1.4", + "@mongodb-js/oidc-plugin": "^0.1.5", "@mongodb-js/ssh-tunnel": "^2.0.5", "lodash": "^4.17.21", "mongodb-build-info": "^1.5.0", diff --git a/packages/data-service/src/connect-mongo-client.spec.ts b/packages/data-service/src/connect-mongo-client.spec.ts index c3e63b00bc1..0e4f521fb21 100644 --- a/packages/data-service/src/connect-mongo-client.spec.ts +++ b/packages/data-service/src/connect-mongo-client.spec.ts @@ -3,7 +3,7 @@ import asyncHooks from 'async_hooks'; import { expect } from 'chai'; import type { CommandStartedEvent } from 'mongodb'; -import connectMongoClient from './connect-mongo-client'; +import { connectMongoClientCompass as connectMongoClient } from './connect-mongo-client'; import type { ConnectionOptions } from './connection-options'; const defaultOptions = { @@ -16,7 +16,11 @@ const setupListeners = () => { }; describe('connectMongoClient', function () { - const toBeClosed = new Set Promise }>(); + const toBeClosed = new Set< + | undefined + | { close: () => Promise } + | { destroy: () => Promise } + >(); beforeEach(function () { toBeClosed.clear(); @@ -24,7 +28,9 @@ describe('connectMongoClient', function () { afterEach(async function () { for (const mongoClientOrTunnel of toBeClosed) { - await mongoClientOrTunnel?.close(); + if (mongoClientOrTunnel && 'close' in mongoClientOrTunnel) + await mongoClientOrTunnel.close(); + else await mongoClientOrTunnel?.destroy(); } }); @@ -40,7 +46,7 @@ describe('connectMongoClient', function () { }); it('should return connection config when connected successfully', async function () { - const [metadataClient, crudClient, tunnel, { url, options }] = + const [metadataClient, crudClient, tunnel, state, { url, options }] = await connectMongoClient( { connectionString: 'mongodb://localhost:27021', @@ -48,17 +54,19 @@ describe('connectMongoClient', function () { setupListeners ); - for (const closeLater of [metadataClient, crudClient, tunnel]) { + for (const closeLater of [metadataClient, crudClient, tunnel, state]) { toBeClosed.add(closeLater); } expect(metadataClient).to.equal(crudClient); expect(url).to.equal('mongodb://localhost:27021'); + expect(options.parentHandle).to.be.a('string'); expect(options).to.deep.equal({ monitorCommands: true, useSystemCA: undefined, autoEncryption: undefined, + parentHandle: options.parentHandle, ...defaultOptions, }); }); @@ -71,7 +79,7 @@ describe('connectMongoClient', function () { }, bypassAutoEncryption: true, }; - const [metadataClient, crudClient, tunnel, { url, options }] = + const [metadataClient, crudClient, tunnel, state, { url, options }] = await connectMongoClient( { connectionString: 'mongodb://localhost:27021', @@ -83,7 +91,7 @@ describe('connectMongoClient', function () { setupListeners ); - for (const closeLater of [metadataClient, crudClient, tunnel]) { + for (const closeLater of [metadataClient, crudClient, tunnel, state]) { toBeClosed.add(closeLater); } @@ -92,16 +100,18 @@ describe('connectMongoClient', function () { expect(crudClient.options.autoEncryption).to.be.an('object'); expect(url).to.equal('mongodb://localhost:27021'); + expect(options.parentHandle).to.be.a('string'); expect(options).to.deep.equal({ monitorCommands: true, useSystemCA: undefined, autoEncryption, + parentHandle: options.parentHandle, ...defaultOptions, }); }); it('should not override a user-specified directConnection option', async function () { - const [metadataClient, crudClient, tunnel, { url, options }] = + const [metadataClient, crudClient, tunnel, state, { url, options }] = await connectMongoClient( { connectionString: @@ -110,7 +120,7 @@ describe('connectMongoClient', function () { setupListeners ); - for (const closeLater of [metadataClient, crudClient, tunnel]) { + for (const closeLater of [metadataClient, crudClient, tunnel, state]) { toBeClosed.add(closeLater); } @@ -119,10 +129,12 @@ describe('connectMongoClient', function () { 'mongodb://localhost:27021/?directConnection=false' ); + expect(options.parentHandle).to.be.a('string'); assert.deepStrictEqual(options, { monitorCommands: true, useSystemCA: undefined, autoEncryption: undefined, + parentHandle: options.parentHandle, ...defaultOptions, }); }); @@ -143,7 +155,7 @@ describe('connectMongoClient', function () { it('should run the ping command with the specified ReadPreference', async function () { const commands: CommandStartedEvent[] = []; - const [metadataClient, crudClient] = await connectMongoClient( + const [metadataClient, crudClient, , state] = await connectMongoClient( { connectionString: 'mongodb://localhost:27021/?readPreference=secondaryPreferred', @@ -156,7 +168,7 @@ describe('connectMongoClient', function () { mode: 'secondaryPreferred', }); - for (const closeLater of [metadataClient, crudClient]) { + for (const closeLater of [metadataClient, crudClient, state]) { toBeClosed.add(closeLater); } }); diff --git a/packages/data-service/src/connect-mongo-client.ts b/packages/data-service/src/connect-mongo-client.ts index be2ee8a4ae2..87e6b05a721 100644 --- a/packages/data-service/src/connect-mongo-client.ts +++ b/packages/data-service/src/connect-mongo-client.ts @@ -1,7 +1,10 @@ import type { MongoClientOptions } from 'mongodb'; import { MongoClient } from 'mongodb'; import { connectMongoClient, hookLogger } from '@mongodb-js/devtools-connect'; -import type { DevtoolsConnectOptions } from '@mongodb-js/devtools-connect'; +import type { + DevtoolsConnectOptions, + DevtoolsConnectionState, +} from '@mongodb-js/devtools-connect'; import type SSHTunnel from '@mongodb-js/ssh-tunnel'; import EventEmitter from 'events'; import { redactConnectionOptions, redactConnectionString } from './redact'; @@ -23,7 +26,7 @@ export type CloneableMongoClient = MongoClient & { [createClonedClient](): Promise; }; -export default async function connectMongoClientCompass( +export async function connectMongoClientCompass( connectionOptions: Readonly, setupListeners: (client: MongoClient) => void, logger?: UnboundDataServiceImplLogger @@ -32,6 +35,7 @@ export default async function connectMongoClientCompass( metadataClient: CloneableMongoClient, crudClient: CloneableMongoClient, sshTunnel: SSHTunnel | undefined, + connectionState: DevtoolsConnectionState, options: { url: string; options: DevtoolsConnectOptions } ] > { @@ -49,6 +53,14 @@ export default async function connectMongoClientCompass( autoEncryption: connectionOptions.fleOptions?.autoEncryption, }; + // TODO(COMPASS-6849): Add a way to properly override openBrowser + if (process.env.COMPASS_TEST_OIDC_BROWSER_DUMMY) { + options.oidc ??= {}; + options.oidc.openBrowser = { + command: process.env.COMPASS_TEST_OIDC_BROWSER_DUMMY, + }; + } + if (options.autoEncryption && process.env.COMPASS_CRYPT_LIBRARY_PATH) { options.autoEncryption = { ...options.autoEncryption, @@ -88,26 +100,34 @@ export default async function connectMongoClientCompass( async function connectSingleClient( overrideOptions: Partial - ): Promise { + ): Promise<{ client: CloneableMongoClient; state: DevtoolsConnectionState }> { // Deep clone because of https://jira.mongodb.org/browse/NODE-4124, // the options here are being mutated. const connectOptions = _.cloneDeep({ ...options, ...overrideOptions }); - const { client } = await connectMongoClient( + const { client, state } = await connectMongoClient( url, connectOptions, connectLogger, CompassMongoClient ); await runCommand(client.db('admin'), { ping: 1 }); - return Object.assign(client, { - async [createClonedClient]() { - return await connectSingleClient(connectOptions); - }, - }); + return { + client: Object.assign(client, { + async [createClonedClient]() { + const { client } = await connectSingleClient({ + ...connectOptions, + parentState: state, + }); + return client; + }, + }), + state, + }; } let metadataClient: CloneableMongoClient | undefined; let crudClient: CloneableMongoClient | undefined; + let state: DevtoolsConnectionState; try { debug('waiting for MongoClient to connect ...'); // Create one or two clients, depending on whether CSFLE @@ -115,18 +135,57 @@ export default async function connectMongoClientCompass( // server metadata (e.g. build info, instance data, etc.) // and one for interacting with the actual CRUD data. // If CSFLE is disabled, use a single client for both cases. - [metadataClient, crudClient] = await Promise.race([ - Promise.all([ - connectSingleClient({ autoEncryption: undefined }), - options.autoEncryption ? connectSingleClient({}) : undefined, - ]), + [metadataClient, crudClient, state] = await Promise.race([ + (async () => { + const { client: metadataClient, state } = await connectSingleClient({ + autoEncryption: undefined, + }); + const parentHandlePromise = state.getStateShareServer(); + parentHandlePromise.catch(() => { + /* handled below */ + }); + let crudClient; + + // This used to happen in parallel, but since the introduction of OIDC connection + // state needs to be shared and managed on the longest-lived client instance, + // so we need to use the DevtoolsConnectionState instance created for the metadata + // client here. + if (options.autoEncryption) { + try { + crudClient = ( + await connectSingleClient({ + parentState: state, + }) + ).client; + } catch (err) { + await metadataClient.close(); + throw err; + } + } + + try { + // Make sure that if this failed, we clean up properly. + await parentHandlePromise; + } catch (err) { + await metadataClient.close(); + await crudClient?.close(); + throw err; + } + + // Return the parentHandle here so that it's included in the options that + // are passed to compass-shell. + return [metadataClient, crudClient, state] as const; + })(), waitForTunnelError(tunnel), ]); // waitForTunnel always throws, never resolves + options.parentHandle = await state.getStateShareServer(); + return [ metadataClient, crudClient ?? metadataClient, tunnel, + state, { url, options }, ]; } catch (err: any) { diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index e42b94e9e56..53855f1cfa9 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -29,7 +29,6 @@ import type { InsertOneOptions, InsertOneResult, MongoClient, - MongoClientOptions, ServerDescriptionChangedEvent, ServerOpeningEvent, ServerClosedEvent, @@ -67,7 +66,10 @@ import { } from './instance-detail-helper'; import { redactConnectionString } from './redact'; import type { CloneableMongoClient } from './connect-mongo-client'; -import connectMongoClient, { createClonedClient } from './connect-mongo-client'; +import { + connectMongoClientCompass as connectMongoClient, + createClonedClient, +} from './connect-mongo-client'; import type { CollectionStats } from './types'; import type { ConnectionStatusWithPrivileges } from './run-command'; import { runCommand } from './run-command'; @@ -93,6 +95,10 @@ import type { UnboundDataServiceImplLogger, } from './logger'; import { WithLogContext, debug, mongoLogId } from './logger'; +import type { + DevtoolsConnectOptions, + DevtoolsConnectionState, +} from '@mongodb-js/devtools-connect'; // TODO: remove try/catch and refactor encryption related types // when the Node bundles native binding distributions @@ -172,7 +178,7 @@ export interface DataService { * Returns connection options passed to the driver on connection */ getMongoClientConnectionOptions(): - | { url: string; options: MongoClientOptions } + | { url: string; options: DevtoolsConnectOptions } | undefined; /** @@ -783,7 +789,7 @@ class DataServiceImpl extends WithLogContext implements DataService { private _isConnecting = false; private _mongoClientConnectionOptions?: { url: string; - options: MongoClientOptions; + options: DevtoolsConnectOptions; }; // Use two separate clients in the CSFLE case, one with CSFLE @@ -797,6 +803,7 @@ class DataServiceImpl extends WithLogContext implements DataService { private _csfleCollectionTracker?: CSFLECollectionTracker; private _tunnel?: SshTunnel; + private _state?: DevtoolsConnectionState; /** * Stores the most recent topology description from the server's SDAM events: @@ -862,7 +869,7 @@ class DataServiceImpl extends WithLogContext implements DataService { } getMongoClientConnectionOptions(): - | { url: string; options: MongoClientOptions } + | { url: string; options: DevtoolsConnectOptions } | undefined { return this._mongoClientConnectionOptions; } @@ -1253,7 +1260,7 @@ class DataServiceImpl extends WithLogContext implements DataService { }); try { - const [metadataClient, crudClient, tunnel, connectionOptions] = + const [metadataClient, crudClient, tunnel, state, connectionOptions] = await connectMongoClient( this._connectionOptions, this._setupListeners.bind(this), @@ -1271,6 +1278,7 @@ class DataServiceImpl extends WithLogContext implements DataService { this._metadataClient = metadataClient; this._crudClient = crudClient; this._tunnel = tunnel; + this._state = state; this._mongoClientConnectionOptions = connectionOptions; this._csfleCollectionTracker = new CSFLECollectionTrackerImpl( this, @@ -1381,6 +1389,11 @@ class DataServiceImpl extends WithLogContext implements DataService { this._tunnel ?.close() .catch((err) => debug('failed to close tunnel', err)), + this._state + ?.destroy() + .catch((err) => + debug('failed to destroy DevtoolsConnectionState', err) + ), ]); } finally { this._cleanup(); @@ -2208,6 +2221,7 @@ class DataServiceImpl extends WithLogContext implements DataService { this._metadataClient = undefined; this._crudClient = undefined; this._tunnel = undefined; + this._state = undefined; this._mongoClientConnectionOptions = undefined; this._isWritable = false; this._isConnecting = false;