clear master password unlock state on Key Connector migration

libs/angular/src/services/jslib-services.module.ts

@@ -1154,6 +1154,7 @@ const safeProviders: SafeProvider[] = [
       RegisterSdkService,
       SecurityStateService,
       AccountCryptographicStateService,
+      InternalUserDecryptionOptionsServiceAbstraction,
     ],
   }),
   safeProvider({

libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts

@@ -1,6 +1,9 @@
 import { mock } from "jest-mock-extended";
 import { firstValueFrom, of, timeout, TimeoutError } from "rxjs";
 
+// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
+// eslint-disable-next-line no-restricted-imports
+import { InternalUserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
 import { SetKeyConnectorKeyRequest } from "@bitwarden/common/key-management/key-connector/models/set-key-connector-key.request";
@@ -53,6 +56,7 @@ describe("KeyConnectorService", () => {
   const registerSdkService = mock<RegisterSdkService>();
   const securityStateService = mock<SecurityStateService>();
   const accountCryptographicStateService = mock<AccountCryptographicStateService>();
+  const userDecryptionOptionsService = mock<InternalUserDecryptionOptionsServiceAbstraction>();
 
   let stateProvider: FakeStateProvider;
 
@@ -97,6 +101,7 @@ describe("KeyConnectorService", () => {
       registerSdkService,
       securityStateService,
       accountCryptographicStateService,
+      userDecryptionOptionsService,
     );
   });
 
@@ -265,7 +270,15 @@ describe("KeyConnectorService", () => {
         Utils.fromBufferToB64(masterKey.inner().encryptionKey),
       );
 
+      const mockUserDecryptionOptions = {
+        hasMasterPassword: true,
+        keyConnectorOption: undefined,
+      };
+
       jest.spyOn(apiService, "postUserKeyToKeyConnector").mockResolvedValue();
+      userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(
+        of(mockUserDecryptionOptions),
+      );
 
       // Act
       await keyConnectorService.migrateUser(keyConnectorUrl, mockUserId);
@@ -276,6 +289,22 @@ describe("KeyConnectorService", () => {
         keyConnectorRequest,
       );
       expect(apiService.postConvertToKeyConnector).toHaveBeenCalled();
+      expect(masterPasswordService.mock.clearMasterKeyHash).toHaveBeenCalledWith(mockUserId);
+      expect(masterPasswordService.mock.clearMasterPasswordUnlockData).toHaveBeenCalledWith(
+        mockUserId,
+      );
+      expect(userDecryptionOptionsService.userDecryptionOptionsById$).toHaveBeenCalledWith(
+        mockUserId,
+      );
+      expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith(
+        mockUserId,
+        {
+          hasMasterPassword: false,
+          keyConnectorOption: {
+            keyConnectorUrl,
+          },
+        },
+      );
     });
 
     it("should handle errors thrown during migration", async () => {
@@ -299,6 +328,10 @@ describe("KeyConnectorService", () => {
           keyConnectorUrl,
           keyConnectorRequest,
         );
+        // Verify state clearing operations were not called due to error
+        expect(masterPasswordService.mock.clearMasterKeyHash).not.toHaveBeenCalled();
+        expect(masterPasswordService.mock.clearMasterPasswordUnlockData).not.toHaveBeenCalled();
+        expect(userDecryptionOptionsService.setUserDecryptionOptionsById).not.toHaveBeenCalled();
       }
     });
   });

libs/common/src/key-management/key-connector/services/key-connector.service.ts

@@ -4,7 +4,10 @@ import { combineLatest, filter, firstValueFrom, map, Observable, of, switchMap }
 
 // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
 // eslint-disable-next-line no-restricted-imports
-import { LogoutReason } from "@bitwarden/auth/common";
+import {
+  InternalUserDecryptionOptionsServiceAbstraction,
+  LogoutReason,
+} from "@bitwarden/auth/common";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { NewSsoUserKeyConnectorConversion } from "@bitwarden/common/key-management/key-connector/models/new-sso-user-key-connector-conversion";
 // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
@@ -93,6 +96,7 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
     private registerSdkService: RegisterSdkService,
     private securityStateService: SecurityStateService,
     private accountCryptographicStateService: AccountCryptographicStateService,
+    private userDecryptionOptionsService: InternalUserDecryptionOptionsServiceAbstraction,
   ) {
     this.convertAccountRequired$ = accountService.activeAccount$.pipe(
       filter((account) => account != null),
@@ -143,6 +147,22 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
     await this.apiService.postConvertToKeyConnector();
 
     await this.setUsesKeyConnector(true, userId);
+
+    // Clear master password unlock from state
+    await this.masterPasswordService.clearMasterKeyHash(userId);
+    await this.masterPasswordService.clearMasterPasswordUnlockData(userId);
+
+    const userDecryptionOptions = await firstValueFrom(
+      this.userDecryptionOptionsService.userDecryptionOptionsById$(userId),
+    );
+    userDecryptionOptions.hasMasterPassword = false;
+    userDecryptionOptions.keyConnectorOption = {
+      keyConnectorUrl,
+    };
+    await this.userDecryptionOptionsService.setUserDecryptionOptionsById(
+      userId,
+      userDecryptionOptions,
+    );
   }
 
   // TODO: UserKey should be renamed to MasterKey and typed accordingly

libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts

@@ -196,6 +196,13 @@ export abstract class InternalMasterPasswordServiceAbstraction extends MasterPas
     userId: UserId,
   ): Promise<void>;
 
+  /**
+   * Clears the master password unlock data for the user.
+   * @param userId The user ID.
+   * @throws Error If the user ID is missing.
+   */
+  abstract clearMasterPasswordUnlockData(userId: UserId): Promise<void>;
+
   /**
    * An observable that emits the master password unlock data for the target user.
    * @param userId The user ID.

libs/common/src/key-management/master-password/services/fake-master-password.service.ts

@@ -124,6 +124,10 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA
     return this.mock.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
   }
 
+  clearMasterPasswordUnlockData(userId: UserId): Promise<void> {
+    return this.mock.clearMasterPasswordUnlockData(userId);
+  }
+
   masterPasswordUnlockData$(userId: UserId): Observable<MasterPasswordUnlockData | null> {
     return this.mock.masterPasswordUnlockData$(userId);
   }

libs/common/src/key-management/master-password/services/master-password.service.spec.ts

@@ -416,6 +416,36 @@ describe("MasterPasswordService", () => {
     );
   });
 
+  describe("clearMasterPasswordUnlockData", () => {
+    it("clears the master password unlock data from state", async () => {
+      const masterKeyWrappedUserKey = makeEncString().toSdk() as MasterKeyWrappedUserKey;
+      const masterPasswordUnlockData = new MasterPasswordUnlockData(
+        salt,
+        kdfPBKDF2,
+        masterKeyWrappedUserKey,
+      );
+      stateProvider.singleUser
+        .getFake(userId, MASTER_PASSWORD_UNLOCK_KEY)
+        .nextState(masterPasswordUnlockData.toJSON());
+
+      await sut.clearMasterPasswordUnlockData(userId);
+
+      const state = await firstValueFrom(
+        stateProvider.getUser(userId, MASTER_PASSWORD_UNLOCK_KEY).state$,
+      );
+      expect(state).toBeNull();
+    });
+
+    test.each([null as unknown as UserId, undefined as unknown as UserId])(
+      "throws when the provided userId is %s",
+      async (userId) => {
+        await expect(sut.clearMasterPasswordUnlockData(userId)).rejects.toThrow(
+          "userId is null or undefined.",
+        );
+      },
+    );
+  });
+
   describe("setLegacyMasterKeyFromUnlockData", () => {
     const password = "test-password";
 

libs/common/src/key-management/master-password/services/master-password.service.ts

@@ -338,6 +338,12 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr
       .update(() => masterPasswordUnlockData.toJSON());
   }
 
+  async clearMasterPasswordUnlockData(userId: UserId): Promise<void> {
+    assertNonNullish(userId, "userId");
+
+    await this.stateProvider.getUser(userId, MASTER_PASSWORD_UNLOCK_KEY).update(() => null);
+  }
+
   masterPasswordUnlockData$(userId: UserId): Observable<MasterPasswordUnlockData | null> {
     assertNonNullish(userId, "userId");