From 6f169534cac85d7edb85a54129b89a9485833f44 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 25 Oct 2024 16:55:13 -0500 Subject: [PATCH 01/30] add api service --- ...asymmetric-key-regeneration-api.service.ts | 8 +++++ .../requests/key-regeneration.request.ts | 6 ++++ ...asymmetric-key-regeneration-api.service.ts | 29 +++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts create mode 100644 libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts create mode 100644 libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration-api.service.ts diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts new file mode 100644 index 00000000000..aac47be71b0 --- /dev/null +++ b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts @@ -0,0 +1,8 @@ +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; + +export abstract class UserAsymmetricKeysRegenerationApiService { + abstract regenerateUserAsymmetricKeys: ( + userPublicKey: string, + userKeyEncryptedUserPrivateKey: EncString, + ) => Promise; +} diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts b/libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts new file mode 100644 index 00000000000..044f58bb51f --- /dev/null +++ b/libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts @@ -0,0 +1,6 @@ +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; + +export class KeyRegenerationRequest { + userPublicKey: string; + userKeyEncryptedUserPrivateKey: EncString; +} diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration-api.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration-api.service.ts new file mode 100644 index 00000000000..d1fe89a74eb --- /dev/null +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration-api.service.ts @@ -0,0 +1,29 @@ +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; + +import { UserAsymmetricKeysRegenerationApiService } from "../abstractions/user-asymmetric-key-regeneration-api.service"; +import { KeyRegenerationRequest } from "../models/requests/key-regeneration.request"; + +export class DefaultUserAsymmetricKeysRegenerationApiService + implements UserAsymmetricKeysRegenerationApiService +{ + constructor(private apiService: ApiService) {} + + async regenerateUserAsymmetricKeys( + userPublicKey: string, + userKeyEncryptedUserPrivateKey: EncString, + ): Promise { + const request: KeyRegenerationRequest = { + userPublicKey, + userKeyEncryptedUserPrivateKey, + }; + + await this.apiService.send( + "POST", + "/accounts/key-management/regenerate-keys", + request, + true, + true, + ); + } +} From fc654494299e970c7b3e01fb484b7b2e2396f7ad Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 25 Oct 2024 16:56:24 -0500 Subject: [PATCH 02/30] Add user asymmetric key regen service --- ...ser-asymmetric-key-regeneration.service.ts | 6 + ...ser-asymmetric-key-regeneration.service.ts | 106 ++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts create mode 100644 libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts new file mode 100644 index 00000000000..c565e593a4e --- /dev/null +++ b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts @@ -0,0 +1,6 @@ +import { UserId } from "@bitwarden/common/types/guid"; + +export abstract class UserAsymmetricKeysRegenerationService { + abstract shouldRegenerate: (userId: UserId) => Promise; + abstract regenerateUserAsymmetricKeys: (userId: UserId) => Promise; +} diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts new file mode 100644 index 00000000000..82a39ef5555 --- /dev/null +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -0,0 +1,106 @@ +import { firstValueFrom } from "rxjs"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { StateProvider } from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; +import { UserKey } from "@bitwarden/common/types/key"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; + +import { KeyService } from "../../abstractions/key.service"; +import { UserAsymmetricKeysRegenerationApiService } from "../abstractions/user-asymmetric-key-regeneration-api.service"; +import { UserAsymmetricKeysRegenerationService } from "../abstractions/user-asymmetric-key-regeneration.service"; + +export class DefaultUserAsymmetricKeysRegenerationService + implements UserAsymmetricKeysRegenerationService +{ + constructor( + protected keyService: KeyService, + protected cipherService: CipherService, + protected regenerateUserAsymmetricKeysApiService: UserAsymmetricKeysRegenerationApiService, + protected stateProvider: StateProvider, + protected logService: LogService, + protected sdkService: SdkService, + protected apiService: ApiService, + ) {} + + async shouldRegenerate(userId: UserId): Promise { + const [userKey, userKeyEncryptedPrivateKey, publicKeyResponse] = await Promise.all([ + firstValueFrom(this.keyService.userKey$(userId)), + firstValueFrom(this.keyService.userEncryptedPrivateKey$(userId)), + this.apiService.getUserPublicKey(userId), + ]); + + const sdk = await firstValueFrom(this.sdkService.userClient$(userId)); + const verificationResponse = sdk.crypto().verify_asymmetric_keys({ + userPublicKey: publicKeyResponse.publicKey, + userKeyEncryptedPrivateKey: userKeyEncryptedPrivateKey, + }); + + if (verificationResponse.privateKeyDecryptable && verificationResponse.validPrivateKey) { + // Private key is decryptable and valid. Should not regenerate. + return false; + } + + if (!verificationResponse.privateKeyDecryptable) { + // Check to see if we can decrypt something with the userKey. + // If we can decrypt something with the userKey then return true. + const userKeyCanDecrypt = await this.userKeyCanDecrypt(userKey); + if (userKeyCanDecrypt) { + return true; + } + + this.logService.warning( + "[UserAsymmetricKeyRegeneration] User Asymmetric Key decryption failure detected, but unable to determine User Symmetric Key validity.", + ); + return false; + } + + if (verificationResponse.privateKeyDecryptable && !verificationResponse.validPrivateKey) { + // The private key is decryptable but not valid so we should regenerate it. + return true; + } + + return false; + } + + async regenerateUserAsymmetricKeys(userId: UserId): Promise { + const sdk = await firstValueFrom(this.sdkService.userClient$(userId)); + const response = sdk.crypto().make_key_pair(); + + try { + await this.regenerateUserAsymmetricKeysApiService.regenerateUserAsymmetricKeys( + response.userPublicKey, + new EncString(response.userKeyEncryptedPrivateKey), + ); + } catch (error) { + this.logService.error( + "[UserAsymmetricKeyRegeneration] User Key regeneration error when submitting the request to the server: " + + error, + ); + return; + } + + await this.keyService.setPrivateKey(response.userKeyEncryptedPrivateKey, userId); + } + + private async userKeyCanDecrypt(userKey: UserKey): Promise { + const ciphers = await this.cipherService.getAll(); + const cipher = ciphers.find((cipher) => cipher.organizationId == null); + + if (cipher != null) { + try { + await cipher.decrypt(userKey); + return true; + } catch (error) { + this.logService.error( + "[UserAsymmetricKeyRegeneration] User Key decryption error: " + error, + ); + return false; + } + } + return false; + } +} From 3a709cb77b5cb14a0a1248c61b19c603cf7b1b44 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 25 Oct 2024 16:56:46 -0500 Subject: [PATCH 03/30] add index files --- libs/key-management/src/index.ts | 2 ++ .../src/user-asymmetric-key-regeneration/index.ts | 5 +++++ 2 files changed, 7 insertions(+) create mode 100644 libs/key-management/src/user-asymmetric-key-regeneration/index.ts diff --git a/libs/key-management/src/index.ts b/libs/key-management/src/index.ts index f2bb5e30166..a815ff417ee 100644 --- a/libs/key-management/src/index.ts +++ b/libs/key-management/src/index.ts @@ -7,3 +7,5 @@ export * from "./biometrics/biometric.state"; export { KeyService } from "./abstractions/key.service"; export { DefaultKeyService } from "./key.service"; + +export * from "./user-asymmetric-key-regeneration"; diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/index.ts b/libs/key-management/src/user-asymmetric-key-regeneration/index.ts new file mode 100644 index 00000000000..8147d76b492 --- /dev/null +++ b/libs/key-management/src/user-asymmetric-key-regeneration/index.ts @@ -0,0 +1,5 @@ +export { UserAsymmetricKeysRegenerationService } from "./abstractions/user-asymmetric-key-regeneration.service"; +export { DefaultUserAsymmetricKeysRegenerationService } from "./services/default-user-asymmetric-key-regeneration.service"; + +export { UserAsymmetricKeysRegenerationApiService } from "./abstractions/user-asymmetric-key-regeneration-api.service"; +export { DefaultUserAsymmetricKeysRegenerationApiService } from "./services/default-user-asymmetric-key-regeneration-api.service"; From e0647a2c20ed5099db8436b26c2ac8923320ef0e Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 25 Oct 2024 16:56:56 -0500 Subject: [PATCH 04/30] add feature flag --- libs/common/src/enums/feature-flag.enum.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 905d7299489..8aa813c452d 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -37,6 +37,7 @@ export enum FeatureFlag { AccessIntelligence = "pm-13227-access-intelligence", Pm13322AddPolicyDefinitions = "pm-13322-add-policy-definitions", LimitCollectionCreationDeletionSplit = "pm-10863-limit-collection-creation-deletion-split", + PrivateKeyRegeneration = "pm-12241-private-key-regeneration", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -84,6 +85,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.AccessIntelligence]: FALSE, [FeatureFlag.Pm13322AddPolicyDefinitions]: FALSE, [FeatureFlag.LimitCollectionCreationDeletionSplit]: FALSE, + [FeatureFlag.PrivateKeyRegeneration]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; From 7c32f5c52a16747106b7059928fa32b55d9f264e Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 25 Oct 2024 17:04:05 -0500 Subject: [PATCH 05/30] wire up service call in login flow. --- .../src/services/jslib-services.module.ts | 24 +++++++++++++++++++ .../common/login-strategies/login.strategy.ts | 18 +++++++++++++- .../login-strategy.service.ts | 7 +++++- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 444fb55deec..40a19260151 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -274,6 +274,10 @@ import { DefaultKeyService as KeyService, BiometricStateService, DefaultBiometricStateService, + UserAsymmetricKeysRegenerationService, + DefaultUserAsymmetricKeysRegenerationService, + UserAsymmetricKeysRegenerationApiService, + DefaultUserAsymmetricKeysRegenerationApiService, } from "@bitwarden/key-management"; import { PasswordRepromptService } from "@bitwarden/vault"; import { @@ -435,6 +439,8 @@ const safeProviders: SafeProvider[] = [ VaultTimeoutSettingsServiceAbstraction, KdfConfigServiceAbstraction, TaskSchedulerService, + UserAsymmetricKeysRegenerationService, + ConfigService, ], }), safeProvider({ @@ -1365,6 +1371,24 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultCipherAuthorizationService, deps: [CollectionService, OrganizationServiceAbstraction], }), + safeProvider({ + provide: UserAsymmetricKeysRegenerationApiService, + useClass: DefaultUserAsymmetricKeysRegenerationApiService, + deps: [ApiServiceAbstraction], + }), + safeProvider({ + provide: UserAsymmetricKeysRegenerationService, + useClass: DefaultUserAsymmetricKeysRegenerationService, + deps: [ + KeyServiceAbstraction, + CipherServiceAbstraction, + UserAsymmetricKeysRegenerationApiService, + StateProvider, + LogService, + SdkService, + ApiServiceAbstraction, + ], + }), ]; @NgModule({ diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 67a286d8195..87449d7bb36 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -22,9 +22,11 @@ import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/id import { IdentityTwoFactorResponse } from "@bitwarden/common/auth/models/response/identity-two-factor.response"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { ClientType } from "@bitwarden/common/enums"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; import { KeysRequest } from "@bitwarden/common/models/request/keys.request"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -33,7 +35,7 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { KdfType } from "@bitwarden/common/platform/enums"; import { Account, AccountProfile } from "@bitwarden/common/platform/models/domain/account"; import { UserId } from "@bitwarden/common/types/guid"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { @@ -80,6 +82,8 @@ export abstract class LoginStrategy { protected billingAccountProfileStateService: BillingAccountProfileStateService, protected vaultTimeoutSettingsService: VaultTimeoutSettingsService, protected KdfConfigService: KdfConfigService, + protected UserAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, + protected configService: ConfigService, ) {} abstract exportCache(): CacheData; @@ -266,6 +270,18 @@ export abstract class LoginStrategy { await this.setUserKey(response, userId); await this.setPrivateKey(response, userId); + const privateKeyRegenerationFlag = await this.configService.getFeatureFlag( + FeatureFlag.PrivateKeyRegeneration, + ); + + if (privateKeyRegenerationFlag) { + const shouldRegenerate = + await this.UserAsymmetricKeysRegenerationService.shouldRegenerate(userId); + if (shouldRegenerate) { + await this.UserAsymmetricKeysRegenerationService.regenerateUserAsymmetricKeys(userId); + } + } + this.messagingService.send("loggedIn"); return result; 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 721ee984974..2613ab2fe30 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 @@ -29,6 +29,7 @@ import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abs import { PreloginRequest } from "@bitwarden/common/models/request/prelogin.request"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -42,7 +43,7 @@ import { GlobalState, GlobalStateProvider } from "@bitwarden/common/platform/sta import { DeviceTrustServiceAbstraction } from "@bitwarden/common/src/auth/abstractions/device-trust.service.abstraction"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { MasterKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { AuthRequestServiceAbstraction, LoginStrategyServiceAbstraction } from "../../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../../abstractions/user-decryption-options.service.abstraction"; @@ -114,6 +115,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { protected vaultTimeoutSettingsService: VaultTimeoutSettingsService, protected kdfConfigService: KdfConfigService, protected taskSchedulerService: TaskSchedulerService, + protected userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, + protected configService: ConfigService, ) { this.currentAuthnTypeState = this.stateProvider.get(CURRENT_LOGIN_STRATEGY_KEY); this.loginStrategyCacheState = this.stateProvider.get(CACHE_KEY); @@ -333,6 +336,8 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { this.billingAccountProfileStateService, this.vaultTimeoutSettingsService, this.kdfConfigService, + this.userAsymmetricKeysRegenerationService, + this.configService, ]; return source.pipe( From 654ae056b34964c2fc6006541b427093b630ee2f Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Thu, 31 Oct 2024 17:18:06 -0500 Subject: [PATCH 06/30] fix sdk calls --- ...ser-asymmetric-key-regeneration.service.ts | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index 82a39ef5555..93ee2d8c608 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, map } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -33,11 +33,16 @@ export class DefaultUserAsymmetricKeysRegenerationService this.apiService.getUserPublicKey(userId), ]); - const sdk = await firstValueFrom(this.sdkService.userClient$(userId)); - const verificationResponse = sdk.crypto().verify_asymmetric_keys({ - userPublicKey: publicKeyResponse.publicKey, - userKeyEncryptedPrivateKey: userKeyEncryptedPrivateKey, - }); + const verificationResponse = await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + map((sdk) => + sdk.crypto().verify_asymmetric_keys({ + userPublicKey: publicKeyResponse.publicKey, + userKeyEncryptedPrivateKey: userKeyEncryptedPrivateKey, + }), + ), + ), + ); if (verificationResponse.privateKeyDecryptable && verificationResponse.validPrivateKey) { // Private key is decryptable and valid. Should not regenerate. @@ -67,13 +72,14 @@ export class DefaultUserAsymmetricKeysRegenerationService } async regenerateUserAsymmetricKeys(userId: UserId): Promise { - const sdk = await firstValueFrom(this.sdkService.userClient$(userId)); - const response = sdk.crypto().make_key_pair(); + const makeKeyPairResponse = await firstValueFrom( + this.sdkService.userClient$(userId).pipe(map((sdk) => sdk.crypto().make_key_pair())), + ); try { await this.regenerateUserAsymmetricKeysApiService.regenerateUserAsymmetricKeys( - response.userPublicKey, - new EncString(response.userKeyEncryptedPrivateKey), + makeKeyPairResponse.userPublicKey, + new EncString(makeKeyPairResponse.userKeyEncryptedPrivateKey), ); } catch (error) { this.logService.error( @@ -83,7 +89,7 @@ export class DefaultUserAsymmetricKeysRegenerationService return; } - await this.keyService.setPrivateKey(response.userKeyEncryptedPrivateKey, userId); + await this.keyService.setPrivateKey(makeKeyPairResponse.userKeyEncryptedPrivateKey, userId); } private async userKeyCanDecrypt(userKey: UserKey): Promise { From 5c090ce40dca975a5f04bbfe5bdb5ad96a42ccb8 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 1 Nov 2024 15:38:14 -0500 Subject: [PATCH 07/30] change interface --- .../src/services/jslib-services.module.ts | 2 +- .../login-strategy.service.ts | 3 --- ...ser-asymmetric-key-regeneration.service.ts | 6 ++++-- ...ser-asymmetric-key-regeneration.service.ts | 20 +++++++++++++++++-- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 04a99947a90..953a6aa2ebf 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -440,7 +440,6 @@ const safeProviders: SafeProvider[] = [ KdfConfigServiceAbstraction, TaskSchedulerService, UserAsymmetricKeysRegenerationService, - ConfigService, ], }), safeProvider({ @@ -1387,6 +1386,7 @@ const safeProviders: SafeProvider[] = [ LogService, SdkService, ApiServiceAbstraction, + ConfigService, ], }), ]; 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 2613ab2fe30..26d6b60c3db 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 @@ -29,7 +29,6 @@ import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abs import { PreloginRequest } from "@bitwarden/common/models/request/prelogin.request"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -116,7 +115,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { protected kdfConfigService: KdfConfigService, protected taskSchedulerService: TaskSchedulerService, protected userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, - protected configService: ConfigService, ) { this.currentAuthnTypeState = this.stateProvider.get(CURRENT_LOGIN_STRATEGY_KEY); this.loginStrategyCacheState = this.stateProvider.get(CACHE_KEY); @@ -337,7 +335,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { this.vaultTimeoutSettingsService, this.kdfConfigService, this.userAsymmetricKeysRegenerationService, - this.configService, ]; return source.pipe( diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts index c565e593a4e..407338f845b 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts @@ -1,6 +1,8 @@ import { UserId } from "@bitwarden/common/types/guid"; export abstract class UserAsymmetricKeysRegenerationService { - abstract shouldRegenerate: (userId: UserId) => Promise; - abstract regenerateUserAsymmetricKeys: (userId: UserId) => Promise; + /** + * Handle regeneration of the user's asymmetric keys if they are invalid. + */ + abstract handleUserAsymmetricKeysRegeneration: (userId: UserId) => Promise; } diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index 93ee2d8c608..cc894dd8263 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -1,6 +1,8 @@ import { firstValueFrom, map } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; @@ -24,9 +26,23 @@ export class DefaultUserAsymmetricKeysRegenerationService protected logService: LogService, protected sdkService: SdkService, protected apiService: ApiService, + protected configService: ConfigService, ) {} - async shouldRegenerate(userId: UserId): Promise { + async handleUserAsymmetricKeysRegeneration(userId: UserId): Promise { + const privateKeyRegenerationFlag = await this.configService.getFeatureFlag( + FeatureFlag.PrivateKeyRegeneration, + ); + + if (privateKeyRegenerationFlag) { + const shouldRegenerate = await this.shouldRegenerate(userId); + if (shouldRegenerate) { + await this.regenerateUserAsymmetricKeys(userId); + } + } + } + + private async shouldRegenerate(userId: UserId): Promise { const [userKey, userKeyEncryptedPrivateKey, publicKeyResponse] = await Promise.all([ firstValueFrom(this.keyService.userKey$(userId)), firstValueFrom(this.keyService.userEncryptedPrivateKey$(userId)), @@ -71,7 +87,7 @@ export class DefaultUserAsymmetricKeysRegenerationService return false; } - async regenerateUserAsymmetricKeys(userId: UserId): Promise { + private async regenerateUserAsymmetricKeys(userId: UserId): Promise { const makeKeyPairResponse = await firstValueFrom( this.sdkService.userClient$(userId).pipe(map((sdk) => sdk.crypto().make_key_pair())), ); From 630eb146867d9ddaa7b9b99ded121e4aa887be13 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 1 Nov 2024 15:38:30 -0500 Subject: [PATCH 08/30] fix unit tests --- .../auth-request-login.strategy.spec.ts | 5 ++++- .../login-strategies/login.strategy.spec.ts | 6 +++++- .../common/login-strategies/login.strategy.ts | 17 ++--------------- .../password-login.strategy.spec.ts | 5 ++++- .../login-strategies/sso-login.strategy.spec.ts | 5 ++++- .../user-api-login.strategy.spec.ts | 5 ++++- .../webauthn-login.strategy.spec.ts | 5 ++++- .../login-strategy.service.spec.ts | 5 ++++- 8 files changed, 31 insertions(+), 22 deletions(-) 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 c0e7d2c00ae..a62c1b71689 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 @@ -23,7 +23,7 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { AuthRequestLoginCredentials } from "../models/domain/login-credentials"; @@ -52,6 +52,7 @@ describe("AuthRequestLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; + let userAsymmetricKeysRegenerationService: MockProxy; const mockUserId = Utils.newGuid() as UserId; let accountService: FakeAccountService; @@ -87,6 +88,7 @@ describe("AuthRequestLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); + userAsymmetricKeysRegenerationService = mock(); accountService = mockAccountServiceWith(mockUserId); masterPasswordService = new FakeMasterPasswordService(); @@ -116,6 +118,7 @@ describe("AuthRequestLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, + userAsymmetricKeysRegenerationService, ); tokenResponse = identityTokenResponseFactory(); diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 49140cc2cc0..15b4d5dc813 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -38,7 +38,7 @@ import { import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { LoginStrategyServiceAbstraction } from "../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; @@ -120,6 +120,7 @@ describe("LoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; + let userAsymmetricKeysRegenerationService: MockProxy; let passwordLoginStrategy: PasswordLoginStrategy; let credentials: PasswordLoginCredentials; @@ -144,6 +145,7 @@ describe("LoginStrategy", () => { policyService = mock(); passwordStrengthService = mock(); billingAccountProfileStateService = mock(); + userAsymmetricKeysRegenerationService = mock(); vaultTimeoutSettingsService = mock(); @@ -172,6 +174,7 @@ describe("LoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, + userAsymmetricKeysRegenerationService, ); credentials = new PasswordLoginCredentials(email, masterPassword); }); @@ -484,6 +487,7 @@ describe("LoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, + userAsymmetricKeysRegenerationService, ); apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory()); diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 87449d7bb36..c77b62e8dca 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -22,11 +22,9 @@ import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/id import { IdentityTwoFactorResponse } from "@bitwarden/common/auth/models/response/identity-two-factor.response"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { ClientType } from "@bitwarden/common/enums"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; import { KeysRequest } from "@bitwarden/common/models/request/keys.request"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -82,8 +80,7 @@ export abstract class LoginStrategy { protected billingAccountProfileStateService: BillingAccountProfileStateService, protected vaultTimeoutSettingsService: VaultTimeoutSettingsService, protected KdfConfigService: KdfConfigService, - protected UserAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, - protected configService: ConfigService, + protected userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) {} abstract exportCache(): CacheData; @@ -270,17 +267,7 @@ export abstract class LoginStrategy { await this.setUserKey(response, userId); await this.setPrivateKey(response, userId); - const privateKeyRegenerationFlag = await this.configService.getFeatureFlag( - FeatureFlag.PrivateKeyRegeneration, - ); - - if (privateKeyRegenerationFlag) { - const shouldRegenerate = - await this.UserAsymmetricKeysRegenerationService.shouldRegenerate(userId); - if (shouldRegenerate) { - await this.UserAsymmetricKeysRegenerationService.regenerateUserAsymmetricKeys(userId); - } - } + await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); this.messagingService.send("loggedIn"); 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 4da6272ccab..7663e64b089 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 @@ -32,7 +32,7 @@ import { import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { LoginStrategyServiceAbstraction } from "../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; @@ -79,6 +79,7 @@ describe("PasswordLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; + let userAsymmetricKeysRegenerationService: MockProxy; let passwordLoginStrategy: PasswordLoginStrategy; let credentials: PasswordLoginCredentials; @@ -105,6 +106,7 @@ describe("PasswordLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); + userAsymmetricKeysRegenerationService = mock(); appIdService.getAppId.mockResolvedValue(deviceId); tokenService.decodeAccessToken.mockResolvedValue({ @@ -143,6 +145,7 @@ describe("PasswordLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, + userAsymmetricKeysRegenerationService, ); credentials = new PasswordLoginCredentials(email, masterPassword); tokenResponse = identityTokenResponseFactory(masterPasswordPolicy); 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 d9827c2e287..56e2b61eb4e 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 @@ -29,7 +29,7 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { DeviceKey, UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { AuthRequestServiceAbstraction, @@ -62,6 +62,7 @@ describe("SsoLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; + let userAsymmetricKeysRegenerationService: MockProxy; let ssoLoginStrategy: SsoLoginStrategy; let credentials: SsoLoginCredentials; @@ -97,6 +98,7 @@ describe("SsoLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); + userAsymmetricKeysRegenerationService = mock(); tokenService.getTwoFactorToken.mockResolvedValue(null); appIdService.getAppId.mockResolvedValue(deviceId); @@ -141,6 +143,7 @@ describe("SsoLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, + userAsymmetricKeysRegenerationService, ); credentials = new SsoLoginCredentials(ssoCode, ssoCodeVerifier, ssoRedirectUrl, ssoOrgId); }); 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 14fafcb58c3..97fcde560d5 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 @@ -26,7 +26,7 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { UserApiLoginCredentials } from "../models/domain/login-credentials"; @@ -55,6 +55,7 @@ describe("UserApiLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; + let userAsymmetricKeysRegenerationService: MockProxy; let apiLogInStrategy: UserApiLoginStrategy; let credentials: UserApiLoginCredentials; @@ -87,6 +88,7 @@ describe("UserApiLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); + userAsymmetricKeysRegenerationService = mock(); appIdService.getAppId.mockResolvedValue(deviceId); tokenService.getTwoFactorToken.mockResolvedValue(null); @@ -114,6 +116,7 @@ describe("UserApiLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, + userAsymmetricKeysRegenerationService, ); credentials = new UserApiLoginCredentials(apiClientId, apiClientSecret); 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 88392b57c53..b7cdc91d63f 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 @@ -24,7 +24,7 @@ import { VaultTimeoutSettingsService } from "@bitwarden/common/services/vault-ti import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; import { PrfKey, UserKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { WebAuthnLoginCredentials } from "../models/domain/login-credentials"; @@ -51,6 +51,7 @@ describe("WebAuthnLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; + let userAsymmetricKeysRegenerationService: MockProxy; let webAuthnLoginStrategy!: WebAuthnLoginStrategy; @@ -94,6 +95,7 @@ describe("WebAuthnLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); + userAsymmetricKeysRegenerationService = mock(); tokenService.getTwoFactorToken.mockResolvedValue(null); appIdService.getAppId.mockResolvedValue(deviceId); @@ -119,6 +121,7 @@ describe("WebAuthnLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, + userAsymmetricKeysRegenerationService, ); // Create credentials diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts index b0d9228f446..35a1d87ac5b 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts @@ -37,7 +37,7 @@ import { } from "@bitwarden/common/spec"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { UserId } from "@bitwarden/common/types/guid"; -import { KeyService } from "@bitwarden/key-management"; +import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { AuthRequestServiceAbstraction, @@ -76,6 +76,7 @@ describe("LoginStrategyService", () => { let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; let taskSchedulerService: MockProxy; + let userAsymmetricKeysRegenerationService: MockProxy; let stateProvider: FakeGlobalStateProvider; let loginStrategyCacheExpirationState: FakeGlobalState; @@ -108,6 +109,7 @@ describe("LoginStrategyService", () => { vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); taskSchedulerService = mock(); + userAsymmetricKeysRegenerationService = mock(); sut = new LoginStrategyService( accountService, @@ -135,6 +137,7 @@ describe("LoginStrategyService", () => { vaultTimeoutSettingsService, kdfConfigService, taskSchedulerService, + userAsymmetricKeysRegenerationService, ); loginStrategyCacheExpirationState = stateProvider.getFake(CACHE_EXPIRATION_KEY); From c6ce0c1766d6b6fa309f91f66c569691aae1d0b5 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 1 Nov 2024 16:07:18 -0500 Subject: [PATCH 09/30] Add try catch to handler --- ...ser-asymmetric-key-regeneration.service.ts | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index cc894dd8263..d2d2e58775d 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -30,15 +30,23 @@ export class DefaultUserAsymmetricKeysRegenerationService ) {} async handleUserAsymmetricKeysRegeneration(userId: UserId): Promise { - const privateKeyRegenerationFlag = await this.configService.getFeatureFlag( - FeatureFlag.PrivateKeyRegeneration, - ); + try { + const privateKeyRegenerationFlag = await this.configService.getFeatureFlag( + FeatureFlag.PrivateKeyRegeneration, + ); - if (privateKeyRegenerationFlag) { - const shouldRegenerate = await this.shouldRegenerate(userId); - if (shouldRegenerate) { - await this.regenerateUserAsymmetricKeys(userId); + if (privateKeyRegenerationFlag) { + const shouldRegenerate = await this.shouldRegenerate(userId); + if (shouldRegenerate) { + await this.regenerateUserAsymmetricKeys(userId); + } } + } catch (error) { + this.logService.error( + "[UserAsymmetricKeyRegeneration] User Key regeneration error: " + + error + + " Skipping regeneration for the user.", + ); } } From 42ce721707737ea650936e12b91ad6c2eda9af1d Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 1 Nov 2024 16:55:59 -0500 Subject: [PATCH 10/30] Add call to lock component --- libs/angular/src/auth/components/lock.component.ts | 11 ++++++++++- libs/auth/src/angular/lock/lock.component.ts | 10 +++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index bc9c667bc87..79635dbf6f0 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -34,7 +34,12 @@ import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { DialogService, ToastService } from "@bitwarden/components"; -import { KeyService, BiometricStateService, BiometricsService } from "@bitwarden/key-management"; +import { + KeyService, + BiometricStateService, + BiometricsService, + UserAsymmetricKeysRegenerationService, +} from "@bitwarden/key-management"; @Directive() export class LockComponent implements OnInit, OnDestroy { @@ -89,6 +94,7 @@ export class LockComponent implements OnInit, OnDestroy { protected kdfConfigService: KdfConfigService, protected syncService: SyncService, protected toastService: ToastService, + protected userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) {} async ngOnInit() { @@ -319,6 +325,9 @@ export class LockComponent implements OnInit, OnDestroy { // Vault can be de-synced since notifications get ignored while locked. Need to check whether sync is required using the sync service. await this.syncService.fullSync(false); + const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); + if (this.onSuccessfulSubmit != null) { await this.onSuccessfulSubmit(); } else if (this.router != null) { diff --git a/libs/auth/src/angular/lock/lock.component.ts b/libs/auth/src/angular/lock/lock.component.ts index 3d4bf51e804..72430502ed7 100644 --- a/libs/auth/src/angular/lock/lock.component.ts +++ b/libs/auth/src/angular/lock/lock.component.ts @@ -35,7 +35,11 @@ import { IconButtonModule, ToastService, } from "@bitwarden/components"; -import { KeyService, BiometricStateService } from "@bitwarden/key-management"; +import { + KeyService, + BiometricStateService, + UserAsymmetricKeysRegenerationService, +} from "@bitwarden/key-management"; import { PinServiceAbstraction } from "../../common/abstractions"; import { AnonLayoutWrapperDataService } from "../anon-layout/anon-layout-wrapper-data.service"; @@ -137,6 +141,7 @@ export class LockV2Component implements OnInit, OnDestroy { private passwordStrengthService: PasswordStrengthServiceAbstraction, private formBuilder: FormBuilder, private toastService: ToastService, + private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, private lockComponentService: LockComponentService, private anonLayoutWrapperDataService: AnonLayoutWrapperDataService, @@ -528,6 +533,9 @@ export class LockV2Component implements OnInit, OnDestroy { // Vault can be de-synced since notifications get ignored while locked. Need to check whether sync is required using the sync service. await this.syncService.fullSync(false); + const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); + if (this.clientType === "browser") { const previousUrl = this.lockComponentService.getPreviousUrl(); if (previousUrl) { From 9f1f8cf04a81ff95182ad1efbfe5c28705154517 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Tue, 5 Nov 2024 18:07:25 -0600 Subject: [PATCH 11/30] Add km libs to jest config --- jest.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/jest.config.js b/jest.config.js index 829adf1bf72..3ed082bcbc3 100644 --- a/jest.config.js +++ b/jest.config.js @@ -41,6 +41,7 @@ module.exports = { "/libs/platform/jest.config.js", "/libs/node/jest.config.js", "/libs/vault/jest.config.js", + "/libs/key-management/jest.config.js", ], // Workaround for a memory leak that crashes tests in CI: From ac1e08a04d0d3c48ba52a39e3cc8d16fecf53c43 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Tue, 5 Nov 2024 18:07:52 -0600 Subject: [PATCH 12/30] Add missing service to lock component --- apps/browser/src/auth/popup/lock.component.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/browser/src/auth/popup/lock.component.ts b/apps/browser/src/auth/popup/lock.component.ts index c7fb108de80..3c88c13361f 100644 --- a/apps/browser/src/auth/popup/lock.component.ts +++ b/apps/browser/src/auth/popup/lock.component.ts @@ -25,7 +25,12 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { DialogService, ToastService } from "@bitwarden/components"; -import { KeyService, BiometricsService, BiometricStateService } from "@bitwarden/key-management"; +import { + KeyService, + BiometricsService, + BiometricStateService, + UserAsymmetricKeysRegenerationService, +} from "@bitwarden/key-management"; import { BiometricErrors, BiometricErrorTypes } from "../../models/biometricErrors"; import { BrowserRouterService } from "../../platform/popup/services/browser-router.service"; @@ -71,6 +76,7 @@ export class LockComponent extends BaseLockComponent implements OnInit { kdfConfigService: KdfConfigService, syncService: SyncService, toastService: ToastService, + userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( masterPasswordService, @@ -100,6 +106,7 @@ export class LockComponent extends BaseLockComponent implements OnInit { kdfConfigService, syncService, toastService, + userAsymmetricKeysRegenerationService, ); this.successRoute = "/tabs/current"; this.isInitialLockScreen = (window as any).previousPopupUrl == null; From 8ac7d64cde8f8e576d48f41dec14bdfb732aadec Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Tue, 5 Nov 2024 18:08:12 -0600 Subject: [PATCH 13/30] cleanup and refactoring of service --- .../src/services/jslib-services.module.ts | 1 - ...ser-asymmetric-key-regeneration.service.ts | 49 +++++++++---------- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 953a6aa2ebf..ca4c2282098 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1382,7 +1382,6 @@ const safeProviders: SafeProvider[] = [ KeyServiceAbstraction, CipherServiceAbstraction, UserAsymmetricKeysRegenerationApiService, - StateProvider, LogService, SdkService, ApiServiceAbstraction, diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index d2d2e58775d..ef170383c03 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -6,7 +6,6 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; -import { StateProvider } from "@bitwarden/common/platform/state"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -21,8 +20,7 @@ export class DefaultUserAsymmetricKeysRegenerationService constructor( protected keyService: KeyService, protected cipherService: CipherService, - protected regenerateUserAsymmetricKeysApiService: UserAsymmetricKeysRegenerationApiService, - protected stateProvider: StateProvider, + protected userAsymmetricKeysRegenerationApiService: UserAsymmetricKeysRegenerationApiService, protected logService: LogService, protected sdkService: SdkService, protected apiService: ApiService, @@ -68,30 +66,25 @@ export class DefaultUserAsymmetricKeysRegenerationService ), ); - if (verificationResponse.privateKeyDecryptable && verificationResponse.validPrivateKey) { - // Private key is decryptable and valid. Should not regenerate. - return false; - } - - if (!verificationResponse.privateKeyDecryptable) { - // Check to see if we can decrypt something with the userKey. - // If we can decrypt something with the userKey then return true. - const userKeyCanDecrypt = await this.userKeyCanDecrypt(userKey); - if (userKeyCanDecrypt) { + if (verificationResponse.privateKeyDecryptable) { + if (verificationResponse.validPrivateKey) { + // The private key is decryptable and valid. Should not regenerate. + return false; + } else { + // The private key is decryptable but not valid so we should regenerate it. return true; } - - this.logService.warning( - "[UserAsymmetricKeyRegeneration] User Asymmetric Key decryption failure detected, but unable to determine User Symmetric Key validity.", - ); - return false; } - if (verificationResponse.privateKeyDecryptable && !verificationResponse.validPrivateKey) { - // The private key is decryptable but not valid so we should regenerate it. + // The private isn't decryptable, check to see if we can decrypt something with the userKey. + const userKeyCanDecrypt = await this.userKeyCanDecrypt(userKey); + if (userKeyCanDecrypt) { return true; } + this.logService.warning( + "[UserAsymmetricKeyRegeneration] User Asymmetric Key decryption failure detected, but unable to determine User Symmetric Key validity.", + ); return false; } @@ -101,15 +94,21 @@ export class DefaultUserAsymmetricKeysRegenerationService ); try { - await this.regenerateUserAsymmetricKeysApiService.regenerateUserAsymmetricKeys( + await this.userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys( makeKeyPairResponse.userPublicKey, new EncString(makeKeyPairResponse.userKeyEncryptedPrivateKey), ); } catch (error) { - this.logService.error( - "[UserAsymmetricKeyRegeneration] User Key regeneration error when submitting the request to the server: " + - error, - ); + if (error.message === "Key regeneration not supported for this user.") { + this.logService.info( + "[UserAsymmetricKeyRegeneration] User Key regeneration not supported for this user at this time.", + ); + } else { + this.logService.error( + "[UserAsymmetricKeyRegeneration] User Key regeneration error when submitting the request to the server: " + + error, + ); + } return; } From c049384d8380f886a2149b20610d8f5acb2b3294 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Tue, 5 Nov 2024 18:08:40 -0600 Subject: [PATCH 14/30] Add unit tests to service --- ...symmetric-key-regeneration.service.spec.ts | 309 ++++++++++++++++++ 1 file changed, 309 insertions(+) create mode 100644 libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts new file mode 100644 index 00000000000..337a7eb9f18 --- /dev/null +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -0,0 +1,309 @@ +import { MockProxy, mock } from "jest-mock-extended"; +import { of, throwError } from "rxjs"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { ContainerService } from "@bitwarden/common/platform/services/container.service"; +import { makeStaticByteArray, mockEnc } from "@bitwarden/common/spec"; +import { CsprngArray } from "@bitwarden/common/types/csprng"; +import { UserId } from "@bitwarden/common/types/guid"; +import { UserKey } from "@bitwarden/common/types/key"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; +import { BitwardenClient, VerifyAsymmetricKeysResponse } from "@bitwarden/sdk-internal"; + +import { KeyService } from "../../abstractions/key.service"; +import { UserAsymmetricKeysRegenerationApiService } from "../abstractions/user-asymmetric-key-regeneration-api.service"; + +import { DefaultUserAsymmetricKeysRegenerationService } from "./default-user-asymmetric-key-regeneration.service"; + +function setupVerificationResponse( + mockVerificationResponse: VerifyAsymmetricKeysResponse, + sdkService: MockProxy, +) { + const mockKeyPairResponse = { + userPublicKey: "userPublicKey", + userKeyEncryptedPrivateKey: "userKeyEncryptedPrivateKey", + }; + + sdkService.userClient$.mockReturnValue( + of({ + crypto: () => ({ + verify_asymmetric_keys: jest.fn().mockReturnValue(mockVerificationResponse), + make_key_pair: jest.fn().mockReturnValue(mockKeyPairResponse), + }), + free: jest.fn(), + echo: jest.fn(), + version: jest.fn(), + throw: jest.fn(), + catch: jest.fn(), + } as unknown as BitwardenClient), + ); +} + +function setupUserKeyValidation( + cipherService: MockProxy, + keyService: MockProxy, + encryptService: MockProxy, +) { + const cipher = new Cipher(); + cipher.id = "id"; + cipher.edit = true; + cipher.viewPassword = true; + cipher.favorite = false; + cipher.name = mockEnc("EncryptedString"); + cipher.notes = mockEnc("EncryptedString"); + cipher.deletedDate = null; + cipher.key = mockEnc("EncKey"); + cipherService.getAll.mockResolvedValue([cipher]); + encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); + (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); +} + +describe("handleUserAsymmetricKeysRegeneration", () => { + let sut: DefaultUserAsymmetricKeysRegenerationService; + const userId = "userId" as UserId; + + let keyService: MockProxy; + let cipherService: MockProxy; + let userAsymmetricKeysRegenerationApiService: MockProxy; + let logService: MockProxy; + let sdkService: MockProxy; + let apiService: MockProxy; + let configService: MockProxy; + let encryptService: MockProxy; + + beforeEach(() => { + keyService = mock(); + cipherService = mock(); + userAsymmetricKeysRegenerationApiService = mock(); + logService = mock(); + sdkService = mock(); + apiService = mock(); + configService = mock(); + encryptService = mock(); + + sut = new DefaultUserAsymmetricKeysRegenerationService( + keyService, + cipherService, + userAsymmetricKeysRegenerationApiService, + logService, + sdkService, + apiService, + configService, + ); + + configService.getFeatureFlag.mockResolvedValue(true); + + const mockRandomBytes = new Uint8Array(64) as CsprngArray; + const mockEncryptedString = new SymmetricCryptoKey( + mockRandomBytes, + ).toString() as EncryptedString; + const mockUserKey = new SymmetricCryptoKey(mockRandomBytes) as UserKey; + keyService.userKey$.mockReturnValue(of(mockUserKey)); + keyService.userEncryptedPrivateKey$.mockReturnValue(of(mockEncryptedString)); + apiService.getUserPublicKey.mockResolvedValue({ + userId: "userId", + publicKey: "publicKey", + } as any); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it("should not call regeneration code when feature flag is off", async () => { + configService.getFeatureFlag.mockResolvedValue(false); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect(keyService.userKey$).not.toHaveBeenCalled(); + }); + + it("should not regenerate when top level error is thrown", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: true, + validPrivateKey: false, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + keyService.userKey$.mockReturnValue(throwError(() => new Error("error"))); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); + + it("should not regenerate when private key is decryptable and valid", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: true, + validPrivateKey: true, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); + + it("should regenerate when private key is decryptable and invalid", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: true, + validPrivateKey: false, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).toHaveBeenCalled(); + expect(keyService.setPrivateKey).toHaveBeenCalled(); + }); + + it("should not set private key on known API error", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: true, + validPrivateKey: false, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys.mockRejectedValue( + new Error("Key regeneration not supported for this user."), + ); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); + + it("should not set private key on unknown API error", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: true, + validPrivateKey: false, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys.mockRejectedValue( + new Error("error"), + ); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); + + it("should regenerate when private key is not decryptable and user key is valid", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: false, + validPrivateKey: true, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + setupUserKeyValidation(cipherService, keyService, encryptService); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).toHaveBeenCalled(); + expect(keyService.setPrivateKey).toHaveBeenCalled(); + }); + + it("should not regenerate when private key is not decryptable and user key is invalid", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: false, + validPrivateKey: true, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + setupUserKeyValidation(cipherService, keyService, encryptService); + encryptService.decryptToBytes.mockRejectedValue(new Error("error")); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); + + it("should not regenerate when private key is not decryptable and no ciphers to check", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: false, + validPrivateKey: true, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + cipherService.getAll.mockResolvedValue([]); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); + + it("should regenerate when private key is not decryptable and invalid and user key is valid", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: false, + validPrivateKey: false, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + setupUserKeyValidation(cipherService, keyService, encryptService); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).toHaveBeenCalled(); + expect(keyService.setPrivateKey).toHaveBeenCalled(); + }); + + it("should not regenerate when private key is not decryptable and invalid and user key is invalid", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: false, + validPrivateKey: false, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + setupUserKeyValidation(cipherService, keyService, encryptService); + encryptService.decryptToBytes.mockRejectedValue(new Error("error")); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); + + it("should not regenerate when private key is not decryptable and invalid and no ciphers to check", async () => { + const mockVerificationResponse: VerifyAsymmetricKeysResponse = { + privateKeyDecryptable: false, + validPrivateKey: false, + }; + setupVerificationResponse(mockVerificationResponse, sdkService); + cipherService.getAll.mockResolvedValue([]); + + await sut.handleUserAsymmetricKeysRegeneration(userId); + + expect( + userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, + ).not.toHaveBeenCalled(); + expect(keyService.setPrivateKey).not.toHaveBeenCalled(); + }); +}); From e877375158cb2282b832570f68a9ee54a9c96a04 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Wed, 13 Nov 2024 17:41:00 -0600 Subject: [PATCH 15/30] update sdk calls --- ...symmetric-key-regeneration.service.spec.ts | 24 +++++++++---------- ...ser-asymmetric-key-regeneration.service.ts | 6 +++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts index 337a7eb9f18..5bf258d0e70 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -31,19 +31,17 @@ function setupVerificationResponse( userKeyEncryptedPrivateKey: "userKeyEncryptedPrivateKey", }; - sdkService.userClient$.mockReturnValue( - of({ - crypto: () => ({ - verify_asymmetric_keys: jest.fn().mockReturnValue(mockVerificationResponse), - make_key_pair: jest.fn().mockReturnValue(mockKeyPairResponse), - }), - free: jest.fn(), - echo: jest.fn(), - version: jest.fn(), - throw: jest.fn(), - catch: jest.fn(), - } as unknown as BitwardenClient), - ); + sdkService.client$ = of({ + crypto: () => ({ + verify_asymmetric_keys: jest.fn().mockReturnValue(mockVerificationResponse), + make_key_pair: jest.fn().mockReturnValue(mockKeyPairResponse), + }), + free: jest.fn(), + echo: jest.fn(), + version: jest.fn(), + throw: jest.fn(), + catch: jest.fn(), + } as unknown as BitwardenClient); } function setupUserKeyValidation( diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index ef170383c03..1490496198e 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -56,9 +56,10 @@ export class DefaultUserAsymmetricKeysRegenerationService ]); const verificationResponse = await firstValueFrom( - this.sdkService.userClient$(userId).pipe( + this.sdkService.client$.pipe( map((sdk) => sdk.crypto().verify_asymmetric_keys({ + userKey: userKey.keyB64, userPublicKey: publicKeyResponse.publicKey, userKeyEncryptedPrivateKey: userKeyEncryptedPrivateKey, }), @@ -89,8 +90,9 @@ export class DefaultUserAsymmetricKeysRegenerationService } private async regenerateUserAsymmetricKeys(userId: UserId): Promise { + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); const makeKeyPairResponse = await firstValueFrom( - this.sdkService.userClient$(userId).pipe(map((sdk) => sdk.crypto().make_key_pair())), + this.sdkService.client$.pipe(map((sdk) => sdk.crypto().make_key_pair(userKey.keyB64))), ); try { From 4769f861905bd46827060b85341d85a88df57d88 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Tue, 19 Nov 2024 15:54:51 -0600 Subject: [PATCH 16/30] remove from login strategy --- libs/angular/src/services/jslib-services.module.ts | 1 - .../login-strategies/auth-request-login.strategy.spec.ts | 5 +---- .../auth/src/common/login-strategies/login.strategy.spec.ts | 6 +----- libs/auth/src/common/login-strategies/login.strategy.ts | 5 +---- .../common/login-strategies/password-login.strategy.spec.ts | 5 +---- .../src/common/login-strategies/sso-login.strategy.spec.ts | 5 +---- .../common/login-strategies/user-api-login.strategy.spec.ts | 5 +---- .../common/login-strategies/webauthn-login.strategy.spec.ts | 5 +---- .../login-strategies/login-strategy.service.spec.ts | 5 +---- .../services/login-strategies/login-strategy.service.ts | 4 +--- 10 files changed, 9 insertions(+), 37 deletions(-) diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 5a1f5227816..d17d3450432 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -440,7 +440,6 @@ const safeProviders: SafeProvider[] = [ VaultTimeoutSettingsServiceAbstraction, KdfConfigServiceAbstraction, TaskSchedulerService, - UserAsymmetricKeysRegenerationService, ], }), safeProvider({ 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 a62c1b71689..c0e7d2c00ae 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 @@ -23,7 +23,7 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { AuthRequestLoginCredentials } from "../models/domain/login-credentials"; @@ -52,7 +52,6 @@ describe("AuthRequestLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; - let userAsymmetricKeysRegenerationService: MockProxy; const mockUserId = Utils.newGuid() as UserId; let accountService: FakeAccountService; @@ -88,7 +87,6 @@ describe("AuthRequestLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); - userAsymmetricKeysRegenerationService = mock(); accountService = mockAccountServiceWith(mockUserId); masterPasswordService = new FakeMasterPasswordService(); @@ -118,7 +116,6 @@ describe("AuthRequestLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, - userAsymmetricKeysRegenerationService, ); tokenResponse = identityTokenResponseFactory(); diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 15b4d5dc813..49140cc2cc0 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -38,7 +38,7 @@ import { import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { LoginStrategyServiceAbstraction } from "../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; @@ -120,7 +120,6 @@ describe("LoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; - let userAsymmetricKeysRegenerationService: MockProxy; let passwordLoginStrategy: PasswordLoginStrategy; let credentials: PasswordLoginCredentials; @@ -145,7 +144,6 @@ describe("LoginStrategy", () => { policyService = mock(); passwordStrengthService = mock(); billingAccountProfileStateService = mock(); - userAsymmetricKeysRegenerationService = mock(); vaultTimeoutSettingsService = mock(); @@ -174,7 +172,6 @@ describe("LoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, - userAsymmetricKeysRegenerationService, ); credentials = new PasswordLoginCredentials(email, masterPassword); }); @@ -487,7 +484,6 @@ describe("LoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, - userAsymmetricKeysRegenerationService, ); apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory()); diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index c77b62e8dca..67a286d8195 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -33,7 +33,7 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { KdfType } from "@bitwarden/common/platform/enums"; import { Account, AccountProfile } from "@bitwarden/common/platform/models/domain/account"; import { UserId } from "@bitwarden/common/types/guid"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { @@ -80,7 +80,6 @@ export abstract class LoginStrategy { protected billingAccountProfileStateService: BillingAccountProfileStateService, protected vaultTimeoutSettingsService: VaultTimeoutSettingsService, protected KdfConfigService: KdfConfigService, - protected userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) {} abstract exportCache(): CacheData; @@ -267,8 +266,6 @@ export abstract class LoginStrategy { await this.setUserKey(response, userId); await this.setPrivateKey(response, userId); - await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); - this.messagingService.send("loggedIn"); return result; 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 7663e64b089..4da6272ccab 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 @@ -32,7 +32,7 @@ import { import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { MasterKey, UserKey } from "@bitwarden/common/types/key"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { LoginStrategyServiceAbstraction } from "../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; @@ -79,7 +79,6 @@ describe("PasswordLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; - let userAsymmetricKeysRegenerationService: MockProxy; let passwordLoginStrategy: PasswordLoginStrategy; let credentials: PasswordLoginCredentials; @@ -106,7 +105,6 @@ describe("PasswordLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); - userAsymmetricKeysRegenerationService = mock(); appIdService.getAppId.mockResolvedValue(deviceId); tokenService.decodeAccessToken.mockResolvedValue({ @@ -145,7 +143,6 @@ describe("PasswordLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, - userAsymmetricKeysRegenerationService, ); credentials = new PasswordLoginCredentials(email, masterPassword); tokenResponse = identityTokenResponseFactory(masterPasswordPolicy); 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 9cd0c987a83..7b5ad4a31b6 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 @@ -29,7 +29,7 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { DeviceKey, UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { AuthRequestServiceAbstraction, @@ -62,7 +62,6 @@ describe("SsoLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; - let userAsymmetricKeysRegenerationService: MockProxy; let ssoLoginStrategy: SsoLoginStrategy; let credentials: SsoLoginCredentials; @@ -98,7 +97,6 @@ describe("SsoLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); - userAsymmetricKeysRegenerationService = mock(); tokenService.getTwoFactorToken.mockResolvedValue(null); appIdService.getAppId.mockResolvedValue(deviceId); @@ -143,7 +141,6 @@ describe("SsoLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, - userAsymmetricKeysRegenerationService, ); credentials = new SsoLoginCredentials(ssoCode, ssoCodeVerifier, ssoRedirectUrl, ssoOrgId); }); 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 a803da162e7..07d06a7567d 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 @@ -26,7 +26,7 @@ import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/sp import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey, MasterKey } from "@bitwarden/common/types/key"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { UserApiLoginCredentials } from "../models/domain/login-credentials"; @@ -55,7 +55,6 @@ describe("UserApiLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; - let userAsymmetricKeysRegenerationService: MockProxy; let apiLogInStrategy: UserApiLoginStrategy; let credentials: UserApiLoginCredentials; @@ -88,7 +87,6 @@ describe("UserApiLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); - userAsymmetricKeysRegenerationService = mock(); appIdService.getAppId.mockResolvedValue(deviceId); tokenService.getTwoFactorToken.mockResolvedValue(null); @@ -116,7 +114,6 @@ describe("UserApiLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, - userAsymmetricKeysRegenerationService, ); credentials = new UserApiLoginCredentials(apiClientId, apiClientSecret); 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 b7cdc91d63f..88392b57c53 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 @@ -24,7 +24,7 @@ import { VaultTimeoutSettingsService } from "@bitwarden/common/services/vault-ti import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; import { PrfKey, UserKey } from "@bitwarden/common/types/key"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { WebAuthnLoginCredentials } from "../models/domain/login-credentials"; @@ -51,7 +51,6 @@ describe("WebAuthnLoginStrategy", () => { let billingAccountProfileStateService: MockProxy; let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; - let userAsymmetricKeysRegenerationService: MockProxy; let webAuthnLoginStrategy!: WebAuthnLoginStrategy; @@ -95,7 +94,6 @@ describe("WebAuthnLoginStrategy", () => { billingAccountProfileStateService = mock(); vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); - userAsymmetricKeysRegenerationService = mock(); tokenService.getTwoFactorToken.mockResolvedValue(null); appIdService.getAppId.mockResolvedValue(deviceId); @@ -121,7 +119,6 @@ describe("WebAuthnLoginStrategy", () => { billingAccountProfileStateService, vaultTimeoutSettingsService, kdfConfigService, - userAsymmetricKeysRegenerationService, ); // Create credentials diff --git a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts index 35a1d87ac5b..b0d9228f446 100644 --- a/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts +++ b/libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts @@ -37,7 +37,7 @@ import { } from "@bitwarden/common/spec"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { UserId } from "@bitwarden/common/types/guid"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { AuthRequestServiceAbstraction, @@ -76,7 +76,6 @@ describe("LoginStrategyService", () => { let vaultTimeoutSettingsService: MockProxy; let kdfConfigService: MockProxy; let taskSchedulerService: MockProxy; - let userAsymmetricKeysRegenerationService: MockProxy; let stateProvider: FakeGlobalStateProvider; let loginStrategyCacheExpirationState: FakeGlobalState; @@ -109,7 +108,6 @@ describe("LoginStrategyService", () => { vaultTimeoutSettingsService = mock(); kdfConfigService = mock(); taskSchedulerService = mock(); - userAsymmetricKeysRegenerationService = mock(); sut = new LoginStrategyService( accountService, @@ -137,7 +135,6 @@ describe("LoginStrategyService", () => { vaultTimeoutSettingsService, kdfConfigService, taskSchedulerService, - userAsymmetricKeysRegenerationService, ); loginStrategyCacheExpirationState = stateProvider.getFake(CACHE_EXPIRATION_KEY); 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 26d6b60c3db..721ee984974 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 @@ -42,7 +42,7 @@ import { GlobalState, GlobalStateProvider } from "@bitwarden/common/platform/sta import { DeviceTrustServiceAbstraction } from "@bitwarden/common/src/auth/abstractions/device-trust.service.abstraction"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { MasterKey } from "@bitwarden/common/types/key"; -import { KeyService, UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; +import { KeyService } from "@bitwarden/key-management"; import { AuthRequestServiceAbstraction, LoginStrategyServiceAbstraction } from "../../abstractions"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../../abstractions/user-decryption-options.service.abstraction"; @@ -114,7 +114,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { protected vaultTimeoutSettingsService: VaultTimeoutSettingsService, protected kdfConfigService: KdfConfigService, protected taskSchedulerService: TaskSchedulerService, - protected userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { this.currentAuthnTypeState = this.stateProvider.get(CURRENT_LOGIN_STRATEGY_KEY); this.loginStrategyCacheState = this.stateProvider.get(CACHE_KEY); @@ -334,7 +333,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction { this.billingAccountProfileStateService, this.vaultTimeoutSettingsService, this.kdfConfigService, - this.userAsymmetricKeysRegenerationService, ]; return source.pipe( From 6d2e7eef3a1ceac4e1187bb17d8a14ab30ef5dd0 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Thu, 21 Nov 2024 13:19:31 -0600 Subject: [PATCH 17/30] [Proposal] Add private key regen to onSuccessfulLogin --- apps/browser/src/auth/popup/login-v1.component.ts | 6 +++++- apps/desktop/src/auth/lock.component.ts | 9 ++++++++- apps/desktop/src/auth/login/login-v1.component.ts | 9 +++++++-- apps/web/src/app/auth/login/login-v1.component.ts | 8 ++++++++ libs/angular/src/auth/components/lock.component.ts | 5 +++-- libs/angular/src/auth/components/login-v1.component.ts | 4 ++-- libs/auth/src/angular/lock/lock.component.ts | 5 +++-- libs/auth/src/angular/login/login.component.ts | 5 +++++ 8 files changed, 41 insertions(+), 10 deletions(-) diff --git a/apps/browser/src/auth/popup/login-v1.component.ts b/apps/browser/src/auth/popup/login-v1.component.ts index eee1bcc4d3f..ee761d0e231 100644 --- a/apps/browser/src/auth/popup/login-v1.component.ts +++ b/apps/browser/src/auth/popup/login-v1.component.ts @@ -21,9 +21,11 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" 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 { UserId } from "@bitwarden/common/types/guid"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; +import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { flagEnabled } from "../../platform/flags"; @@ -55,6 +57,7 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { webAuthnLoginService: WebAuthnLoginServiceAbstraction, registerRouteService: RegisterRouteService, toastService: ToastService, + userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( devicesApiService, @@ -78,8 +81,9 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { registerRouteService, toastService, ); - this.onSuccessfulLogin = async () => { + this.onSuccessfulLogin = async (userId: UserId) => { await syncService.fullSync(true); + await userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); }; this.successRoute = "/tabs/vault"; this.showPasswordless = flagEnabled("showPasswordless"); diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index cc062965f35..26a61250df7 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -26,7 +26,12 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { DialogService, ToastService } from "@bitwarden/components"; -import { KeyService, BiometricsService, BiometricStateService } from "@bitwarden/key-management"; +import { + KeyService, + BiometricsService, + BiometricStateService, + UserAsymmetricKeysRegenerationService, +} from "@bitwarden/key-management"; const BroadcasterSubscriptionId = "LockComponent"; @@ -71,6 +76,7 @@ export class LockComponent extends BaseLockComponent implements OnInit, OnDestro kdfConfigService: KdfConfigService, syncService: SyncService, toastService: ToastService, + userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( masterPasswordService, @@ -100,6 +106,7 @@ export class LockComponent extends BaseLockComponent implements OnInit, OnDestro kdfConfigService, syncService, toastService, + userAsymmetricKeysRegenerationService, ); } diff --git a/apps/desktop/src/auth/login/login-v1.component.ts b/apps/desktop/src/auth/login/login-v1.component.ts index 132b430f327..1fc6fb8d7dc 100644 --- a/apps/desktop/src/auth/login/login-v1.component.ts +++ b/apps/desktop/src/auth/login/login-v1.component.ts @@ -26,9 +26,11 @@ 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 { UserId } from "@bitwarden/common/types/guid"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; +import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { EnvironmentComponent } from "../environment.component"; @@ -79,6 +81,7 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe registerRouteService: RegisterRouteService, toastService: ToastService, private configService: ConfigService, + userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( devicesApiService, @@ -102,8 +105,10 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe registerRouteService, toastService, ); - this.onSuccessfulLogin = () => { - return syncService.fullSync(true); + this.onSuccessfulLogin = (userId: UserId) => { + return syncService.fullSync(true).then(() => { + userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); + }); }; } diff --git a/apps/web/src/app/auth/login/login-v1.component.ts b/apps/web/src/app/auth/login/login-v1.component.ts index f556c5b65c6..69b7411ac80 100644 --- a/apps/web/src/app/auth/login/login-v1.component.ts +++ b/apps/web/src/app/auth/login/login-v1.component.ts @@ -29,8 +29,10 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { UserId } from "@bitwarden/common/types/guid"; +import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; +import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { flagEnabled } from "../../../utils/flags"; import { RouterService } from "../../core"; @@ -74,6 +76,8 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { webAuthnLoginService: WebAuthnLoginServiceAbstraction, registerRouteService: RegisterRouteService, toastService: ToastService, + userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, + syncService: SyncService, ) { super( devicesApiService, @@ -97,6 +101,10 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { registerRouteService, toastService, ); + this.onSuccessfulLogin = async (userId: UserId) => { + await syncService.fullSync(true); + await userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); + }; this.onSuccessfulLoginNavigate = this.goAfterLogIn; this.showPasswordless = flagEnabled("showPasswordless"); } diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index 29b0c984bf4..68252c5e97c 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -326,8 +326,9 @@ export class LockComponent implements OnInit, OnDestroy { // Vault can be de-synced since notifications get ignored while locked. Need to check whether sync is required using the sync service. await this.syncService.fullSync(false); - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); + await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration( + this.activeUserId, + ); if (this.onSuccessfulSubmit != null) { await this.onSuccessfulSubmit(); diff --git a/libs/angular/src/auth/components/login-v1.component.ts b/libs/angular/src/auth/components/login-v1.component.ts index 31145191898..7341d587851 100644 --- a/libs/angular/src/auth/components/login-v1.component.ts +++ b/libs/angular/src/auth/components/login-v1.component.ts @@ -41,7 +41,7 @@ export class LoginComponentV1 extends CaptchaProtectedComponent implements OnIni showPassword = false; formPromise: Promise; - onSuccessfulLogin: () => Promise; + onSuccessfulLogin: (userId: UserId) => Promise; onSuccessfulLoginNavigate: (userId: UserId) => Promise; onSuccessfulLoginTwoFactorNavigate: () => Promise; onSuccessfulLoginForceResetNavigate: () => Promise; @@ -208,7 +208,7 @@ export class LoginComponentV1 extends CaptchaProtectedComponent implements OnIni if (this.onSuccessfulLogin != null) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.onSuccessfulLogin(); + this.onSuccessfulLogin(response.userId); } if (this.onSuccessfulLoginNavigate != null) { diff --git a/libs/auth/src/angular/lock/lock.component.ts b/libs/auth/src/angular/lock/lock.component.ts index 7af1b4cbf85..9f9ae568864 100644 --- a/libs/auth/src/angular/lock/lock.component.ts +++ b/libs/auth/src/angular/lock/lock.component.ts @@ -534,8 +534,9 @@ export class LockV2Component implements OnInit, OnDestroy { // Vault can be de-synced since notifications get ignored while locked. Need to check whether sync is required using the sync service. await this.syncService.fullSync(false); - const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); + await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration( + this.activeAccount.id, + ); if (this.clientType === "browser") { const previousUrl = this.lockComponentService.getPreviousUrl(); diff --git a/libs/auth/src/angular/login/login.component.ts b/libs/auth/src/angular/login/login.component.ts index 0193e4c4035..1fc35fc98d6 100644 --- a/libs/auth/src/angular/login/login.component.ts +++ b/libs/auth/src/angular/login/login.component.ts @@ -42,6 +42,7 @@ import { LinkModule, ToastService, } from "@bitwarden/components"; +import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { AnonLayoutWrapperDataService } from "../anon-layout/anon-layout-wrapper-data.service"; import { VaultIcon, WaveIcon } from "../icons"; @@ -142,6 +143,7 @@ export class LoginComponent implements OnInit, OnDestroy { private logService: LogService, private validationService: ValidationService, private configService: ConfigService, + private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { this.clientType = this.platformUtilsService.getClientType(); this.loginViaAuthRequestSupported = this.loginComponentService.isLoginViaAuthRequestSupported(); @@ -298,6 +300,9 @@ export class LoginComponent implements OnInit, OnDestroy { } await this.syncService.fullSync(true); + await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration( + authResult.userId, + ); if (authResult.forcePasswordReset != ForceSetPasswordReason.None) { this.loginEmailService.clearValues(); From 7fa765cc2bfc52ce4cba559f6cf345a5ec5d06cf Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Thu, 21 Nov 2024 16:05:22 -0600 Subject: [PATCH 18/30] revert login v1 proposal --- apps/browser/src/auth/popup/login-v1.component.ts | 6 +----- apps/desktop/src/auth/login/login-v1.component.ts | 9 ++------- apps/web/src/app/auth/login/login-v1.component.ts | 8 -------- libs/angular/src/auth/components/login-v1.component.ts | 4 ++-- 4 files changed, 5 insertions(+), 22 deletions(-) diff --git a/apps/browser/src/auth/popup/login-v1.component.ts b/apps/browser/src/auth/popup/login-v1.component.ts index ee761d0e231..eee1bcc4d3f 100644 --- a/apps/browser/src/auth/popup/login-v1.component.ts +++ b/apps/browser/src/auth/popup/login-v1.component.ts @@ -21,11 +21,9 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" 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 { UserId } from "@bitwarden/common/types/guid"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; -import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { flagEnabled } from "../../platform/flags"; @@ -57,7 +55,6 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { webAuthnLoginService: WebAuthnLoginServiceAbstraction, registerRouteService: RegisterRouteService, toastService: ToastService, - userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( devicesApiService, @@ -81,9 +78,8 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { registerRouteService, toastService, ); - this.onSuccessfulLogin = async (userId: UserId) => { + this.onSuccessfulLogin = async () => { await syncService.fullSync(true); - await userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); }; this.successRoute = "/tabs/vault"; this.showPasswordless = flagEnabled("showPasswordless"); diff --git a/apps/desktop/src/auth/login/login-v1.component.ts b/apps/desktop/src/auth/login/login-v1.component.ts index 1fc6fb8d7dc..e77110b53da 100644 --- a/apps/desktop/src/auth/login/login-v1.component.ts +++ b/apps/desktop/src/auth/login/login-v1.component.ts @@ -26,11 +26,9 @@ 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 { UserId } from "@bitwarden/common/types/guid"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; -import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { EnvironmentComponent } from "../environment.component"; @@ -81,7 +79,6 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe registerRouteService: RegisterRouteService, toastService: ToastService, private configService: ConfigService, - userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( devicesApiService, @@ -105,10 +102,8 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe registerRouteService, toastService, ); - this.onSuccessfulLogin = (userId: UserId) => { - return syncService.fullSync(true).then(() => { - userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); - }); + this.onSuccessfulLogin = () => { + return syncService.fullSync(true).then(() => {}); }; } diff --git a/apps/web/src/app/auth/login/login-v1.component.ts b/apps/web/src/app/auth/login/login-v1.component.ts index 69b7411ac80..f556c5b65c6 100644 --- a/apps/web/src/app/auth/login/login-v1.component.ts +++ b/apps/web/src/app/auth/login/login-v1.component.ts @@ -29,10 +29,8 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { UserId } from "@bitwarden/common/types/guid"; -import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; -import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { flagEnabled } from "../../../utils/flags"; import { RouterService } from "../../core"; @@ -76,8 +74,6 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { webAuthnLoginService: WebAuthnLoginServiceAbstraction, registerRouteService: RegisterRouteService, toastService: ToastService, - userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, - syncService: SyncService, ) { super( devicesApiService, @@ -101,10 +97,6 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit { registerRouteService, toastService, ); - this.onSuccessfulLogin = async (userId: UserId) => { - await syncService.fullSync(true); - await userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration(userId); - }; this.onSuccessfulLoginNavigate = this.goAfterLogIn; this.showPasswordless = flagEnabled("showPasswordless"); } diff --git a/libs/angular/src/auth/components/login-v1.component.ts b/libs/angular/src/auth/components/login-v1.component.ts index 7341d587851..31145191898 100644 --- a/libs/angular/src/auth/components/login-v1.component.ts +++ b/libs/angular/src/auth/components/login-v1.component.ts @@ -41,7 +41,7 @@ export class LoginComponentV1 extends CaptchaProtectedComponent implements OnIni showPassword = false; formPromise: Promise; - onSuccessfulLogin: (userId: UserId) => Promise; + onSuccessfulLogin: () => Promise; onSuccessfulLoginNavigate: (userId: UserId) => Promise; onSuccessfulLoginTwoFactorNavigate: () => Promise; onSuccessfulLoginForceResetNavigate: () => Promise; @@ -208,7 +208,7 @@ export class LoginComponentV1 extends CaptchaProtectedComponent implements OnIni if (this.onSuccessfulLogin != null) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.onSuccessfulLogin(response.userId); + this.onSuccessfulLogin(); } if (this.onSuccessfulLoginNavigate != null) { From d3e9610f8c427c6c8a87881239f73ebe91c2e8ac Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Thu, 21 Nov 2024 16:24:04 -0600 Subject: [PATCH 19/30] only target refresh components --- apps/browser/src/auth/popup/lock.component.ts | 2 -- apps/desktop/src/auth/lock.component.ts | 3 --- apps/desktop/src/auth/login/login-v1.component.ts | 2 +- libs/angular/src/auth/components/lock.component.ts | 2 -- 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/apps/browser/src/auth/popup/lock.component.ts b/apps/browser/src/auth/popup/lock.component.ts index ceb0f364490..50097a3a8b3 100644 --- a/apps/browser/src/auth/popup/lock.component.ts +++ b/apps/browser/src/auth/popup/lock.component.ts @@ -29,7 +29,6 @@ import { KeyService, BiometricsService, BiometricStateService, - UserAsymmetricKeysRegenerationService, } from "@bitwarden/key-management"; import { BiometricErrors, BiometricErrorTypes } from "../../models/biometricErrors"; @@ -76,7 +75,6 @@ export class LockComponent extends BaseLockComponent implements OnInit { kdfConfigService: KdfConfigService, syncService: SyncService, toastService: ToastService, - userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( masterPasswordService, diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index f388b4b8f1b..f0323f3e7da 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -30,7 +30,6 @@ import { KeyService, BiometricsService, BiometricStateService, - UserAsymmetricKeysRegenerationService, } from "@bitwarden/key-management"; const BroadcasterSubscriptionId = "LockComponent"; @@ -76,7 +75,6 @@ export class LockComponent extends BaseLockComponent implements OnInit, OnDestro kdfConfigService: KdfConfigService, syncService: SyncService, toastService: ToastService, - userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) { super( masterPasswordService, @@ -106,7 +104,6 @@ export class LockComponent extends BaseLockComponent implements OnInit, OnDestro kdfConfigService, syncService, toastService, - userAsymmetricKeysRegenerationService, ); } diff --git a/apps/desktop/src/auth/login/login-v1.component.ts b/apps/desktop/src/auth/login/login-v1.component.ts index e77110b53da..132b430f327 100644 --- a/apps/desktop/src/auth/login/login-v1.component.ts +++ b/apps/desktop/src/auth/login/login-v1.component.ts @@ -103,7 +103,7 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe toastService, ); this.onSuccessfulLogin = () => { - return syncService.fullSync(true).then(() => {}); + return syncService.fullSync(true); }; } diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index 565076fe510..f0d1163c24d 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -39,7 +39,6 @@ import { KeyService, BiometricStateService, BiometricsService, - UserAsymmetricKeysRegenerationService, } from "@bitwarden/key-management"; @Directive() @@ -95,7 +94,6 @@ export class LockComponent implements OnInit, OnDestroy { protected kdfConfigService: KdfConfigService, protected syncService: SyncService, protected toastService: ToastService, - protected userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, ) {} async ngOnInit() { From 112a2398a7af581cc093ac773219fa20c1eed68f Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 22 Nov 2024 15:52:43 -0600 Subject: [PATCH 20/30] Rename method --- apps/browser/src/auth/popup/lock.component.ts | 1 - libs/auth/src/angular/lock/lock.component.ts | 4 +-- ...ser-asymmetric-key-regeneration.service.ts | 6 ++-- ...symmetric-key-regeneration.service.spec.ts | 24 +++++++-------- ...ser-asymmetric-key-regeneration.service.ts | 29 ++++++++++++------- 5 files changed, 36 insertions(+), 28 deletions(-) diff --git a/apps/browser/src/auth/popup/lock.component.ts b/apps/browser/src/auth/popup/lock.component.ts index 50097a3a8b3..1d1ed619995 100644 --- a/apps/browser/src/auth/popup/lock.component.ts +++ b/apps/browser/src/auth/popup/lock.component.ts @@ -104,7 +104,6 @@ export class LockComponent extends BaseLockComponent implements OnInit { kdfConfigService, syncService, toastService, - userAsymmetricKeysRegenerationService, ); this.successRoute = "/tabs/current"; this.isInitialLockScreen = (window as any).previousPopupUrl == null; diff --git a/libs/auth/src/angular/lock/lock.component.ts b/libs/auth/src/angular/lock/lock.component.ts index 9f9ae568864..adf35373e5e 100644 --- a/libs/auth/src/angular/lock/lock.component.ts +++ b/libs/auth/src/angular/lock/lock.component.ts @@ -534,9 +534,7 @@ export class LockV2Component implements OnInit, OnDestroy { // Vault can be de-synced since notifications get ignored while locked. Need to check whether sync is required using the sync service. await this.syncService.fullSync(false); - await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration( - this.activeAccount.id, - ); + await this.userAsymmetricKeysRegenerationService.regenerateIfNeeded(this.activeAccount.id); if (this.clientType === "browser") { const previousUrl = this.lockComponentService.getPreviousUrl(); diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts index 407338f845b..c59aa6f29c1 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts @@ -2,7 +2,9 @@ import { UserId } from "@bitwarden/common/types/guid"; export abstract class UserAsymmetricKeysRegenerationService { /** - * Handle regeneration of the user's asymmetric keys if they are invalid. + * Attempts to regenerate the user's asymmetric keys if they are invalid. + * Requires the PrivateKeyRegeneration feature flag to be enabled if not the method will do nothing. + * @param userId The user id. */ - abstract handleUserAsymmetricKeysRegeneration: (userId: UserId) => Promise; + abstract regenerateIfNeeded: (userId: UserId) => Promise; } diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts index 5bf258d0e70..dec25282b4a 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -118,7 +118,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { it("should not call regeneration code when feature flag is off", async () => { configService.getFeatureFlag.mockResolvedValue(false); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect(keyService.userKey$).not.toHaveBeenCalled(); }); @@ -131,7 +131,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { setupVerificationResponse(mockVerificationResponse, sdkService); keyService.userKey$.mockReturnValue(throwError(() => new Error("error"))); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -146,7 +146,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { }; setupVerificationResponse(mockVerificationResponse, sdkService); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -161,7 +161,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { }; setupVerificationResponse(mockVerificationResponse, sdkService); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -180,7 +180,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { new Error("Key regeneration not supported for this user."), ); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -199,7 +199,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { new Error("error"), ); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -215,7 +215,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { setupVerificationResponse(mockVerificationResponse, sdkService); setupUserKeyValidation(cipherService, keyService, encryptService); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -232,7 +232,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { setupUserKeyValidation(cipherService, keyService, encryptService); encryptService.decryptToBytes.mockRejectedValue(new Error("error")); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -248,7 +248,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { setupVerificationResponse(mockVerificationResponse, sdkService); cipherService.getAll.mockResolvedValue([]); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -264,7 +264,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { setupVerificationResponse(mockVerificationResponse, sdkService); setupUserKeyValidation(cipherService, keyService, encryptService); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -281,7 +281,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { setupUserKeyValidation(cipherService, keyService, encryptService); encryptService.decryptToBytes.mockRejectedValue(new Error("error")); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, @@ -297,7 +297,7 @@ describe("handleUserAsymmetricKeysRegeneration", () => { setupVerificationResponse(mockVerificationResponse, sdkService); cipherService.getAll.mockResolvedValue([]); - await sut.handleUserAsymmetricKeysRegeneration(userId); + await sut.regenerateIfNeeded(userId); expect( userAsymmetricKeysRegenerationApiService.regenerateUserAsymmetricKeys, diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index 1490496198e..36345d15abd 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -18,16 +18,16 @@ export class DefaultUserAsymmetricKeysRegenerationService implements UserAsymmetricKeysRegenerationService { constructor( - protected keyService: KeyService, - protected cipherService: CipherService, - protected userAsymmetricKeysRegenerationApiService: UserAsymmetricKeysRegenerationApiService, - protected logService: LogService, - protected sdkService: SdkService, - protected apiService: ApiService, - protected configService: ConfigService, + private keyService: KeyService, + private cipherService: CipherService, + private userAsymmetricKeysRegenerationApiService: UserAsymmetricKeysRegenerationApiService, + private logService: LogService, + private sdkService: SdkService, + private apiService: ApiService, + private configService: ConfigService, ) {} - async handleUserAsymmetricKeysRegeneration(userId: UserId): Promise { + async regenerateIfNeeded(userId: UserId): Promise { try { const privateKeyRegenerationFlag = await this.configService.getFeatureFlag( FeatureFlag.PrivateKeyRegeneration, @@ -41,7 +41,7 @@ export class DefaultUserAsymmetricKeysRegenerationService } } catch (error) { this.logService.error( - "[UserAsymmetricKeyRegeneration] User Key regeneration error: " + + "[UserAsymmetricKeyRegeneration] An error occurred: " + error + " Skipping regeneration for the user.", ); @@ -73,6 +73,9 @@ export class DefaultUserAsymmetricKeysRegenerationService return false; } else { // The private key is decryptable but not valid so we should regenerate it. + this.logService.info( + "[UserAsymmetricKeyRegeneration] User's private key is decryptable but not a valid key, attempting regeneration.", + ); return true; } } @@ -80,11 +83,14 @@ export class DefaultUserAsymmetricKeysRegenerationService // The private isn't decryptable, check to see if we can decrypt something with the userKey. const userKeyCanDecrypt = await this.userKeyCanDecrypt(userKey); if (userKeyCanDecrypt) { + this.logService.info( + "[UserAsymmetricKeyRegeneration] User Asymmetric Key decryption failure detected, attempting regeneration.", + ); return true; } this.logService.warning( - "[UserAsymmetricKeyRegeneration] User Asymmetric Key decryption failure detected, but unable to determine User Symmetric Key validity.", + "[UserAsymmetricKeyRegeneration] User Asymmetric Key decryption failure detected, but unable to determine User Symmetric Key validity, skipping regeneration.", ); return false; } @@ -115,6 +121,9 @@ export class DefaultUserAsymmetricKeysRegenerationService } await this.keyService.setPrivateKey(makeKeyPairResponse.userKeyEncryptedPrivateKey, userId); + this.logService.info( + "[UserAsymmetricKeyRegeneration] User's asymmetric keys successfully regenerated.", + ); } private async userKeyCanDecrypt(userKey: UserKey): Promise { From 77b525cbf1b60963852d8a4327416fddff5526d6 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Fri, 22 Nov 2024 17:10:49 -0600 Subject: [PATCH 21/30] Add LoginSuccessHandlerService --- .../src/services/jslib-services.module.ts | 7 +++++++ .../login-via-auth-request.component.ts | 12 ++++++------ libs/auth/src/angular/login/login.component.ts | 9 +++------ libs/auth/src/common/abstractions/index.ts | 1 + .../login-success-handler.service.ts | 10 ++++++++++ libs/auth/src/common/services/index.ts | 1 + .../default-login-success-handler.service.ts | 16 ++++++++++++++++ 7 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 libs/auth/src/common/abstractions/login-success-handler.service.ts create mode 100644 libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 2f9cddf96be..32f5facca49 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -35,6 +35,8 @@ import { RegisterRouteService, AuthRequestApiService, DefaultAuthRequestApiService, + DefaultLoginSuccessHandlerService, + LoginSuccessHandlerService, } from "@bitwarden/auth/common"; import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service"; import { AuditService as AuditServiceAbstraction } from "@bitwarden/common/abstractions/audit.service"; @@ -1413,6 +1415,11 @@ const safeProviders: SafeProvider[] = [ ConfigService, ], }), + safeProvider({ + provide: LoginSuccessHandlerService, + useClass: DefaultLoginSuccessHandlerService, + deps: [SyncService, UserAsymmetricKeysRegenerationService], + }), ]; @NgModule({ diff --git a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts index 38614a9046a..4e991274eec 100644 --- a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts +++ b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts @@ -10,6 +10,7 @@ import { AuthRequestServiceAbstraction, LoginEmailServiceAbstraction, LoginStrategyServiceAbstraction, + LoginSuccessHandlerService, } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AnonymousHubService } from "@bitwarden/common/auth/abstractions/anonymous-hub.service"; @@ -32,7 +33,6 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { UserId } from "@bitwarden/common/types/guid"; -import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; import { ButtonModule, LinkModule, ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; @@ -86,9 +86,9 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { private passwordGenerationService: PasswordGenerationServiceAbstraction, private platformUtilsService: PlatformUtilsService, private router: Router, - private syncService: SyncService, private toastService: ToastService, private validationService: ValidationService, + private loginSuccessHandlerService: LoginSuccessHandlerService, ) { this.clientType = this.platformUtilsService.getClientType(); @@ -483,7 +483,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { const activeAccount = await firstValueFrom(this.accountService.activeAccount$); await this.deviceTrustService.trustDeviceIfRequired(activeAccount.id); - await this.handleSuccessfulLoginNavigation(); + await this.handleSuccessfulLoginNavigation(userId); } /** @@ -553,17 +553,17 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { } else if (loginResponse.forcePasswordReset != ForceSetPasswordReason.None) { await this.router.navigate(["update-temp-password"]); } else { - await this.handleSuccessfulLoginNavigation(); + await this.handleSuccessfulLoginNavigation(loginResponse.userId); } } - private async handleSuccessfulLoginNavigation() { + private async handleSuccessfulLoginNavigation(userId: UserId) { if (this.flow === Flow.StandardAuthRequest) { // Only need to set remembered email on standard login with auth req flow await this.loginEmailService.saveEmailSettings(); } - await this.syncService.fullSync(true); + await this.loginSuccessHandlerService.run(userId); await this.router.navigate(["vault"]); } } diff --git a/libs/auth/src/angular/login/login.component.ts b/libs/auth/src/angular/login/login.component.ts index 7aeca1a53b7..0e4d0bebfe7 100644 --- a/libs/auth/src/angular/login/login.component.ts +++ b/libs/auth/src/angular/login/login.component.ts @@ -8,6 +8,7 @@ import { JslibModule } from "@bitwarden/angular/jslib.module"; import { LoginEmailServiceAbstraction, LoginStrategyServiceAbstraction, + LoginSuccessHandlerService, PasswordLoginCredentials, RegisterRouteService, } from "@bitwarden/auth/common"; @@ -42,7 +43,6 @@ import { LinkModule, ToastService, } from "@bitwarden/components"; -import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; import { AnonLayoutWrapperDataService } from "../anon-layout/anon-layout-wrapper-data.service"; import { VaultIcon, WaveIcon } from "../icons"; @@ -137,7 +137,7 @@ export class LoginComponent implements OnInit, OnDestroy { private logService: LogService, private validationService: ValidationService, private configService: ConfigService, - private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, + private loginSuccessHandlerService: LoginSuccessHandlerService, ) { this.clientType = this.platformUtilsService.getClientType(); } @@ -292,10 +292,7 @@ export class LoginComponent implements OnInit, OnDestroy { return; } - await this.syncService.fullSync(true); - await this.userAsymmetricKeysRegenerationService.handleUserAsymmetricKeysRegeneration( - authResult.userId, - ); + await this.loginSuccessHandlerService.run(authResult.userId); if (authResult.forcePasswordReset != ForceSetPasswordReason.None) { this.loginEmailService.clearValues(); diff --git a/libs/auth/src/common/abstractions/index.ts b/libs/auth/src/common/abstractions/index.ts index 093d703b74a..e0781c4f017 100644 --- a/libs/auth/src/common/abstractions/index.ts +++ b/libs/auth/src/common/abstractions/index.ts @@ -4,3 +4,4 @@ export * from "./login-email.service"; export * from "./login-strategy.service"; export * from "./user-decryption-options.service.abstraction"; export * from "./auth-request.service.abstraction"; +export * from "./login-success-handler.service"; diff --git a/libs/auth/src/common/abstractions/login-success-handler.service.ts b/libs/auth/src/common/abstractions/login-success-handler.service.ts new file mode 100644 index 00000000000..c74db74fe30 --- /dev/null +++ b/libs/auth/src/common/abstractions/login-success-handler.service.ts @@ -0,0 +1,10 @@ +import { UserId } from "@bitwarden/common/types/guid"; + +export abstract class LoginSuccessHandlerService { + /** + * Runs any service calls required after a successful login. + * Service calls that should be included in this method are only those required to be awaited after successful login. + * @param userId The user id. + */ + abstract run: (userId: UserId) => Promise; +} diff --git a/libs/auth/src/common/services/index.ts b/libs/auth/src/common/services/index.ts index 41e0ba087ae..d1cedebcf36 100644 --- a/libs/auth/src/common/services/index.ts +++ b/libs/auth/src/common/services/index.ts @@ -6,3 +6,4 @@ export * from "./auth-request/auth-request.service"; export * from "./auth-request/auth-request-api.service"; export * from "./register-route.service"; export * from "./accounts/lock.service"; +export * from "./login-success-handler/default-login-success-handler.service"; diff --git a/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts new file mode 100644 index 00000000000..215329051df --- /dev/null +++ b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts @@ -0,0 +1,16 @@ +import { SyncService } from "@bitwarden/common/platform/sync"; +import { UserId } from "@bitwarden/common/types/guid"; +import { UserAsymmetricKeysRegenerationService } from "@bitwarden/key-management"; + +import { LoginSuccessHandlerService } from "../../abstractions/login-success-handler.service"; + +export class DefaultLoginSuccessHandlerService implements LoginSuccessHandlerService { + constructor( + private syncService: SyncService, + private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, + ) {} + async run(userId: UserId): Promise { + await this.syncService.fullSync(true); + await this.userAsymmetricKeysRegenerationService.regenerateIfNeeded(userId); + } +} From 960ece29031ac017d44f41afb37d1cdcc0a2049d Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Tue, 26 Nov 2024 15:50:41 -0600 Subject: [PATCH 22/30] add loginSuccessHandlerService to BaseLoginViaWebAuthnComponent --- .../base-login-via-webauthn.component.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/libs/angular/src/auth/components/base-login-via-webauthn.component.ts b/libs/angular/src/auth/components/base-login-via-webauthn.component.ts index c2bd1becf28..3396169dc8a 100644 --- a/libs/angular/src/auth/components/base-login-via-webauthn.component.ts +++ b/libs/angular/src/auth/components/base-login-via-webauthn.component.ts @@ -1,6 +1,7 @@ import { Directive, OnInit } from "@angular/core"; import { Router } from "@angular/router"; +import { LoginSuccessHandlerService } from "@bitwarden/auth/common"; import { WebAuthnLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login.service.abstraction"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { WebAuthnLoginCredentialAssertionView } from "@bitwarden/common/auth/models/view/webauthn-login/webauthn-login-credential-assertion.view"; @@ -24,6 +25,7 @@ export class BaseLoginViaWebAuthnComponent implements OnInit { private logService: LogService, private validationService: ValidationService, private i18nService: I18nService, + private loginSuccessHandlerService: LoginSuccessHandlerService, ) {} ngOnInit(): void { @@ -57,11 +59,17 @@ export class BaseLoginViaWebAuthnComponent implements OnInit { this.i18nService.t("twoFactorForPasskeysNotSupportedOnClientUpdateToLogIn"), ); this.currentState = "assertFailed"; - } else if (authResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { + return; + } + + await this.loginSuccessHandlerService.run(authResult.userId); + + if (authResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { await this.router.navigate([this.forcePasswordResetRoute]); - } else { - await this.router.navigate([this.successRoute]); + return; } + + await this.router.navigate([this.successRoute]); } catch (error) { if (error instanceof ErrorResponse) { this.validationService.showError(this.i18nService.t("invalidPasskeyPleaseTryAgain")); From 03bab1cf7a87980ed608d21c31c646510bc29cdc Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Mon, 9 Dec 2024 15:21:38 -0600 Subject: [PATCH 23/30] Only run loginSuccessHandlerService if webAuthn is used for vault decryption. --- .../auth/components/base-login-via-webauthn.component.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libs/angular/src/auth/components/base-login-via-webauthn.component.ts b/libs/angular/src/auth/components/base-login-via-webauthn.component.ts index 3396169dc8a..5fa394cef7c 100644 --- a/libs/angular/src/auth/components/base-login-via-webauthn.component.ts +++ b/libs/angular/src/auth/components/base-login-via-webauthn.component.ts @@ -1,5 +1,6 @@ import { Directive, OnInit } from "@angular/core"; import { Router } from "@angular/router"; +import { firstValueFrom } from "rxjs"; import { LoginSuccessHandlerService } from "@bitwarden/auth/common"; import { WebAuthnLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/webauthn/webauthn-login.service.abstraction"; @@ -9,6 +10,7 @@ import { ErrorResponse } from "@bitwarden/common/models/response/error.response" import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; +import { KeyService } from "@bitwarden/key-management"; export type State = "assert" | "assertFailed"; @@ -26,6 +28,7 @@ export class BaseLoginViaWebAuthnComponent implements OnInit { private validationService: ValidationService, private i18nService: I18nService, private loginSuccessHandlerService: LoginSuccessHandlerService, + private keyService: KeyService, ) {} ngOnInit(): void { @@ -62,7 +65,11 @@ export class BaseLoginViaWebAuthnComponent implements OnInit { return; } - await this.loginSuccessHandlerService.run(authResult.userId); + // Only run loginSuccessHandlerService if webAuthn is used for vault decryption. + const userKey = await firstValueFrom(this.keyService.userKey$(authResult.userId)); + if (userKey) { + await this.loginSuccessHandlerService.run(authResult.userId); + } if (authResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { await this.router.navigate([this.forcePasswordResetRoute]); From b0cd1e2606e749f6ad00cdedd3170a21300de7d0 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Mon, 9 Dec 2024 15:21:54 -0600 Subject: [PATCH 24/30] Update logging wording --- .../default-user-asymmetric-key-regeneration.service.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index 36345d15abd..08f4d0fa171 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -109,11 +109,11 @@ export class DefaultUserAsymmetricKeysRegenerationService } catch (error) { if (error.message === "Key regeneration not supported for this user.") { this.logService.info( - "[UserAsymmetricKeyRegeneration] User Key regeneration not supported for this user at this time.", + "[UserAsymmetricKeyRegeneration] Regeneration not supported for this user at this time.", ); } else { this.logService.error( - "[UserAsymmetricKeyRegeneration] User Key regeneration error when submitting the request to the server: " + + "[UserAsymmetricKeyRegeneration] Regeneration error when submitting the request to the server: " + error, ); } @@ -136,7 +136,7 @@ export class DefaultUserAsymmetricKeysRegenerationService return true; } catch (error) { this.logService.error( - "[UserAsymmetricKeyRegeneration] User Key decryption error: " + error, + "[UserAsymmetricKeyRegeneration] User Symmetric Key validation error: " + error, ); return false; } From e5870629fa7272cabdb74a871302cfeca26a8471 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Mon, 9 Dec 2024 15:22:49 -0600 Subject: [PATCH 25/30] Remove unused service injection --- libs/auth/src/angular/login/login.component.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/auth/src/angular/login/login.component.ts b/libs/auth/src/angular/login/login.component.ts index 629abdefc73..0b82eec0de5 100644 --- a/libs/auth/src/angular/login/login.component.ts +++ b/libs/auth/src/angular/login/login.component.ts @@ -30,7 +30,6 @@ import { MessagingService } from "@bitwarden/common/platform/abstractions/messag import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SyncService } from "@bitwarden/common/platform/sync"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { AsyncActionsModule, @@ -126,7 +125,6 @@ export class LoginComponent implements OnInit, OnDestroy { private policyService: InternalPolicyService, private registerRouteService: RegisterRouteService, private router: Router, - private syncService: SyncService, private toastService: ToastService, private logService: LogService, private validationService: ValidationService, From 36322f77056cbd229f5e6dd30d9f3304651694c5 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Mon, 9 Dec 2024 15:30:07 -0600 Subject: [PATCH 26/30] update test naming --- .../default-user-asymmetric-key-regeneration.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts index dec25282b4a..fcdf96bc16c 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -63,7 +63,7 @@ function setupUserKeyValidation( (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); } -describe("handleUserAsymmetricKeysRegeneration", () => { +describe("regenerateIfNeeded", () => { let sut: DefaultUserAsymmetricKeysRegenerationService; const userId = "userId" as UserId; From b6637fa1429fbcbd5f06eb5e3ab6187ea6480081 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Mon, 9 Dec 2024 16:25:15 -0600 Subject: [PATCH 27/30] Updates for TS strict --- .../requests/key-regeneration.request.ts | 5 ++++ ...symmetric-key-regeneration.service.spec.ts | 1 - ...ser-asymmetric-key-regeneration.service.ts | 24 +++++++++++++------ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts b/libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts index 044f58bb51f..2d3b62aedad 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/models/requests/key-regeneration.request.ts @@ -3,4 +3,9 @@ import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; export class KeyRegenerationRequest { userPublicKey: string; userKeyEncryptedUserPrivateKey: EncString; + + constructor(userPublicKey: string, userKeyEncryptedUserPrivateKey: EncString) { + this.userPublicKey = userPublicKey; + this.userKeyEncryptedUserPrivateKey = userKeyEncryptedUserPrivateKey; + } } diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts index fcdf96bc16c..77d7ebbb814 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -56,7 +56,6 @@ function setupUserKeyValidation( cipher.favorite = false; cipher.name = mockEnc("EncryptedString"); cipher.notes = mockEnc("EncryptedString"); - cipher.deletedDate = null; cipher.key = mockEnc("EncKey"); cipherService.getAll.mockResolvedValue([cipher]); encryptService.decryptToBytes.mockResolvedValue(makeStaticByteArray(64)); diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index 08f4d0fa171..5574199aa36 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -57,13 +57,16 @@ export class DefaultUserAsymmetricKeysRegenerationService const verificationResponse = await firstValueFrom( this.sdkService.client$.pipe( - map((sdk) => - sdk.crypto().verify_asymmetric_keys({ + map((sdk) => { + if (sdk === undefined) { + throw new Error("SDK is undefined"); + } + return sdk.crypto().verify_asymmetric_keys({ userKey: userKey.keyB64, userPublicKey: publicKeyResponse.publicKey, userKeyEncryptedPrivateKey: userKeyEncryptedPrivateKey, - }), - ), + }); + }), ), ); @@ -98,7 +101,14 @@ export class DefaultUserAsymmetricKeysRegenerationService private async regenerateUserAsymmetricKeys(userId: UserId): Promise { const userKey = await firstValueFrom(this.keyService.userKey$(userId)); const makeKeyPairResponse = await firstValueFrom( - this.sdkService.client$.pipe(map((sdk) => sdk.crypto().make_key_pair(userKey.keyB64))), + this.sdkService.client$.pipe( + map((sdk) => { + if (sdk === undefined) { + throw new Error("SDK is undefined"); + } + return sdk.crypto().make_key_pair(userKey.keyB64); + }), + ), ); try { @@ -106,8 +116,8 @@ export class DefaultUserAsymmetricKeysRegenerationService makeKeyPairResponse.userPublicKey, new EncString(makeKeyPairResponse.userKeyEncryptedPrivateKey), ); - } catch (error) { - if (error.message === "Key regeneration not supported for this user.") { + } catch (error: any) { + if (error?.message === "Key regeneration not supported for this user.") { this.logService.info( "[UserAsymmetricKeyRegeneration] Regeneration not supported for this user at this time.", ); From 4e39e0181a4cfb616dd74640da7bf701cf735184 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Wed, 11 Dec 2024 15:39:18 -0600 Subject: [PATCH 28/30] bump SDK version --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 64a7c926ca2..7d03643c266 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,7 @@ "@angular/platform-browser": "17.3.12", "@angular/platform-browser-dynamic": "17.3.12", "@angular/router": "17.3.12", - "@bitwarden/sdk-internal": "0.2.0-main.3", + "@bitwarden/sdk-internal": "0.2.0-main.38", "@electron/fuses": "1.8.0", "@koa/multer": "3.0.2", "@koa/router": "13.1.0", @@ -4298,9 +4298,9 @@ "link": true }, "node_modules/@bitwarden/sdk-internal": { - "version": "0.2.0-main.3", - "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.3.tgz", - "integrity": "sha512-CYp98uaVMSFp6nr/QLw+Qw8ttnVtWark/bMpw59OhwMVhrCDKmpCgcR9G4oEdVO11IuFcYZieTBmtOEPhCpGaw==", + "version": "0.2.0-main.38", + "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.38.tgz", + "integrity": "sha512-bkN+BZC0YA4k0To8QiT33UTZX8peKDXud8Gzq3UHNPlU/vMSkP3Wn8q0GezzmYN3UNNIWXfreNCS0mJ+S51j/Q==", "license": "GPL-3.0" }, "node_modules/@bitwarden/vault": { diff --git a/package.json b/package.json index 5573332db1a..56d1bfbc347 100644 --- a/package.json +++ b/package.json @@ -154,7 +154,7 @@ "@angular/platform-browser": "17.3.12", "@angular/platform-browser-dynamic": "17.3.12", "@angular/router": "17.3.12", - "@bitwarden/sdk-internal": "0.2.0-main.3", + "@bitwarden/sdk-internal": "0.2.0-main.38", "@electron/fuses": "1.8.0", "@koa/multer": "3.0.2", "@koa/router": "13.1.0", From d677b9e9359d23f1806b82542d3e0bde0b6779ab Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Thu, 12 Dec 2024 11:44:35 -0600 Subject: [PATCH 29/30] swap to combineLatest --- ...ult-user-asymmetric-key-regeneration.service.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts index 5574199aa36..ffaa3a82608 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, map } from "rxjs"; +import { combineLatest, firstValueFrom, map } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; @@ -49,11 +49,13 @@ export class DefaultUserAsymmetricKeysRegenerationService } private async shouldRegenerate(userId: UserId): Promise { - const [userKey, userKeyEncryptedPrivateKey, publicKeyResponse] = await Promise.all([ - firstValueFrom(this.keyService.userKey$(userId)), - firstValueFrom(this.keyService.userEncryptedPrivateKey$(userId)), - this.apiService.getUserPublicKey(userId), - ]); + const [userKey, userKeyEncryptedPrivateKey, publicKeyResponse] = await firstValueFrom( + combineLatest([ + this.keyService.userKey$(userId), + this.keyService.userEncryptedPrivateKey$(userId), + this.apiService.getUserPublicKey(userId), + ]), + ); const verificationResponse = await firstValueFrom( this.sdkService.client$.pipe( From 5466be05bfd3339f4929839c84ac949e8d31ee90 Mon Sep 17 00:00:00 2001 From: Thomas Avery Date: Thu, 12 Dec 2024 12:18:09 -0600 Subject: [PATCH 30/30] Update abstractions --- .../src/common/abstractions/login-success-handler.service.ts | 2 +- .../user-asymmetric-key-regeneration-api.service.ts | 4 ++-- .../abstractions/user-asymmetric-key-regeneration.service.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/auth/src/common/abstractions/login-success-handler.service.ts b/libs/auth/src/common/abstractions/login-success-handler.service.ts index c74db74fe30..8dee1dd32b9 100644 --- a/libs/auth/src/common/abstractions/login-success-handler.service.ts +++ b/libs/auth/src/common/abstractions/login-success-handler.service.ts @@ -6,5 +6,5 @@ export abstract class LoginSuccessHandlerService { * Service calls that should be included in this method are only those required to be awaited after successful login. * @param userId The user id. */ - abstract run: (userId: UserId) => Promise; + abstract run(userId: UserId): Promise; } diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts index aac47be71b0..2b6e093d796 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration-api.service.ts @@ -1,8 +1,8 @@ import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; export abstract class UserAsymmetricKeysRegenerationApiService { - abstract regenerateUserAsymmetricKeys: ( + abstract regenerateUserAsymmetricKeys( userPublicKey: string, userKeyEncryptedUserPrivateKey: EncString, - ) => Promise; + ): Promise; } diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts index c59aa6f29c1..4703d836db7 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/abstractions/user-asymmetric-key-regeneration.service.ts @@ -6,5 +6,5 @@ export abstract class UserAsymmetricKeysRegenerationService { * Requires the PrivateKeyRegeneration feature flag to be enabled if not the method will do nothing. * @param userId The user id. */ - abstract regenerateIfNeeded: (userId: UserId) => Promise; + abstract regenerateIfNeeded(userId: UserId): Promise; }