Auth/PM-7467- Fix Refresh token issues (#8757)

* PM-7467 - Login Strategy bug - VaultTimeoutSettings will be undefined before the account is activated unless you pass in user ids to retrieve the data. This resulted in refresh tokens always being set into secure storage regardless of a user's vault timeout settings (logout should translate to memory)

* PM-7467 - TokenSvc - Fix bug in getRefreshToken which would retrieve the user's refresh token from secure storage even if the user had changed their vault timeout setting to log out which moved the refresh token into memory. Includes a migration to remove the no longer required REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE state provider flag.

* PM-7467 - Per PR feedback, use IRREVERSIBLE for rollback.

Co-authored-by: Jake Fink <jfink@bitwarden.com>

* PM-7467 - fix tests

* PM-7467 - Fix migrator based on PR feedback.

* PM-7467 - Bump migration version

---------

Co-authored-by: Jake Fink <jfink@bitwarden.com>

libs/common/src/state-migrations/migrate.ts

@@ -54,6 +54,7 @@ import { SendMigrator } from "./migrations/54-move-encrypted-sends";
 import { MoveMasterKeyStateToProviderMigrator } from "./migrations/55-move-master-key-state-to-provider";
 import { AuthRequestMigrator } from "./migrations/56-move-auth-requests";
 import { CipherServiceMigrator } from "./migrations/57-move-cipher-service-to-state-provider";
+import { RemoveRefreshTokenMigratedFlagMigrator } from "./migrations/58-remove-refresh-token-migrated-state-provider-flag";
 import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key";
 import { MoveBiometricAutoPromptToAccount } from "./migrations/7-move-biometric-auto-prompt-to-account";
 import { MoveStateVersionMigrator } from "./migrations/8-move-state-version";
@@ -61,7 +62,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting
 import { MinVersionMigrator } from "./migrations/min-version";
 
 export const MIN_VERSION = 3;
-export const CURRENT_VERSION = 57;
+export const CURRENT_VERSION = 58;
 export type MinVersion = typeof MIN_VERSION;
 
 export function createMigrationBuilder() {
@@ -120,7 +121,8 @@ export function createMigrationBuilder() {
     .with(SendMigrator, 53, 54)
     .with(MoveMasterKeyStateToProviderMigrator, 54, 55)
     .with(AuthRequestMigrator, 55, 56)
-    .with(CipherServiceMigrator, 56, CURRENT_VERSION);
+    .with(CipherServiceMigrator, 56, 57)
+    .with(RemoveRefreshTokenMigratedFlagMigrator, 57, CURRENT_VERSION);
 }
 
 export async function currentVersion(

libs/common/src/state-migrations/migrations/58-remove-refresh-token-migrated-state-provider-flag.spec.ts

@@ -0,0 +1,72 @@
+import { MockProxy, any } from "jest-mock-extended";
+
+import { MigrationHelper } from "../migration-helper";
+import { mockMigrationHelper } from "../migration-helper.spec";
+import { IRREVERSIBLE } from "../migrator";
+
+import {
+  REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE,
+  RemoveRefreshTokenMigratedFlagMigrator,
+} from "./58-remove-refresh-token-migrated-state-provider-flag";
+
+// Represents data in state service pre-migration
+function preMigrationJson() {
+  return {
+    global: {
+      otherStuff: "otherStuff1",
+    },
+    authenticatedAccounts: ["user1", "user2", "user3"],
+
+    user_user1_token_refreshTokenMigratedToSecureStorage: true,
+    user_user2_token_refreshTokenMigratedToSecureStorage: false,
+  };
+}
+
+function rollbackJSON() {
+  return {
+    global: {
+      otherStuff: "otherStuff1",
+    },
+    authenticatedAccounts: ["user1", "user2", "user3"],
+  };
+}
+
+describe("RemoveRefreshTokenMigratedFlagMigrator", () => {
+  let helper: MockProxy<MigrationHelper>;
+  let sut: RemoveRefreshTokenMigratedFlagMigrator;
+
+  describe("migrate", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(preMigrationJson(), 57);
+      sut = new RemoveRefreshTokenMigratedFlagMigrator(57, 58);
+    });
+
+    it("should remove refreshTokenMigratedToSecureStorage from state provider for all accounts that have it", async () => {
+      await sut.migrate(helper);
+
+      expect(helper.removeFromUser).toHaveBeenCalledWith(
+        "user1",
+        REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE,
+      );
+      expect(helper.removeFromUser).toHaveBeenCalledWith(
+        "user2",
+        REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE,
+      );
+
+      expect(helper.removeFromUser).toHaveBeenCalledTimes(2);
+
+      expect(helper.removeFromUser).not.toHaveBeenCalledWith("user3", any());
+    });
+  });
+
+  describe("rollback", () => {
+    beforeEach(() => {
+      helper = mockMigrationHelper(rollbackJSON(), 58);
+      sut = new RemoveRefreshTokenMigratedFlagMigrator(57, 58);
+    });
+
+    it("should not add data back and throw IRREVERSIBLE error on call", async () => {
+      await expect(sut.rollback(helper)).rejects.toThrow(IRREVERSIBLE);
+    });
+  });
+});

libs/common/src/state-migrations/migrations/58-remove-refresh-token-migrated-state-provider-flag.ts

@@ -0,0 +1,34 @@
+import { KeyDefinitionLike, MigrationHelper } from "../migration-helper";
+import { IRREVERSIBLE, Migrator } from "../migrator";
+
+type ExpectedAccountType = NonNullable<unknown>;
+
+export const REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE: KeyDefinitionLike = {
+  key: "refreshTokenMigratedToSecureStorage", // matches KeyDefinition.key in DeviceTrustCryptoService
+  stateDefinition: {
+    name: "token", // matches StateDefinition.name in StateDefinitions
+  },
+};
+
+export class RemoveRefreshTokenMigratedFlagMigrator extends Migrator<57, 58> {
+  async migrate(helper: MigrationHelper): Promise<void> {
+    const accounts = await helper.getAccounts<ExpectedAccountType>();
+    async function migrateAccount(userId: string, account: ExpectedAccountType): Promise<void> {
+      const refreshTokenMigratedFlag = await helper.getFromUser(
+        userId,
+        REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE,
+      );
+
+      if (refreshTokenMigratedFlag != null) {
+        // Only delete the flag if it exists
+        await helper.removeFromUser(userId, REFRESH_TOKEN_MIGRATED_TO_SECURE_STORAGE);
+      }
+    }
+
+    await Promise.all([...accounts.map(({ userId, account }) => migrateAccount(userId, account))]);
+  }
+
+  async rollback(helper: MigrationHelper): Promise<void> {
+    throw IRREVERSIBLE;
+  }
+}