From bf081b2f7b21422334bd485f8f5713ff7025ffa0 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:51:31 -0600 Subject: [PATCH] [PM-30812] Update userKey rotation to use saltForUser (#18697) --- .../user-key-rotation.service.spec.ts | 33 ++++++++++--------- .../key-rotation/user-key-rotation.service.ts | 19 +++++++---- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts index c0b734f17cc..a2330025c92 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.spec.ts +++ b/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; let mockWebauthnLoginAdminService: MockProxy; let mockLogService: MockProxy; - let mockVaultTimeoutService: MockProxy; + let mockLogoutService: MockProxy; let mockDialogService: MockProxy; let mockToastService: MockProxy; let mockI18nService: MockProxy; @@ -284,6 +286,7 @@ describe("KeyRotationService", () => { let mockKdfConfigService: MockProxy; let mockSdkClientFactory: MockProxy; let mockSecurityStateService: MockProxy; + let mockMasterPasswordService: MockProxy; 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(); mockWebauthnLoginAdminService = mock(); mockLogService = mock(); - mockVaultTimeoutService = mock(); + mockLogoutService = mock(); mockToastService = mock(); mockI18nService = mock(); mockDialogService = mock(); @@ -354,6 +359,7 @@ describe("KeyRotationService", () => { }, } as BitwardenClient); mockSecurityStateService = mock(); + mockMasterPasswordService = mock(); 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; 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, diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts index b9bd23b12de..68253a4a35d 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts +++ b/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 { // 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 { // 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(