From 609baece05a046f6679989e55f6e029dd57fb9ee Mon Sep 17 00:00:00 2001
From: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
Date: Mon, 14 Feb 2022 23:16:07 +1000
Subject: [PATCH] Clear stale everBeenUnlocked value from onDisk storage (#682)

* Add StateVersion.Four to remove old everBeenUnlocked key

* Save new state properly

* Add unit tests

* Fix linting
---
 common/src/enums/stateVersion.ts              |  3 +-
 common/src/services/stateMigration.service.ts | 20 +++++
 .../common/services/stateMigration.service.ts | 88 +++++++++++++++++++
 3 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 spec/common/services/stateMigration.service.ts

diff --git a/common/src/enums/stateVersion.ts b/common/src/enums/stateVersion.ts
index f966c323243..5aeb02e5409 100644
--- a/common/src/enums/stateVersion.ts
+++ b/common/src/enums/stateVersion.ts
@@ -2,5 +2,6 @@ export enum StateVersion {
   One = 1, // Original flat key/value pair store
   Two = 2, // Move to a typed State object
   Three = 3, // Fix migration of users' premium status
-  Latest = Three,
+  Four = 4, // Fix 'Never Lock' option by removing stale data
+  Latest = Four,
 }
diff --git a/common/src/services/stateMigration.service.ts b/common/src/services/stateMigration.service.ts
index cc40583cf35..f585b0cd533 100644
--- a/common/src/services/stateMigration.service.ts
+++ b/common/src/services/stateMigration.service.ts
@@ -158,6 +158,9 @@ export class StateMigrationService<
         case StateVersion.Two:
           await this.migrateStateFrom2To3();
           break;
+        case StateVersion.Three:
+          await this.migrateStateFrom3To4();
+          break;
       }
 
       currentStateVersion += 1;
@@ -474,6 +477,23 @@ export class StateMigrationService<
     await this.set(keys.global, globals);
   }
 
+  protected async migrateStateFrom3To4(): Promise<void> {
+    const authenticatedUserIds = await this.get<string[]>(keys.authenticatedAccounts);
+    await Promise.all(
+      authenticatedUserIds.map(async (userId) => {
+        const account = await this.get<TAccount>(userId);
+        if (account?.profile?.everBeenUnlocked != null) {
+          delete account.profile.everBeenUnlocked;
+          return this.set(userId, account);
+        }
+      })
+    );
+
+    const globals = await this.getGlobals();
+    globals.stateVersion = StateVersion.Four;
+    await this.set(keys.global, globals);
+  }
+
   protected get options(): StorageOptions {
     return { htmlStorageLocation: HtmlStorageLocation.Local };
   }
diff --git a/spec/common/services/stateMigration.service.ts b/spec/common/services/stateMigration.service.ts
new file mode 100644
index 00000000000..1f488b1246d
--- /dev/null
+++ b/spec/common/services/stateMigration.service.ts
@@ -0,0 +1,88 @@
+import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
+
+import { StorageService } from "jslib-common/abstractions/storage.service";
+
+import { StateMigrationService } from "jslib-common/services/stateMigration.service";
+
+import { StateFactory } from "jslib-common/factories/stateFactory";
+
+import { Account } from "jslib-common/models/domain/account";
+import { GlobalState } from "jslib-common/models/domain/globalState";
+
+import { StateVersion } from "jslib-common/enums/stateVersion";
+
+const userId = "USER_ID";
+
+describe("State Migration Service", () => {
+  let storageService: SubstituteOf<StorageService>;
+  let secureStorageService: SubstituteOf<StorageService>;
+  let stateFactory: SubstituteOf<StateFactory>;
+
+  let stateMigrationService: StateMigrationService;
+
+  beforeEach(() => {
+    storageService = Substitute.for<StorageService>();
+    secureStorageService = Substitute.for<StorageService>();
+    stateFactory = Substitute.for<StateFactory>();
+
+    stateMigrationService = new StateMigrationService(
+      storageService,
+      secureStorageService,
+      stateFactory
+    );
+  });
+
+  describe("StateVersion 3 to 4 migration", async () => {
+    beforeEach(() => {
+      const globalVersion3: Partial<GlobalState> = {
+        stateVersion: StateVersion.Three,
+      };
+
+      storageService.get("global", Arg.any()).resolves(globalVersion3);
+      storageService.get("authenticatedAccounts", Arg.any()).resolves([userId]);
+    });
+
+    it("clears everBeenUnlocked", async () => {
+      const accountVersion3: Account = {
+        profile: {
+          apiKeyClientId: null,
+          convertAccountToKeyConnector: null,
+          email: "EMAIL",
+          emailVerified: true,
+          everBeenUnlocked: true,
+          hasPremiumPersonally: false,
+          kdfIterations: 100000,
+          kdfType: 0,
+          keyHash: "KEY_HASH",
+          lastSync: "LAST_SYNC",
+          userId: userId,
+          usesKeyConnector: false,
+          forcePasswordReset: false,
+        },
+      };
+
+      const expectedAccountVersion4: Account = {
+        profile: {
+          ...accountVersion3.profile,
+        },
+      };
+      delete expectedAccountVersion4.profile.everBeenUnlocked;
+
+      storageService.get(userId, Arg.any()).resolves(accountVersion3);
+
+      await stateMigrationService.migrate();
+
+      storageService.received(1).save(userId, expectedAccountVersion4, Arg.any());
+    });
+
+    it("updates StateVersion number", async () => {
+      await stateMigrationService.migrate();
+
+      storageService.received(1).save(
+        "global",
+        Arg.is((globals: GlobalState) => globals.stateVersion === StateVersion.Four),
+        Arg.any()
+      );
+    });
+  });
+});