diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 55a2bee9b1a2..8e4880f67f00 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -770,7 +770,6 @@ export default class MainBackground { this.masterPasswordUnlockService = new DefaultMasterPasswordUnlockService( this.masterPasswordService, - this.keyService, this.logService, ); diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts index e0e0315ab12d..4e4d6eb94bcc 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts @@ -507,8 +507,6 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi ); await this.kdfConfigService.setKdfConfig(userId, kdfConfig); // [PM-23246] "Legacy" master key setting path - to be removed once unlock path migration is complete - await this.masterPasswordService.setMasterKey(masterKey, userId); - // [PM-23246] "Legacy" master key setting path - to be removed once unlock path migration is complete await this.masterPasswordService.setMasterKeyEncryptedUserKey( masterKeyEncryptedUserKey[1], userId, @@ -538,13 +536,6 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi await this.kdfConfigService.setKdfConfig(userId, kdfConfig); // TODO Remove master key memory state https://bitwarden.atlassian.net/browse/PM-23477 await this.masterPasswordService.setMasterKeyEncryptedUserKey(masterKeyWrappedUserKey, userId); - - // TODO Removed with https://bitwarden.atlassian.net/browse/PM-30676 - await this.masterPasswordService.setLegacyMasterKeyFromUnlockData( - newPassword, - masterPasswordUnlockData, - userId, - ); } /** diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts index 7974ee58214b..3bdb4689ce5c 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts @@ -410,10 +410,6 @@ describe("DefaultSetInitialPasswordService", () => { userDecryptionOptions, ); expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig); - expect(masterPasswordService.setMasterKey).toHaveBeenCalledWith( - credentials.newMasterKey, - userId, - ); expect(keyService.setUserKey).toHaveBeenCalledWith(masterKeyEncryptedUserKey[0], userId); }); @@ -644,10 +640,6 @@ describe("DefaultSetInitialPasswordService", () => { userDecryptionOptions, ); expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig); - expect(masterPasswordService.setMasterKey).toHaveBeenCalledWith( - credentials.newMasterKey, - userId, - ); expect(masterPasswordService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( masterKeyEncryptedUserKey[1], userId, @@ -1144,12 +1136,6 @@ describe("DefaultSetInitialPasswordService", () => { new EncString(sdkRegistrationResult.master_password_unlock.masterKeyWrappedUserKey), userId, ); - - expect(masterPasswordService.setLegacyMasterKeyFromUnlockData).toHaveBeenCalledWith( - credentials.newPassword, - MasterPasswordUnlockData.fromSdk(sdkRegistrationResult.master_password_unlock), - userId, - ); }); describe("input validation", () => { @@ -1433,11 +1419,6 @@ describe("DefaultSetInitialPasswordService", () => { new EncString(unlockData.masterKeyWrappedUserKey), userId, ); - expect(masterPasswordService.setLegacyMasterKeyFromUnlockData).toHaveBeenCalledWith( - credentials.newPassword, - unlockData, - userId, - ); }); describe("given resetPasswordAutoEnroll is false", () => { diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 5aae2e2855a1..a71b41d8032f 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -539,7 +539,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: ChangeKdfService, useClass: DefaultChangeKdfService, - deps: [ChangeKdfApiService, SdkService, KeyService, InternalMasterPasswordServiceAbstraction], + deps: [ChangeKdfApiService, SdkService], }), safeProvider({ provide: EncryptedMigrator, @@ -1195,7 +1195,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: MasterPasswordUnlockService, useClass: DefaultMasterPasswordUnlockService, - deps: [InternalMasterPasswordServiceAbstraction, KeyService, LogService], + deps: [InternalMasterPasswordServiceAbstraction, LogService], }), safeProvider({ provide: KeyConnectorServiceAbstraction, @@ -1413,7 +1413,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: ChangeKdfService, useClass: DefaultChangeKdfService, - deps: [ChangeKdfApiService, SdkService, KeyService, InternalMasterPasswordServiceAbstraction], + deps: [ChangeKdfApiService, SdkService], }), safeProvider({ provide: AuthRequestServiceAbstraction, diff --git a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts index 1c7cdfac6753..540451506a0e 100644 --- a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts +++ b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.spec.ts @@ -163,7 +163,6 @@ describe("LoginDecryptionOptionsComponent", () => { jest.mock("@bitwarden/common/platform/abstractions/sdk/sdk.service", () => ({ asUuid: (x: any) => x, })); - (Symbol as any).dispose = Symbol("dispose"); mockPrivateKey = "mock-private-key"; mockSignedPublicKey = "mock-signed-public-key"; diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts index 20374be2b09d..f86dc7f5452d 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts @@ -160,9 +160,6 @@ describe("AuthRequestLoginStrategy", () => { // Call logIn await authRequestLoginStrategy.logIn(credentials); - // setMasterKey and setMasterKeyHash should not be called - expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); - // setMasterKeyEncryptedUserKey, setUserKey, and setPrivateKey should still be called expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( tokenResponse.key, @@ -175,7 +172,7 @@ describe("AuthRequestLoginStrategy", () => { ); // trustDeviceIfRequired should be called - expect(deviceTrustService.trustDeviceIfRequired).not.toHaveBeenCalled(); + expect(deviceTrustService.trustDeviceIfRequired).toHaveBeenCalledWith(mockUserId); }); it("sets account cryptographic state when accountKeysResponseModel is present", async () => { diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts index 1f3eaf7c1646..47885f8e2ab9 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.ts @@ -86,25 +86,8 @@ export class AuthRequestLoginStrategy extends LoginStrategy { await this.masterPasswordService.setMasterKeyEncryptedUserKey(response.key, userId); } - if (authRequestCredentials.decryptedUserKey) { - await this.keyService.setUserKey(authRequestCredentials.decryptedUserKey, userId); - } else { - await this.trySetUserKeyWithMasterKey(userId); - - // Establish trust if required after setting user key - await this.deviceTrustService.trustDeviceIfRequired(userId); - } - } - - private async trySetUserKeyWithMasterKey(userId: UserId): Promise { - const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - if (masterKey) { - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( - masterKey, - userId, - ); - await this.keyService.setUserKey(userKey, userId); - } + await this.keyService.setUserKey(authRequestCredentials.decryptedUserKey, userId); + await this.deviceTrustService.trustDeviceIfRequired(userId); } protected override async setAccountCryptographicState( diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index 34f5a5301130..b41656f88992 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -17,7 +17,6 @@ import { } from "@bitwarden/common/auth/password-prelogin"; import { TwoFactorService } from "@bitwarden/common/auth/two-factor"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service"; @@ -200,76 +199,20 @@ describe("PasswordLoginStrategy", () => { ); }); - it("sets keys after a successful authentication", async () => { - const userKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; - - masterPasswordService.masterKeySubject.next(masterKey); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); - tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); - - await passwordLoginStrategy.logIn(credentials); - - expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(masterKey, userId); - expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( - tokenResponse.key, - userId, - ); - expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); - expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( - { V1: { private_key: tokenResponse.privateKey } }, - userId, - ); - }); - - it("uses master password unlock service when feature flag is enabled", async () => { - configService.getFeatureFlag.mockImplementation(async (flag: FeatureFlag) => { - if (flag === FeatureFlag.UseUnlockServiceForPasswordLogin) { - return true; - } - return false; - }); - - // Re-create he strategy and wait a bit to settle the feature flag - passwordLoginStrategy = new PasswordLoginStrategy( - new PasswordLoginStrategyData(), - passwordStrengthService, - policyService, - passwordPreloginService, - unlockService, - accountService, - masterPasswordService, - keyService, - encryptService, - apiService, - tokenService, - appIdService, - platformUtilsService, - messagingService, - logService, - stateService, - twoFactorService, - userDecryptionOptionsService, - billingAccountProfileStateService, - vaultTimeoutSettingsService, - kdfConfigService, - environmentService, - configService, - accountCryptographicStateService, - ); - + it("unlocks with the master password and sets account cryptographic state after a successful authentication", async () => { unlockService.unlockWithMasterPassword.mockResolvedValue(undefined); tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); await passwordLoginStrategy.logIn(credentials); - expect(configService.getFeatureFlag).toHaveBeenCalledWith( - FeatureFlag.UseUnlockServiceForPasswordLogin, - ); - expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); expect(unlockService.unlockWithMasterPassword).toHaveBeenCalledWith(userId, masterPassword); + expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled(); expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).not.toHaveBeenCalled(); expect(keyService.setUserKey).not.toHaveBeenCalled(); - expect(passwordLoginStrategy.exportCache().password.unlockServiceForPasswordLogin).toBe(true); + expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( + { V1: { private_key: tokenResponse.privateKey } }, + userId, + ); }); describe("makePasswordPreloginMasterKey", () => { diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index c698244d5c08..e11d8df89879 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -17,7 +17,6 @@ import { PasswordPreloginData, PasswordPreloginService, } from "@bitwarden/common/auth/password-prelogin"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { UserId } from "@bitwarden/common/types/guid"; @@ -38,8 +37,6 @@ export class PasswordLoginStrategyData implements LoginStrategyData { masterKey: MasterKey; /** The user's master password */ masterPassword: string; - /** Whether unlock service should be used for this login flow. */ - unlockServiceForPasswordLogin = false; /** * Tracks if the user needs to update their password due to * a password that does not meet an organization's master password policy. @@ -81,10 +78,6 @@ export class PasswordLoginStrategy extends LoginStrategy { } override async logIn(credentials: PasswordLoginCredentials): Promise { - const unlockServiceForPasswordLogin = await this.configService.getFeatureFlag( - FeatureFlag.UseUnlockServiceForPasswordLogin, - ); - const { email, masterPassword, twoFactor, preFetchedPreloginData } = credentials; const data = new PasswordLoginStrategyData(); @@ -95,7 +88,6 @@ export class PasswordLoginStrategy extends LoginStrategy { ); this.passwordPreloginService.clearCache(); data.masterPassword = masterPassword; - data.unlockServiceForPasswordLogin = unlockServiceForPasswordLogin; data.userEnteredEmail = email; // Hash the password early (before authentication) so we don't persist it in memory in plaintext @@ -123,37 +115,13 @@ export class PasswordLoginStrategy extends LoginStrategy { return result; } - protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { - if (!this.cache.value.unlockServiceForPasswordLogin) { - const { masterKey } = this.cache.value; - await this.masterPasswordService.setMasterKey(masterKey, userId); - } - } + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) {} protected override async setUserKey( response: IdentityTokenResponse, userId: UserId, ): Promise { - if (this.cache.value.unlockServiceForPasswordLogin) { - await this.unlockService.unlockWithMasterPassword(userId, this.cache.value.masterPassword); - } else { - // If migration is required, we won't have a user key to set yet. - if (this.encryptionKeyMigrationRequired(response)) { - return; - } - await this.masterPasswordService.setMasterKeyEncryptedUserKey(response.key, userId); - // Warning: State is accessed right after state is set. This could lead to a race condition - // in some cases where decryptUserKeyWithMasterKey will get a null encrypted user-key!! - // https://github.com/bitwarden/clients/tree/afc45ee0c8fc823301bb361b0dcac581eb0aff0c/libs/state#updating-state-with-update - const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - if (masterKey) { - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( - masterKey, - userId, - ); - await this.keyService.setUserKey(userKey, userId); - } - } + await this.unlockService.unlockWithMasterPassword(userId, this.cache.value.masterPassword); } protected override async setAccountCryptographicState( diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index 8027b24f88f3..105c00040016 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -10,7 +10,6 @@ import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/id import { IUserDecryptionOptionsServerResponse } from "@bitwarden/common/auth/models/response/user-decryption-options/user-decryption-options.response"; import { TwoFactorService } from "@bitwarden/common/auth/two-factor"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncryptedString } from "@bitwarden/common/key-management/crypto/models/enc-string"; @@ -33,7 +32,7 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { FakeAccountService, makeEncString, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; -import { DeviceKey, MasterKey, UserKey } from "@bitwarden/common/types/key"; +import { DeviceKey, UserKey } from "@bitwarden/common/types/key"; import { Argon2KdfConfig, KdfConfigService, KeyService } from "@bitwarden/key-management"; import { UnlockService } from "@bitwarden/unlock"; @@ -201,7 +200,6 @@ describe("SsoLoginStrategy", () => { await ssoLoginStrategy.logIn(credentials); - expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); expect(keyService.setUserKey).not.toHaveBeenCalled(); expect(accountCryptographicStateService.setAccountCryptographicState).not.toHaveBeenCalled(); }); @@ -490,30 +488,14 @@ describe("SsoLoginStrategy", () => { tokenResponse.apiUseKeyConnector = true; }); - it("gets and sets the master key if Key Connector is enabled and the user doesn't have a master password", async () => { - const masterKey = new SymmetricCryptoKey(new Uint8Array(64)) as MasterKey; - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - masterPasswordService.masterKeySubject.next(masterKey); - - await ssoLoginStrategy.logIn(credentials); - - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); - }); - - it("uses unlock service when SDK key connector feature flag is enabled", async () => { + it("uses unlock service", async () => { apiService.postIdentityToken.mockResolvedValue(tokenResponse); - configService.getFeatureFlag - .calledWith(FeatureFlag.UnlockKeyConnectorWithSdk) - .mockResolvedValue(true); - await ssoLoginStrategy.logIn(credentials); expect(unlockService.unlockWithKeyConnector).toHaveBeenCalledWith(userId, { url: keyConnectorUrl, keyConnectorKeyWrappedUserKey: tokenResponse.key!.encryptedString!, }); - expect(keyConnectorService.setMasterKeyFromUrl).not.toHaveBeenCalled(); }); it("converts new SSO user with no master password to Key Connector on first login", async () => { @@ -534,23 +516,6 @@ describe("SsoLoginStrategy", () => { ); }); - it("decrypts and sets the user key if Key Connector is enabled and the user doesn't have a master password", async () => { - const userKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; - const masterKey = new SymmetricCryptoKey(new Uint8Array(64)) as MasterKey; - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - masterPasswordService.masterKeySubject.next(masterKey); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); - - await ssoLoginStrategy.logIn(credentials); - - expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( - masterKey, - userId, - undefined, - ); - expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); - }); }); it("sets account cryptographic state when accountKeysResponseModel is present", async () => { diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index f9f52f848f95..1d65e158cb75 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -24,8 +24,6 @@ import { LoginStrategyData, LoginStrategy } from "./login.strategy"; export class SsoLoginStrategyData implements LoginStrategyData { tokenRequest: SsoTokenRequest; - /** Whether unlock service should be used for Key Connector in this login flow. */ - unlockServiceForKeyConnectorLogin = false; /** * User's entered email obtained pre-login. Present in most SSO flows, but not CLI + SSO Flow. */ @@ -87,9 +85,6 @@ export class SsoLoginStrategy extends LoginStrategy { async logIn(credentials: SsoLoginCredentials): Promise { const data = new SsoLoginStrategyData(); - data.unlockServiceForKeyConnectorLogin = await this.configService.getFeatureFlag( - FeatureFlag.UnlockKeyConnectorWithSdk, - ); data.orgId = credentials.orgId; data.userEnteredEmail = credentials.email; @@ -140,11 +135,6 @@ export class SsoLoginStrategy extends LoginStrategy { }, userId, ); - } else { - const keyConnectorUrl = this.getKeyConnectorUrl(tokenResponse); - if (!this.cache.value.unlockServiceForKeyConnectorLogin) { - await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl, userId); - } } } } @@ -195,10 +185,7 @@ export class SsoLoginStrategy extends LoginStrategy { const userDecryptionOptions = tokenResponse?.userDecryptionOptions; - if ( - tokenResponse.canUnlockWithKeyConnector() && - this.cache.value.unlockServiceForKeyConnectorLogin - ) { + if (tokenResponse.canUnlockWithKeyConnector()) { await this.unlockService.unlockWithKeyConnector( userId, tokenResponse.intoKeyConnectorUnlockData(), @@ -222,13 +209,6 @@ export class SsoLoginStrategy extends LoginStrategy { await this.trySetUserKeyWithDeviceKey(tokenResponse, userId); } - } else if ( - masterKeyEncryptedUserKey != null && - this.getKeyConnectorUrl(tokenResponse) != null && - !this.cache.value.unlockServiceForKeyConnectorLogin - ) { - // Key connector enabled for user - await this.trySetUserKeyWithMasterKey(userId); } // Note: In the traditional SSO flow with MP without key connector, the lock component @@ -326,23 +306,6 @@ export class SsoLoginStrategy extends LoginStrategy { } } - private async trySetUserKeyWithMasterKey(userId: UserId): Promise { - const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - - // There are two scenarios in which the master key is not set here: - // 1. If the user has a master password and is using Key Connector. In that case, we cannot set the master key - // because the user hasn't entered their master password yet. - // 2. For new users with Key Connector, we will not have a master key yet, since Key Connector domain - // has to be confirmed first. - // In both cases, we'll return here and let the migration to Key Connector handle setting the master key. - if (!masterKey) { - return; - } - - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey(masterKey, userId); - await this.keyService.setUserKey(userKey, userId); - } - protected override async setAccountCryptographicState( tokenResponse: IdentityTokenResponse, userId: UserId, diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts index a74a600791e0..32e26c827e9b 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts @@ -5,7 +5,6 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { TwoFactorService } from "@bitwarden/common/auth/two-factor"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; @@ -25,10 +24,8 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; -import { UserKey, MasterKey } from "@bitwarden/common/types/key"; import { KdfConfigService, KeyService } from "@bitwarden/key-management"; import { UnlockService } from "@bitwarden/unlock"; @@ -110,7 +107,6 @@ describe("UserApiLoginStrategy", () => { apiLogInStrategy = new UserApiLoginStrategy( cache, - keyConnectorService, unlockService, accountService, masterPasswordService, @@ -185,55 +181,20 @@ describe("UserApiLoginStrategy", () => { expect(environmentService.seedUserEnvironment).toHaveBeenCalled(); }); - it("sets the encrypted user key and private key from the identity token response", async () => { + it("sets account cryptographic state from the identity token response", async () => { const tokenResponse = identityTokenResponseFactory(); apiService.postIdentityToken.mockResolvedValue(tokenResponse); await apiLogInStrategy.logIn(credentials); - expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( - tokenResponse.key, - userId, - ); expect(accountCryptographicStateService.setAccountCryptographicState).toHaveBeenCalledWith( { V1: { private_key: tokenResponse.privateKey } }, userId, ); }); - it("gets and sets the master key if Key Connector is enabled", async () => { - const tokenResponse = identityTokenResponseFactory(); - tokenResponse.apiUseKeyConnector = true; - - const env = mock(); - env.getKeyConnectorUrl.mockReturnValue(keyConnectorUrl); - environmentService.environment$ = new BehaviorSubject(env); - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - - await apiLogInStrategy.logIn(credentials); - - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); - }); - - it("uses the legacy Key Connector master key path when SDK handling is disabled", async () => { - const tokenResponse = identityTokenResponseFactory(); - tokenResponse.apiUseKeyConnector = true; - tokenResponse.canUnlockWithKeyConnector = jest.fn().mockReturnValue(false); - - const env = mock(); - env.getKeyConnectorUrl.mockReturnValue(keyConnectorUrl); - environmentService.environment$ = new BehaviorSubject(env); - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - - await apiLogInStrategy.logIn(credentials); - - expect(keyConnectorService.setMasterKeyFromUrl).toHaveBeenCalledWith(keyConnectorUrl, userId); - }); - - it("uses unlock service when SDK key connector feature flag is enabled", async () => { + it("unlocks with key connector when the identity token response supports it", async () => { const tokenResponse = identityTokenResponseFactory(undefined, { HasMasterPassword: false, KeyConnectorOption: { KeyConnectorUrl: keyConnectorUrl }, @@ -243,9 +204,6 @@ describe("UserApiLoginStrategy", () => { const env = mock(); env.getKeyConnectorUrl.mockReturnValue(keyConnectorUrl); environmentService.environment$ = new BehaviorSubject(env); - configService.getFeatureFlag - .calledWith(FeatureFlag.UnlockKeyConnectorWithSdk) - .mockResolvedValue(true); apiService.postIdentityToken.mockResolvedValue(tokenResponse); @@ -255,62 +213,6 @@ describe("UserApiLoginStrategy", () => { url: keyConnectorUrl, keyConnectorKeyWrappedUserKey: tokenResponse.key!.encryptedString!, }); - expect(keyConnectorService.setMasterKeyFromUrl).not.toHaveBeenCalled(); - }); - - it("decrypts and sets the user key if Key Connector is enabled", async () => { - const userKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; - const masterKey = new SymmetricCryptoKey(new Uint8Array(32)) as MasterKey; - - const tokenResponse = identityTokenResponseFactory(); - tokenResponse.apiUseKeyConnector = true; - - const env = mock(); - env.getKeyConnectorUrl.mockReturnValue(keyConnectorUrl); - environmentService.environment$ = new BehaviorSubject(env); - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - masterPasswordService.masterKeySubject.next(masterKey); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); - - await apiLogInStrategy.logIn(credentials); - - expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( - masterKey, - userId, - undefined, - ); - expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); - }); - - it("uses the legacy Key Connector user key path when SDK handling is disabled", async () => { - const userKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; - const masterKey = new SymmetricCryptoKey(new Uint8Array(32)) as MasterKey; - - const tokenResponse = identityTokenResponseFactory(); - tokenResponse.apiUseKeyConnector = true; - tokenResponse.canUnlockWithKeyConnector = jest.fn().mockReturnValue(false); - - const env = mock(); - env.getKeyConnectorUrl.mockReturnValue(keyConnectorUrl); - environmentService.environment$ = new BehaviorSubject(env); - - apiService.postIdentityToken.mockResolvedValue(tokenResponse); - masterPasswordService.masterKeySubject.next(masterKey); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); - - await apiLogInStrategy.logIn(credentials); - - expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( - tokenResponse.key, - userId, - ); - expect(masterPasswordService.mock.decryptUserKeyWithMasterKey).toHaveBeenCalledWith( - masterKey, - userId, - undefined, - ); - expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, userId); }); it("sets account cryptographic state when accountKeysResponseModel is present", async () => { diff --git a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts index e48145b91ac3..e90220ff9803 100644 --- a/libs/auth/src/common/login-strategies/user-api-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/user-api-login.strategy.ts @@ -31,7 +31,6 @@ export class UserApiLoginStrategy extends LoginStrategy { constructor( data: UserApiLoginStrategyData, - private keyConnectorService: KeyConnectorService, private unlockService: UnlockService, ...sharedDeps: ConstructorParameters ) { @@ -54,48 +53,19 @@ export class UserApiLoginStrategy extends LoginStrategy { return authResult; } - protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) { - const sdkHandledKeyConnector = - response.canUnlockWithKeyConnector() && - (await this.configService.getFeatureFlag(FeatureFlag.UnlockKeyConnectorWithSdk)); - - if (!sdkHandledKeyConnector && response.apiUseKeyConnector) { - const env = await firstValueFrom(this.environmentService.environment$); - const keyConnectorUrl = env.getKeyConnectorUrl(); - await this.keyConnectorService.setMasterKeyFromUrl(keyConnectorUrl, userId); - } - } + protected override async setMasterKey(response: IdentityTokenResponse, userId: UserId) {} protected override async setUserKey( response: IdentityTokenResponse, userId: UserId, ): Promise { - const sdkHandledKeyConnector = - response.canUnlockWithKeyConnector() && - (await this.configService.getFeatureFlag(FeatureFlag.UnlockKeyConnectorWithSdk)); - - if (sdkHandledKeyConnector) { + if (response.canUnlockWithKeyConnector()) { await this.unlockService.unlockWithKeyConnector( userId, response.intoKeyConnectorUnlockData(), ); return; } - - if (response.key) { - await this.masterPasswordService.setMasterKeyEncryptedUserKey(response.key, userId); - } - - if (response.apiUseKeyConnector) { - const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - if (masterKey) { - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( - masterKey, - userId, - ); - await this.keyService.setUserKey(userKey, userId); - } - } } protected override async setAccountCryptographicState( diff --git a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts index c31e2b74dc11..2a054e803f52 100644 --- a/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts @@ -267,9 +267,6 @@ describe("WebAuthnLoginStrategy", () => { { V1: { private_key: idTokenResponse.privateKey } }, userId, ); - - // Master key and private key should not be set - expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); }); it("does not try to set the user key when prfKey is missing", async () => { diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts index 49bac94d6696..076b4e039c9d 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.ts @@ -401,7 +401,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { case AuthenticationType.UserApiKey: return new UserApiLoginStrategy( data?.userApiKey ?? new UserApiLoginStrategyData(), - this.keyConnectorService, this.unlockService, ...sharedDeps, ); diff --git a/libs/common/src/abstractions/api.service.ts b/libs/common/src/abstractions/api.service.ts index a8209d9357bc..0d9e5b86d5de 100644 --- a/libs/common/src/abstractions/api.service.ts +++ b/libs/common/src/abstractions/api.service.ts @@ -168,7 +168,6 @@ export abstract class ApiService { id: string, request: SecretVerificationRequest, ): Promise; - abstract postConvertToKeyConnector(): Promise; //passwordless abstract getAuthRequest(id: string): Promise; abstract putAuthRequest( diff --git a/libs/common/src/auth/models/response/identity-token.response.ts b/libs/common/src/auth/models/response/identity-token.response.ts index 7f9382792801..8e409fd3297e 100644 --- a/libs/common/src/auth/models/response/identity-token.response.ts +++ b/libs/common/src/auth/models/response/identity-token.response.ts @@ -103,7 +103,7 @@ export class IdentityTokenResponse extends BaseResponse { } canUnlockWithKeyConnector(): boolean { - return this.userDecryptionOptions?.keyConnectorOption != null; + return this.userDecryptionOptions?.keyConnectorOption != null && this.key != null; } intoKeyConnectorUnlockData(): KeyConnectorUnlockData { diff --git a/libs/common/src/auth/services/change-email/default-change-email.service.spec.ts b/libs/common/src/auth/services/change-email/default-change-email.service.spec.ts index f4d269edc1db..8e5396ee0762 100644 --- a/libs/common/src/auth/services/change-email/default-change-email.service.spec.ts +++ b/libs/common/src/auth/services/change-email/default-change-email.service.spec.ts @@ -310,7 +310,6 @@ describe("DefaultChangeEmailService", () => { .mockResolvedValueOnce(existingAuthData) .mockResolvedValueOnce(newAuthData); masterPasswordService.mock.makeMasterPasswordUnlockData.mockResolvedValue(newUnlockData); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockResolvedValue(undefined); apiService.send.mockResolvedValue(undefined); // Act @@ -403,7 +402,6 @@ describe("DefaultChangeEmailService", () => { .mockResolvedValueOnce(existingAuthData) .mockResolvedValueOnce(newAuthData); masterPasswordService.mock.makeMasterPasswordUnlockData.mockResolvedValue(newUnlockData); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockResolvedValue(undefined); apiService.send.mockResolvedValue(undefined); }); @@ -474,7 +472,6 @@ describe("DefaultChangeEmailService", () => { .mockResolvedValueOnce(existingAuthData) .mockResolvedValueOnce(newAuthData); masterPasswordService.mock.makeMasterPasswordUnlockData.mockResolvedValue(newUnlockData); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockResolvedValue(undefined); apiService.send.mockResolvedValue(undefined); // Act @@ -583,104 +580,10 @@ describe("DefaultChangeEmailService", () => { .mockResolvedValueOnce(existingAuthData) .mockResolvedValueOnce(newAuthData); masterPasswordService.mock.makeMasterPasswordUnlockData.mockResolvedValue(newUnlockData); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockResolvedValue(undefined); apiService.send.mockResolvedValue(undefined); // Act await sut.confirmEmailChange(mockMasterPassword, mockNewEmail, mockToken, mockUserId); - - // Assert: Sets legacy master key for backwards compat (remove in PM-30676) - expect(masterPasswordService.mock.setLegacyMasterKeyFromUnlockData).toHaveBeenCalledWith( - mockMasterPassword, - newUnlockData, - mockUserId, - ); - }); - - /** - * The legacy master key MUST be set AFTER the API call succeeds. - * If set before and the API fails, local state would be inconsistent with the server, - * making the operation non-retry-able without logging out. - */ - it("should set legacy master key AFTER the API call succeeds", async () => { - // Arrange - configService.getFeatureFlag.mockResolvedValue(true); - kdfConfigService.getKdfConfig$.mockReturnValue(of(kdfConfig)); - - const mockUserKey = new SymmetricCryptoKey( - new Uint8Array(64).fill(3) as CsprngArray, - ) as UserKey; - keyService.userKey$.mockReturnValue(of(mockUserKey)); - - const newSalt = "new@example.com" as MasterPasswordSalt; - masterPasswordService.mock.saltForUser$.mockReturnValue(of(existingSalt)); - masterPasswordService.mock.emailToSalt.mockReturnValue(newSalt); - - masterPasswordService.mock.makeMasterPasswordAuthenticationData.mockResolvedValue({ - salt: existingSalt, - kdf: kdfConfig, - masterPasswordAuthenticationHash: "auth-hash" as MasterPasswordAuthenticationHash, - }); - masterPasswordService.mock.makeMasterPasswordUnlockData.mockResolvedValue({ - salt: newSalt, - kdf: kdfConfig, - masterKeyWrappedUserKey: "wrapped-key" as MasterKeyWrappedUserKey, - } as MasterPasswordUnlockData); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockResolvedValue(undefined); - apiService.send.mockResolvedValue(undefined); - - // Track call order - const callOrder: string[] = []; - apiService.send.mockImplementation(async () => { - callOrder.push("apiService.send"); - }); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockImplementation(async () => { - callOrder.push("setLegacyMasterKeyFromUnlockData"); - }); - - // Act - await sut.confirmEmailChange(mockMasterPassword, mockNewEmail, mockToken, mockUserId); - - // Assert: API call must happen BEFORE legacy key update - expect(callOrder).toEqual(["apiService.send", "setLegacyMasterKeyFromUnlockData"]); - }); - - it("should NOT set legacy master key if API call fails", async () => { - // Arrange - configService.getFeatureFlag.mockResolvedValue(true); - kdfConfigService.getKdfConfig$.mockReturnValue(of(kdfConfig)); - - const mockUserKey = new SymmetricCryptoKey( - new Uint8Array(64).fill(3) as CsprngArray, - ) as UserKey; - keyService.userKey$.mockReturnValue(of(mockUserKey)); - - const newSalt = "new@example.com" as MasterPasswordSalt; - masterPasswordService.mock.saltForUser$.mockReturnValue(of(existingSalt)); - masterPasswordService.mock.emailToSalt.mockReturnValue(newSalt); - - masterPasswordService.mock.makeMasterPasswordAuthenticationData.mockResolvedValue({ - salt: existingSalt, - kdf: kdfConfig, - masterPasswordAuthenticationHash: "auth-hash" as MasterPasswordAuthenticationHash, - }); - masterPasswordService.mock.makeMasterPasswordUnlockData.mockResolvedValue({ - salt: newSalt, - kdf: kdfConfig, - masterKeyWrappedUserKey: "wrapped-key" as MasterKeyWrappedUserKey, - } as MasterPasswordUnlockData); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockResolvedValue(undefined); - - // API call fails - apiService.send.mockRejectedValue(new Error("Server error")); - - // Act & Assert - await expect( - sut.confirmEmailChange(mockMasterPassword, mockNewEmail, mockToken, mockUserId), - ).rejects.toThrow("Server error"); - - // Legacy key should NOT have been set (preserves retry-ability) - expect(masterPasswordService.mock.setLegacyMasterKeyFromUnlockData).not.toHaveBeenCalled(); }); }); @@ -786,7 +689,6 @@ describe("DefaultChangeEmailService", () => { kdf: kdfConfig, masterKeyWrappedUserKey: "wrapped-key" as MasterKeyWrappedUserKey, } as MasterPasswordUnlockData); - masterPasswordService.mock.setLegacyMasterKeyFromUnlockData.mockResolvedValue(undefined); apiService.send.mockResolvedValue(undefined); // Act diff --git a/libs/common/src/auth/services/change-email/default-change-email.service.ts b/libs/common/src/auth/services/change-email/default-change-email.service.ts index 544c7e3182ba..ceb644136552 100644 --- a/libs/common/src/auth/services/change-email/default-change-email.service.ts +++ b/libs/common/src/auth/services/change-email/default-change-email.service.ts @@ -148,16 +148,5 @@ export class DefaultChangeEmailService implements ChangeEmailService { } await this.apiService.send("POST", "/accounts/email", request, userId, false); - - // Set legacy master key only AFTER successful API call to prevent inconsistent state on failure. - // This ensures the operation is retry-able if the server request fails. - // Remove in PM-30676. - if (unlockDataForLegacyUpdate != null) { - await this.masterPasswordService.setLegacyMasterKeyFromUnlockData( - masterPassword, - unlockDataForLegacyUpdate, - userId, - ); - } } } diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 89e8fe8135c7..f315daf35690 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -20,7 +20,6 @@ export enum FeatureFlag { SafariAccountSwitching = "pm-5594-safari-account-switching", PM30811_ChangeEmailNewAuthenticationApis = "pm-30811-change-email-new-authentication-apis", PM31088_MasterPasswordServiceEmitSalt = "pm-31088-master-password-service-emit-salt", - UseUnlockServiceForPasswordLogin = "use-unlock-service-for-password-login", PM32413_MultiClientPasswordManagement = "pm-32413-multi-client-password-management", /* Autofill */ @@ -52,8 +51,6 @@ export enum FeatureFlag { PM27279_V2RegistrationTdeJit = "pm-27279-v2-registration-tde-jit", EnableAccountEncryptionV2KeyConnectorRegistration = "enable-account-encryption-v2-key-connector-registration", EnableAccountEncryptionV2JitPasswordRegistration = "enable-account-encryption-v2-jit-password-registration", - UnlockKeyConnectorWithSdk = "use-unlock-service-for-key-connector-login", - SdkKeyConnectorMigration = "use-sdk-for-key-connector-migration", UnlockViaSDK = "unlock-via-sdk", /* Tools */ @@ -163,7 +160,6 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.SafariAccountSwitching]: FALSE, [FeatureFlag.PM30811_ChangeEmailNewAuthenticationApis]: FALSE, [FeatureFlag.PM31088_MasterPasswordServiceEmitSalt]: FALSE, - [FeatureFlag.UseUnlockServiceForPasswordLogin]: FALSE, [FeatureFlag.PM32413_MultiClientPasswordManagement]: FALSE, /* Billing */ @@ -187,8 +183,6 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PM27279_V2RegistrationTdeJit]: FALSE, [FeatureFlag.EnableAccountEncryptionV2KeyConnectorRegistration]: FALSE, [FeatureFlag.EnableAccountEncryptionV2JitPasswordRegistration]: FALSE, - [FeatureFlag.UnlockKeyConnectorWithSdk]: FALSE, - [FeatureFlag.SdkKeyConnectorMigration]: FALSE, [FeatureFlag.UnlockViaSDK]: FALSE, /* Platform */ diff --git a/libs/common/src/key-management/kdf/change-kdf.service.spec.ts b/libs/common/src/key-management/kdf/change-kdf.service.spec.ts index b697569f68ba..89c219738eac 100644 --- a/libs/common/src/key-management/kdf/change-kdf.service.spec.ts +++ b/libs/common/src/key-management/kdf/change-kdf.service.spec.ts @@ -51,12 +51,7 @@ describe("ChangeKdfService", () => { beforeEach(() => { sdkService.userClient$ = jest.fn((userId: UserId) => of(mockSdk)) as any; - sut = new DefaultChangeKdfService( - changeKdfApiService, - sdkService, - keyService, - masterPasswordService, - ); + sut = new DefaultChangeKdfService(changeKdfApiService, sdkService); }); afterEach(() => { @@ -171,19 +166,6 @@ describe("ChangeKdfService", () => { expect(changeKdfApiService.updateUserKdfParams).toHaveBeenCalledWith(expectedRequest); }); - it("should set master key and hash after KDF update", async () => { - const masterPassword = "masterPassword"; - const mockMasterKey = {} as any; - const mockHash = "localHash"; - - keyService.makeMasterKey.mockResolvedValue(mockMasterKey); - keyService.hashMasterKey.mockResolvedValue(mockHash); - - await sut.updateUserKdfParams(masterPassword, mockNewKdfConfig, mockUserId); - - expect(masterPasswordService.setMasterKey).toHaveBeenCalledWith(mockMasterKey, mockUserId); - }); - it("should properly dispose of SDK resources", async () => { const masterPassword = "masterPassword"; jest.spyOn(mockNewKdfConfig, "toSdkConfig").mockReturnValue({} as any); diff --git a/libs/common/src/key-management/kdf/change-kdf.service.ts b/libs/common/src/key-management/kdf/change-kdf.service.ts index 20f8f0486544..f0bc0980263b 100644 --- a/libs/common/src/key-management/kdf/change-kdf.service.ts +++ b/libs/common/src/key-management/kdf/change-kdf.service.ts @@ -21,8 +21,6 @@ export class DefaultChangeKdfService implements ChangeKdfService { constructor( private changeKdfApiService: ChangeKdfApiService, private sdkService: SdkService, - private keyService: KeyService, - private masterPasswordService: InternalMasterPasswordServiceAbstraction, ) {} async updateUserKdfParams(masterPassword: string, kdf: KdfConfig, userId: UserId): Promise { @@ -59,13 +57,5 @@ export class DefaultChangeKdfService implements ChangeKdfService { const request = new KdfRequest(authenticationData, unlockData); request.authenticateWith(oldAuthenticationData); await this.changeKdfApiService.updateUserKdfParams(request); - - // Update the locally stored master key and hash, so that UV, etc. still works - const masterKey = await this.keyService.makeMasterKey( - masterPassword, - unlockData.salt, - unlockData.kdf, - ); - await this.masterPasswordService.setMasterKey(masterKey, userId); } } diff --git a/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts b/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts index c3fedc3333ab..51da3f139015 100644 --- a/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts +++ b/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts @@ -7,8 +7,6 @@ import { UserId } from "../../../types/guid"; import { KeyConnectorDomainConfirmation } from "../models/key-connector-domain-confirmation"; export abstract class KeyConnectorService { - abstract setMasterKeyFromUrl(keyConnectorUrl: string, userId: UserId): Promise; - abstract getManagingOrganization(userId: UserId): Promise; abstract getUsesKeyConnector(userId: UserId): Promise; diff --git a/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts b/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts index b3277fa5e147..457bbdc308a5 100644 --- a/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts +++ b/libs/common/src/key-management/key-connector/services/key-connector.service.spec.ts @@ -226,63 +226,36 @@ describe("KeyConnectorService", () => { }); }); - describe("setMasterKeyFromUrl", () => { - it("should set the master key from the provided URL", async () => { - // Arrange - const url = keyConnectorUrl; - - apiService.getMasterKeyFromKeyConnector.mockResolvedValue(mockMasterKeyResponse); - - // Hard to mock these, but we can generate the same keys - const keyArr = Utils.fromB64ToArray(mockMasterKeyResponse.key); - const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey; - - // Act - await keyConnectorService.setMasterKeyFromUrl(url, mockUserId); - - // Assert - expect(apiService.getMasterKeyFromKeyConnector).toHaveBeenCalledWith(url); - expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(masterKey, mockUserId); - }); - - it("should handle errors thrown during the process", async () => { - // Arrange - const url = keyConnectorUrl; - - const error = new Error("Failed to get master key"); - apiService.getMasterKeyFromKeyConnector.mockRejectedValue(error); - jest.spyOn(logService, "error"); - - try { - // Act - await keyConnectorService.setMasterKeyFromUrl(url, mockUserId); - } catch { - // Assert - expect(logService.error).toHaveBeenCalledWith(error); - expect(apiService.getMasterKeyFromKeyConnector).toHaveBeenCalledWith(url); - } - }); - }); - describe("migrateUser", () => { + let migrateToKeyConnector: jest.Mock; + let mockSdkRef: any; + let mockSdk: any; + beforeEach(() => { configService.getFeatureFlag$.mockReturnValue(of(false)); + + migrateToKeyConnector = jest.fn().mockResolvedValue(undefined); + mockSdkRef = { + value: { + user_crypto_management: jest.fn().mockReturnValue({ + migrate_to_key_connector: migrateToKeyConnector, + }), + }, + [Symbol.dispose]: jest.fn(), + }; + mockSdk = { + take: jest.fn().mockReturnValue(mockSdkRef), + }; + sdkService.userClient$.mockReturnValue(of(mockSdk)); }); it("should migrate the user to the key connector", async () => { // Arrange - const masterKey = getMockMasterKey(); - masterPasswordService.masterKeySubject.next(masterKey); - const keyConnectorRequest = new KeyConnectorUserKeyRequest( - Utils.fromBufferToB64(masterKey.inner().encryptionKey), - ); - const mockUserDecryptionOptions = { hasMasterPassword: true, keyConnectorOption: undefined, } as UserDecryptionOptions; - jest.spyOn(apiService, "postUserKeyToKeyConnector").mockResolvedValue(); userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( of(mockUserDecryptionOptions), ); @@ -291,11 +264,8 @@ describe("KeyConnectorService", () => { await keyConnectorService.migrateUser(keyConnectorUrl, mockUserId); // Assert - expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith( - keyConnectorUrl, - keyConnectorRequest, - ); - expect(apiService.postConvertToKeyConnector).toHaveBeenCalled(); + expect(sdkService.userClient$).toHaveBeenCalledWith(mockUserId); + expect(migrateToKeyConnector).toHaveBeenCalledWith(keyConnectorUrl); expect(masterPasswordService.mock.clearMasterPasswordUnlockData).toHaveBeenCalledWith( mockUserId, ); @@ -315,29 +285,21 @@ describe("KeyConnectorService", () => { it("should handle errors thrown during migration", async () => { // Arrange - const masterKey = getMockMasterKey(); - const keyConnectorRequest = new KeyConnectorUserKeyRequest( - Utils.fromBufferToB64(masterKey.inner().encryptionKey), - ); - masterPasswordService.masterKeySubject.next(masterKey); - const error = new Error("Failed to post user key to key connector"); - jest.spyOn(apiService, "postUserKeyToKeyConnector").mockRejectedValue(error); + const error = new Error("Failed to migrate to key connector"); + migrateToKeyConnector.mockRejectedValue(error); jest.spyOn(logService, "error"); - try { - // Act - await keyConnectorService.migrateUser(keyConnectorUrl, mockUserId); - } catch { - // Assert - expect(logService.error).toHaveBeenCalledWith(error); - expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith( - keyConnectorUrl, - keyConnectorRequest, - ); - // Verify state clearing operations were not called due to error - expect(masterPasswordService.mock.clearMasterPasswordUnlockData).not.toHaveBeenCalled(); - expect(userDecryptionOptionsService.setUserDecryptionOptionsById).not.toHaveBeenCalled(); - } + // Act + await expect( + keyConnectorService.migrateUser(keyConnectorUrl, mockUserId), + ).rejects.toThrow("Key Connector error"); + + // Assert + expect(logService.error).toHaveBeenCalledWith(error); + expect(migrateToKeyConnector).toHaveBeenCalledWith(keyConnectorUrl); + // Verify state clearing operations were not called due to error + expect(masterPasswordService.mock.clearMasterPasswordUnlockData).not.toHaveBeenCalled(); + expect(userDecryptionOptionsService.setUserDecryptionOptionsById).not.toHaveBeenCalled(); }); }); @@ -535,10 +497,6 @@ describe("KeyConnectorService", () => { .registration().post_keys_for_key_connector_registration; expect(mockRegistration).toHaveBeenCalledWith(keyConnectorUrl, mockSsoOrgIdentifier); - expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith( - expect.any(SymmetricCryptoKey), - mockUserId, - ); expect(keyService.setUserKey).toHaveBeenCalledWith( expect.any(SymmetricCryptoKey), mockUserId, @@ -577,7 +535,6 @@ describe("KeyConnectorService", () => { ).rejects.toThrow("SDK not available"); expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); - expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); expect(keyService.setUserKey).not.toHaveBeenCalled(); expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled(); expect( @@ -611,7 +568,6 @@ describe("KeyConnectorService", () => { ).rejects.toThrow("Unexpected account cryptographic state version"); expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); - expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); expect(keyService.setUserKey).not.toHaveBeenCalled(); expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled(); expect( @@ -637,7 +593,6 @@ describe("KeyConnectorService", () => { ).rejects.toThrow("Key Connector registration failed"); expect(await firstValueFrom(conversionState.state$)).toEqual(conversion); - expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled(); expect(keyService.setUserKey).not.toHaveBeenCalled(); expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).not.toHaveBeenCalled(); expect( @@ -700,10 +655,6 @@ describe("KeyConnectorService", () => { mockEmail, expectedKdfConfig, ); - expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith( - mockMasterKey, - mockUserId, - ); expect(keyService.makeUserKey).toHaveBeenCalledWith(mockMasterKey); expect(keyService.setUserKey).toHaveBeenCalledWith(mockUserKey, mockUserId); expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( @@ -750,10 +701,6 @@ describe("KeyConnectorService", () => { mockEmail, new PBKDF2KdfConfig(600_000), ); - expect(masterPasswordService.mock.setMasterKey).toHaveBeenCalledWith( - mockMasterKey, - mockUserId, - ); expect(keyService.makeUserKey).toHaveBeenCalledWith(mockMasterKey); expect(keyService.setUserKey).toHaveBeenCalledWith(mockUserKey, mockUserId); expect(masterPasswordService.mock.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith( diff --git a/libs/common/src/key-management/key-connector/services/key-connector.service.ts b/libs/common/src/key-management/key-connector/services/key-connector.service.ts index 91a63303b92a..26697d02b613 100644 --- a/libs/common/src/key-management/key-connector/services/key-connector.service.ts +++ b/libs/common/src/key-management/key-connector/services/key-connector.service.ts @@ -131,40 +131,22 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { } async migrateUser(keyConnectorUrl: string, userId: UserId) { - const sdkKeyConnectorMigration = await firstValueFrom( - this.configService.getFeatureFlag$(FeatureFlag.SdkKeyConnectorMigration), - ); - if (sdkKeyConnectorMigration) { - try { - await firstValueFrom( - this.sdkService.userClient$(userId).pipe( - map((sdk) => { - if (!sdk) { - throw new Error("SDK not available"); - } - - using ref = sdk.take(); - - return ref.value.user_crypto_management().migrate_to_key_connector(keyConnectorUrl); - }), - ), - ); - } catch (e) { - this.handleKeyConnectorError(e); - } - } else { - const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - const keyConnectorRequest = new KeyConnectorUserKeyRequest( - Utils.fromBufferToB64(masterKey.inner().encryptionKey), - ); + try { + await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + map((sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } - try { - await this.apiService.postUserKeyToKeyConnector(keyConnectorUrl, keyConnectorRequest); - } catch (e) { - this.handleKeyConnectorError(e); - } + using ref = sdk.take(); - await this.apiService.postConvertToKeyConnector(); + return ref.value.user_crypto_management().migrate_to_key_connector(keyConnectorUrl); + }), + ), + ); + } catch (e) { + this.handleKeyConnectorError(e); } await this.setUsesKeyConnector(true, userId); @@ -185,18 +167,6 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { ); } - // TODO: UserKey should be renamed to MasterKey and typed accordingly - async setMasterKeyFromUrl(keyConnectorUrl: string, userId: UserId) { - try { - const masterKeyResponse = await this.apiService.getMasterKeyFromKeyConnector(keyConnectorUrl); - const keyArr = Utils.fromB64ToArray(masterKeyResponse.key); - const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey; - await this.masterPasswordService.setMasterKey(masterKey, userId); - } catch (e) { - this.handleKeyConnectorError(e); - } - } - async getManagingOrganization(userId: UserId): Promise { const organizations = await firstValueFrom(this.organizationService.organizations$(userId)); return this.findManagingOrganization(organizations); @@ -265,10 +235,6 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { throw new Error(`Unexpected account cryptographic state version ${version}`); } - await this.masterPasswordService.setMasterKey( - SymmetricCryptoKey.fromString(result.key_connector_key) as MasterKey, - userId, - ); await this.keyService.setUserKey( SymmetricCryptoKey.fromString(result.user_key) as UserKey, userId, @@ -300,7 +266,6 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { const keyConnectorRequest = new KeyConnectorUserKeyRequest( Utils.fromBufferToB64(masterKey.inner().encryptionKey), ); - await this.masterPasswordService.setMasterKey(masterKey, userId); const userKey = await this.keyService.makeUserKey(masterKey); await this.keyService.setUserKey(userKey[0], userId); diff --git a/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts b/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts index fd202aded3a9..62de85b9189a 100644 --- a/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts +++ b/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts @@ -48,13 +48,6 @@ export abstract class MasterPasswordServiceAbstraction { * email, no matter how the email is capitalized. */ abstract emailToSalt(email: string): MasterPasswordSalt; - /** - * An observable that emits the master key for the user. - * @deprecated Interacting with the master-key directly is deprecated. Please use {@link makeMasterPasswordUnlockData}, {@link makeMasterPasswordAuthenticationData} or {@link unwrapUserKeyFromMasterPasswordUnlockData} instead. - * @param userId The user ID. - * @throws If the user ID is missing. - */ - abstract masterKey$: (userId: UserId) => Observable; /** * Returns the master key encrypted user key for the user. * @param userId The user ID. @@ -122,35 +115,9 @@ export abstract class MasterPasswordServiceAbstraction { * @throws If the user ID is missing. */ abstract userHasMasterPassword(userId: UserId): Promise; - - /** - * Derives a master key from the provided password and master password unlock data, - * then sets it to state for the specified user. This is a temporary backwards compatibility function - * to support existing code that relies on direct master key access. - * Note: This will be removed in https://bitwarden.atlassian.net/browse/PM-30676 - * - * @param password The master password. - * @param masterPasswordUnlockData The master password unlock data containing the KDF settings and salt. - * @param userId The user ID. - * @throws If the password, master password unlock data, or user ID is missing. - */ - abstract setLegacyMasterKeyFromUnlockData( - password: string, - masterPasswordUnlockData: MasterPasswordUnlockData, - userId: UserId, - ): Promise; } export abstract class InternalMasterPasswordServiceAbstraction extends MasterPasswordServiceAbstraction { - /** - * Set the master key for the user. - * Note: Use {@link clearMasterKey} to clear the master key. - * @deprecated Interacting with the master-key directly is deprecated. - * @param masterKey The master key. - * @param userId The user ID. - * @throws If the user ID or master key is missing. - */ - abstract setMasterKey: (masterKey: MasterKey, userId: UserId) => Promise; /** * Set the master key encrypted user key for the user. * @param encryptedKey The master key encrypted user key. diff --git a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts index b33813fe2984..5d41428696c2 100644 --- a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts +++ b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.spec.ts @@ -3,13 +3,13 @@ import { of } from "rxjs"; import { newGuid } from "@bitwarden/guid"; // eslint-disable-next-line no-restricted-imports -import { Argon2KdfConfig, KeyService } from "@bitwarden/key-management"; +import { Argon2KdfConfig } from "@bitwarden/key-management"; import { LogService } from "@bitwarden/logging"; import { CryptoError } from "@bitwarden/sdk-internal"; import { UserId } from "@bitwarden/user-core"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; -import { MasterKey, UserKey } from "../../../types/key"; +import { UserKey } from "../../../types/key"; import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction"; import { MasterKeyWrappedUserKey, @@ -23,7 +23,6 @@ describe("DefaultMasterPasswordUnlockService", () => { let sut: DefaultMasterPasswordUnlockService; let masterPasswordService: MockProxy; - let keyService: MockProxy; let logService: MockProxy; const mockMasterPassword = "testExample"; @@ -36,25 +35,16 @@ describe("DefaultMasterPasswordUnlockService", () => { "encryptedMasterKeyWrappedUserKey" as MasterKeyWrappedUserKey, ); - //Legacy data for tests - const mockMasterKey = new SymmetricCryptoKey(new Uint8Array(32)) as MasterKey; - const mockKeyHash = "localKeyHash"; - beforeEach(() => { masterPasswordService = mock(); - keyService = mock(); logService = mock(); - sut = new DefaultMasterPasswordUnlockService(masterPasswordService, keyService, logService); + sut = new DefaultMasterPasswordUnlockService(masterPasswordService, logService); masterPasswordService.masterPasswordUnlockData$.mockReturnValue( of(mockMasterPasswordUnlockData), ); masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData.mockResolvedValue(mockUserKey); - - // Legacy state mocking - keyService.makeMasterKey.mockResolvedValue(mockMasterKey); - keyService.hashMasterKey.mockResolvedValue(mockKeyHash); }); afterEach(() => { @@ -107,45 +97,6 @@ describe("DefaultMasterPasswordUnlockService", () => { ); }); - it("sets legacy state on success", async () => { - const result = await sut.unlockWithMasterPassword(mockMasterPassword, mockUserId); - - expect(result).toEqual(mockUserKey); - expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId); - expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith( - mockMasterPassword, - mockMasterPasswordUnlockData, - ); - - expect(keyService.makeMasterKey).toHaveBeenCalledWith( - mockMasterPassword, - mockMasterPasswordUnlockData.salt, - mockMasterPasswordUnlockData.kdf, - ); - expect(masterPasswordService.setMasterKey).toHaveBeenCalledWith(mockMasterKey, mockUserId); - }); - - it("throws an error if masterKey construction fails", async () => { - keyService.makeMasterKey.mockResolvedValue(null as unknown as MasterKey); - - await expect(sut.unlockWithMasterPassword(mockMasterPassword, mockUserId)).rejects.toThrow( - "Master key could not be created to set legacy master password state.", - ); - - expect(masterPasswordService.masterPasswordUnlockData$).toHaveBeenCalledWith(mockUserId); - expect(masterPasswordService.unwrapUserKeyFromMasterPasswordUnlockData).toHaveBeenCalledWith( - mockMasterPassword, - mockMasterPasswordUnlockData, - ); - - expect(keyService.makeMasterKey).toHaveBeenCalledWith( - mockMasterPassword, - mockMasterPasswordUnlockData.salt, - mockMasterPasswordUnlockData.kdf, - ); - expect(keyService.hashMasterKey).not.toHaveBeenCalled(); - expect(masterPasswordService.setMasterKey).not.toHaveBeenCalled(); - }); }); describe("proofOfDecryption", () => { diff --git a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts index 70acfd272bcd..ed277785dc79 100644 --- a/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts +++ b/libs/common/src/key-management/master-password/services/default-master-password-unlock.service.ts @@ -1,7 +1,5 @@ import { firstValueFrom } from "rxjs"; -// eslint-disable-next-line no-restricted-imports -import { KeyService } from "@bitwarden/key-management"; import { LogService } from "@bitwarden/logging"; import { isCryptoError } from "@bitwarden/sdk-internal"; import { UserId } from "@bitwarden/user-core"; @@ -9,12 +7,10 @@ import { UserId } from "@bitwarden/user-core"; import { UserKey } from "../../../types/key"; import { MasterPasswordUnlockService } from "../abstractions/master-password-unlock.service"; import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction"; -import { MasterPasswordUnlockData } from "../types/master-password.types"; export class DefaultMasterPasswordUnlockService implements MasterPasswordUnlockService { constructor( private readonly masterPasswordService: InternalMasterPasswordServiceAbstraction, - private readonly keyService: KeyService, private readonly logService: LogService, ) {} @@ -34,8 +30,6 @@ export class DefaultMasterPasswordUnlockService implements MasterPasswordUnlockS masterPasswordUnlockData, ); - await this.setLegacyState(masterPassword, masterPasswordUnlockData, userId); - return userKey; } @@ -84,24 +78,4 @@ export class DefaultMasterPasswordUnlockService implements MasterPasswordUnlockS throw new Error("User ID is required"); } } - - // Previously unlocking had the side effect of setting the masterKey and masterPasswordHash in state. - // This is to preserve that behavior, once masterKey and masterPasswordHash state is removed this should be removed as well. - private async setLegacyState( - masterPassword: string, - masterPasswordUnlockData: MasterPasswordUnlockData, - userId: UserId, - ): Promise { - const masterKey = await this.keyService.makeMasterKey( - masterPassword, - masterPasswordUnlockData.salt, - masterPasswordUnlockData.kdf, - ); - - if (!masterKey) { - throw new Error("Master key could not be created to set legacy master password state."); - } - - await this.masterPasswordService.setMasterKey(masterKey, userId); - } } diff --git a/libs/common/src/key-management/master-password/services/fake-master-password.service.ts b/libs/common/src/key-management/master-password/services/fake-master-password.service.ts index 9069c956938d..ea6fea801b45 100644 --- a/libs/common/src/key-management/master-password/services/fake-master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/fake-master-password.service.ts @@ -45,14 +45,6 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA return this.mock.saltForUser$(userId); } - masterKey$(userId: UserId): Observable { - return this.masterKeySubject.asObservable(); - } - - setMasterKey(masterKey: MasterKey, userId: UserId): Promise { - return this.mock.setMasterKey(masterKey, userId); - } - getMasterKeyEncryptedUserKey(userId: UserId): Promise { return this.mock.getMasterKeyEncryptedUserKey(userId); } @@ -115,12 +107,4 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA masterPasswordUnlockData$(userId: UserId): Observable { return this.mock.masterPasswordUnlockData$(userId); } - - setLegacyMasterKeyFromUnlockData( - password: string, - masterPasswordUnlockData: MasterPasswordUnlockData, - userId: UserId, - ): Promise { - return this.mock.setLegacyMasterKeyFromUnlockData(password, masterPasswordUnlockData, userId); - } } diff --git a/libs/common/src/key-management/master-password/services/master-password.service.spec.ts b/libs/common/src/key-management/master-password/services/master-password.service.spec.ts index 518918ee3d22..acbef3789b2b 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.spec.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.spec.ts @@ -34,7 +34,6 @@ import { import { FORCE_SET_PASSWORD_REASON, - MASTER_KEY, MASTER_KEY_ENCRYPTED_USER_KEY, MASTER_PASSWORD_UNLOCK_KEY, MasterPasswordService, @@ -251,18 +250,6 @@ describe("MasterPasswordService", () => { ); expect(decryptedUserKey).toBeNull(); }); - - it("returns error when master key is null", async () => { - stateProvider.singleUser.getFake(userId, MASTER_KEY).nextState(null); - - await expect( - sut.decryptUserKeyWithMasterKey( - null as unknown as MasterKey, - userId, - masterKeyEncryptedUserKey, - ), - ).rejects.toThrow("No master key found."); - }); }); describe("setMasterKeyEncryptedUserKey", () => { @@ -499,115 +486,6 @@ describe("MasterPasswordService", () => { ); }); - describe("setLegacyMasterKeyFromUnlockData", () => { - const password = "test-password"; - - it("derives master key from password and sets it in state", async () => { - const masterKey = makeSymmetricCryptoKey(32, 5) as MasterKey; - keyGenerationService.deriveKeyFromPassword.mockResolvedValue(masterKey); - cryptoFunctionService.pbkdf2.mockResolvedValue(new Uint8Array(32)); - - const masterPasswordUnlockData = new MasterPasswordUnlockData( - salt, - kdfPBKDF2, - makeEncString().toSdk() as MasterKeyWrappedUserKey, - ); - - await sut.setLegacyMasterKeyFromUnlockData(password, masterPasswordUnlockData, userId); - - expect(keyGenerationService.deriveKeyFromPassword).toHaveBeenCalledWith( - password, - masterPasswordUnlockData.salt, - masterPasswordUnlockData.kdf, - ); - - const state = await firstValueFrom(stateProvider.getUser(userId, MASTER_KEY).state$); - expect(state).toEqual(masterKey); - }); - - it("works with argon2 kdf config", async () => { - const masterKey = makeSymmetricCryptoKey(32, 6) as MasterKey; - keyGenerationService.deriveKeyFromPassword.mockResolvedValue(masterKey); - cryptoFunctionService.pbkdf2.mockResolvedValue(new Uint8Array(32)); - - const masterPasswordUnlockData = new MasterPasswordUnlockData( - salt, - kdfArgon2, - makeEncString().toSdk() as MasterKeyWrappedUserKey, - ); - - await sut.setLegacyMasterKeyFromUnlockData(password, masterPasswordUnlockData, userId); - - expect(keyGenerationService.deriveKeyFromPassword).toHaveBeenCalledWith( - password, - masterPasswordUnlockData.salt, - masterPasswordUnlockData.kdf, - ); - - const state = await firstValueFrom(stateProvider.getUser(userId, MASTER_KEY).state$); - expect(state).toEqual(masterKey); - }); - - it("computes and sets master key hash in state", async () => { - const masterKey = makeSymmetricCryptoKey(32, 7) as MasterKey; - const expectedHashBytes = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]); - const expectedHashB64 = "AQIDBAUGBwg="; - keyGenerationService.deriveKeyFromPassword.mockResolvedValue(masterKey); - cryptoFunctionService.pbkdf2.mockResolvedValue(expectedHashBytes); - jest.spyOn(Utils, "fromBufferToB64").mockReturnValue(expectedHashB64); - - const masterPasswordUnlockData = new MasterPasswordUnlockData( - salt, - kdfPBKDF2, - makeEncString().toSdk() as MasterKeyWrappedUserKey, - ); - - await sut.setLegacyMasterKeyFromUnlockData(password, masterPasswordUnlockData, userId); - }); - - it("throws if password is null", async () => { - const masterPasswordUnlockData = new MasterPasswordUnlockData( - salt, - kdfPBKDF2, - makeEncString().toSdk() as MasterKeyWrappedUserKey, - ); - - await expect( - sut.setLegacyMasterKeyFromUnlockData( - null as unknown as string, - masterPasswordUnlockData, - userId, - ), - ).rejects.toThrow("password is null or undefined."); - }); - - it("throws if masterPasswordUnlockData is null", async () => { - await expect( - sut.setLegacyMasterKeyFromUnlockData( - password, - null as unknown as MasterPasswordUnlockData, - userId, - ), - ).rejects.toThrow("masterPasswordUnlockData is null or undefined."); - }); - - it("throws if userId is null", async () => { - const masterPasswordUnlockData = new MasterPasswordUnlockData( - salt, - kdfPBKDF2, - makeEncString().toSdk() as MasterKeyWrappedUserKey, - ); - - await expect( - sut.setLegacyMasterKeyFromUnlockData( - password, - masterPasswordUnlockData, - null as unknown as UserId, - ), - ).rejects.toThrow("userId is null or undefined."); - }); - }); - describe("MASTER_PASSWORD_UNLOCK_KEY", () => { it("has the correct configuration", () => { expect(MASTER_PASSWORD_UNLOCK_KEY.stateDefinition).toBeDefined(); diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index b0f334fbd7b6..e61a4907eaf1 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -37,12 +37,6 @@ import { MasterPasswordUnlockData, } from "../types/master-password.types"; -/** Memory since master key shouldn't be available on lock */ -export const MASTER_KEY = new UserKeyDefinition(MASTER_PASSWORD_MEMORY, "masterKey", { - deserializer: (masterKey) => SymmetricCryptoKey.fromJSON(masterKey) as MasterKey, - clearOn: ["lock", "logout"], -}); - /** Disk to persist through lock */ export const MASTER_KEY_ENCRYPTED_USER_KEY = new UserKeyDefinition( MASTER_PASSWORD_DISK, @@ -140,13 +134,6 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr ); } - masterKey$(userId: UserId): Observable { - if (userId == null) { - throw new Error("User ID is required."); - } - return this.stateProvider.getUser(userId, MASTER_KEY).state$; - } - forceSetPasswordReason$(userId: UserId): Observable { if (userId == null) { throw new Error("User ID is required."); @@ -171,23 +158,6 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr return email.toLowerCase().trim() as MasterPasswordSalt; } - async setMasterKey(masterKey: MasterKey, userId: UserId): Promise { - if (masterKey == null) { - throw new Error("Master key is required."); - } - if (userId == null) { - throw new Error("User ID is required."); - } - await this.stateProvider.getUser(userId, MASTER_KEY).update((_) => masterKey); - } - - async clearMasterKey(userId: UserId): Promise { - if (userId == null) { - throw new Error("User ID is required."); - } - await this.stateProvider.getUser(userId, MASTER_KEY).update((_) => null); - } - async setMasterKeyEncryptedUserKey(encryptedKey: EncString, userId: UserId): Promise { if (encryptedKey == null || encryptedKey.encryptedString == null) { throw new Error("Encrypted Key is required."); @@ -227,7 +197,6 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr userKey?: EncString, ): Promise { userKey ??= await this.getMasterKeyEncryptedUserKey(userId); - masterKey ??= await firstValueFrom(this.masterKey$(userId)); if (masterKey == null) { throw new Error("No master key found."); @@ -353,22 +322,4 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr return this.stateProvider.getUser(userId, MASTER_PASSWORD_UNLOCK_KEY).state$; } - - async setLegacyMasterKeyFromUnlockData( - password: string, - masterPasswordUnlockData: MasterPasswordUnlockData, - userId: UserId, - ): Promise { - assertNonNullish(password, "password"); - assertNonNullish(masterPasswordUnlockData, "masterPasswordUnlockData"); - assertNonNullish(userId, "userId"); - - const masterKey = (await this.keyGenerationService.deriveKeyFromPassword( - password, - masterPasswordUnlockData.salt, - masterPasswordUnlockData.kdf, - )) as MasterKey; - - await this.setMasterKey(masterKey, userId); - } } diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index b1dae16e138b..b3170a4decaf 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -371,10 +371,6 @@ export class ApiService implements ApiServiceAbstraction { return new ApiKeyResponse(r); } - postConvertToKeyConnector(): Promise { - return this.send("POST", "/accounts/convert-to-key-connector", null, true, false); - } - // Account Billing APIs async getUserBillingHistory(): Promise { diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index ee930ba0cb0f..ef352449e69c 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -711,16 +711,6 @@ describe("keyService", () => { }, ); - it("returns the master key if it is already available", async () => { - const masterKey = makeSymmetricCryptoKey(32) as MasterKey; - masterPasswordService.masterKeySubject.next(masterKey); - - const result = await keyService.getOrDeriveMasterKey("password", mockUserId); - - expect(kdfConfigService.getKdfConfig$).not.toHaveBeenCalledWith(mockUserId); - expect(result).toEqual(masterKey); - }); - it("throws an error if user's email is not available", async () => { accountService.accounts$ = of({}); diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 4dc63fc951c7..c26741f2d423 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -223,11 +223,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { throw new Error("User ID is required."); } - const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - if (masterKey != null) { - return masterKey; - } - const email = await firstValueFrom( this.accountService.accounts$.pipe(map((accounts) => accounts[userId]?.email)), ); diff --git a/libs/unlock/src/default-unlock.service.spec.ts b/libs/unlock/src/default-unlock.service.spec.ts index 96bf00f4a8df..21d4c3e22662 100644 --- a/libs/unlock/src/default-unlock.service.spec.ts +++ b/libs/unlock/src/default-unlock.service.spec.ts @@ -8,7 +8,6 @@ import { ClientType } from "@bitwarden/client-type"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; -import { MASTER_KEY } from "@bitwarden/common/key-management/master-password/services/master-password.service"; import { PinStateServiceAbstraction } from "@bitwarden/common/key-management/pin/pin-state.service.abstraction"; import { VaultTimeoutStringType } from "@bitwarden/common/key-management/vault-timeout"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -25,7 +24,6 @@ import { KdfConfigService, } from "@bitwarden/key-management"; import { LogService } from "@bitwarden/logging"; -import { PureCrypto } from "@bitwarden/sdk-internal"; import { StateProvider, StateService } from "@bitwarden/state"; import { DefaultUnlockService } from "./default-unlock.service"; @@ -57,7 +55,6 @@ describe("DefaultUnlockService", () => { let mockSdkRef: any; let mockSdk: any; let mockCrypto: any; - let setLegacyMasterKeyFromUnlockDataSpy: jest.SpyInstance; beforeEach(() => { jest.resetAllMocks(); @@ -107,8 +104,6 @@ describe("DefaultUnlockService", () => { configurable: true, }); - jest.spyOn(PureCrypto, "derive_kdf_material").mockReturnValue(new Uint8Array(32)); - const mockStateUpdate = jest.fn().mockResolvedValue(undefined); stateProvider.getUser.mockReturnValue({ update: mockStateUpdate } as any); stateProvider.setUserState.mockResolvedValue(undefined); @@ -127,10 +122,6 @@ describe("DefaultUnlockService", () => { stateService, biometricStateService, ); - - setLegacyMasterKeyFromUnlockDataSpy = jest - .spyOn(service as any, "setLegacyMasterKeyFromUnlockData") - .mockResolvedValue(undefined); }); describe("unlockWithPin", () => { @@ -323,28 +314,4 @@ describe("DefaultUnlockService", () => { }); }); - describe("setLegacyMasterKeyFromUnlockData", () => { - it("derives legacy master key and stores key", async () => { - setLegacyMasterKeyFromUnlockDataSpy.mockRestore(); - const derivedMasterKey = new Uint8Array(32); - const updateMasterKey = jest.fn().mockResolvedValue(undefined); - - jest.spyOn(PureCrypto, "derive_kdf_material").mockReturnValue(derivedMasterKey); - stateProvider.getUser.mockReturnValueOnce({ update: updateMasterKey } as any); - - await (service as any).setLegacyMasterKeyFromUnlockData( - mockMasterPassword, - mockMasterPasswordUnlockData, - mockUserId, - ); - - expect(PureCrypto.derive_kdf_material).toHaveBeenCalledWith( - new TextEncoder().encode(mockMasterPassword), - new TextEncoder().encode(mockMasterPasswordUnlockData.salt), - mockMasterPasswordUnlockData.kdf, - ); - expect(stateProvider.getUser).toHaveBeenCalledWith(mockUserId, MASTER_KEY); - expect(updateMasterKey).toHaveBeenCalledTimes(1); - }); - }); }); diff --git a/libs/unlock/src/default-unlock.service.ts b/libs/unlock/src/default-unlock.service.ts index 65a47c776841..500b7997dc9c 100644 --- a/libs/unlock/src/default-unlock.service.ts +++ b/libs/unlock/src/default-unlock.service.ts @@ -5,7 +5,6 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { assertNonNullish } from "@bitwarden/common/auth/utils"; import { AccountCryptographicStateService } from "@bitwarden/common/key-management/account-cryptography/account-cryptographic-state.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; -import { MASTER_KEY } from "@bitwarden/common/key-management/master-password/services/master-password.service"; import { PinStateServiceAbstraction } from "@bitwarden/common/key-management/pin/pin-state.service.abstraction"; import { VAULT_TIMEOUT, @@ -13,12 +12,10 @@ import { } from "@bitwarden/common/key-management/vault-timeout"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { RegisterSdkService } from "@bitwarden/common/platform/abstractions/sdk/register-sdk.service"; -import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { asUuid } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { Ref } from "@bitwarden/common/platform/misc/reference-counting/rc"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { USER_EVER_HAD_USER_KEY } from "@bitwarden/common/platform/services/key-state/user-key.state"; -import { MasterKey } from "@bitwarden/common/types/key"; import { BiometricsService, BiometricStateService, @@ -32,7 +29,6 @@ import { MasterPasswordUnlockData, PasswordManagerClient, PasswordProtectedKeyEnvelope, - PureCrypto, WrappedAccountCryptographicState, } from "@bitwarden/sdk-internal"; import { StateProvider, StateService } from "@bitwarden/state"; @@ -120,11 +116,6 @@ export class DefaultUnlockService implements UnlockService { }), ), ); - await this.setLegacyMasterKeyFromUnlockData( - masterPassword, - await this.getMasterPasswordUnlockData(userId), - userId, - ); this.logService.measure( startTime, "Unlock", @@ -250,32 +241,6 @@ export class DefaultUnlockService implements UnlockService { return unlockData.toSdk(); } - private async setLegacyMasterKeyFromUnlockData( - password: string, - masterPasswordUnlockData: MasterPasswordUnlockData, - userId: UserId, - ): Promise { - assertNonNullish(password, "password"); - assertNonNullish(masterPasswordUnlockData, "masterPasswordUnlockData"); - assertNonNullish(userId, "userId"); - this.logService.info("[DefaultUnlockService] Setting legacy master key from unlock data"); - - // NOTE: This entire section is deprecated and will be removed as soon as - // the masterkey is dropped from state. It is very temporary. - await SdkLoadService.Ready; - - const passwordBuffer = new TextEncoder().encode(password); - const saltBuffer = new TextEncoder().encode(masterPasswordUnlockData.salt); - const masterKey = PureCrypto.derive_kdf_material( - passwordBuffer, - saltBuffer, - masterPasswordUnlockData.kdf, - ); - await this.stateProvider - .getUser(userId, MASTER_KEY) - .update((_) => new SymmetricCryptoKey(masterKey) as MasterKey); - } - // When unlocking, certain side-effects must be run, such as setting the never-lock key and the biometrics key. // Currently this does not happen from within the SDK but form here instead. private async runOnUnlockSideEffects(