From 85485a28fd177c9774dbe595f83bab00b1ebbeb4 Mon Sep 17 00:00:00 2001 From: Basit <1305718+mabaasit@users.noreply.github.com> Date: Wed, 29 Nov 2023 00:08:26 +0100 Subject: [PATCH] test(connection-storage): exported connections tests COMPASS-7464 COMPASS-7392 (#5125) --- .../src/connection-storage.spec.ts | 489 ++++++++++++++++-- .../test/fixtures/compass-connections.ts | 106 ++++ 2 files changed, 555 insertions(+), 40 deletions(-) create mode 100644 packages/connection-storage/test/fixtures/compass-connections.ts diff --git a/packages/connection-storage/src/connection-storage.spec.ts b/packages/connection-storage/src/connection-storage.spec.ts index c684f8b523f..ba93ca74aaf 100644 --- a/packages/connection-storage/src/connection-storage.spec.ts +++ b/packages/connection-storage/src/connection-storage.spec.ts @@ -16,6 +16,7 @@ import Sinon from 'sinon'; import connection1270 from './../test/fixtures/favorite_connection_1.27.0.json'; import connection1310 from './../test/fixtures/favorite_connection_1.31.0.json'; import connection1380 from './../test/fixtures/favorite_connection_1.38.0.json'; +import * as exportedConnectionFixtures from './../test/fixtures/compass-connections'; function getConnectionFilePath(tmpDir: string, id: string): string { const connectionsDir = path.join(tmpDir, 'Connections'); @@ -56,7 +57,6 @@ async function readConnection(tmpDir: string, id: string) { const maxAllowedConnections = 10; describe('ConnectionStorage', function () { - const initialKeytarEnvValue = process.env.COMPASS_E2E_DISABLE_KEYCHAIN_USAGE; const ipcMain = ConnectionStorage['ipcMain']; let tmpDir: string; @@ -69,8 +69,6 @@ describe('ConnectionStorage', function () { createHandle: Sinon.stub(), }; ConnectionStorage.init(tmpDir); - - process.env.COMPASS_E2E_DISABLE_KEYCHAIN_USAGE = 'true'; }); afterEach(async function () { @@ -79,12 +77,341 @@ describe('ConnectionStorage', function () { ConnectionStorage['calledOnce'] = false; ConnectionStorage['ipcMain'] = ipcMain; + }); + + it('reminds us to remove keytar in', function () { + const testDate = new Date('2023-11-17T00:00:00.000Z'); + + const endOfKeytarDate = new Date(testDate); + endOfKeytarDate.setMonth(endOfKeytarDate.getMonth() + 3); + + // Based on milestone #2 of Move from keytar to Electron safeStorage, + // we should remove keytar in 3 months from now. And as such, we are + // intentionally failing this test to remind us to remove keytar. + // If we want to continue using keytar for now, check with the product and + // please update this test accordingly. + expect( + new Date(), + 'Expected to have keytar removed completely by now. If we want to continue using it for now, please update this test.' + ).to.be.lessThan(endOfKeytarDate); + }); + + describe('migrateToSafeStorage', function () { + let sandbox: Sinon.SinonSandbox; + beforeEach(function () { + sandbox = Sinon.createSandbox(); + }); + + afterEach(function () { + sandbox.restore(); + }); + + context('does not migrate connections', function () { + it('when there are no connections', async function () { + await ConnectionStorage.migrateToSafeStorage(); + const connections = await ConnectionStorage.loadAll(); + expect(connections).to.deep.equal([]); + }); + + it('when there are only legacy connections', async function () { + await writeFakeConnection(tmpDir, connection1270); + + const encryptSecretsSpy = sandbox.spy( + ConnectionStorage, + 'encryptSecrets' as any + ); + const getKeytarCredentialsSpy = sandbox.spy( + ConnectionStorage, + 'getKeytarCredentials' as any + ); + + await ConnectionStorage.migrateToSafeStorage(); + + expect( + encryptSecretsSpy.called, + 'it does not try to encrypt' + ).to.be.false; + expect( + getKeytarCredentialsSpy.called, + 'it does not try to get secrets from keychain' + ).to.be.false; + }); + + it('when there are only migrated connections', async function () { + const connectionInfo1 = getConnectionInfo(); + const connectionInfo2 = getConnectionInfo(); + + await writeFakeConnection(tmpDir, { + _id: connectionInfo1.id, + connectionInfo: connectionInfo1, + version: 1, + }); + await writeFakeConnection(tmpDir, { + _id: connectionInfo2.id, + connectionInfo: connectionInfo2, + version: 1, + }); + + const encryptSecretsSpy = sandbox.spy( + ConnectionStorage, + 'encryptSecrets' as any + ); + const getKeytarCredentialsSpy = sandbox.spy( + ConnectionStorage, + 'getKeytarCredentials' as any + ); + + await ConnectionStorage.migrateToSafeStorage(); + + expect( + encryptSecretsSpy.called, + 'it does not try to encrypt' + ).to.be.false; + expect( + getKeytarCredentialsSpy.called, + 'it does not try to get secrets from keychain' + ).to.be.false; + + const expectedConnection1 = await readConnection( + tmpDir, + connectionInfo1.id + ); + const expectedConnection2 = await readConnection( + tmpDir, + connectionInfo2.id + ); + + expect(expectedConnection1).to.deep.equal({ + _id: connectionInfo1.id, + connectionInfo: connectionInfo1, + version: 1, + }); + + expect(expectedConnection2).to.deep.equal({ + _id: connectionInfo2.id, + connectionInfo: connectionInfo2, + version: 1, + }); + }); + }); + + context('migrates connections', function () { + it('when there are connections and secrets in keychain', async function () { + const connectionInfo1 = getConnectionInfo(); + const connectionInfo2 = getConnectionInfo(); + await writeFakeConnection(tmpDir, { + connectionInfo: connectionInfo1, + }); + await writeFakeConnection(tmpDir, { + connectionInfo: connectionInfo2, + }); + + // Keytar stub + sandbox.stub(ConnectionStorage, 'getKeytarCredentials' as any).returns({ + [connectionInfo1.id]: { + password: 'password1', + }, + [connectionInfo2.id]: { + password: 'password2', + }, + }); + + // safeStorage.encryptString stub + sandbox + .stub(ConnectionStorage, 'encryptSecrets' as any) + .returns('encrypted-password'); + + await ConnectionStorage.migrateToSafeStorage(); + + const expectedConnection1 = await readConnection( + tmpDir, + connectionInfo1.id + ); + const expectedConnection2 = await readConnection( + tmpDir, + connectionInfo2.id + ); + + expect(expectedConnection1).to.deep.equal({ + _id: connectionInfo1.id, + connectionInfo: connectionInfo1, + connectionSecrets: 'encrypted-password', + version: 1, + }); + expect(expectedConnection2).to.deep.equal({ + _id: connectionInfo2.id, + connectionInfo: connectionInfo2, + connectionSecrets: 'encrypted-password', + version: 1, + }); + }); + it('when there are connections and no secrets in keychain', async function () { + const connectionInfo1 = getConnectionInfo(); + const connectionInfo2 = getConnectionInfo(); + await writeFakeConnection(tmpDir, { + connectionInfo: connectionInfo1, + }); + await writeFakeConnection(tmpDir, { + connectionInfo: connectionInfo2, + }); + + // Keytar fake + sandbox + .stub(ConnectionStorage, 'getKeytarCredentials' as any) + .returns({}); + + // Since there're no secrets in keychain, we do not expect to call safeStorage.encryptString + // and connection.connectionSecrets should be undefined + + await ConnectionStorage.migrateToSafeStorage(); + + const expectedConnection1 = await readConnection( + tmpDir, + connectionInfo1.id + ); + const expectedConnection2 = await readConnection( + tmpDir, + connectionInfo2.id + ); + + expect(expectedConnection1).to.deep.equal({ + _id: connectionInfo1.id, + connectionInfo: connectionInfo1, + version: 1, + }); + expect(expectedConnection2).to.deep.equal({ + _id: connectionInfo2.id, + connectionInfo: connectionInfo2, + version: 1, + }); + }); + + it('when there are any unmigrated connections', async function () { + const connectionInfo1 = getConnectionInfo(); + const connectionInfo2 = getConnectionInfo(); + await writeFakeConnection(tmpDir, { + _id: connectionInfo1.id, + connectionInfo: connectionInfo1, + version: 1, + }); + await writeFakeConnection(tmpDir, { + connectionInfo: connectionInfo2, + }); - if (initialKeytarEnvValue) { - process.env.COMPASS_E2E_DISABLE_KEYCHAIN_USAGE = initialKeytarEnvValue; - } else { - delete process.env.COMPASS_E2E_DISABLE_KEYCHAIN_USAGE; - } + // Keytar stub + sandbox + .stub(ConnectionStorage, 'getKeytarCredentials' as any) + .returns({}); + + await ConnectionStorage.migrateToSafeStorage(); + + const expectedConnection1 = await readConnection( + tmpDir, + connectionInfo1.id + ); + const expectedConnection2 = await readConnection( + tmpDir, + connectionInfo2.id + ); + + expect(expectedConnection1).to.deep.equal({ + _id: connectionInfo1.id, + connectionInfo: connectionInfo1, + version: 1, + }); + expect(expectedConnection2).to.deep.equal({ + _id: connectionInfo2.id, + connectionInfo: connectionInfo2, + version: 1, + }); + }); + }); + + context( + 'imports already exported connections from compass using keytar', + function () { + const exportedConnections = [ + { + label: 'connection with plain secrets', + data: exportedConnectionFixtures.connectionsWithPlainSecrets, + }, + { + label: 'connection with encrypted secrets', + data: exportedConnectionFixtures.connectionsWithEncryptedSecrets, + importOptions: { + passphrase: 'password', + }, + }, + ]; + + for (const exportUseCase of exportedConnections) { + it(exportUseCase.label, async function () { + const countOfConnections = exportUseCase.data.connections.length; + // Import + { + // When importing connections, we will encrypt secrets using safeStorage. + // So, we mock the *encryptSecrets* implementation to not trigger keychain. + const encryptSecretsStub = sandbox + .stub(ConnectionStorage, 'encryptSecrets' as any) + .returns('encrypted-password'); + // Import + await ConnectionStorage.importConnections({ + content: JSON.stringify(exportUseCase.data), + options: exportUseCase.importOptions, + }); + expect( + encryptSecretsStub.getCalls().map((x) => x.args), + `makes ${countOfConnections} calls with correct arguments when encrypting` + ).to.deep.equal( + Array.from({ length: countOfConnections }).map(() => [ + { password: 'password' }, + ]) + ); + } + + // Read imported connections using ConnectionStorage + { + // When reading, we will mock *decryptSecrets* implementation. + const decryptSecretsStub = sandbox + .stub(ConnectionStorage, 'decryptSecrets' as any) + .returns({ password: 'password' }); + + const connections = await ConnectionStorage.loadAll(); + + expect(connections).to.have.lengthOf(countOfConnections); + + expect( + decryptSecretsStub.getCalls().map((x) => x.args), + `makes ${countOfConnections} calls with correct arguments when decrypting` + ).to.deep.equal( + Array.from({ length: countOfConnections }).map(() => [ + 'encrypted-password', + ]) + ); + } + + // Read imported connections using fs from disk + { + const connections = await Promise.all( + exportUseCase.data.connections.map(({ id }) => + readConnection(tmpDir, id) + ) + ); + + expect(connections).to.have.lengthOf(countOfConnections); + + for (const connection of connections) { + expect(connection).to.have.a.property('version', 1); + expect(connection).to.have.a.property( + 'connectionSecrets', + 'encrypted-password' + ); + } + } + }); + } + } + ); }); describe('migrateToSafeStorage', function () { @@ -525,42 +852,112 @@ describe('ConnectionStorage', function () { }); expect.fail('Expected connection string to be required.'); } catch (e) { - expect(e).to.have.nested.property( - 'errors[0].message', - 'Connection string is required.' + expect(e).to.have.property( + 'message', + 'Invalid scheme, expected connection string to start with "mongodb://" or "mongodb+srv://"' ); } }); - // In tests we can not use keytar and have it disabled. When saving any data, - // its completely stored on disk without anything removed. - it('it stores all the fleOptions on disk', async function () { - const id = new UUID().toString(); - const connectionInfo = { - id, - connectionOptions: { - connectionString: 'mongodb://localhost:27017', - fleOptions: { - storeCredentials: false, - autoEncryption: { - keyVaultNamespace: 'db.coll', - kmsProviders: { - local: { - key: 'my-key', + context('it stores all the fleOptions on disk', function () { + it('removes secrets when storeCredentials is false', async function () { + const id = new UUID().toString(); + const connectionInfo = { + id, + connectionOptions: { + connectionString: 'mongodb://localhost:27017/', + fleOptions: { + storeCredentials: false, + autoEncryption: { + keyVaultNamespace: 'db.coll', + kmsProviders: { + local: { + key: 'my-key', + }, }, }, }, }, - }, - }; - await ConnectionStorage.save({ connectionInfo }); + }; - const content = await fs.readFile( - getConnectionFilePath(tmpDir, id), - 'utf-8' - ); - const { connectionInfo: expectedConnectionInfo } = JSON.parse(content); - expect(expectedConnectionInfo).to.deep.equal(connectionInfo); + // // Stub encryptSecrets so that we do not call electron.safeStorage.encrypt + // // and make assertions on that. + const encryptSecretsStub = Sinon.stub( + ConnectionStorage, + 'encryptSecrets' as any + ).returns(undefined); + + await ConnectionStorage.save({ connectionInfo }); + + const expectedConnection = await readConnection(tmpDir, id); + connectionInfo.connectionOptions.fleOptions.autoEncryption.kmsProviders = + {} as any; + expect(expectedConnection).to.deep.equal({ + _id: connectionInfo.id, + connectionInfo, + version: 1, + }); + + expect(encryptSecretsStub.calledOnce).to.be.true; + expect( + encryptSecretsStub.firstCall.firstArg, + 'it should not store any secrets' + ).to.deep.equal({}); + }); + + it('encrypt and store secrets when storeCredentials is true', async function () { + const id = new UUID().toString(); + const connectionInfo = { + id, + connectionOptions: { + connectionString: 'mongodb://localhost:27017/', + fleOptions: { + storeCredentials: true, + autoEncryption: { + keyVaultNamespace: 'db.coll', + kmsProviders: { + local: { + key: 'my-key', + }, + }, + }, + }, + }, + }; + + // Stub encryptSecrets so that we do not call electron.safeStorage.encrypt + // and make assertions on that. + const encryptSecretsStub = Sinon.stub( + ConnectionStorage, + 'encryptSecrets' as any + ).returns('encrypted-data'); + + await ConnectionStorage.save({ connectionInfo }); + + const expectedConnection = await readConnection(tmpDir, id); + connectionInfo.connectionOptions.fleOptions.autoEncryption.kmsProviders = + {} as any; + expect(expectedConnection).to.deep.equal({ + _id: connectionInfo.id, + connectionInfo, + connectionSecrets: 'encrypted-data', + version: 1, + }); + + expect(encryptSecretsStub.calledOnce).to.be.true; + expect( + encryptSecretsStub.firstCall.firstArg, + 'it should store secrets' + ).to.deep.equal({ + autoEncryption: { + kmsProviders: { + local: { + key: 'my-key', + }, + }, + }, + }); + }); }); it('saves a connection with _id', async function () { @@ -571,6 +968,13 @@ describe('ConnectionStorage', function () { expect(e).to.be.instanceOf(Error); } + // Stub encryptSecrets so that we do not call electron.safeStorage.encrypt + // and make assertions on that. + const encryptSecretsStub = Sinon.stub( + ConnectionStorage, + 'encryptSecrets' as any + ).returns('encrypted-password'); + await ConnectionStorage.save({ connectionInfo: { id, @@ -580,19 +984,24 @@ describe('ConnectionStorage', function () { }, }); - const savedConnection = JSON.parse( - await fs.readFile(getConnectionFilePath(tmpDir, id), 'utf-8') - ); - + const savedConnection = await readConnection(tmpDir, id); expect(savedConnection).to.deep.equal({ connectionInfo: { id, connectionOptions: { - connectionString: 'mongodb://root:password@localhost:27017', + connectionString: 'mongodb://root@localhost:27017/', }, }, + connectionSecrets: 'encrypted-password', + version: 1, _id: id, }); + + expect(encryptSecretsStub.calledOnce).to.be.true; + expect( + encryptSecretsStub.firstCall.firstArg, + 'it encrypts the connection secrets correctly' + ).to.deep.equal({ password: 'password' }); }); context(`max allowed connections ${maxAllowedConnections}`, function () { diff --git a/packages/connection-storage/test/fixtures/compass-connections.ts b/packages/connection-storage/test/fixtures/compass-connections.ts new file mode 100644 index 00000000000..f9cc255d108 --- /dev/null +++ b/packages/connection-storage/test/fixtures/compass-connections.ts @@ -0,0 +1,106 @@ +// These connections have been encrypted with *password* passphrase. +// The password of these connections is *password* +// Exported on Compass 1.40.4 +export const connectionsWithEncryptedSecrets = { + type: 'Compass Connections', + version: { + $numberInt: '1', + }, + connections: [ + { + id: '30bc83dd-a988-4553-b379-89fac70f3f5a', + favorite: { + name: 'Compass 1.39.3', + color: 'color6', + }, + connectionOptions: { + connectionString: + 'mongodb://compass1393@localhost:27017/?authMechanism=DEFAULT', + }, + connectionSecrets: + 'AAFSEEDlKaJFiXzDE1UFQ9/iokaH3j1t7TOCXUsy6sG+yxASG2WOfTP8rbg0g5XqrOcm6D+IpTni5QxpTXChGkrwngRVOWwmGmHkAItGNlZaR0AGaw==', + }, + { + id: '5231cafe-b280-43ac-977b-f95619fb0892', + favorite: { + name: 'Compass 1.39.0', + color: 'color3', + }, + connectionOptions: { + connectionString: + 'mongodb://compass1390@localhost:27017/?authMechanism=DEFAULT', + }, + connectionSecrets: + 'AAFj3cDh/I7e17O0rMczLJBqzNaSHBwbhlzPCek+G43M5H61EOLXGcvMUZpMr+0CYYWl5xSFBaWOX+qCDZDYCsd3Md+HL32F4o78QZDd5XeXA2fjFw==', + }, + { + id: '594f1301-30db-4b4b-a341-432bb915cc0b', + favorite: { + name: 'Compass 1.38.2', + color: 'color10', + }, + connectionOptions: { + connectionString: + 'mongodb://compass1382@localhost:27017/?authMechanism=DEFAULT', + }, + connectionSecrets: + 'AAEqe3aNaVhTg+90sm2ESKElVyFDwAoRiXu6D2C+Saelo4jDAqwzH+jOQqmwF0CTLDBjsnKjlKJKxfrzhakgZxy6LgyxGUmM/yrICy/eUjIL75oJOA==', + }, + { + id: '9005e859-cf99-4b29-8837-ca9d4bf3b8bf', + favorite: { + name: 'Compass 1.39.2', + color: 'color6', + }, + connectionOptions: { + connectionString: + 'mongodb://compass1392@localhost:27017/?authMechanism=DEFAULT', + }, + connectionSecrets: + 'AAHK7dlGMo8VUT8owt7rbtOhaevPP2ArrAkg+5WablYAAcMQFrxDlFUAambct6uolnpVhTEzfQED2kM5tdmgj8wQiuM0yFvmHNPIU3x/ukZthDJWfA==', + }, + ], +} as const; + +// Exported on Compass 1.40.4 +export const connectionsWithPlainSecrets = { + type: 'Compass Connections', + version: { + $numberInt: '1', + }, + connections: [ + { + id: '3985f01b-417a-448c-91f5-6b04303a26be', + favorite: { + name: 'Compass 1.36.4', + color: 'color5', + }, + connectionOptions: { + connectionString: + 'mongodb://compass1364:password@localhost:27017/?authMechanism=DEFAULT', + }, + }, + { + id: '5231cafe-b280-43ac-977b-f95619fb0892', + favorite: { + name: 'Compass 1.39.0', + color: 'color3', + }, + connectionOptions: { + connectionString: + 'mongodb://compass1390:password@localhost:27017/?authMechanism=DEFAULT', + }, + }, + { + id: '594f1301-30db-4b4b-a341-432bb915cc0b', + favorite: { + name: 'Compass 1.38.2', + color: 'color10', + }, + connectionOptions: { + connectionString: + 'mongodb://compass1382:password@localhost:27017/?authMechanism=DEFAULT', + }, + }, + ], +} as const;