[PM-30812] Update userKey rotation to use saltForUser (#18697)

apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts

@@ -1,7 +1,8 @@
 import { mock, MockProxy } from "jest-mock-extended";
-import { BehaviorSubject } from "rxjs";
+import { BehaviorSubject, of } from "rxjs";
 
 import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/admin-console/common";
+import { LogoutService } from "@bitwarden/auth/common";
 import { Account } from "@bitwarden/common/auth/abstractions/account.service";
 import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
@@ -12,6 +13,8 @@ import {
   EncString,
 } from "@bitwarden/common/key-management/crypto/models/enc-string";
 import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
+import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
+import { MasterPasswordSalt } from "@bitwarden/common/key-management/master-password/types/master-password.types";
 import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service";
 import {
   SignedPublicKey,
@@ -21,7 +24,6 @@ import {
   WrappedPrivateKey,
   WrappedSigningKey,
 } from "@bitwarden/common/key-management/types";
-import { VaultTimeoutService } from "@bitwarden/common/key-management/vault-timeout";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
@@ -276,7 +278,7 @@ describe("KeyRotationService", () => {
   let mockSyncService: MockProxy<SyncService>;
   let mockWebauthnLoginAdminService: MockProxy<WebauthnLoginAdminService>;
   let mockLogService: MockProxy<LogService>;
-  let mockVaultTimeoutService: MockProxy<VaultTimeoutService>;
+  let mockLogoutService: MockProxy<LogoutService>;
   let mockDialogService: MockProxy<DialogService>;
   let mockToastService: MockProxy<ToastService>;
   let mockI18nService: MockProxy<I18nService>;
@@ -284,6 +286,7 @@ describe("KeyRotationService", () => {
   let mockKdfConfigService: MockProxy<KdfConfigService>;
   let mockSdkClientFactory: MockProxy<SdkClientFactory>;
   let mockSecurityStateService: MockProxy<SecurityStateService>;
+  let mockMasterPasswordService: MockProxy<MasterPasswordServiceAbstraction>;
 
   const mockUser = {
     id: "mockUserId" as UserId,
@@ -293,6 +296,8 @@ describe("KeyRotationService", () => {
     }),
   };
 
+  const mockUserSalt = "usersalt";
+
   const mockTrustedPublicKeys = [Utils.fromUtf8ToArray("test-public-key")];
 
   const mockMakeKeysForUserCryptoV2 = jest.fn();
@@ -337,7 +342,7 @@ describe("KeyRotationService", () => {
     mockSyncService = mock<SyncService>();
     mockWebauthnLoginAdminService = mock<WebauthnLoginAdminService>();
     mockLogService = mock<LogService>();
-    mockVaultTimeoutService = mock<VaultTimeoutService>();
+    mockLogoutService = mock<LogoutService>();
     mockToastService = mock<ToastService>();
     mockI18nService = mock<I18nService>();
     mockDialogService = mock<DialogService>();
@@ -354,6 +359,7 @@ describe("KeyRotationService", () => {
       },
     } as BitwardenClient);
     mockSecurityStateService = mock<SecurityStateService>();
+    mockMasterPasswordService = mock<MasterPasswordServiceAbstraction>();
 
     keyRotationService = new TestUserKeyRotationService(
       mockApiService,
@@ -368,7 +374,7 @@ describe("KeyRotationService", () => {
       mockSyncService,
       mockWebauthnLoginAdminService,
       mockLogService,
-      mockVaultTimeoutService,
+      mockLogoutService,
       mockToastService,
       mockI18nService,
       mockDialogService,
@@ -377,6 +383,7 @@ describe("KeyRotationService", () => {
       mockKdfConfigService,
       mockSdkClientFactory,
       mockSecurityStateService,
+      mockMasterPasswordService,
     );
   });
 
@@ -391,10 +398,10 @@ describe("KeyRotationService", () => {
       value: Promise.resolve(),
       configurable: true,
     });
+    mockMasterPasswordService.saltForUser$.mockReturnValue(of(mockUserSalt as MasterPasswordSalt));
   });
 
   describe("rotateUserKeyMasterPasswordAndEncryptedData", () => {
-    let privateKey: BehaviorSubject<UserPrivateKey | null>;
     let keyPair: BehaviorSubject<{ privateKey: UserPrivateKey; publicKey: UserPublicKey }>;
 
     beforeEach(() => {
@@ -420,10 +427,6 @@ describe("KeyRotationService", () => {
 
       mockKeyService.getFingerprint.mockResolvedValue(["a", "b"]);
 
-      // Mock private key
-      privateKey = new BehaviorSubject("mockPrivateKey" as any);
-      mockKeyService.userPrivateKeyWithLegacySupport$.mockReturnValue(privateKey);
-
       keyPair = new BehaviorSubject({
         privateKey: "mockPrivateKey",
         publicKey: "mockPublicKey",
@@ -543,7 +546,7 @@ describe("KeyRotationService", () => {
       expect(spy).toHaveBeenCalledWith(
         mockUser.id,
         expect.any(PBKDF2KdfConfig),
-        mockUser.email,
+        mockUserSalt,
         expect.objectContaining({ version: 1 }),
         true,
       );
@@ -683,7 +686,7 @@ describe("KeyRotationService", () => {
           },
           signingKey: TEST_VECTOR_SIGNING_KEY_V2 as WrappedSigningKey,
           securityState: TEST_VECTOR_SECURITY_STATE_V2 as SignedSecurityState,
-        },
+        } as V2CryptographicStateParameters,
       );
       expect(mockGetV2RotatedAccountKeys).toHaveBeenCalled();
       expect(result).toEqual({
@@ -810,7 +813,7 @@ describe("KeyRotationService", () => {
           masterPasswordHash: "omitted",
           otp: undefined,
           authRequestAccessCode: undefined,
-        },
+        } as OrganizationUserResetPasswordWithIdRequest,
       ]);
       mockKeyService.makeMasterKey.mockResolvedValue(
         new SymmetricCryptoKey(new Uint8Array(32)) as MasterKey,
@@ -1122,7 +1125,7 @@ describe("KeyRotationService", () => {
       const cryptographicState = await keyRotationService.getCryptographicStateForUser(mockUser);
       expect(cryptographicState).toEqual({
         masterKeyKdfConfig: new PBKDF2KdfConfig(100000),
-        masterKeySalt: "mockemail", // the email is lowercased to become the salt
+        masterKeySalt: mockUserSalt,
         cryptographicStateParameters: {
           version: 1,
           userKey: TEST_VECTOR_USER_KEY_V1,
@@ -1138,7 +1141,7 @@ describe("KeyRotationService", () => {
       const cryptographicState = await keyRotationService.getCryptographicStateForUser(mockUser);
       expect(cryptographicState).toEqual({
         masterKeyKdfConfig: new PBKDF2KdfConfig(100000),
-        masterKeySalt: "mockemail", // the email is lowercased to become the salt
+        masterKeySalt: mockUserSalt,
         cryptographicStateParameters: {
           version: 2,
           userKey: TEST_VECTOR_USER_KEY_V2,

apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts

@@ -8,6 +8,7 @@ import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/a
 import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
 import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
 import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction";
+import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
 import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service";
 import {
   SignedPublicKey,
@@ -99,6 +100,7 @@ export class UserKeyRotationService {
     private kdfConfigService: KdfConfigService,
     private sdkClientFactory: SdkClientFactory,
     private securityStateService: SecurityStateService,
+    private masterPasswordService: MasterPasswordServiceAbstraction,
   ) {}
 
   /**
@@ -146,7 +148,7 @@ export class UserKeyRotationService {
     const { userKey: newUserKey, accountKeysRequest } = await this.getRotatedAccountKeysFlagged(
       user.id,
       masterKeyKdfConfig,
-      user.email,
+      masterKeySalt,
       currentCryptographicStateParameters,
       upgradeToV2FeatureFlagEnabled,
     );
@@ -300,7 +302,7 @@ export class UserKeyRotationService {
   protected async upgradeV1UserToV2UserAccountKeys(
     userId: UserId,
     kdfConfig: KdfConfig,
-    email: string,
+    masterKeySalt: string,
     cryptographicStateParameters: V1CryptographicStateParameters,
   ): Promise<V2UserCryptographicState> {
     // Initialize an SDK with the current cryptographic state
@@ -308,7 +310,7 @@ export class UserKeyRotationService {
     await sdk.crypto().initialize_user_crypto({
       userId: asUuid(userId),
       kdfParams: kdfConfig.toSdkConfig(),
-      email: email,
+      email: masterKeySalt,
       accountCryptographicState: {
         V1: {
           private_key: cryptographicStateParameters.publicKeyEncryptionKeyPair.wrappedPrivateKey,
@@ -328,7 +330,7 @@ export class UserKeyRotationService {
   protected async rotateV2UserAccountKeys(
     userId: UserId,
     kdfConfig: KdfConfig,
-    email: string,
+    masterKeySalt: string,
     cryptographicStateParameters: V2CryptographicStateParameters,
   ): Promise<V2UserCryptographicState> {
     // Initialize an SDK with the current cryptographic state
@@ -336,7 +338,7 @@ export class UserKeyRotationService {
     await sdk.crypto().initialize_user_crypto({
       userId: asUuid(userId),
       kdfParams: kdfConfig.toSdkConfig(),
-      email: email,
+      email: masterKeySalt,
       accountCryptographicState: {
         V2: {
           private_key: cryptographicStateParameters.publicKeyEncryptionKeyPair.wrappedPrivateKey,
@@ -598,8 +600,11 @@ export class UserKeyRotationService {
       this.kdfConfigService.getKdfConfig$(user.id),
       "KDF config",
     ))!;
-    // The master key salt used for deriving the masterkey always needs to be trimmed and lowercased.
-    const masterKeySalt = user.email.trim().toLowerCase();
+
+    const masterKeySalt = await this.firstValueFromOrThrow(
+      this.masterPasswordService.saltForUser$(user.id),
+      "Master key salt",
+    );
 
     // V1 and V2 users both have a user key and a private key
     const currentUserKey: UserKey = (await this.firstValueFromOrThrow(