fix: check secretStorageLocation instead of storageLocation (#583)

* fix: check secretStorageLocation instead of storageLocation * feat: perform a second attempt to migrate connections
mongodb-js · Aug 23, 2023 · d1ba961 · d1ba961
1 parent 801c9b7
commit d1ba961
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 19 deletions.
diff --git a/src/connectionController.ts b/src/connectionController.ts
@@ -164,7 +164,7 @@ export default class ConnectionController {
         (ids, [connectionId, connectionInfo]) => {
           if (
             connectionInfo.secretStorageLocation ===
-            SecretStorageLocation.Keytar
+            SecretStorageLocation.KeytarSecondAttempt
           ) {
             return [...ids, connectionId];
           }
@@ -175,7 +175,9 @@ export default class ConnectionController {
 
     const hasConnectionsThatDidNotMigrateEarlier =
       !!globalAndWorkspaceConnections.some(
-        ([, connectionInfo]) => !connectionInfo.storageLocation
+        ([, connectionInfo]) =>
+          !connectionInfo.secretStorageLocation ||
+          connectionInfo.secretStorageLocation === 'vscode.Keytar'
       );
 
     if (hasConnectionsThatDidNotMigrateEarlier) {
@@ -203,7 +205,7 @@ export default class ConnectionController {
           this._connections[connectionId] = connectionInfoWithSecrets;
           const connectionSecretsInKeytar =
             connectionInfoWithSecrets.secretStorageLocation ===
-            SecretStorageLocation.Keytar;
+            SecretStorageLocation.KeytarSecondAttempt;
           if (
             connectionSecretsInKeytar &&
             !connectionIdsThatDidNotMigrateEarlier.includes(connectionId)
@@ -229,7 +231,9 @@ export default class ConnectionController {
       loaded_connections: loadedConnections.length,
       connections_with_secrets_in_keytar: loadedConnections.filter(
         (connection) =>
-          connection.secretStorageLocation === SecretStorageLocation.Keytar
+          connection.secretStorageLocation === SecretStorageLocation.Keytar ||
+          connection.secretStorageLocation ===
+            SecretStorageLocation.KeytarSecondAttempt
       ).length,
       connections_with_secrets_in_secret_storage: loadedConnections.filter(
         (connection) =>
@@ -265,7 +269,10 @@ export default class ConnectionController {
         );
       }
 
-      if (!connectionInfo.secretStorageLocation) {
+      if (
+        !connectionInfo.secretStorageLocation ||
+        connectionInfo.secretStorageLocation === 'vscode.Keytar'
+      ) {
         return await this._migrateConnectionWithKeytarSecrets(
           connectionInfo as StoreConnectionInfoWithConnectionOptions
         );
@@ -274,7 +281,8 @@ export default class ConnectionController {
       // We tried migrating this connection earlier but failed because Keytar was not
       // available. So we return simply the connection without secrets.
       if (
-        connectionInfo.secretStorageLocation === SecretStorageLocation.Keytar
+        connectionInfo.secretStorageLocation ===
+        SecretStorageLocation.KeytarSecondAttempt
       ) {
         return connectionInfo as MigratedStoreConnectionInfoWithConnectionOptions;
       }
@@ -322,7 +330,7 @@ export default class ConnectionController {
       return await this._storageController.saveConnection<MigratedStoreConnectionInfoWithConnectionOptions>(
         {
           ...savedConnectionInfo,
-          secretStorageLocation: SecretStorageLocation.Keytar,
+          secretStorageLocation: SecretStorageLocation.KeytarSecondAttempt,
         }
       );
     }

diff --git a/src/storage/storageController.ts b/src/storage/storageController.ts
@@ -34,11 +34,13 @@ export type ConnectionsFromStorage = {
 
 export const SecretStorageLocation = {
   Keytar: 'vscode.Keytar',
+  KeytarSecondAttempt: 'vscode.KeytarSecondAttempt',
   SecretStorage: 'vscode.SecretStorage',
 } as const;
 
 export type SecretStorageLocationType =
   | typeof SecretStorageLocation.Keytar
+  | typeof SecretStorageLocation.KeytarSecondAttempt
   | typeof SecretStorageLocation.SecretStorage;
 
 interface StorageVariableContents {

diff --git a/src/test/suite/connectionController.test.ts b/src/test/suite/connectionController.test.ts
@@ -1464,7 +1464,7 @@ suite('Connection Controller Test Suite', function () {
           );
           assert.strictEqual(
             updatedConnection.secretStorageLocation,
-            SecretStorageLocation.Keytar
+            SecretStorageLocation.KeytarSecondAttempt
           );
         });
 
@@ -1661,6 +1661,16 @@ suite('Connection Controller Test Suite', function () {
           },
           'random-connection-2': {
             id: 'random-connection-2',
+            name: 'localhost:27017',
+            storageLocation: 'GLOBAL',
+            secretStorageLocation: SecretStorageLocation.KeytarSecondAttempt,
+            connectionOptions: {
+              connectionString:
+                'mongodb://localhost:27017/?readPreference=primary&ssl=false',
+            },
+          },
+          'random-connection-3': {
+            id: 'random-connection-3',
             name: 'localhost:27018',
             storageLocation: 'GLOBAL',
             connectionOptions: {
@@ -1676,7 +1686,7 @@ suite('Connection Controller Test Suite', function () {
         (connectionInfo) =>
           Promise.resolve({
             ...connectionInfo,
-            secretStorageLocation: SecretStorageLocation.Keytar,
+            secretStorageLocation: SecretStorageLocation.KeytarSecondAttempt,
           } as any)
       );
       const trackStub = testSandbox.stub(
@@ -1691,16 +1701,16 @@ suite('Connection Controller Test Suite', function () {
       // Notified to user
       assert.strictEqual(showInformationMessageStub.calledOnce, true);
       assert.deepStrictEqual(showInformationMessageStub.lastCall.args, [
-        keytarMigrationFailedMessage(1),
+        keytarMigrationFailedMessage(2),
       ]);
 
       // Tracked
       assert.strictEqual(trackStub.calledOnce, true);
       assert.deepStrictEqual(trackStub.lastCall.args, [
         {
-          saved_connections: 2,
-          loaded_connections: 2,
-          connections_with_failed_keytar_migration: 1,
+          saved_connections: 3,
+          loaded_connections: 3,
+          connections_with_failed_keytar_migration: 2,
         },
       ]);
     });
@@ -1719,7 +1729,7 @@ suite('Connection Controller Test Suite', function () {
             id: 'random-connection-1',
             name: 'localhost:27017',
             storageLocation: 'GLOBAL',
-            secretStorageLocation: SecretStorageLocation.Keytar,
+            secretStorageLocation: SecretStorageLocation.KeytarSecondAttempt,
             connectionOptions: {
               connectionString:
                 'mongodb://localhost:27017/?readPreference=primary&ssl=false',
@@ -1729,7 +1739,7 @@ suite('Connection Controller Test Suite', function () {
             id: 'random-connection-2',
             name: 'localhost:27018',
             storageLocation: 'GLOBAL',
-            secretStorageLocation: SecretStorageLocation.Keytar,
+            secretStorageLocation: SecretStorageLocation.KeytarSecondAttempt,
             connectionOptions: {
               connectionString:
                 'mongodb://localhost:27018/?readPreference=primary&ssl=false',
@@ -1743,7 +1753,7 @@ suite('Connection Controller Test Suite', function () {
         (connectionInfo) =>
           Promise.resolve({
             ...connectionInfo,
-            secretStorageLocation: SecretStorageLocation.Keytar,
+            secretStorageLocation: SecretStorageLocation.KeytarSecondAttempt,
           } as any)
       );
       const trackStub = testSandbox.stub(
@@ -1802,6 +1812,16 @@ suite('Connection Controller Test Suite', function () {
                 'mongodb://localhost:27018/?readPreference=primary&ssl=false',
             },
           },
+          'random-connection-4': {
+            id: 'random-connection-4',
+            name: 'localhost:27018',
+            storageLocation: 'GLOBAL',
+            secretStorageLocation: SecretStorageLocation.KeytarSecondAttempt,
+            connectionOptions: {
+              connectionString:
+                'mongodb://localhost:27018/?readPreference=primary&ssl=false',
+            },
+          },
         } as any;
       });
       testSandbox.replace(
@@ -1824,10 +1844,10 @@ suite('Connection Controller Test Suite', function () {
       assert.strictEqual(trackStub.calledOnce, true);
       assert.deepStrictEqual(trackStub.lastCall.args, [
         {
-          connections_with_secrets_in_keytar: 1,
+          connections_with_secrets_in_keytar: 2,
           connections_with_secrets_in_secret_storage: 2,
-          saved_connections: 3,
-          loaded_connections: 3,
+          saved_connections: 4,
+          loaded_connections: 4,
         },
       ]);
     });