Skip to content

Commit 6f122ee

Browse files
mabaasitmabaasit
authored
feat(connection-storage): migrate from keytar COMPASS-6854 (#5115)
1 parent 8a5885c commit 6f122ee

File tree

5 files changed

+417
-63
lines changed

5 files changed

+417
-63
lines changed

‎package-lock.json‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/compass/src/main/application.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ import { setupTheme } from './theme';
2121
import { setupProtocolHandlers } from './protocol-handling';
2222
import { ConnectionStorage } from '@mongodb-js/connection-storage/main';
2323

24-
const { debug, track } = createLoggerAndTelemetry('COMPASS-MAIN');
24+
const { debug, log, track, mongoLogId } =
25+
createLoggerAndTelemetry('COMPASS-MAIN');
2526

2627
type ExitHandler = () => Promise<unknown>;
2728
type CompassApplicationMode = 'CLI' | 'GUI';
@@ -86,6 +87,17 @@ class CompassApplication {
8687
// ConnectionStorage offers import/export which is used via CLI as well.
8788
ConnectionStorage.init();
8889

90+
try {
91+
await ConnectionStorage.migrateToSafeStorage();
92+
} catch (e) {
93+
log.error(
94+
mongoLogId(1_001_000_275),
95+
'SafeStorage Migration',
96+
'Failed to migrate connections',
97+
{ message: (e as Error).message }
98+
);
99+
}
100+
89101
if (mode === 'CLI') {
90102
return;
91103
}

‎packages/connection-storage/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
"@mongodb-js/compass-user-data": "^0.1.9",
5757
"@mongodb-js/compass-utils": "^0.5.5",
5858
"bson": "^6.2.0",
59+
"electron": "^25.9.6",
5960
"hadron-ipc": "^3.2.4",
6061
"keytar": "^7.9.0",
6162
"lodash": "^4.17.21",

‎packages/connection-storage/src/connection-storage.spec.ts‎

Lines changed: 255 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import os from 'os';
66
import { UUID } from 'bson';
77
import { sortBy } from 'lodash';
88

9-
import { ConnectionStorage } from './connection-storage';
9+
import {
10+
ConnectionStorage,
11+
type ConnectionWithLegacyProps,
12+
} from './connection-storage';
1013
import type { ConnectionInfo } from './connection-info';
1114
import Sinon from 'sinon';
1215

@@ -33,14 +36,23 @@ function getConnectionInfo(props: Partial<ConnectionInfo> = {}) {
3336

3437
async function writeFakeConnection(
3538
tmpDir: string,
36-
connection: { connectionInfo: ConnectionInfo }
39+
connection: ConnectionWithLegacyProps
3740
) {
38-
const filePath = getConnectionFilePath(tmpDir, connection.connectionInfo.id);
41+
const _id = connection._id || connection.connectionInfo?.id;
42+
if (!_id) {
43+
throw new Error('Connection should have _id or connectionInfo.id');
44+
}
45+
const filePath = getConnectionFilePath(tmpDir, _id);
3946
const connectionsDir = path.dirname(filePath);
4047
await fs.mkdir(connectionsDir, { recursive: true });
4148
await fs.writeFile(filePath, JSON.stringify(connection));
4249
}
4350

51+
async function readConnection(tmpDir: string, id: string) {
52+
const content = await fs.readFile(getConnectionFilePath(tmpDir, id), 'utf-8');
53+
return JSON.parse(content);
54+
}
55+
4456
const maxAllowedConnections = 10;
4557

4658
describe('ConnectionStorage', function () {
@@ -75,6 +87,238 @@ describe('ConnectionStorage', function () {
7587
}
7688
});
7789

90+
describe('migrateToSafeStorage', function () {
91+
let sandbox: Sinon.SinonSandbox;
92+
beforeEach(function () {
93+
sandbox = Sinon.createSandbox();
94+
});
95+
96+
afterEach(function () {
97+
sandbox.restore();
98+
});
99+
100+
context('does not migrate connections', function () {
101+
it('when there are no connections', async function () {
102+
await ConnectionStorage.migrateToSafeStorage();
103+
const connections = await ConnectionStorage.loadAll();
104+
expect(connections).to.deep.equal([]);
105+
});
106+
107+
it('when there are only legacy connections', async function () {
108+
await writeFakeConnection(tmpDir, connection1270);
109+
110+
const encryptSecretsSpy = sandbox.spy(
111+
ConnectionStorage,
112+
'encryptSecrets' as any
113+
);
114+
const getKeytarCredentialsSpy = sandbox.spy(
115+
ConnectionStorage,
116+
'getKeytarCredentials' as any
117+
);
118+
119+
await ConnectionStorage.migrateToSafeStorage();
120+
121+
expect(
122+
encryptSecretsSpy.called,
123+
'it does not try to encrypt'
124+
).to.be.false;
125+
expect(
126+
getKeytarCredentialsSpy.called,
127+
'it does not try to get secrets from keychain'
128+
).to.be.false;
129+
});
130+
131+
it('when there are only migrated connections', async function () {
132+
const connectionInfo1 = getConnectionInfo();
133+
const connectionInfo2 = getConnectionInfo();
134+
135+
await writeFakeConnection(tmpDir, {
136+
_id: connectionInfo1.id,
137+
connectionInfo: connectionInfo1,
138+
version: 1,
139+
});
140+
await writeFakeConnection(tmpDir, {
141+
_id: connectionInfo2.id,
142+
connectionInfo: connectionInfo2,
143+
version: 1,
144+
});
145+
146+
const encryptSecretsSpy = sandbox.spy(
147+
ConnectionStorage,
148+
'encryptSecrets' as any
149+
);
150+
const getKeytarCredentialsSpy = sandbox.spy(
151+
ConnectionStorage,
152+
'getKeytarCredentials' as any
153+
);
154+
155+
await ConnectionStorage.migrateToSafeStorage();
156+
157+
expect(
158+
encryptSecretsSpy.called,
159+
'it does not try to encrypt'
160+
).to.be.false;
161+
expect(
162+
getKeytarCredentialsSpy.called,
163+
'it does not try to get secrets from keychain'
164+
).to.be.false;
165+
166+
const expectedConnection1 = await readConnection(
167+
tmpDir,
168+
connectionInfo1.id
169+
);
170+
const expectedConnection2 = await readConnection(
171+
tmpDir,
172+
connectionInfo2.id
173+
);
174+
175+
expect(expectedConnection1).to.deep.equal({
176+
_id: connectionInfo1.id,
177+
connectionInfo: connectionInfo1,
178+
version: 1,
179+
});
180+
181+
expect(expectedConnection2).to.deep.equal({
182+
_id: connectionInfo2.id,
183+
connectionInfo: connectionInfo2,
184+
version: 1,
185+
});
186+
});
187+
});
188+
189+
context('migrates connections', function () {
190+
it('when there are connections and secrets in keychain', async function () {
191+
const connectionInfo1 = getConnectionInfo();
192+
const connectionInfo2 = getConnectionInfo();
193+
await writeFakeConnection(tmpDir, {
194+
connectionInfo: connectionInfo1,
195+
});
196+
await writeFakeConnection(tmpDir, {
197+
connectionInfo: connectionInfo2,
198+
});
199+
200+
// Keytar stub
201+
sandbox.stub(ConnectionStorage, 'getKeytarCredentials' as any).returns({
202+
[connectionInfo1.id]: {
203+
password: 'password1',
204+
},
205+
[connectionInfo2.id]: {
206+
password: 'password2',
207+
},
208+
});
209+
210+
// safeStorage.encryptString stub
211+
sandbox
212+
.stub(ConnectionStorage, 'encryptSecrets' as any)
213+
.returns('encrypted-password');
214+
215+
await ConnectionStorage.migrateToSafeStorage();
216+
217+
const expectedConnection1 = await readConnection(
218+
tmpDir,
219+
connectionInfo1.id
220+
);
221+
const expectedConnection2 = await readConnection(
222+
tmpDir,
223+
connectionInfo2.id
224+
);
225+
226+
expect(expectedConnection1).to.deep.equal({
227+
_id: connectionInfo1.id,
228+
connectionInfo: connectionInfo1,
229+
connectionSecrets: 'encrypted-password',
230+
version: 1,
231+
});
232+
expect(expectedConnection2).to.deep.equal({
233+
_id: connectionInfo2.id,
234+
connectionInfo: connectionInfo2,
235+
connectionSecrets: 'encrypted-password',
236+
version: 1,
237+
});
238+
});
239+
it('when there are connections and no secrets in keychain', async function () {
240+
const connectionInfo1 = getConnectionInfo();
241+
const connectionInfo2 = getConnectionInfo();
242+
await writeFakeConnection(tmpDir, {
243+
connectionInfo: connectionInfo1,
244+
});
245+
await writeFakeConnection(tmpDir, {
246+
connectionInfo: connectionInfo2,
247+
});
248+
249+
// Keytar fake
250+
sandbox
251+
.stub(ConnectionStorage, 'getKeytarCredentials' as any)
252+
.returns({});
253+
254+
// Since there're no secrets in keychain, we do not expect to call safeStorage.encryptString
255+
// and connection.connectionSecrets should be undefined
256+
257+
await ConnectionStorage.migrateToSafeStorage();
258+
259+
const expectedConnection1 = await readConnection(
260+
tmpDir,
261+
connectionInfo1.id
262+
);
263+
const expectedConnection2 = await readConnection(
264+
tmpDir,
265+
connectionInfo2.id
266+
);
267+
268+
expect(expectedConnection1).to.deep.equal({
269+
_id: connectionInfo1.id,
270+
connectionInfo: connectionInfo1,
271+
version: 1,
272+
});
273+
expect(expectedConnection2).to.deep.equal({
274+
_id: connectionInfo2.id,
275+
connectionInfo: connectionInfo2,
276+
version: 1,
277+
});
278+
});
279+
280+
it('when there are any unmigrated connections', async function () {
281+
const connectionInfo1 = getConnectionInfo();
282+
const connectionInfo2 = getConnectionInfo();
283+
await writeFakeConnection(tmpDir, {
284+
_id: connectionInfo1.id,
285+
connectionInfo: connectionInfo1,
286+
version: 1,
287+
});
288+
await writeFakeConnection(tmpDir, {
289+
connectionInfo: connectionInfo2,
290+
});
291+
292+
// Keytar stub
293+
sandbox
294+
.stub(ConnectionStorage, 'getKeytarCredentials' as any)
295+
.returns({});
296+
297+
await ConnectionStorage.migrateToSafeStorage();
298+
299+
const expectedConnection1 = await readConnection(
300+
tmpDir,
301+
connectionInfo1.id
302+
);
303+
const expectedConnection2 = await readConnection(
304+
tmpDir,
305+
connectionInfo2.id
306+
);
307+
308+
expect(expectedConnection1).to.deep.equal({
309+
_id: connectionInfo1.id,
310+
connectionInfo: connectionInfo1,
311+
version: 1,
312+
});
313+
expect(expectedConnection2).to.deep.equal({
314+
_id: connectionInfo2.id,
315+
connectionInfo: connectionInfo2,
316+
version: 1,
317+
});
318+
});
319+
});
320+
});
321+
78322
describe('loadAll', function () {
79323
it('should load an empty array with no connections', async function () {
80324
const connections = await ConnectionStorage.loadAll();
@@ -463,27 +707,12 @@ describe('ConnectionStorage', function () {
463707
});
464708

465709
it('returns true if there are favorite legacy connections', async function () {
466-
const _id = new UUID().toString();
467-
468-
// Save a legacy connection (connection without connectionInfo)
469-
const filePath = getConnectionFilePath(tmpDir, _id);
470-
// As we are saving a legacy connection here, we can not use Storage.save as
471-
// it requries connectionInfo. And the internals of fs ensure the subdir exists,
472-
// here we are creating it manually.
473-
await fs.mkdir(path.join(tmpDir, 'Connections'), { recursive: true });
474-
await fs.writeFile(
475-
filePath,
476-
JSON.stringify({
477-
_id,
478-
isFavorite: true,
479-
name: 'Local 1',
480-
hosts: [{ host: 'localhost', port: 27017 }],
481-
readPreference: 'primary',
482-
port: 27017,
483-
hostname: 'localhost',
484-
})
485-
);
486-
710+
// Write a legacy connection (connection without connectionInfo, which is favorite and has a name)
711+
await writeFakeConnection(tmpDir, {
712+
_id: new UUID().toString(),
713+
isFavorite: true,
714+
name: 'Local 1',
715+
});
487716
const getLegacyConnections =
488717
await ConnectionStorage.getLegacyConnections();
489718
expect(getLegacyConnections).to.deep.equal([{ name: 'Local 1' }]);
@@ -583,18 +812,8 @@ describe('ConnectionStorage', function () {
583812
});
584813

585814
describe('supports connections from older version of compass', function () {
586-
const storeConnection = async (connection: any) => {
587-
const connectionFolder = path.join(tmpDir, 'Connections');
588-
const connectionPath = path.join(
589-
connectionFolder,
590-
`${connection._id}.json`
591-
);
592-
await fs.mkdir(connectionFolder, { recursive: true });
593-
await fs.writeFile(connectionPath, JSON.stringify(connection));
594-
};
595-
596815
it('correctly identifies connection as legacy connection', async function () {
597-
await storeConnection(connection1270);
816+
await writeFakeConnection(tmpDir, connection1270);
598817
const expectedConnection = await ConnectionStorage.load({
599818
id: connection1270._id,
600819
});
@@ -612,7 +831,7 @@ describe('ConnectionStorage', function () {
612831

613832
for (const version in connections) {
614833
const connection = connections[version];
615-
await storeConnection(connection);
834+
await writeFakeConnection(tmpDir, connection);
616835
const expectedConnection = await ConnectionStorage.load({
617836
id: connection._id,
618837
});

0 commit comments

Comments
 (0)