clean up

Author: BTreston

libs/services/state-service/stateMigration.service.spec.ts

@@ -617,29 +617,48 @@ describe("StateMigrationService", () => {
         secureStorage.store.set(`${userId}_oktaToken`, "okta-token");
       });
 
-      it("calls migrateKeytarPassword for each old {userId}_* key", async () => {
+      it("does not call migrateKeytarPassword for keys that are already readable", async () => {
+        // beforeEach seeds ldapPassword, entraIdKey, entraKey, oktaToken directly so
+        // secureStorageService.get returns a value — migrateKeytarPassword should be skipped
+        // for those keys.
         await svc.migrate();
 
         const calledKeys = passwords.migrateKeytarPassword.mock.calls.map(
           (c: [string, string]) => c[1],
         );
-        expect(calledKeys).toEqual(
-          expect.arrayContaining([
-            `${userId}_ldapPassword`,
-            `${userId}_entraIdKey`,
-            `${userId}_entraKey`,
-            `${userId}_oktaToken`,
-          ]),
+        expect(calledKeys).not.toContain(`${userId}_ldapPassword`);
+        expect(calledKeys).not.toContain(`${userId}_entraIdKey`);
+        expect(calledKeys).not.toContain(`${userId}_oktaToken`);
+      });
+
+      it("calls migrateKeytarPassword when old key is unreadable (UTF-8 keytar blob)", async () => {
+        // Remove the readable value to simulate a key that exists in the credential store
+        // but cannot be decoded by desktop_core (UTF-8 blob).
+        secureStorage.store.delete(`${userId}_ldapPassword`);
+
+        await svc.migrate();
+
+        const calledKeys = passwords.migrateKeytarPassword.mock.calls.map(
+          (c: [string, string]) => c[1],
         );
+        expect(calledKeys).toContain(`${userId}_ldapPassword`);
       });
 
-      it("copies old keys to flat keys via secureStorageService", async () => {
+      it("copies old keys to flat keys when the flat key is absent", async () => {
         await svc.migrate();
 
         expect(secureStorage.store.get(SecureStorageKeys.ldap)).toBe("ldap-pass");
         expect(secureStorage.store.get(SecureStorageKeys.okta)).toBe("okta-token");
       });
 
+      it("does not overwrite a flat key that already has a current value", async () => {
+        secureStorage.store.set(SecureStorageKeys.ldap, "current-ldap-pass");
+
+        await svc.migrate();
+
+        expect(secureStorage.store.get(SecureStorageKeys.ldap)).toBe("current-ldap-pass");
+      });
+
       it("prefers _entraIdKey over _entraKey and keeps both old keys intact", async () => {
         await svc.migrate();
 

libs/services/state-service/stateMigration.service.ts

@@ -275,8 +275,9 @@ export class StateMigrationService {
    *
    * Strategy: call migrateKeytarPassword on each old {userId}_* key first, which re-encodes
    * the blob from UTF-8 to UTF-16 in-place (making it readable by desktop_core). Then copy
-   * to the flat key via secureStorageService and remove the old key. On macOS/Linux,
-   * migrateKeytarPassword is a no-op so the get/save/remove path handles everything.
+   * to the flat key only if it is not already set — preserving any current value written by
+   * the running app. Old keys are kept intact.
+   * On macOS/Linux, migrateKeytarPassword is a no-op so the get/save path handles everything.
    */
   protected async migrateStateFrom6To7(): Promise<void> {
     const clientId = await this.storageService.get<string>("activeUserId");
@@ -297,18 +298,18 @@ export class StateMigrationService {
 
       const written = new Set<string>();
       for (const { old: oldKey, new: newKey } of oldSecretKeys) {
-        // Re-encode the UTF-8 keytar blob to UTF-16 in-place so desktop_core can read it.
-        await passwords.migrateKeytarPassword(SECURE_STORAGE_SERVICE_NAME, oldKey);
-
-        if (written.has(newKey)) {
-          continue;
-        }
-
-        const value = await this.secureStorageService.get<string>(oldKey);
-        if (value != null) {
-          await this.secureStorageService.save(newKey, value);
-          written.add(newKey);
+        const existing = await this.secureStorageService.get<string>(newKey);
+        if (existing == null) {
+          let value = await this.secureStorageService.get<string>(oldKey);
+          if (value == null) {
+            await passwords.migrateKeytarPassword(SECURE_STORAGE_SERVICE_NAME, oldKey);
+            value = await this.secureStorageService.get<string>(oldKey);
+          }
+          if (value != null && !written.has(newKey)) {
+            await this.secureStorageService.save(newKey, value);
+          }
         }
+        written.add(newKey);
       }
     }