From b03ab7a9cf257cce4e41fe875a5ba215ede1ca13 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 9 Jan 2024 19:57:54 +0100 Subject: [PATCH 1/9] chore: added integration tests for oidc in vscode --- package-lock.json | 110 ++++++++- package.json | 7 + src/connectionController.ts | 30 ++- src/test/fixture/curl.js | 13 ++ src/test/suite/oidc.test.ts | 433 ++++++++++++++++++++++++++++++++++++ 5 files changed, 575 insertions(+), 18 deletions(-) create mode 100644 src/test/fixture/curl.js create mode 100644 src/test/suite/oidc.test.ts diff --git a/package-lock.json b/package-lock.json index 081f2119d..106c6a2a8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -54,6 +54,7 @@ }, "devDependencies": { "@babel/preset-typescript": "^7.22.5", + "@mongodb-js/oidc-mock-provider": "^0.6.9", "@mongodb-js/oidc-plugin": "^0.3.0", "@mongodb-js/prettier-config-compass": "^1.0.0", "@mongodb-js/sbom-tools": "^0.5.4", @@ -107,6 +108,7 @@ "mocha-multi": "^1.1.7", "mongodb-client-encryption": "^6.0.0", "mongodb-runner": "^5.4.5", + "node-fetch": "^2.7.0", "node-loader": "^0.6.0", "npm-run-all": "^4.1.5", "ora": "^5.4.1", @@ -4885,6 +4887,59 @@ "tar": "^6.1.15" } }, + "node_modules/@mongodb-js/oidc-mock-provider": { + "version": "0.6.9", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.6.9.tgz", + "integrity": "sha512-4D9y7w7k0f7z6OkFJ8Ux5UhMG7Tg287CC1KmpW43BMzMx5gPXhostYK+OtpZNBlOoB9yrlHLusLKtpqQywMaog==", + "dev": true, + "dependencies": { + "yargs": "17.7.2" + }, + "bin": { + "oidc-mock-provider": "bin/oidc-mock-provider.js" + } + }, + "node_modules/@mongodb-js/oidc-mock-provider/node_modules/cliui": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/cliui/-/cliui-8.0.1.tgz", + "integrity": "sha512-BSeNnyus75C4//NQ9gQt1/csTXyo/8Sb+afLAkzAptFuMsod9HFokGNudZpi/oQV73hnVK+sR+5PVRMd+Dr7YQ==", + "dev": true, + "dependencies": { + "string-width": "^4.2.0", + "strip-ansi": "^6.0.1", + "wrap-ansi": "^7.0.0" + }, + "engines": { + "node": ">=12" + } + }, + "node_modules/@mongodb-js/oidc-mock-provider/node_modules/yargs": { + "version": "17.7.2", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.7.2.tgz", + "integrity": "sha512-7dSzzRQ++CKnNI/krKnYRV7JKKPUXMEh61soaHKg9mrWEhzFWhFnxPxGl+69cD1Ou63C13NUPCnmIcrvqCuM6w==", + "dev": true, + "dependencies": { + "cliui": "^8.0.1", + "escalade": "^3.1.1", + "get-caller-file": "^2.0.5", + "require-directory": "^2.1.1", + "string-width": "^4.2.3", + "y18n": "^5.0.5", + "yargs-parser": "^21.1.1" + }, + "engines": { + "node": ">=12" + } + }, + "node_modules/@mongodb-js/oidc-mock-provider/node_modules/yargs-parser": { + "version": "21.1.1", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-21.1.1.tgz", + "integrity": "sha512-tVpsJW7DdjecAiFpbIB1e3qxIQsE6NoPc5/eTdrbbIC4h0LVsWhnoa3g+m2HclBIujHzsxZ4VJVA+GUuc2/LBw==", + "dev": true, + "engines": { + "node": ">=12" + } + }, "node_modules/@mongodb-js/oidc-plugin": { "version": "0.3.0", "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-plugin/-/oidc-plugin-0.3.0.tgz", @@ -18252,9 +18307,9 @@ "integrity": "sha512-73sE9+3UaLYYFmDsFZnqCInzPyh3MqIwZO9cw58yIqAZhONrrabrYyYe3TuIqtIiOuTXVhsGau8hcrhhwSsDIQ==" }, "node_modules/node-fetch": { - "version": "2.6.12", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.12.tgz", - "integrity": "sha512-C/fGU2E8ToujUivIO0H+tpQ6HWo4eEmchoPIoXtxCrVghxdKq+QOHqEZW7tuP3KlV3bC8FRMO5nMCC7Zm1VP6g==", + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", + "integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==", "dependencies": { "whatwg-url": "^5.0.0" }, @@ -28774,6 +28829,49 @@ "tar": "^6.1.15" } }, + "@mongodb-js/oidc-mock-provider": { + "version": "0.6.9", + "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-mock-provider/-/oidc-mock-provider-0.6.9.tgz", + "integrity": "sha512-4D9y7w7k0f7z6OkFJ8Ux5UhMG7Tg287CC1KmpW43BMzMx5gPXhostYK+OtpZNBlOoB9yrlHLusLKtpqQywMaog==", + "dev": true, + "requires": { + "yargs": "17.7.2" + }, + "dependencies": { + "cliui": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/cliui/-/cliui-8.0.1.tgz", + "integrity": "sha512-BSeNnyus75C4//NQ9gQt1/csTXyo/8Sb+afLAkzAptFuMsod9HFokGNudZpi/oQV73hnVK+sR+5PVRMd+Dr7YQ==", + "dev": true, + "requires": { + "string-width": "^4.2.0", + "strip-ansi": "^6.0.1", + "wrap-ansi": "^7.0.0" + } + }, + "yargs": { + "version": "17.7.2", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-17.7.2.tgz", + "integrity": "sha512-7dSzzRQ++CKnNI/krKnYRV7JKKPUXMEh61soaHKg9mrWEhzFWhFnxPxGl+69cD1Ou63C13NUPCnmIcrvqCuM6w==", + "dev": true, + "requires": { + "cliui": "^8.0.1", + "escalade": "^3.1.1", + "get-caller-file": "^2.0.5", + "require-directory": "^2.1.1", + "string-width": "^4.2.3", + "y18n": "^5.0.5", + "yargs-parser": "^21.1.1" + } + }, + "yargs-parser": { + "version": "21.1.1", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-21.1.1.tgz", + "integrity": "sha512-tVpsJW7DdjecAiFpbIB1e3qxIQsE6NoPc5/eTdrbbIC4h0LVsWhnoa3g+m2HclBIujHzsxZ4VJVA+GUuc2/LBw==", + "dev": true + } + } + }, "@mongodb-js/oidc-plugin": { "version": "0.3.0", "resolved": "https://registry.npmjs.org/@mongodb-js/oidc-plugin/-/oidc-plugin-0.3.0.tgz", @@ -39202,9 +39300,9 @@ "integrity": "sha512-73sE9+3UaLYYFmDsFZnqCInzPyh3MqIwZO9cw58yIqAZhONrrabrYyYe3TuIqtIiOuTXVhsGau8hcrhhwSsDIQ==" }, "node-fetch": { - "version": "2.6.12", - "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.12.tgz", - "integrity": "sha512-C/fGU2E8ToujUivIO0H+tpQ6HWo4eEmchoPIoXtxCrVghxdKq+QOHqEZW7tuP3KlV3bC8FRMO5nMCC7Zm1VP6g==", + "version": "2.7.0", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", + "integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==", "requires": { "whatwg-url": "^5.0.0" }, diff --git a/package.json b/package.json index 5b616a37f..bd95a641c 100644 --- a/package.json +++ b/package.json @@ -971,6 +971,11 @@ "type": "boolean", "default": false, "description": "The default behavior is to generate a single ObjectId and insert it on all cursors. Set to true to generate a unique ObjectId per cursor instead." + }, + "mdb.browserCommandForOIDCAuth": { + "type": "string", + "default": "", + "description": "Specify a shell command that is run to start the browser for authenticating with the OIDC identity provider for the server connection. Leave this empty for default browser." } } } @@ -1021,6 +1026,7 @@ }, "devDependencies": { "@babel/preset-typescript": "^7.22.5", + "@mongodb-js/oidc-mock-provider": "^0.6.9", "@mongodb-js/oidc-plugin": "^0.3.0", "@mongodb-js/prettier-config-compass": "^1.0.0", "@mongodb-js/sbom-tools": "^0.5.4", @@ -1074,6 +1080,7 @@ "mocha-multi": "^1.1.7", "mongodb-client-encryption": "^6.0.0", "mongodb-runner": "^5.4.5", + "node-fetch": "^2.7.0", "node-loader": "^0.6.0", "npm-run-all": "^4.1.5", "ora": "^5.4.1", diff --git a/src/connectionController.ts b/src/connectionController.ts index f831773a7..3f2309e1c 100644 --- a/src/connectionController.ts +++ b/src/connectionController.ts @@ -335,22 +335,27 @@ export default class ConnectionController { browserCommandForOIDCAuth: undefined, // We overwrite this below. }, }); + const browserAuthCommand = vscode.workspace + .getConfiguration('mdb') + .get('browserCommandForOIDCAuth'); dataService = await connectionAttempt.connect({ ...connectionOptions, oidc: { ...cloneDeep(connectionOptions.oidc), - openBrowser: async ({ signal, url }) => { - try { - await openLink(url); - } catch (err) { - if (signal.aborted) return; - // If opening the link fails we default to regular link opening. - await vscode.commands.executeCommand( - 'vscode.open', - vscode.Uri.parse(url) - ); - } - }, + openBrowser: browserAuthCommand + ? { command: browserAuthCommand } + : async ({ signal, url }) => { + try { + await openLink(url); + } catch (err) { + if (signal.aborted) return; + // If opening the link fails we default to regular link opening. + await vscode.commands.executeCommand( + 'vscode.open', + vscode.Uri.parse(url) + ); + } + }, }, }); @@ -418,6 +423,7 @@ export default class ConnectionController { ); if (removeConfirmationResponse !== 'Confirm') { + await this.disconnect(); throw new Error('Reauthentication declined by user'); } } diff --git a/src/test/fixture/curl.js b/src/test/fixture/curl.js new file mode 100644 index 000000000..d7108c327 --- /dev/null +++ b/src/test/fixture/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/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts new file mode 100644 index 000000000..2f632a042 --- /dev/null +++ b/src/test/suite/oidc.test.ts @@ -0,0 +1,433 @@ +import os from 'os'; +import path from 'path'; +import assert from 'assert'; +import fs from 'fs/promises'; +import sinon from 'sinon'; +import type { SinonStub } from 'sinon'; +import * as vscode from 'vscode'; +import { createHash } from 'crypto'; +import { before, after, afterEach, beforeEach } from 'mocha'; +import EventEmitter, { once } from 'events'; +import { ExtensionContextStub } from './stubs'; +import { StorageController } from '../../storage'; +import { TelemetryService } from '../../telemetry'; +import ConnectionController from '../../connectionController'; +import { StatusView } from '../../views'; + +import { MongoCluster } from 'mongodb-runner'; +import type { MongoClusterOptions } from 'mongodb-runner'; +import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider'; +import type { OIDCMockProviderConfig } from '@mongodb-js/oidc-mock-provider'; +import { ConnectionString } from 'mongodb-connection-string-url'; + +import launchMongoShell from '../../commands/launchMongoShell'; +import { getFullRange } from './suggestTestHelpers'; + +function hash(input: string): string { + return createHash('sha256').update(input).digest('hex').slice(0, 12); +} + +// Need to be provided via CI env +const browserShellCommand = process.env.BROWSER_AUTH_COMMAND; + +const clusters = new Map(); +const defaultClusterOptions: MongoClusterOptions = { + topology: 'standalone', + tmpDir: path.join( + os.tmpdir(), + `vscode-tests-${hash(process.env.EVERGREEN_TASK_ID ?? '')}` + ), + logDir: process.env.MONGODB_RUNNER_LOGDIR, + version: process.env.MONGODB_VERSION, +}; + +const DEFAULT_TOKEN_PAYLOAD = { + expires_in: 3600, + payload: { + // Define the user information stored inside the access tokens + groups: ['testgroup'], + sub: 'testuser', + aud: 'resource-server-audience-value', + }, +}; + +export async function startTestServer( + config: Partial & { alwaysStartNewServer?: boolean } = {} +): Promise { + const key = JSON.stringify(config); + const existing = !config.alwaysStartNewServer && clusters.get(key); + if (existing && !existing.isClosed()) return existing; + const cluster = await MongoCluster.start({ + ...defaultClusterOptions, + ...config, + }); + + clusters.set(key, cluster); + return cluster; +} + +suite('OIDC Tests', function () { + this.timeout(50000); + + const extensionContextStub = new ExtensionContextStub(); + const testStorageController = new StorageController(extensionContextStub); + const testTelemetryService = new TelemetryService( + testStorageController, + extensionContextStub + ); + const testConnectionController = new ConnectionController({ + statusView: new StatusView(extensionContextStub), + storageController: testStorageController, + telemetryService: testTelemetryService, + }); + let showInformationMessageStub: SinonStub; + const sandbox = sinon.createSandbox(); + + // OIDC related variables + let getTokenPayload: typeof oidcMockProviderConfig.getTokenPayload = () => + DEFAULT_TOKEN_PAYLOAD; + let overrideRequestHandler: typeof oidcMockProviderConfig.overrideRequestHandler; + let oidcMockProviderConfig: OIDCMockProviderConfig; + let oidcMockProvider: OIDCMockProvider; + let oidcMockProviderEndpointAccesses: Record; + + let i = 0; + let tmpdir: string; + let cluster: MongoCluster; + let connectionString: string; + + let createTerminalStub: SinonStub; + let sendTextStub: SinonStub; + + before(async function () { + if (process.platform !== 'linux') { + connectionString = + 'mongodb://localhost:27096/?authMechanism=MONGODB-OIDC'; + return; + // OIDC is only supported on Linux in the 7.0+ enterprise server. + return this.skip(); + } + + oidcMockProviderEndpointAccesses = {}; + oidcMockProviderConfig = { + getTokenPayload(metadata: Parameters[0]) { + return getTokenPayload(metadata); + }, + overrideRequestHandler(url, req, res) { + const { pathname } = new URL(url); + oidcMockProviderEndpointAccesses[pathname] ??= 0; + oidcMockProviderEndpointAccesses[pathname]++; + return overrideRequestHandler?.(url, req, res); + }, + }; + oidcMockProvider = await OIDCMockProvider.create(oidcMockProviderConfig); + + tmpdir = path.join( + os.tmpdir(), + `vscode-oidc-${Date.now().toString(32)}-${++i}` + ); + await fs.mkdir(path.join(tmpdir, 'db'), { recursive: true }); + const serverOidcConfig = { + issuer: oidcMockProvider.issuer, + clientId: 'testServer', + requestScopes: ['mongodbGroups'], + authorizationClaim: 'groups', + audience: 'resource-server-audience-value', + authNamePrefix: 'dev', + }; + + cluster = await startTestServer({ + version: '7.0.x', + downloadOptions: { enterprise: true }, + args: [ + '--setParameter', + 'authenticationMechanisms=SCRAM-SHA-256,MONGODB-OIDC', + // enableTestCommands allows using http:// issuers such as http://localhost + '--setParameter', + 'enableTestCommands=true', + '--setParameter', + `oidcIdentityProviders=${JSON.stringify([serverOidcConfig])}`, + ], + }); + + const cs = new ConnectionString(cluster.connectionString); + cs.searchParams.set('authMechanism', 'MONGODB-OIDC'); + + connectionString = cs.toString(); + }); + + after(async function () { + if (process.platform !== 'linux') { + return; + } + + await oidcMockProvider?.close(); + await cluster?.close(); + }); + + beforeEach(async function () { + sandbox.stub(testTelemetryService, 'trackNewConnection'); + showInformationMessageStub = sandbox.stub( + vscode.window, + 'showInformationMessage' + ); + + // This is required to follow through the redirect while establishing + // connection + await vscode.workspace + .getConfiguration('mdb') + .update('browserCommandForOIDCAuth', browserShellCommand); + + createTerminalStub = sandbox.stub(vscode.window, 'createTerminal'); + sendTextStub = sandbox.stub(); + createTerminalStub.returns({ + sendText: sendTextStub, + show: () => {}, + }); + }); + + afterEach(async function () { + // Reset our mock extension's state. + extensionContextStub._workspaceState = {}; + extensionContextStub._globalState = {}; + + await testConnectionController.disconnect(); + testConnectionController.clearAllConnections(); + + sandbox.restore(); + }); + + test('can successfully connect with a connection string', async function () { + const succesfullyConnected = + await testConnectionController.addNewConnectionStringAndConnect( + connectionString + ); + assert.strictEqual(succesfullyConnected, true); + + await launchMongoShell(testConnectionController); + assert.strictEqual(createTerminalStub.called, true); + + const terminalOptions: vscode.TerminalOptions = + createTerminalStub.firstCall.args[0]; + const terminalConnectionString = terminalOptions.env?.MDB_CONNECTION_STRING; + if (!terminalConnectionString) { + assert.fail('Terminal connection string not found'); + } + const terminalCsWithoutAppName = new ConnectionString( + terminalConnectionString + ); + terminalCsWithoutAppName.searchParams.delete('appname'); + + assert.strictEqual( + terminalCsWithoutAppName.toString(), + connectionString, + `Expected open terminal to set shell arg as driver url "${connectionString}" found "${terminalCsWithoutAppName}"` + ); + + const shellCommandText = sendTextStub.firstCall.args[0]; + assert.strictEqual(shellCommandText, 'mongosh $MDB_CONNECTION_STRING;'); + + // Required for shell to share the OIDC state + assert.notStrictEqual( + terminalOptions.env?.MONGOSH_OIDC_PARENT_HANDLE, + undefined + ); + }); + + test('it persists tokens for further attempt if the settings is set to true', async function () { + await vscode.workspace + .getConfiguration('mdb') + .update('persistOIDCTokens', true); + let tokenFetchCalls = 0; + getTokenPayload = () => { + tokenFetchCalls++; + return DEFAULT_TOKEN_PAYLOAD; + }; + + assert.strictEqual( + await testConnectionController.addNewConnectionStringAndConnect( + connectionString + ), + true + ); + const connectionId = testConnectionController.getActiveConnectionId(); + if (!connectionId) { + assert.fail('Connection id not found for active connection'); + } + + await testConnectionController.disconnect(); + + assert.strictEqual( + await testConnectionController.connectWithConnectionId(connectionId), + true + ); + assert.strictEqual(tokenFetchCalls, 1); + }); + + test('it will not persist tokens for further attempt if the settings is set to false', async function () { + await vscode.workspace + .getConfiguration('mdb') + .update('persistOIDCTokens', false); + let tokenFetchCalls = 0; + getTokenPayload = () => { + tokenFetchCalls++; + return DEFAULT_TOKEN_PAYLOAD; + }; + + assert.strictEqual( + await testConnectionController.addNewConnectionStringAndConnect( + connectionString + ), + true + ); + + const connectionId = testConnectionController.getActiveConnectionId(); + if (!connectionId) { + assert.fail('Connection id not found for active connection'); + } + + await testConnectionController.disconnect(); + + assert.strictEqual( + await testConnectionController.connectWithConnectionId(connectionId), + true + ); + assert.strictEqual(tokenFetchCalls, 2); + }); + + test('can cancel a connection attempt and then successfully connect', async function () { + const emitter = new EventEmitter(); + const secondConnectionEstablished = once( + emitter, + 'secondConnectionEstablished' + ); + overrideRequestHandler = async (url) => { + if (new URL(url).pathname === '/authorize') { + emitter.emit('authorizeEndpointCalled'); + // This does effectively mean that our 'fake browser' + // will never get a response from the authorization endpoint + // during the first connection attempt, and that therefore + // the local HTTP server will never have its redirect endpoint + // accessed. + await secondConnectionEstablished; + } + }; + + testConnectionController + .addNewConnectionStringAndConnect(connectionString) + .catch(() => { + // ignored + }); + + await once(emitter, 'authorizeEndpointCalled'); + overrideRequestHandler = () => {}; + const connected = + await testConnectionController.addNewConnectionStringAndConnect( + connectionString + ); + emitter.emit('secondConnectionEstablished'); + assert.strictEqual(connected, true); + }); + + test('can successfully re-authenticate', async function () { + showInformationMessageStub.resolves('Confirm'); + let tokenFetchCalls = 0; + let afterReauth = false; + getTokenPayload = () => { + tokenFetchCalls++; + return { + ...DEFAULT_TOKEN_PAYLOAD, + ...(afterReauth ? {} : { expires_in: 1 }), + }; + }; + + const connected = + await testConnectionController.addNewConnectionStringAndConnect( + connectionString + ); + assert.strictEqual(connected, true); + afterReauth = true; + + // Wait for auth to expire + await new Promise((resolve) => setTimeout(resolve, 1100)); + + // Trigger a command on data service for reauthentication + await testConnectionController.getActiveDataService()?.count('x.y', {}); + + // Minor pause + await new Promise((resolve) => setTimeout(resolve, 500)); + + assert.strictEqual(tokenFetchCalls, 2); + assert.strictEqual(testConnectionController.isCurrentlyConnected(), true); + }); + + test('can decline re-authentication if wanted', async function () { + showInformationMessageStub.resolves('Declined'); + let tokenFetchCalls = 0; + let afterReauth = false; + getTokenPayload = () => { + tokenFetchCalls++; + return { + ...DEFAULT_TOKEN_PAYLOAD, + ...(afterReauth ? {} : { expires_in: 1 }), + }; + }; + + const connected = + await testConnectionController.addNewConnectionStringAndConnect( + connectionString + ); + assert.strictEqual(connected, true); + afterReauth = true; + + // Wait for auth to expire + await new Promise((resolve) => setTimeout(resolve, 1100)); + + // Trigger a command on data service for reauthentication + await testConnectionController + .getActiveDataService() + ?.count('x.y', {}) + .catch((error) => { + assert.strictEqual(error.message, 'Reauthentication declined by user'); + }); + + // Minor pause + await new Promise((resolve) => setTimeout(resolve, 500)); + + // Because we declined the auth in showInformationMessage above + assert.strictEqual(tokenFetchCalls, 1); + assert.strictEqual(testConnectionController.isCurrentlyConnected(), false); + }); + + test('shares the oidc state also with the playgrounds', async function () { + let tokenFetchCalls = 0; + getTokenPayload = () => { + tokenFetchCalls++; + return DEFAULT_TOKEN_PAYLOAD; + }; + + const succesfullyConnected = + await testConnectionController.addNewConnectionStringAndConnect( + connectionString + ); + assert.strictEqual(succesfullyConnected, true); + + await vscode.commands.executeCommand('mdb.createPlayground'); + + const editor = vscode.window.activeTextEditor; + if (!editor) { + throw new Error('Window active text editor is undefined'); + } + + const testDocumentUri = editor.document.uri; + const edit = new vscode.WorkspaceEdit(); + edit.replace( + testDocumentUri, + getFullRange(editor.document), + "use('random'); db.randomColl.find({}).count();" + ); + await vscode.workspace.applyEdit(edit); + await vscode.commands.executeCommand('mdb.runPlayground'); + assert.strictEqual(tokenFetchCalls, 1); + }); +}); From 3ae0550cd88aaebbf0b2174ccb1f97f47052f4da Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 9 Jan 2024 20:02:23 +0100 Subject: [PATCH 2/9] chore: removed mac specific changes --- src/test/suite/oidc.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts index 2f632a042..7e7470d91 100644 --- a/src/test/suite/oidc.test.ts +++ b/src/test/suite/oidc.test.ts @@ -101,9 +101,6 @@ suite('OIDC Tests', function () { before(async function () { if (process.platform !== 'linux') { - connectionString = - 'mongodb://localhost:27096/?authMechanism=MONGODB-OIDC'; - return; // OIDC is only supported on Linux in the 7.0+ enterprise server. return this.skip(); } From d36c9e2b294110cd17b4528bf7c07d31db0c6e19 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 10 Jan 2024 10:01:19 +0100 Subject: [PATCH 3/9] chore: fix github ci --- .github/workflows/actions/test-and-build/action.yaml | 7 +++++++ src/test/suite/oidc.test.ts | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/actions/test-and-build/action.yaml b/.github/workflows/actions/test-and-build/action.yaml index ad06d2a54..aff0fd975 100644 --- a/.github/workflows/actions/test-and-build/action.yaml +++ b/.github/workflows/actions/test-and-build/action.yaml @@ -69,7 +69,14 @@ runs: if: ${{ runner.os != 'Windows' }} shell: bash + - name: Set BROWSER_AUTH_COMMAND + run: | + BROWSER_AUTH_COMMAND=$(echo "$(which node) $(pwd)/src/test/fixture/curl.js") + echo "BROWSER_AUTH_COMMAND=$BROWSER_AUTH_COMMAND" >> $GITHUB_ENV + - name: Run Tests + env: + BROWSER_AUTH_COMMAND: ${{ env.BROWSER_AUTH_COMMAND }} run: | npm run test shell: bash diff --git a/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts index 7e7470d91..9f30245a2 100644 --- a/src/test/suite/oidc.test.ts +++ b/src/test/suite/oidc.test.ts @@ -27,7 +27,8 @@ function hash(input: string): string { return createHash('sha256').update(input).digest('hex').slice(0, 12); } -// Need to be provided via CI env +// Need to be provided via CI env because we can't get a hold for node.js exec +// path in our tests - they run inside a vscode process const browserShellCommand = process.env.BROWSER_AUTH_COMMAND; const clusters = new Map(); From 2ea6449bddd7b38f241d38f11f330c19375b9772 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 10 Jan 2024 10:02:21 +0100 Subject: [PATCH 4/9] chore: action fixup --- .github/workflows/actions/test-and-build/action.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/actions/test-and-build/action.yaml b/.github/workflows/actions/test-and-build/action.yaml index aff0fd975..6d51d8cc9 100644 --- a/.github/workflows/actions/test-and-build/action.yaml +++ b/.github/workflows/actions/test-and-build/action.yaml @@ -73,6 +73,7 @@ runs: run: | BROWSER_AUTH_COMMAND=$(echo "$(which node) $(pwd)/src/test/fixture/curl.js") echo "BROWSER_AUTH_COMMAND=$BROWSER_AUTH_COMMAND" >> $GITHUB_ENV + shell: bash - name: Run Tests env: From 0d2e2d12b810ce262117d18c0398baa585951641 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 10 Jan 2024 10:26:58 +0100 Subject: [PATCH 5/9] chore: minor cleanup --- src/test/suite/oidc.test.ts | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts index 9f30245a2..b7feb6d26 100644 --- a/src/test/suite/oidc.test.ts +++ b/src/test/suite/oidc.test.ts @@ -31,13 +31,13 @@ function hash(input: string): string { // path in our tests - they run inside a vscode process const browserShellCommand = process.env.BROWSER_AUTH_COMMAND; -const clusters = new Map(); +const UNIQUE_TASK_ID = + process.env.GITHUB_RUN_ID && process.env.GITHUB_RUN_NUMBER + ? `${process.env.GITHUB_RUN_ID}-${process.env.GITHUB_RUN_NUMBER}` + : ''; const defaultClusterOptions: MongoClusterOptions = { topology: 'standalone', - tmpDir: path.join( - os.tmpdir(), - `vscode-tests-${hash(process.env.EVERGREEN_TASK_ID ?? '')}` - ), + tmpDir: path.join(os.tmpdir(), `vscode-tests-${hash(UNIQUE_TASK_ID)}-data`), logDir: process.env.MONGODB_RUNNER_LOGDIR, version: process.env.MONGODB_VERSION, }; @@ -52,21 +52,6 @@ const DEFAULT_TOKEN_PAYLOAD = { }, }; -export async function startTestServer( - config: Partial & { alwaysStartNewServer?: boolean } = {} -): Promise { - const key = JSON.stringify(config); - const existing = !config.alwaysStartNewServer && clusters.get(key); - if (existing && !existing.isClosed()) return existing; - const cluster = await MongoCluster.start({ - ...defaultClusterOptions, - ...config, - }); - - clusters.set(key, cluster); - return cluster; -} - suite('OIDC Tests', function () { this.timeout(50000); @@ -134,7 +119,8 @@ suite('OIDC Tests', function () { authNamePrefix: 'dev', }; - cluster = await startTestServer({ + cluster = await MongoCluster.start({ + ...defaultClusterOptions, version: '7.0.x', downloadOptions: { enterprise: true }, args: [ From 45d5c895902b8df1713ea4b9c45a08f348432fe7 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 10 Jan 2024 15:12:34 +0100 Subject: [PATCH 6/9] chore: remove the timeout and instead wait for promise resolve --- src/test/suite/oidc.test.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts index b7feb6d26..070e275af 100644 --- a/src/test/suite/oidc.test.ts +++ b/src/test/suite/oidc.test.ts @@ -315,6 +315,20 @@ suite('OIDC Tests', function () { test('can successfully re-authenticate', async function () { showInformationMessageStub.resolves('Confirm'); + const originalReAuthHandler = + testConnectionController._reauthenticationHandler.bind( + testConnectionController + ); + let resolveReAuthPromise: (value?: unknown) => void; + const reAuthPromise = new Promise((resolve) => { + resolveReAuthPromise = resolve; + }); + sandbox + .stub(testConnectionController, '_reauthenticationHandler') + .callsFake(async () => { + resolveReAuthPromise(); + await originalReAuthHandler(); + }); let tokenFetchCalls = 0; let afterReauth = false; getTokenPayload = () => { @@ -332,14 +346,14 @@ suite('OIDC Tests', function () { assert.strictEqual(connected, true); afterReauth = true; - // Wait for auth to expire + // Wait for auth to expire, our auth expire in 1 second await new Promise((resolve) => setTimeout(resolve, 1100)); // Trigger a command on data service for reauthentication await testConnectionController.getActiveDataService()?.count('x.y', {}); - // Minor pause - await new Promise((resolve) => setTimeout(resolve, 500)); + // Wait for reauthentication promise to resolve + await reAuthPromise; assert.strictEqual(tokenFetchCalls, 2); assert.strictEqual(testConnectionController.isCurrentlyConnected(), true); From 913225bccac0575ff938dc1a00786c1b4c7a8229 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 10 Jan 2024 18:25:28 +0100 Subject: [PATCH 7/9] chore: review fixup --- src/test/suite/oidc.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts index 070e275af..f6562c4e0 100644 --- a/src/test/suite/oidc.test.ts +++ b/src/test/suite/oidc.test.ts @@ -77,7 +77,6 @@ suite('OIDC Tests', function () { let oidcMockProvider: OIDCMockProvider; let oidcMockProviderEndpointAccesses: Record; - let i = 0; let tmpdir: string; let cluster: MongoCluster; let connectionString: string; @@ -105,10 +104,7 @@ suite('OIDC Tests', function () { }; oidcMockProvider = await OIDCMockProvider.create(oidcMockProviderConfig); - tmpdir = path.join( - os.tmpdir(), - `vscode-oidc-${Date.now().toString(32)}-${++i}` - ); + tmpdir = path.join(os.tmpdir(), `vscode-oidc-${Date.now().toString(32)}`); await fs.mkdir(path.join(tmpdir, 'db'), { recursive: true }); const serverOidcConfig = { issuer: oidcMockProvider.issuer, From 58da0987e06c71637c1d7f2ddffdc9b5442d4d9b Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 11 Jan 2024 11:42:35 +0100 Subject: [PATCH 8/9] chore: changed expectations to use chai --- src/test/suite/oidc.test.ts | 94 +++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 50 deletions(-) diff --git a/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts index f6562c4e0..f17b136ff 100644 --- a/src/test/suite/oidc.test.ts +++ b/src/test/suite/oidc.test.ts @@ -1,6 +1,7 @@ import os from 'os'; import path from 'path'; -import assert from 'assert'; +import chai, { expect } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; import fs from 'fs/promises'; import sinon from 'sinon'; import type { SinonStub } from 'sinon'; @@ -23,6 +24,8 @@ import { ConnectionString } from 'mongodb-connection-string-url'; import launchMongoShell from '../../commands/launchMongoShell'; import { getFullRange } from './suggestTestHelpers'; +chai.use(chaiAsPromised); + function hash(input: string): string { return createHash('sha256').update(input).digest('hex').slice(0, 12); } @@ -182,36 +185,30 @@ suite('OIDC Tests', function () { await testConnectionController.addNewConnectionStringAndConnect( connectionString ); - assert.strictEqual(succesfullyConnected, true); + expect(succesfullyConnected).to.be.true; await launchMongoShell(testConnectionController); - assert.strictEqual(createTerminalStub.called, true); + expect(createTerminalStub).to.be.called; const terminalOptions: vscode.TerminalOptions = createTerminalStub.firstCall.args[0]; const terminalConnectionString = terminalOptions.env?.MDB_CONNECTION_STRING; + if (!terminalConnectionString) { - assert.fail('Terminal connection string not found'); + expect.fail('Terminal connection string not found'); } const terminalCsWithoutAppName = new ConnectionString( terminalConnectionString ); terminalCsWithoutAppName.searchParams.delete('appname'); - assert.strictEqual( - terminalCsWithoutAppName.toString(), - connectionString, - `Expected open terminal to set shell arg as driver url "${connectionString}" found "${terminalCsWithoutAppName}"` - ); + expect(terminalCsWithoutAppName.toString()).to.equal(connectionString); const shellCommandText = sendTextStub.firstCall.args[0]; - assert.strictEqual(shellCommandText, 'mongosh $MDB_CONNECTION_STRING;'); + expect(shellCommandText).to.equal('mongosh $MDB_CONNECTION_STRING;'); // Required for shell to share the OIDC state - assert.notStrictEqual( - terminalOptions.env?.MONGOSH_OIDC_PARENT_HANDLE, - undefined - ); + expect(terminalOptions.env?.MONGOSH_OIDC_PARENT_HANDLE).to.not.be.undefined; }); test('it persists tokens for further attempt if the settings is set to true', async function () { @@ -224,24 +221,23 @@ suite('OIDC Tests', function () { return DEFAULT_TOKEN_PAYLOAD; }; - assert.strictEqual( + expect( await testConnectionController.addNewConnectionStringAndConnect( connectionString - ), - true - ); + ) + ).to.be.true; + const connectionId = testConnectionController.getActiveConnectionId(); if (!connectionId) { - assert.fail('Connection id not found for active connection'); + expect.fail('Connection id not found for active connection'); } await testConnectionController.disconnect(); - assert.strictEqual( - await testConnectionController.connectWithConnectionId(connectionId), - true - ); - assert.strictEqual(tokenFetchCalls, 1); + expect( + await testConnectionController.connectWithConnectionId(connectionId) + ).to.be.true; + expect(tokenFetchCalls).to.equal(1); }); test('it will not persist tokens for further attempt if the settings is set to false', async function () { @@ -254,25 +250,23 @@ suite('OIDC Tests', function () { return DEFAULT_TOKEN_PAYLOAD; }; - assert.strictEqual( + expect( await testConnectionController.addNewConnectionStringAndConnect( connectionString - ), - true - ); + ) + ).to.be.true; const connectionId = testConnectionController.getActiveConnectionId(); if (!connectionId) { - assert.fail('Connection id not found for active connection'); + expect.fail('Connection id not found for active connection'); } await testConnectionController.disconnect(); - assert.strictEqual( - await testConnectionController.connectWithConnectionId(connectionId), - true - ); - assert.strictEqual(tokenFetchCalls, 2); + expect( + await testConnectionController.connectWithConnectionId(connectionId) + ).to.be.true; + expect(tokenFetchCalls).to.equal(2); }); test('can cancel a connection attempt and then successfully connect', async function () { @@ -306,7 +300,7 @@ suite('OIDC Tests', function () { connectionString ); emitter.emit('secondConnectionEstablished'); - assert.strictEqual(connected, true); + expect(connected).to.be.true; }); test('can successfully re-authenticate', async function () { @@ -335,11 +329,11 @@ suite('OIDC Tests', function () { }; }; - const connected = + expect( await testConnectionController.addNewConnectionStringAndConnect( connectionString - ); - assert.strictEqual(connected, true); + ) + ).to.be.true; afterReauth = true; // Wait for auth to expire, our auth expire in 1 second @@ -351,8 +345,8 @@ suite('OIDC Tests', function () { // Wait for reauthentication promise to resolve await reAuthPromise; - assert.strictEqual(tokenFetchCalls, 2); - assert.strictEqual(testConnectionController.isCurrentlyConnected(), true); + expect(tokenFetchCalls).to.equal(2); + expect(testConnectionController.isCurrentlyConnected()).to.be.true; }); test('can decline re-authentication if wanted', async function () { @@ -367,11 +361,11 @@ suite('OIDC Tests', function () { }; }; - const connected = + expect( await testConnectionController.addNewConnectionStringAndConnect( connectionString - ); - assert.strictEqual(connected, true); + ) + ).to.be.true; afterReauth = true; // Wait for auth to expire @@ -382,15 +376,15 @@ suite('OIDC Tests', function () { .getActiveDataService() ?.count('x.y', {}) .catch((error) => { - assert.strictEqual(error.message, 'Reauthentication declined by user'); + expect(error.message).to.equal('Reauthentication declined by user'); }); // Minor pause await new Promise((resolve) => setTimeout(resolve, 500)); // Because we declined the auth in showInformationMessage above - assert.strictEqual(tokenFetchCalls, 1); - assert.strictEqual(testConnectionController.isCurrentlyConnected(), false); + expect(tokenFetchCalls).to.equal(1); + expect(testConnectionController.isCurrentlyConnected()).to.be.false; }); test('shares the oidc state also with the playgrounds', async function () { @@ -400,11 +394,11 @@ suite('OIDC Tests', function () { return DEFAULT_TOKEN_PAYLOAD; }; - const succesfullyConnected = + expect( await testConnectionController.addNewConnectionStringAndConnect( connectionString - ); - assert.strictEqual(succesfullyConnected, true); + ) + ).to.be.true; await vscode.commands.executeCommand('mdb.createPlayground'); @@ -422,6 +416,6 @@ suite('OIDC Tests', function () { ); await vscode.workspace.applyEdit(edit); await vscode.commands.executeCommand('mdb.runPlayground'); - assert.strictEqual(tokenFetchCalls, 1); + expect(tokenFetchCalls).to.equal(1); }); }); From 3019cf69d2a0f531000bc193c5ae95da6c0981c2 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 11 Jan 2024 12:04:43 +0100 Subject: [PATCH 9/9] chore: remove reliance on timeouts entirely --- src/test/suite/oidc.test.ts | 45 ++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/test/suite/oidc.test.ts b/src/test/suite/oidc.test.ts index f17b136ff..fc6a1450c 100644 --- a/src/test/suite/oidc.test.ts +++ b/src/test/suite/oidc.test.ts @@ -309,6 +309,7 @@ suite('OIDC Tests', function () { testConnectionController._reauthenticationHandler.bind( testConnectionController ); + let reAuthCalled = false; let resolveReAuthPromise: (value?: unknown) => void; const reAuthPromise = new Promise((resolve) => { resolveReAuthPromise = resolve; @@ -316,6 +317,7 @@ suite('OIDC Tests', function () { sandbox .stub(testConnectionController, '_reauthenticationHandler') .callsFake(async () => { + reAuthCalled = true; resolveReAuthPromise(); await originalReAuthHandler(); }); @@ -336,11 +338,10 @@ suite('OIDC Tests', function () { ).to.be.true; afterReauth = true; - // Wait for auth to expire, our auth expire in 1 second - await new Promise((resolve) => setTimeout(resolve, 1100)); - // Trigger a command on data service for reauthentication - await testConnectionController.getActiveDataService()?.count('x.y', {}); + while (reAuthCalled === false) { + await testConnectionController.getActiveDataService()?.count('x.y', {}); + } // Wait for reauthentication promise to resolve await reAuthPromise; @@ -351,6 +352,22 @@ suite('OIDC Tests', function () { test('can decline re-authentication if wanted', async function () { showInformationMessageStub.resolves('Declined'); + const originalReAuthHandler = + testConnectionController._reauthenticationHandler.bind( + testConnectionController + ); + let reAuthCalled = false; + let resolveReAuthPromise: (value?: unknown) => void; + const reAuthPromise = new Promise((resolve) => { + resolveReAuthPromise = resolve; + }); + sandbox + .stub(testConnectionController, '_reauthenticationHandler') + .callsFake(async () => { + reAuthCalled = true; + resolveReAuthPromise(); + await originalReAuthHandler(); + }); let tokenFetchCalls = 0; let afterReauth = false; getTokenPayload = () => { @@ -368,19 +385,17 @@ suite('OIDC Tests', function () { ).to.be.true; afterReauth = true; - // Wait for auth to expire - await new Promise((resolve) => setTimeout(resolve, 1100)); - // Trigger a command on data service for reauthentication - await testConnectionController - .getActiveDataService() - ?.count('x.y', {}) - .catch((error) => { - expect(error.message).to.equal('Reauthentication declined by user'); - }); + while (reAuthCalled === false) { + await testConnectionController + .getActiveDataService() + ?.count('x.y', {}) + .catch((error) => { + expect(error.message).to.equal('Reauthentication declined by user'); + }); + } - // Minor pause - await new Promise((resolve) => setTimeout(resolve, 500)); + await reAuthPromise; // Because we declined the auth in showInformationMessage above expect(tokenFetchCalls).to.equal(1);