From ca3987739d180a0b8edba303311747ed5af87df6 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 21 Oct 2025 16:13:14 +0200 Subject: [PATCH] Implement automatic kdf upgrades --- apps/browser/src/_locales/en/messages.json | 18 ++ apps/cli/src/auth/commands/login.command.ts | 4 + apps/cli/src/base-program.ts | 1 + .../key-management/commands/unlock.command.ts | 4 + apps/cli/src/oss-serve-configurator.ts | 1 + apps/cli/src/program.ts | 2 + .../service-container/service-container.ts | 19 ++ apps/desktop/src/locales/en/messages.json | 18 ++ .../app/auth/recover-two-factor.component.ts | 2 +- .../change-kdf-confirmation.component.ts | 2 +- apps/web/src/locales/en/messages.json | 18 ++ .../login-via-webauthn.component.ts | 2 +- ...igrations-scheduler.service.abstraction.ts | 9 + ...ypted-migrations-scheduler.service.spec.ts | 266 ++++++++++++++++++ .../encrypted-migrations-scheduler.service.ts | 162 +++++++++++ .../prompt-migration-password.component.html | 49 ++++ .../prompt-migration-password.component.ts | 54 ++++ .../src/services/jslib-services.module.ts | 49 +++- .../login-via-auth-request.component.ts | 2 +- .../auth/src/angular/login/login.component.ts | 2 +- .../new-device-verification.component.ts | 3 +- .../registration-finish.component.ts | 5 +- libs/auth/src/angular/sso/sso.component.ts | 2 +- .../two-factor-auth.component.ts | 2 +- .../login-success-handler.service.ts | 3 +- .../login-strategies/login.strategy.spec.ts | 2 + .../common/login-strategies/login.strategy.ts | 4 + .../password-login.strategy.ts | 6 + ...ault-login-success-handler.service.spec.ts | 16 +- .../default-login-success-handler.service.ts | 10 +- .../src/auth/models/domain/auth-result.ts | 2 + .../default-encrypted-migrator.spec.ts | 191 +++++++++++++ .../default-encrypted-migrator.ts | 96 +++++++ .../encrypted-migrator.abstraction.ts | 27 ++ .../migrations/encrypted-migration.ts | 36 +++ .../migrations/minimum-kdf-migration.spec.ts | 185 ++++++++++++ .../migrations/minimum-kdf-migration.ts | 69 +++++ ...n.ts => change-kdf.service.abstraction.ts} | 0 ...ice.spec.ts => change-kdf.service.spec.ts} | 0 ...e-kdf-service.ts => change-kdf.service.ts} | 2 +- .../master-password.service.abstraction.ts | 7 + .../services/fake-master-password.service.ts | 4 + .../services/master-password.service.ts | 15 + .../lock/components/lock.component.spec.ts | 3 + .../src/lock/components/lock.component.ts | 13 + libs/key-management/src/index.ts | 1 + libs/key-management/src/models/kdf-config.ts | 1 + libs/state/src/core/state-definitions.ts | 18 +- 48 files changed, 1379 insertions(+), 28 deletions(-) create mode 100644 libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts create mode 100644 libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts create mode 100644 libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts create mode 100644 libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html create mode 100644 libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts rename libs/common/src/key-management/kdf/{change-kdf-service.abstraction.ts => change-kdf.service.abstraction.ts} (100%) rename libs/common/src/key-management/kdf/{change-kdf-service.spec.ts => change-kdf.service.spec.ts} (100%) rename libs/common/src/key-management/kdf/{change-kdf-service.ts => change-kdf.service.ts} (97%) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index ce4c5c76b81..e7a75dd537a 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -1373,6 +1373,24 @@ "learnMore": { "message": "Learn more" }, + "migrationsFailed": { + "message": "An error occurred updating the encryption settings." + }, + "updateEncryptionSettingsTitle": { + "message": "Update your encryption settings" + }, + "updateEncryptionSettingsDesc": { + "message": "The new recommended encryption settings will improve your account security. Enter your master password to update now." + }, + "confirmIdentityToContinue": { + "message": "Confirm your identity to continue" + }, + "enterYourMasterPassword": { + "message": "Enter your master password" + }, + "updateSettings": { + "message": "Update settings" + }, "authenticatorKeyTotp": { "message": "Authenticator key (TOTP)" }, diff --git a/apps/cli/src/auth/commands/login.command.ts b/apps/cli/src/auth/commands/login.command.ts index aa43b353f9c..bf79e3e3c27 100644 --- a/apps/cli/src/auth/commands/login.command.ts +++ b/apps/cli/src/auth/commands/login.command.ts @@ -32,6 +32,7 @@ import { TwoFactorApiService } from "@bitwarden/common/auth/two-factor"; import { ClientType } from "@bitwarden/common/enums"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; @@ -82,6 +83,7 @@ export class LoginCommand { protected ssoUrlService: SsoUrlService, protected i18nService: I18nService, protected masterPasswordService: MasterPasswordServiceAbstraction, + protected encryptedMigrator: EncryptedMigrator, ) {} async run(email: string, password: string, options: OptionValues) { @@ -368,6 +370,8 @@ export class LoginCommand { } } + await this.encryptedMigrator.runMigrations(response.userId, password); + return await this.handleSuccessResponse(response); } catch (e) { if ( diff --git a/apps/cli/src/base-program.ts b/apps/cli/src/base-program.ts index 69a5e4e1bde..71c3830b4cc 100644 --- a/apps/cli/src/base-program.ts +++ b/apps/cli/src/base-program.ts @@ -182,6 +182,7 @@ export abstract class BaseProgram { this.serviceContainer.organizationApiService, this.serviceContainer.logout, this.serviceContainer.i18nService, + this.serviceContainer.encryptedMigrator, this.serviceContainer.masterPasswordUnlockService, this.serviceContainer.configService, ); diff --git a/apps/cli/src/key-management/commands/unlock.command.ts b/apps/cli/src/key-management/commands/unlock.command.ts index 4ae8ce823a4..c88d9ae1cc4 100644 --- a/apps/cli/src/key-management/commands/unlock.command.ts +++ b/apps/cli/src/key-management/commands/unlock.command.ts @@ -9,6 +9,7 @@ import { VerificationType } from "@bitwarden/common/auth/enums/verification-type import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; @@ -38,6 +39,7 @@ export class UnlockCommand { private organizationApiService: OrganizationApiServiceAbstraction, private logout: () => Promise, private i18nService: I18nService, + private encryptedMigrator: EncryptedMigrator, private masterPasswordUnlockService: MasterPasswordUnlockService, private configService: ConfigService, ) {} @@ -116,6 +118,8 @@ export class UnlockCommand { } } + await this.encryptedMigrator.runMigrations(userId, password); + return this.successResponse(); } diff --git a/apps/cli/src/oss-serve-configurator.ts b/apps/cli/src/oss-serve-configurator.ts index d318a44c677..1c31f2a42ce 100644 --- a/apps/cli/src/oss-serve-configurator.ts +++ b/apps/cli/src/oss-serve-configurator.ts @@ -173,6 +173,7 @@ export class OssServeConfigurator { this.serviceContainer.organizationApiService, async () => await this.serviceContainer.logout(), this.serviceContainer.i18nService, + this.serviceContainer.encryptedMigrator, this.serviceContainer.masterPasswordUnlockService, this.serviceContainer.configService, ); diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts index 41368269faf..8d941099886 100644 --- a/apps/cli/src/program.ts +++ b/apps/cli/src/program.ts @@ -195,6 +195,7 @@ export class Program extends BaseProgram { this.serviceContainer.ssoUrlService, this.serviceContainer.i18nService, this.serviceContainer.masterPasswordService, + this.serviceContainer.encryptedMigrator, ); const response = await command.run(email, password, options); this.processResponse(response, true); @@ -303,6 +304,7 @@ export class Program extends BaseProgram { this.serviceContainer.organizationApiService, async () => await this.serviceContainer.logout(), this.serviceContainer.i18nService, + this.serviceContainer.encryptedMigrator, this.serviceContainer.masterPasswordUnlockService, this.serviceContainer.configService, ); diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index d10849bae0f..14d7a1dac33 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -69,6 +69,10 @@ import { import { EncryptServiceImplementation } from "@bitwarden/common/key-management/crypto/services/encrypt.service.implementation"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; import { DeviceTrustService } from "@bitwarden/common/key-management/device-trust/services/device-trust.service.implementation"; +import { DefaultEncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/default-encrypted-migrator"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; +import { DefaultChangeKdfApiService } from "@bitwarden/common/key-management/kdf/change-kdf-api.service"; +import { DefaultChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/services/key-connector.service"; import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; @@ -315,6 +319,7 @@ export class ServiceContainer { cipherEncryptionService: CipherEncryptionService; restrictedItemTypesService: RestrictedItemTypesService; cliRestrictedItemTypesService: CliRestrictedItemTypesService; + encryptedMigrator: EncryptedMigrator; securityStateService: SecurityStateService; masterPasswordUnlockService: MasterPasswordUnlockService; cipherArchiveService: CipherArchiveService; @@ -948,6 +953,20 @@ export class ServiceContainer { ); this.masterPasswordApiService = new MasterPasswordApiService(this.apiService, this.logService); + const changeKdfApiService = new DefaultChangeKdfApiService(this.apiService); + const changeKdfService = new DefaultChangeKdfService( + this.masterPasswordService, + this.keyService, + this.kdfConfigService, + changeKdfApiService, + ); + this.encryptedMigrator = new DefaultEncryptedMigrator( + this.kdfConfigService, + changeKdfService, + this.logService, + this.configService, + this.masterPasswordService, + ); } async logout() { diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 3e004e270a3..19519b9bca1 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -1084,6 +1084,24 @@ "learnMore": { "message": "Learn more" }, + "migrationsFailed": { + "message": "An error occurred updating the encryption settings." + }, + "updateEncryptionSettingsTitle": { + "message": "Update your encryption settings" + }, + "updateEncryptionSettingsDesc": { + "message": "The new recommended encryption settings will improve your account security. Enter your master password to update now." + }, + "confirmIdentityToContinue": { + "message": "Confirm your identity to continue" + }, + "enterYourMasterPassword": { + "message": "Enter your master password" + }, + "updateSettings": { + "message": "Update settings" + }, "featureUnavailable": { "message": "Feature unavailable" }, diff --git a/apps/web/src/app/auth/recover-two-factor.component.ts b/apps/web/src/app/auth/recover-two-factor.component.ts index f606e803df3..f1b23f6fb14 100644 --- a/apps/web/src/app/auth/recover-two-factor.component.ts +++ b/apps/web/src/app/auth/recover-two-factor.component.ts @@ -106,7 +106,7 @@ export class RecoverTwoFactorComponent implements OnInit { message: this.i18nService.t("twoStepRecoverDisabled"), }); - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, this.masterPassword); await this.router.navigate(["/settings/security/two-factor"]); } catch (error: unknown) { diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts index 3e09f710062..ae782cbbf45 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts @@ -6,7 +6,7 @@ import { firstValueFrom } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service.abstraction"; +import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { DIALOG_DATA, ToastService } from "@bitwarden/components"; diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 5eab55e2301..9f714b22950 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -4486,6 +4486,24 @@ "learnMore": { "message": "Learn more" }, + "migrationsFailed": { + "message": "An error occurred updating the encryption settings." + }, + "updateEncryptionSettingsTitle": { + "message": "Update your encryption settings" + }, + "updateEncryptionSettingsDesc": { + "message": "The new recommended encryption settings will improve your account security. Enter your master password to update now." + }, + "confirmIdentityToContinue": { + "message": "Confirm your identity to continue" + }, + "enterYourMasterPassword": { + "message": "Enter your master password" + }, + "updateSettings": { + "message": "Update settings" + }, "deleteRecoverDesc": { "message": "Enter your email address below to recover and delete your account." }, diff --git a/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts b/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts index f795b66d916..7ffeceb8415 100644 --- a/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts +++ b/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts @@ -118,7 +118,7 @@ export class LoginViaWebAuthnComponent implements OnInit { // 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); + await this.loginSuccessHandlerService.run(authResult.userId, null); } await this.router.navigate([this.successRoute]); diff --git a/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts new file mode 100644 index 00000000000..565cbb02cf0 --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts @@ -0,0 +1,9 @@ +import { UserId } from "@bitwarden/common/types/guid"; + +export abstract class EncryptedMigrationsSchedulerService { + /** + * Runs migrations for a user if needed, handling both interactive and non-interactive cases + * @param userId The user ID to run migrations for + */ + abstract runMigrationsIfNeeded(userId: UserId): Promise; +} diff --git a/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts new file mode 100644 index 00000000000..72702331ea1 --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts @@ -0,0 +1,266 @@ +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AccountInfo } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { SingleUserState, StateProvider } from "@bitwarden/common/platform/state"; +import { SyncService } from "@bitwarden/common/platform/sync"; +import { FakeAccountService } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; +import { DialogService, ToastService } from "@bitwarden/components"; +import { LogService } from "@bitwarden/logging"; + +import { + DefaultEncryptedMigrationsSchedulerService, + ENCRYPTED_MIGRATION_DISMISSED, +} from "./encrypted-migrations-scheduler.service"; +import { PromptMigrationPasswordComponent } from "./prompt-migration-password.component"; + +const SomeUser = "SomeUser" as UserId; +const AnotherUser = "SomeOtherUser" as UserId; +const accounts: Record = { + [SomeUser]: { + name: "some user", + email: "some.user@example.com", + emailVerified: true, + }, + [AnotherUser]: { + name: "some other user", + email: "some.other.user@example.com", + emailVerified: true, + }, +}; + +describe("DefaultEncryptedMigrationsSchedulerService", () => { + let service: DefaultEncryptedMigrationsSchedulerService; + const mockAccountService = new FakeAccountService(accounts); + const mockAuthService = mock(); + const mockEncryptedMigrator = mock(); + const mockStateProvider = mock(); + const mockSyncService = mock(); + const mockDialogService = mock(); + const mockToastService = mock(); + const mockI18nService = mock(); + const mockLogService = mock(); + + const mockUserId = "test-user-id" as UserId; + const mockMasterPassword = "test-master-password"; + + const createMockUserState = (value: T): jest.Mocked> => + ({ + state$: of(value), + userId: mockUserId, + update: jest.fn(), + combinedState$: of([mockUserId, value]), + }) as any; + + beforeEach(() => { + const mockDialogRef = { + closed: of(mockMasterPassword), + }; + + jest.spyOn(PromptMigrationPasswordComponent, "open").mockReturnValue(mockDialogRef as any); + mockI18nService.t.mockReturnValue("translated_migrationsFailed"); + + service = new DefaultEncryptedMigrationsSchedulerService( + mockSyncService, + mockAccountService, + mockStateProvider, + mockEncryptedMigrator, + mockAuthService, + mockLogService, + mockDialogService, + mockToastService, + mockI18nService, + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("runMigrationsIfNeeded", () => { + it("should return early if user is not unlocked", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Locked)); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).not.toHaveBeenCalled(); + expect(mockLogService.info).not.toHaveBeenCalled(); + }); + + it("should log and return when no migration is needed", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("noMigrationNeeded"); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).toHaveBeenCalledWith(mockUserId); + expect(mockLogService.info).toHaveBeenCalledWith( + `[EncryptedMigrationsScheduler] No migrations needed for user ${mockUserId}`, + ); + expect(mockEncryptedMigrator.runMigrations).not.toHaveBeenCalled(); + }); + + it("should run migrations without interaction when master password is not required", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigration"); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).toHaveBeenCalledWith(mockUserId); + expect(mockLogService.info).toHaveBeenCalledWith( + `[EncryptedMigrationsScheduler] User ${mockUserId} needs migrations with master password`, + ); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith(mockUserId, null); + }); + + it("should run migrations with interaction when migration is needed", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigrationWithMasterPassword"); + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).toHaveBeenCalledWith(mockUserId); + expect(mockLogService.info).toHaveBeenCalledWith( + `[EncryptedMigrationsScheduler] User ${mockUserId} needs migrations with master password`, + ); + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + }); + + describe("runMigrationsWithoutInteraction", () => { + it("should run migrations without master password", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigration"); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith(mockUserId, null); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + + it("should handle errors during migration without interaction", async () => { + const mockError = new Error("Migration failed"); + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigration"); + mockEncryptedMigrator.runMigrations.mockRejectedValue(mockError); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith(mockUserId, null); + expect(mockLogService.error).toHaveBeenCalledWith( + "[EncryptedMigrationsInitiator] Error during migration without interaction", + mockError, + ); + }); + }); + + describe("runMigrationsWithInteraction", () => { + beforeEach(() => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigrationWithMasterPassword"); + }); + + it("should skip if migration was dismissed recently", async () => { + const recentDismissDate = new Date(Date.now() - 12 * 60 * 60 * 1000); // 12 hours ago + const mockUserState = createMockUserState(recentDismissDate); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockStateProvider.getUser).toHaveBeenCalledWith( + mockUserId, + ENCRYPTED_MIGRATION_DISMISSED, + ); + expect(mockLogService.info).toHaveBeenCalledWith( + "[EncryptedMigrationsInitiator] Migration prompt dismissed recently, skipping for now.", + ); + expect(PromptMigrationPasswordComponent.open).not.toHaveBeenCalled(); + }); + + it("should prompt for migration if dismissed date is older than 24 hours", async () => { + const oldDismissDate = new Date(Date.now() - 25 * 60 * 60 * 1000); // 25 hours ago + const mockUserState = createMockUserState(oldDismissDate); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockStateProvider.getUser).toHaveBeenCalledWith( + mockUserId, + ENCRYPTED_MIGRATION_DISMISSED, + ); + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + + it("should prompt for migration if no dismiss date exists", async () => { + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + + it("should set dismiss date when empty password is provided", async () => { + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + const mockDialogRef = { + closed: of(""), // Empty password + }; + jest.spyOn(PromptMigrationPasswordComponent, "open").mockReturnValue(mockDialogRef as any); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).not.toHaveBeenCalled(); + expect(mockStateProvider.setUserState).toHaveBeenCalledWith( + ENCRYPTED_MIGRATION_DISMISSED, + expect.any(Date), + mockUserId, + ); + }); + + it("should handle errors during migration prompt and show toast", async () => { + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + const mockError = new Error("Migration failed"); + mockEncryptedMigrator.runMigrations.mockRejectedValue(mockError); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + expect(mockLogService.error).toHaveBeenCalledWith( + "[EncryptedMigrationsInitiator] Error during migration prompt", + mockError, + ); + expect(mockToastService.showToast).toHaveBeenCalledWith({ + variant: "error", + message: "translated_migrationsFailed", + }); + }); + }); +}); diff --git a/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts new file mode 100644 index 00000000000..a7eae5a5136 --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts @@ -0,0 +1,162 @@ +import { combineLatest, map, switchMap, of, firstValueFrom, filter, debounceTime, tap } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { + UserKeyDefinition, + ENCRYPTED_MIGRATION_DISK, + StateProvider, +} from "@bitwarden/common/platform/state"; +import { SyncService } from "@bitwarden/common/platform/sync"; +import { UserId } from "@bitwarden/common/types/guid"; +import { DialogService, ToastService } from "@bitwarden/components"; +import { LogService } from "@bitwarden/logging"; + +import { EncryptedMigrationsSchedulerService } from "./encrypted-migrations-scheduler.service.abstraction"; +import { PromptMigrationPasswordComponent } from "./prompt-migration-password.component"; + +export const ENCRYPTED_MIGRATION_DISMISSED = new UserKeyDefinition( + ENCRYPTED_MIGRATION_DISK, + "encryptedMigrationDismissed", + { + deserializer: (obj: any) => (obj != null ? new Date(obj) : null), + clearOn: [], + }, +); +const DISMISS_TIME_HOURS = 24; + +type UserSyncData = { + userId: UserId; + lastSync: Date | null; +}; + +/** + * This services schedules encrypted migrations for users on clients that are interactive (non-cli), and handles manual interaction, + * if it is required by showing a UI prompt. It is only one means of triggering migrations, in case the user stays unlocked for a while, + * or regularly logs in without a master-password, when the migrations do require a master-password to run. + */ +export class DefaultEncryptedMigrationsSchedulerService + implements EncryptedMigrationsSchedulerService +{ + constructor( + private syncService: SyncService, + private accountService: AccountService, + private stateProvider: StateProvider, + private encryptedMigrator: EncryptedMigrator, + private authService: AuthService, + private logService: LogService, + private dialogService: DialogService, + private toastService: ToastService, + private i18nService: I18nService, + ) { + // For all accounts, if the auth status changes to unlocked or a sync happens, prompt for migration + this.accountService.accounts$ + .pipe( + switchMap((accounts) => { + const userIds = Object.keys(accounts) as UserId[]; + + if (userIds.length === 0) { + return of([]); + } + + return combineLatest( + userIds.map((userId) => + combineLatest([ + this.authService.authStatusFor$(userId), + this.syncService.lastSync$(userId), + ]).pipe( + // Prevent double emissions from frequent state changes during login/unlock from showing overlapping prompts + debounceTime(2000), + filter(([authStatus]) => authStatus === AuthenticationStatus.Unlocked), + map(([, lastSync]) => ({ userId, lastSync }) as UserSyncData), + tap(({ userId }) => this.runMigrationsIfNeeded(userId)), + ), + ), + ); + }), + ) + .subscribe(); + } + + async runMigrationsIfNeeded(userId: UserId): Promise { + const authStatus = await firstValueFrom(this.authService.authStatusFor$(userId)); + if (authStatus !== AuthenticationStatus.Unlocked) { + return; + } + + switch (await this.encryptedMigrator.needsMigrations(userId)) { + case "noMigrationNeeded": + this.logService.info( + `[EncryptedMigrationsScheduler] No migrations needed for user ${userId}`, + ); + break; + case "needsMigrationWithMasterPassword": + this.logService.info( + `[EncryptedMigrationsScheduler] User ${userId} needs migrations with master password`, + ); + // If the user is unlocked, we can run migrations with the master password + await this.runMigrationsWithInteraction(userId); + break; + case "needsMigration": + this.logService.info( + `[EncryptedMigrationsScheduler] User ${userId} needs migrations with master password`, + ); + // If the user is unlocked, we can prompt for the master password + await this.runMigrationsWithoutInteraction(userId); + break; + } + } + + private async runMigrationsWithoutInteraction(userId: UserId): Promise { + try { + await this.encryptedMigrator.runMigrations(userId, null); + } catch (error) { + this.logService.error( + "[EncryptedMigrationsInitiator] Error during migration without interaction", + error, + ); + } + } + + private async runMigrationsWithInteraction(userId: UserId): Promise { + // A dialog can be dismissed for a certain amount of time + const dismissedDate = await firstValueFrom( + this.stateProvider.getUser(userId, ENCRYPTED_MIGRATION_DISMISSED).state$, + ); + if (dismissedDate != null) { + const now = new Date(); + const timeDiff = now.getTime() - (dismissedDate as Date).getTime(); + const hoursDiff = timeDiff / (1000 * 60 * 60); + + if (hoursDiff < DISMISS_TIME_HOURS) { + this.logService.info( + "[EncryptedMigrationsInitiator] Migration prompt dismissed recently, skipping for now.", + ); + return; + } + } + + try { + const dialog = PromptMigrationPasswordComponent.open(this.dialogService); + const masterPassword = await firstValueFrom(dialog.closed); + if (masterPassword == "") { + await this.stateProvider.setUserState(ENCRYPTED_MIGRATION_DISMISSED, new Date(), userId); + } else { + await this.encryptedMigrator.runMigrations( + userId, + masterPassword === undefined ? null : masterPassword, + ); + } + } catch (error) { + this.logService.error("[EncryptedMigrationsInitiator] Error during migration prompt", error); + // If migrations failed when the user actively was prompted, show a toast + this.toastService.showToast({ + variant: "error", + message: this.i18nService.t("migrationsFailed"), + }); + } + } +} diff --git a/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html new file mode 100644 index 00000000000..e75734c136d --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html @@ -0,0 +1,49 @@ +
+ +
+ {{ "updateEncryptionSettingsTitle" | i18n }} +
+
+

+ {{ "updateEncryptionSettingsDesc" | i18n }} + + {{ "learnMore" | i18n }} + + +

+ + {{ "masterPass" | i18n }} + {{ "confirmIdentityToContinue" | i18n }} + + + +
+ + + + + +
diff --git a/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts new file mode 100644 index 00000000000..bb7547826b1 --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts @@ -0,0 +1,54 @@ +import { CommonModule } from "@angular/common"; +import { Component, inject } from "@angular/core"; +import { FormBuilder, ReactiveFormsModule, Validators } from "@angular/forms"; + +import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { + AsyncActionsModule, + ButtonModule, + DialogModule, + DialogRef, + DialogService, + FormFieldModule, + IconButtonModule, +} from "@bitwarden/components"; + +/** + * This is a generic prompt to run encryption migrations that require the master password. + */ +@Component({ + templateUrl: "prompt-migration-password.component.html", + imports: [ + DialogModule, + CommonModule, + JslibModule, + ButtonModule, + IconButtonModule, + ReactiveFormsModule, + AsyncActionsModule, + FormFieldModule, + ], +}) +export class PromptMigrationPasswordComponent { + private dialogRef = inject(DialogRef); + private formBuilder = inject(FormBuilder); + + migrationPasswordForm = this.formBuilder.group({ + masterPassword: ["", [Validators.required]], + }); + + static open(dialogService: DialogService) { + return dialogService.open(PromptMigrationPasswordComponent); + } + + submit = async () => { + const masterPasswordControl = this.migrationPasswordForm.controls.masterPassword; + + if (!masterPasswordControl.value || masterPasswordControl.invalid) { + return; + } + + // Return the master password to the caller + this.dialogRef.close(masterPasswordControl.value); + }; +} diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index f5642f45b2e..8b45937b1e4 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { ErrorHandler, LOCALE_ID, NgModule } from "@angular/core"; +import { APP_INITIALIZER, ErrorHandler, LOCALE_ID, NgModule } from "@angular/core"; import { Subject } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. @@ -167,10 +167,12 @@ import { EncryptServiceImplementation } from "@bitwarden/common/key-management/c import { WebCryptoFunctionService } from "@bitwarden/common/key-management/crypto/services/web-crypto-function.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; import { DeviceTrustService } from "@bitwarden/common/key-management/device-trust/services/device-trust.service.implementation"; +import { DefaultEncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/default-encrypted-migrator"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { DefaultChangeKdfApiService } from "@bitwarden/common/key-management/kdf/change-kdf-api.service"; import { ChangeKdfApiService } from "@bitwarden/common/key-management/kdf/change-kdf-api.service.abstraction"; -import { DefaultChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service"; -import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service.abstraction"; +import { DefaultChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service"; +import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service.abstraction"; import { KeyConnectorService as KeyConnectorServiceAbstraction } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/services/key-connector.service"; import { KeyApiService } from "@bitwarden/common/key-management/keys/services/abstractions/key-api-service.abstraction"; @@ -310,6 +312,7 @@ import { DefaultTaskService, TaskService } from "@bitwarden/common/vault/tasks"; import { AnonLayoutWrapperDataService, DefaultAnonLayoutWrapperDataService, + DialogService, ToastService, } from "@bitwarden/components"; import { @@ -376,6 +379,8 @@ import { DefaultSetInitialPasswordService } from "../auth/password-management/se import { SetInitialPasswordService } from "../auth/password-management/set-initial-password/set-initial-password.service.abstraction"; import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "../auth/services/device-trust-toast.service.abstraction"; import { DeviceTrustToastService } from "../auth/services/device-trust-toast.service.implementation"; +import { DefaultEncryptedMigrationsSchedulerService } from "../key-management/encrypted-migration/encrypted-migrations-scheduler.service"; +import { EncryptedMigrationsSchedulerService } from "../key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction"; import { FormValidationErrorsService as FormValidationErrorsServiceAbstraction } from "../platform/abstractions/form-validation-errors.service"; import { DocumentLangSetter } from "../platform/i18n"; import { FormValidationErrorsService } from "../platform/services/form-validation-errors.service"; @@ -501,6 +506,22 @@ const safeProviders: SafeProvider[] = [ TokenServiceAbstraction, ], }), + safeProvider({ + provide: ChangeKdfService, + useClass: DefaultChangeKdfService, + deps: [ChangeKdfApiService, SdkService], + }), + safeProvider({ + provide: EncryptedMigrator, + useClass: DefaultEncryptedMigrator, + deps: [ + KdfConfigService, + ChangeKdfService, + LogService, + ConfigService, + MasterPasswordServiceAbstraction, + ], + }), safeProvider({ provide: LoginStrategyServiceAbstraction, useClass: LoginStrategyService, @@ -1618,6 +1639,7 @@ const safeProviders: SafeProvider[] = [ SsoLoginServiceAbstraction, SyncService, UserAsymmetricKeysRegenerationService, + EncryptedMigrator, LogService, ], }), @@ -1688,6 +1710,27 @@ const safeProviders: SafeProvider[] = [ InternalMasterPasswordServiceAbstraction, ], }), + safeProvider({ + provide: EncryptedMigrationsSchedulerService, + useClass: DefaultEncryptedMigrationsSchedulerService, + deps: [ + SyncService, + AccountService, + StateProvider, + EncryptedMigrator, + AuthServiceAbstraction, + LogService, + DialogService, + ToastService, + I18nServiceAbstraction, + ], + }), + safeProvider({ + provide: APP_INITIALIZER as SafeInjectionToken<() => Promise>, + useFactory: (encryptedMigrationsScheduler: EncryptedMigrationsSchedulerService) => () => {}, + deps: [EncryptedMigrationsSchedulerService], + multi: true, + }), safeProvider({ provide: CipherArchiveService, useClass: DefaultCipherArchiveService, 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 10b19567946..93d02228a51 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 @@ -820,7 +820,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { } private async handleSuccessfulLoginNavigation(userId: UserId) { - await this.loginSuccessHandlerService.run(userId); + await this.loginSuccessHandlerService.run(userId, null); 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 9ade2c1d0af..0a4569ea9e7 100644 --- a/libs/auth/src/angular/login/login.component.ts +++ b/libs/auth/src/angular/login/login.component.ts @@ -381,7 +381,7 @@ export class LoginComponent implements OnInit, OnDestroy { } // User logged in successfully so execute side effects - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, authResult.masterPassword); // Determine where to send the user next // The AuthGuard will handle routing to change-password based on state diff --git a/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts b/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts index 2211b3390a7..830e4107b31 100644 --- a/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts +++ b/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts @@ -151,8 +151,9 @@ export class NewDeviceVerificationComponent implements OnInit, OnDestroy { } // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. + // TODO // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.loginSuccessHandlerService.run(authResult.userId); + this.loginSuccessHandlerService.run(authResult.userId, authResult.masterPassword); // TODO: PM-22663 use the new service to handle routing. const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); diff --git a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts index 952b9e9ce75..7cd03fa87ff 100644 --- a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts +++ b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts @@ -186,7 +186,10 @@ export class RegistrationFinishComponent implements OnInit, OnDestroy { return; } - await this.loginSuccessHandlerService.run(authenticationResult.userId); + await this.loginSuccessHandlerService.run( + authenticationResult.userId, + authenticationResult?.masterPassword ?? null, + ); await this.router.navigate(["/vault"]); } catch (e) { diff --git a/libs/auth/src/angular/sso/sso.component.ts b/libs/auth/src/angular/sso/sso.component.ts index 8636595759e..11fcdfdcd59 100644 --- a/libs/auth/src/angular/sso/sso.component.ts +++ b/libs/auth/src/angular/sso/sso.component.ts @@ -435,7 +435,7 @@ export class SsoComponent implements OnInit { // Everything after the 2FA check is considered a successful login // Just have to figure out where to send the user - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, null); // Save off the OrgSsoIdentifier for use in the TDE flows (or elsewhere) // - TDE login decryption options component diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index 4c0784928d4..e6c07947e8c 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts @@ -444,7 +444,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { } // User is fully logged in so handle any post login logic before executing navigation - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, authResult.masterPassword); // Save off the OrgSsoIdentifier for use in the TDE flows // - TDE login decryption options component 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 8dee1dd32b9..479e2dfa9cb 100644 --- a/libs/auth/src/common/abstractions/login-success-handler.service.ts +++ b/libs/auth/src/common/abstractions/login-success-handler.service.ts @@ -5,6 +5,7 @@ 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. + * @param masterPassword */ - abstract run(userId: UserId): Promise; + abstract run(userId: UserId, masterPassword: string | null): Promise; } 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 a23f8034238..e9f3d20e305 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -307,6 +307,7 @@ describe("LoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); const expected = new AuthResult(); + expected.masterPassword = "password"; expected.userId = userId; expected.resetMasterPassword = true; expected.twoFactorProviders = null; @@ -322,6 +323,7 @@ describe("LoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); const expected = new AuthResult(); + expected.masterPassword = "password"; expected.userId = userId; expected.resetMasterPassword = false; expected.twoFactorProviders = null; diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 7ad5cd24353..276e6e11e7d 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -108,6 +108,8 @@ export abstract class LoginStrategy { data.tokenRequest.setTwoFactor(twoFactor); this.cache.next(data); const [authResult] = await this.startLogIn(); + // There is an import cycle between PasswordLoginStrategyData and LoginStrategy, which means this cast is necessary, which is solved by extracting the data classes. + authResult.masterPassword = (this.cache.value as any)["masterPassword"] ?? null; return authResult; } @@ -263,6 +265,8 @@ export abstract class LoginStrategy { await this.processForceSetPasswordReason(response.forcePasswordReset, userId); this.messagingService.send("loggedIn"); + // There is an import cycle between PasswordLoginStrategyData and LoginStrategy, which means this cast is necessary, which is solved by extracting the data classes. + result.masterPassword = (this.cache.value as any)["masterPassword"] ?? null; return result; } diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index 3482e73d5d7..67f27471571 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -33,6 +33,8 @@ export class PasswordLoginStrategyData implements LoginStrategyData { localMasterKeyHash: string; /** The user's master key */ masterKey: MasterKey; + /** The user's master password */ + masterPassword: string; /** * Tracks if the user needs to update their password due to * a password that does not meet an organization's master password policy. @@ -80,6 +82,7 @@ export class PasswordLoginStrategy extends LoginStrategy { const data = new PasswordLoginStrategyData(); data.masterKey = await this.loginStrategyService.makePreloginKey(masterPassword, email); + data.masterPassword = masterPassword; data.userEnteredEmail = email; // Hash the password early (before authentication) so we don't persist it in memory in plaintext @@ -248,6 +251,9 @@ export class PasswordLoginStrategy extends LoginStrategy { this.cache.next(data); const [authResult] = await this.startLogIn(); + if (this.cache.value instanceof PasswordLoginStrategyData) { + authResult.masterPassword = this.cache.value["masterPassword"] ?? null; + } return authResult; } diff --git a/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts index 86f7be8dfc7..988bf01f05d 100644 --- a/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts +++ b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts @@ -2,6 +2,7 @@ import { MockProxy, mock } from "jest-mock-extended"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; @@ -20,6 +21,7 @@ describe("DefaultLoginSuccessHandlerService", () => { let ssoLoginService: MockProxy; let syncService: MockProxy; let userAsymmetricKeysRegenerationService: MockProxy; + let encryptedMigrator: MockProxy; let logService: MockProxy; const userId = "USER_ID" as UserId; @@ -31,6 +33,7 @@ describe("DefaultLoginSuccessHandlerService", () => { ssoLoginService = mock(); syncService = mock(); userAsymmetricKeysRegenerationService = mock(); + encryptedMigrator = mock(); logService = mock(); service = new DefaultLoginSuccessHandlerService( @@ -39,6 +42,7 @@ describe("DefaultLoginSuccessHandlerService", () => { ssoLoginService, syncService, userAsymmetricKeysRegenerationService, + encryptedMigrator, logService, ); @@ -51,7 +55,7 @@ describe("DefaultLoginSuccessHandlerService", () => { describe("run", () => { it("should call required services on successful login", async () => { - await service.run(userId); + await service.run(userId, null); expect(syncService.fullSync).toHaveBeenCalledWith(true, { skipTokenRefresh: true }); expect(userAsymmetricKeysRegenerationService.regenerateIfNeeded).toHaveBeenCalledWith(userId); @@ -64,7 +68,7 @@ describe("DefaultLoginSuccessHandlerService", () => { }); it("should not check SSO requirements", async () => { - await service.run(userId); + await service.run(userId, null); expect(ssoLoginService.getSsoEmail).not.toHaveBeenCalled(); expect(ssoLoginService.updateSsoRequiredCache).not.toHaveBeenCalled(); @@ -77,7 +81,7 @@ describe("DefaultLoginSuccessHandlerService", () => { }); it("should check feature flag", async () => { - await service.run(userId); + await service.run(userId, null); expect(configService.getFeatureFlag).toHaveBeenCalledWith( FeatureFlag.PM22110_DisableAlternateLoginMethods, @@ -85,7 +89,7 @@ describe("DefaultLoginSuccessHandlerService", () => { }); it("should get SSO email", async () => { - await service.run(userId); + await service.run(userId, null); expect(ssoLoginService.getSsoEmail).toHaveBeenCalled(); }); @@ -96,7 +100,7 @@ describe("DefaultLoginSuccessHandlerService", () => { }); it("should log error and return early", async () => { - await service.run(userId); + await service.run(userId, null); expect(logService.error).toHaveBeenCalledWith("SSO login email not found."); expect(ssoLoginService.updateSsoRequiredCache).not.toHaveBeenCalled(); @@ -109,7 +113,7 @@ describe("DefaultLoginSuccessHandlerService", () => { }); it("should call updateSsoRequiredCache() and clearSsoEmail()", async () => { - await service.run(userId); + await service.run(userId, null); expect(ssoLoginService.updateSsoRequiredCache).toHaveBeenCalledWith(testEmail, userId); expect(ssoLoginService.clearSsoEmail).toHaveBeenCalled(); 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 index 78003a4fca0..9bb1cd6b583 100644 --- 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 @@ -1,5 +1,6 @@ import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; @@ -16,12 +17,19 @@ export class DefaultLoginSuccessHandlerService implements LoginSuccessHandlerSer private ssoLoginService: SsoLoginServiceAbstraction, private syncService: SyncService, private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, + private encryptedMigrator: EncryptedMigrator, private logService: LogService, ) {} - async run(userId: UserId): Promise { + + async run(userId: UserId, masterPassword: string | null): Promise { await this.syncService.fullSync(true, { skipTokenRefresh: true }); await this.userAsymmetricKeysRegenerationService.regenerateIfNeeded(userId); await this.loginEmailService.clearLoginEmail(); + try { + await this.encryptedMigrator.runMigrations(userId, masterPassword); + } catch { + // Don't block login success on migration failure + } const disableAlternateLoginMethodsFlagEnabled = await this.configService.getFeatureFlag( FeatureFlag.PM22110_DisableAlternateLoginMethods, diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index a61a35eeb1d..ae3e9bdeda6 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -18,6 +18,8 @@ export class AuthResult { email: string; requiresEncryptionKeyMigration: boolean; requiresDeviceVerification: boolean; + // The master-password used in the authentication process + masterPassword: string | null; get requiresTwoFactor() { return this.twoFactorProviders != null; diff --git a/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts new file mode 100644 index 00000000000..10fc8c77944 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts @@ -0,0 +1,191 @@ +import { mock } from "jest-mock-extended"; + +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { UserId } from "@bitwarden/common/types/guid"; +// eslint-disable-next-line no-restricted-imports +import { KdfConfigService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { ChangeKdfService } from "../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../master-password/abstractions/master-password.service.abstraction"; + +import { DefaultEncryptedMigrator } from "./default-encrypted-migrator"; +import { EncryptedMigration } from "./migrations/encrypted-migration"; +import { MinimumKdfMigration } from "./migrations/minimum-kdf-migration"; + +jest.mock("./migrations/minimum-kdf-migration"); + +describe("EncryptedMigrator", () => { + const mockKdfConfigService = mock(); + const mockChangeKdfService = mock(); + const mockLogService = mock(); + const configService = mock(); + const masterPasswordService = mock(); + + let sut: DefaultEncryptedMigrator; + const mockMigration = mock(); + + const mockUserId = "00000000-0000-0000-0000-000000000000" as UserId; + const mockMasterPassword = "masterPassword123"; + + beforeEach(() => { + jest.clearAllMocks(); + + // Mock the MinimumKdfMigration constructor to return our mock + (MinimumKdfMigration as jest.MockedClass).mockImplementation( + () => mockMigration, + ); + + sut = new DefaultEncryptedMigrator( + mockKdfConfigService, + mockChangeKdfService, + mockLogService, + configService, + masterPasswordService, + ); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe("runMigrations", () => { + it("should throw error when userId is null", async () => { + await expect(sut.runMigrations(null as any, null)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.runMigrations(undefined as any, null)).rejects.toThrow("userId"); + }); + + it("should not run migration when needsMigration returns 'noMigrationNeeded'", async () => { + mockMigration.needsMigration.mockResolvedValue("noMigrationNeeded"); + + await sut.runMigrations(mockUserId, null); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).not.toHaveBeenCalled(); + }); + + it("should run migration when needsMigration returns 'needsMigration'", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).toHaveBeenCalledWith(mockUserId, mockMasterPassword); + }); + + it("should run migration when needsMigration returns 'needsMigrationWithMasterPassword'", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).toHaveBeenCalledWith(mockUserId, mockMasterPassword); + }); + + it("should throw error when migration needs master password but null is provided", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + await sut.runMigrations(mockUserId, null); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).not.toHaveBeenCalled(); + }); + + it("should run multiple migrations", async () => { + const mockSecondMigration = mock(); + mockSecondMigration.needsMigration.mockResolvedValue("needsMigration"); + + (sut as any).migrations.push({ + name: "Test Second Migration", + migration: mockSecondMigration, + }); + + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockSecondMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).toHaveBeenCalledWith(mockUserId, mockMasterPassword); + expect(mockSecondMigration.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + }); + + describe("needsMigrations", () => { + it("should return 'noMigrationNeeded' when no migrations are needed", async () => { + mockMigration.needsMigration.mockResolvedValue("noMigrationNeeded"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'needsMigration' when at least one migration needs to run", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigration"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'needsMigrationWithMasterPassword' when at least one migration needs master password", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigrationWithMasterPassword"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should prioritize 'needsMigrationWithMasterPassword' over 'needsMigration'", async () => { + const mockSecondMigration = mock(); + mockSecondMigration.needsMigration.mockResolvedValue("needsMigration"); + + (sut as any).migrations.push({ + name: "Test Second Migration", + migration: mockSecondMigration, + }); + + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigrationWithMasterPassword"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockSecondMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'needsMigration' when some migrations need running but none need master password", async () => { + const mockSecondMigration = mock(); + mockSecondMigration.needsMigration.mockResolvedValue("noMigrationNeeded"); + + (sut as any).migrations.push({ + name: "Test Second Migration", + migration: mockSecondMigration, + }); + + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigration"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockSecondMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should throw error when userId is null", async () => { + await expect(sut.needsMigrations(null as any)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.needsMigrations(undefined as any)).rejects.toThrow("userId"); + }); + }); +}); diff --git a/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts new file mode 100644 index 00000000000..afc3b1c13d4 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts @@ -0,0 +1,96 @@ +import { assertNonNullish } from "@bitwarden/common/auth/utils"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { UserId } from "@bitwarden/common/types/guid"; +// eslint-disable-next-line no-restricted-imports +import { KdfConfigService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { ChangeKdfService } from "../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../master-password/abstractions/master-password.service.abstraction"; + +import { EncryptedMigrator } from "./encrypted-migrator.abstraction"; +import { EncryptedMigration, MigrationRequirement } from "./migrations/encrypted-migration"; +import { MinimumKdfMigration } from "./migrations/minimum-kdf-migration"; + +export class DefaultEncryptedMigrator implements EncryptedMigrator { + private migrations: { name: string; migration: EncryptedMigration }[] = []; + private isRunningMigration = false; + + constructor( + readonly kdfConfigService: KdfConfigService, + readonly changeKdfService: ChangeKdfService, + private readonly logService: LogService, + readonly configService: ConfigService, + readonly masterPasswordService: MasterPasswordServiceAbstraction, + ) { + // Register migrations here + this.migrations.push({ + name: "Minimum PBKDF2 Iteration Count Migration", + migration: new MinimumKdfMigration( + kdfConfigService, + changeKdfService, + logService, + configService, + masterPasswordService, + ), + }); + } + + async runMigrations(userId: UserId, masterPassword: string | null): Promise { + assertNonNullish(userId, "userId"); + + // Ensure that the requirements for running all migrations are met + const needsMigration = await this.needsMigrations(userId); + if (needsMigration === "noMigrationNeeded") { + return; + } else if (needsMigration === "needsMigrationWithMasterPassword" && masterPassword == null) { + return; + } + + try { + // No concurrent migrations allowed, so acquire a service-wide lock + if (this.isRunningMigration) { + return; + } + this.isRunningMigration = true; + + // Run all migrations sequentially in the order they were registered + this.logService.mark("[Encrypted Migrator] Start"); + this.logService.info(`[Encrypted Migrator] Starting migrations for user: ${userId}`); + for (const { name, migration } of this.migrations) { + if ((await migration.needsMigration(userId)) !== "noMigrationNeeded") { + this.logService.info(`[Encrypted Migrator] Running migration: ${name}`); + const start = performance.now(); + await migration.runMigrations(userId, masterPassword); + this.logService.measure(start, "[Encrypted Migrator]", name, "ExecutionTime"); + } + } + this.logService.mark("[Encrypted Migrator] Finish"); + this.logService.info(`[Encrypted Migrator] Completed migrations for user: ${userId}`); + } catch (error) { + this.logService.error( + `[Encrypted Migrator] Error running migrations for user: ${userId}`, + error, + ); + throw error; // Re-throw the error to be handled by the caller + } finally { + this.isRunningMigration = false; + } + } + + async needsMigrations(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + const migrationRequirements = await Promise.all( + this.migrations.map(async ({ migration }) => migration.needsMigration(userId)), + ); + + if (migrationRequirements.includes("needsMigrationWithMasterPassword")) { + return "needsMigrationWithMasterPassword"; + } else if (migrationRequirements.includes("needsMigration")) { + return "needsMigration"; + } else { + return "noMigrationNeeded"; + } + } +} diff --git a/libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts b/libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts new file mode 100644 index 00000000000..09732629ba9 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts @@ -0,0 +1,27 @@ +import { UserId } from "@bitwarden/common/types/guid"; + +import { MigrationRequirement } from "./migrations/encrypted-migration"; + +export abstract class EncryptedMigrator { + /** + * Runs migrations on a decrypted user, with the cryptographic state initialized. + * This only runs the migrations that are needed for the user. + * This needs to be run after the decrypted user key has been set to state. + * + * If the master password is required but not provided, the migrations will not run, and the function will return early. + * If migrations are already running, the migrations will not run again, and the function will return early. + * + * @param userId The ID of the user to run migrations for. + * @param masterPassword The user's current master password. + * @throws If the user does not exist + * @throws If the user is locked or logged out + * @throws If a migration fails + */ + abstract runMigrations(userId: UserId, masterPassword: string | null): Promise; + /** + * Checks if the user needs to run any migrations. + * This is used to determine if the user should be prompted to run migrations. + * @param userId The ID of the user to check migrations for. + */ + abstract needsMigrations(userId: UserId): Promise; +} diff --git a/libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts b/libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts new file mode 100644 index 00000000000..3af60873019 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts @@ -0,0 +1,36 @@ +import { UserId } from "@bitwarden/common/types/guid"; + +/** + * @internal + * IMPORTANT: Please read this when implementing new migrations. + * + * An encrypted migration defines an online migration that mutates the persistent state of the user on the server, or locally. + * It should only be run once per user (or for local migrations, once per device). Migrations get scheduled automatically, + * during actions such as login and unlock, or during sync. + * + * Migrations can require the master-password, which is provided by the user if required. + * Migrations are run as soon as possible non-lazily, and MAY block unlock / login, if they have to run. + * + * Most importantly, implementing a migration should be done such that concurrent migrations may fail, but must never + * leave the user in a broken state. Locally, these are scheduled with an application-global lock. However, no such guarantees + * are made for the server, and other devices may run the migration concurrently. + * + * When adding a migration, it *MUST* be feature-flagged for the initial roll-out. + */ +export interface EncryptedMigration { + /** + * Runs the migration. + * @throws If the migration fails, such as when no network is available. + * @throws If the requirements for migration are not met (e.g. the user is locked) + */ + runMigrations(userId: UserId, masterPassword: string | null): Promise; + /** + * Returns whether the migration needs to be run for the user, and if it does, whether the master password is required. + */ + needsMigration(userId: UserId): Promise; +} + +export type MigrationRequirement = + | "needsMigration" + | "needsMigrationWithMasterPassword" + | "noMigrationNeeded"; diff --git a/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts new file mode 100644 index 00000000000..b61e3f7bc85 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts @@ -0,0 +1,185 @@ +import { mock } from "jest-mock-extended"; + +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { UserId } from "@bitwarden/common/types/guid"; +// eslint-disable-next-line no-restricted-imports +import { + Argon2KdfConfig, + KdfConfigService, + KdfType, + MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE, + PBKDF2KdfConfig, +} from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { ChangeKdfService } from "../../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; + +import { MinimumKdfMigration } from "./minimum-kdf-migration"; + +describe("MinimumKdfMigration", () => { + const mockKdfConfigService = mock(); + const mockChangeKdfService = mock(); + const mockLogService = mock(); + const mockConfigService = mock(); + const mockMasterPasswordService = mock(); + + let sut: MinimumKdfMigration; + + const mockUserId = "00000000-0000-0000-0000-000000000000" as UserId; + const mockMasterPassword = "masterPassword"; + + beforeEach(() => { + jest.clearAllMocks(); + + sut = new MinimumKdfMigration( + mockKdfConfigService, + mockChangeKdfService, + mockLogService, + mockConfigService, + mockMasterPasswordService, + ); + }); + + describe("needsMigration", () => { + it("should return 'noMigrationNeeded' when user does not have a master password`", async () => { + mockMasterPasswordService.userHasMasterPassword.mockResolvedValue(false); + const result = await sut.needsMigration(mockUserId); + expect(result).toBe("noMigrationNeeded"); + }); + + it("should return 'noMigrationNeeded' when user uses argon2id`", async () => { + mockMasterPasswordService.userHasMasterPassword.mockResolvedValue(true); + mockKdfConfigService.getKdfConfig.mockResolvedValue(new Argon2KdfConfig(3, 64, 4)); + const result = await sut.needsMigration(mockUserId); + expect(result).toBe("noMigrationNeeded"); + }); + + it("should return 'noMigrationNeeded' when PBKDF2 iterations are already above minimum", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE + 1000, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'noMigrationNeeded' when PBKDF2 iterations equal minimum", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + mockConfigService.getFeatureFlag.mockResolvedValue(true); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'noMigrationNeeded' when feature flag is disabled", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE - 1000, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + mockConfigService.getFeatureFlag.mockResolvedValue(false); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + expect(mockConfigService.getFeatureFlag).toHaveBeenCalledWith( + FeatureFlag.ForceUpdateKDFSettings, + ); + }); + + it("should return 'needsMigrationWithMasterPassword' when PBKDF2 iterations are below minimum and feature flag is enabled", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE - 1000, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + mockConfigService.getFeatureFlag.mockResolvedValue(true); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("needsMigrationWithMasterPassword"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + expect(mockConfigService.getFeatureFlag).toHaveBeenCalledWith( + FeatureFlag.ForceUpdateKDFSettings, + ); + }); + + it("should throw error when userId is null", async () => { + await expect(sut.needsMigration(null as any)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.needsMigration(undefined as any)).rejects.toThrow("userId"); + }); + }); + + describe("runMigrations", () => { + it("should update KDF parameters with minimum PBKDF2 iterations", async () => { + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockLogService.info).toHaveBeenCalledWith( + `[MinimumKdfMigration] Updating user ${mockUserId} to minimum PBKDF2 iteration count ${MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE}`, + ); + expect(mockChangeKdfService.updateUserKdfParams).toHaveBeenCalledWith( + mockMasterPassword, + expect.any(PBKDF2KdfConfig), + mockUserId, + ); + + // Verify the PBKDF2KdfConfig has the correct iteration count + const kdfConfigArg = (mockChangeKdfService.updateUserKdfParams as jest.Mock).mock.calls[0][1]; + expect(kdfConfigArg.iterations).toBe(MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE); + }); + + it("should throw error when userId is null", async () => { + await expect(sut.runMigrations(null as any, mockMasterPassword)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.runMigrations(undefined as any, mockMasterPassword)).rejects.toThrow( + "userId", + ); + }); + + it("should throw error when masterPassword is null", async () => { + await expect(sut.runMigrations(mockUserId, null as any)).rejects.toThrow("masterPassword"); + }); + + it("should throw error when masterPassword is undefined", async () => { + await expect(sut.runMigrations(mockUserId, undefined as any)).rejects.toThrow( + "masterPassword", + ); + }); + + it("should handle errors from changeKdfService", async () => { + const mockError = new Error("KDF update failed"); + mockChangeKdfService.updateUserKdfParams.mockRejectedValue(mockError); + + await expect(sut.runMigrations(mockUserId, mockMasterPassword)).rejects.toThrow( + "KDF update failed", + ); + + expect(mockLogService.info).toHaveBeenCalledWith( + `[MinimumKdfMigration] Updating user ${mockUserId} to minimum PBKDF2 iteration count ${MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE}`, + ); + expect(mockChangeKdfService.updateUserKdfParams).toHaveBeenCalledWith( + mockMasterPassword, + expect.any(PBKDF2KdfConfig), + mockUserId, + ); + }); + }); +}); diff --git a/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts new file mode 100644 index 00000000000..52b95472826 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts @@ -0,0 +1,69 @@ +import { assertNonNullish } from "@bitwarden/common/auth/utils"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { UserId } from "@bitwarden/common/types/guid"; +// eslint-disable-next-line no-restricted-imports +import { + KdfConfigService, + KdfType, + MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE, + PBKDF2KdfConfig, +} from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { ChangeKdfService } from "../../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; + +import { EncryptedMigration, MigrationRequirement } from "./encrypted-migration"; + +/** + * @internal + * This migrator ensures the user's account has a minimum PBKDF2 iteration count. + * It will update the entire account, logging out old clients if necessary. + */ +export class MinimumKdfMigration implements EncryptedMigration { + constructor( + private readonly kdfConfigService: KdfConfigService, + private readonly changeKdfService: ChangeKdfService, + private readonly logService: LogService, + private readonly configService: ConfigService, + private readonly masterPasswordService: MasterPasswordServiceAbstraction, + ) {} + + async runMigrations(userId: UserId, masterPassword: string | null): Promise { + assertNonNullish(userId, "userId"); + assertNonNullish(masterPassword, "masterPassword"); + + this.logService.info( + `[MinimumKdfMigration] Updating user ${userId} to minimum PBKDF2 iteration count ${MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE}`, + ); + await this.changeKdfService.updateUserKdfParams( + masterPassword!, + new PBKDF2KdfConfig(MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE), + userId, + ); + } + + async needsMigration(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + if (!(await this.masterPasswordService.userHasMasterPassword(userId))) { + return "noMigrationNeeded"; + } + + // Only PBKDF2 users below the minimum iteration count need migration + const kdfConfig = await this.kdfConfigService.getKdfConfig(userId); + if ( + kdfConfig.kdfType !== KdfType.PBKDF2_SHA256 || + kdfConfig.iterations >= MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE + ) { + return "noMigrationNeeded"; + } + + if (!(await this.configService.getFeatureFlag(FeatureFlag.ForceUpdateKDFSettings))) { + return "noMigrationNeeded"; + } + + return "needsMigrationWithMasterPassword"; + } +} diff --git a/libs/common/src/key-management/kdf/change-kdf-service.abstraction.ts b/libs/common/src/key-management/kdf/change-kdf.service.abstraction.ts similarity index 100% rename from libs/common/src/key-management/kdf/change-kdf-service.abstraction.ts rename to libs/common/src/key-management/kdf/change-kdf.service.abstraction.ts diff --git a/libs/common/src/key-management/kdf/change-kdf-service.spec.ts b/libs/common/src/key-management/kdf/change-kdf.service.spec.ts similarity index 100% rename from libs/common/src/key-management/kdf/change-kdf-service.spec.ts rename to libs/common/src/key-management/kdf/change-kdf.service.spec.ts diff --git a/libs/common/src/key-management/kdf/change-kdf-service.ts b/libs/common/src/key-management/kdf/change-kdf.service.ts similarity index 97% rename from libs/common/src/key-management/kdf/change-kdf-service.ts rename to libs/common/src/key-management/kdf/change-kdf.service.ts index 64fbd1fce05..89d97e6704f 100644 --- a/libs/common/src/key-management/kdf/change-kdf-service.ts +++ b/libs/common/src/key-management/kdf/change-kdf.service.ts @@ -14,7 +14,7 @@ import { } from "../master-password/types/master-password.types"; import { ChangeKdfApiService } from "./change-kdf-api.service.abstraction"; -import { ChangeKdfService } from "./change-kdf-service.abstraction"; +import { ChangeKdfService } from "./change-kdf.service.abstraction"; export class DefaultChangeKdfService implements ChangeKdfService { constructor( diff --git a/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts b/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts index f982c2c5ce8..6ca3b1ff376 100644 --- a/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts +++ b/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts @@ -106,6 +106,13 @@ export abstract class MasterPasswordServiceAbstraction { password: string, masterPasswordUnlockData: MasterPasswordUnlockData, ) => Promise; + + /** + * Returns whether the user has a master password set. + * @param userId The user ID. + * @throws If the user ID is missing. + */ + abstract userHasMasterPassword: (userId: UserId) => Promise; } export abstract class InternalMasterPasswordServiceAbstraction extends MasterPasswordServiceAbstraction { diff --git a/libs/common/src/key-management/master-password/services/fake-master-password.service.ts b/libs/common/src/key-management/master-password/services/fake-master-password.service.ts index 5db7f178b18..90fcaddb1a5 100644 --- a/libs/common/src/key-management/master-password/services/fake-master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/fake-master-password.service.ts @@ -33,6 +33,10 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA this.masterKeyHashSubject.next(initialMasterKeyHash); } + userHasMasterPassword(userId: UserId): Promise { + return this.mock.userHasMasterPassword(userId); + } + emailToSalt(email: string): MasterPasswordSalt { return this.mock.emailToSalt(email); } diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index 8012a9230e7..c2947b2263d 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -25,6 +25,7 @@ import { MasterKey, UserKey } from "../../../types/key"; import { KeyGenerationService } from "../../crypto"; import { CryptoFunctionService } from "../../crypto/abstractions/crypto-function.service"; import { EncryptedString, EncString } from "../../crypto/models/enc-string"; +import { USES_KEY_CONNECTOR } from "../../key-connector/services/key-connector.service"; import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction"; import { MasterKeyWrappedUserKey, @@ -85,6 +86,19 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr private accountService: AccountService, ) {} + async userHasMasterPassword(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + // A user has a master-password if they have a master-key encrypted user key *but* are not a key connector user + // Note: We can't use the key connector service as an abstraction here because it causes a run-time dependency injection cycle between KC service and MP service. + const usesKeyConnector = await firstValueFrom( + this.stateProvider.getUser(userId, USES_KEY_CONNECTOR).state$, + ); + const usesMasterKey = await firstValueFrom( + this.stateProvider.getUser(userId, MASTER_KEY_ENCRYPTED_USER_KEY).state$, + ); + return usesMasterKey && !usesKeyConnector; + } + saltForUser$(userId: UserId): Observable { assertNonNullish(userId, "userId"); return this.accountService.accounts$.pipe( @@ -307,6 +321,7 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr masterPasswordUnlockData.kdf.toSdkConfig(), ), ); + return userKey as UserKey; } diff --git a/libs/key-management-ui/src/lock/components/lock.component.spec.ts b/libs/key-management-ui/src/lock/components/lock.component.spec.ts index 69f949fb843..ab0a630b0e6 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.spec.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.spec.ts @@ -22,6 +22,7 @@ import { } from "@bitwarden/common/auth/types/verification"; import { ClientType, DeviceType } from "@bitwarden/common/enums"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; @@ -92,6 +93,7 @@ describe("LockComponent", () => { const mockLockComponentService = mock(); const mockAnonLayoutWrapperDataService = mock(); const mockBroadcasterService = mock(); + const mockEncryptedMigrator = mock(); const mockConfigService = mock(); beforeEach(async () => { @@ -150,6 +152,7 @@ describe("LockComponent", () => { { provide: LockComponentService, useValue: mockLockComponentService }, { provide: AnonLayoutWrapperDataService, useValue: mockAnonLayoutWrapperDataService }, { provide: BroadcasterService, useValue: mockBroadcasterService }, + { provide: EncryptedMigrator, useValue: mockEncryptedMigrator }, { provide: ConfigService, useValue: mockConfigService }, ], }) diff --git a/libs/key-management-ui/src/lock/components/lock.component.ts b/libs/key-management-ui/src/lock/components/lock.component.ts index 7fcb050f34e..1d50737e9ab 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.ts @@ -31,6 +31,7 @@ import { import { ClientType, DeviceType } from "@bitwarden/common/enums"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; @@ -175,6 +176,8 @@ export class LockComponent implements OnInit, OnDestroy { private logoutService: LogoutService, private lockComponentService: LockComponentService, private anonLayoutWrapperDataService: AnonLayoutWrapperDataService, + private encryptedMigrator: EncryptedMigrator, + private configService: ConfigService, // desktop deps private broadcasterService: BroadcasterService, @@ -637,6 +640,16 @@ export class LockComponent implements OnInit, OnDestroy { } await this.biometricStateService.resetUserPromptCancelled(); + + try { + await this.encryptedMigrator.runMigrations( + this.activeAccount.id, + afterUnlockActions.passwordEvaluation?.masterPassword ?? null, + ); + } catch { + // Don't block login success on migration failure + } + this.messagingService.send("unlocked"); if (afterUnlockActions.passwordEvaluation) { diff --git a/libs/key-management/src/index.ts b/libs/key-management/src/index.ts index fb551b92cd6..31ddd2f162b 100644 --- a/libs/key-management/src/index.ts +++ b/libs/key-management/src/index.ts @@ -16,6 +16,7 @@ export { Argon2KdfConfig, KdfConfig, DEFAULT_KDF_CONFIG, + MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE, fromSdkKdfConfig, } from "./models/kdf-config"; export { KdfConfigService } from "./abstractions/kdf-config.service"; diff --git a/libs/key-management/src/models/kdf-config.ts b/libs/key-management/src/models/kdf-config.ts index 3296247710e..ca044845b3c 100644 --- a/libs/key-management/src/models/kdf-config.ts +++ b/libs/key-management/src/models/kdf-config.ts @@ -160,3 +160,4 @@ export function fromSdkKdfConfig(sdkKdf: Kdf): KdfConfig { } export const DEFAULT_KDF_CONFIG = new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue); +export const MINIMUM_PBKDF2_ITERATIONS_FOR_UPGRADE = 600_000; diff --git a/libs/state/src/core/state-definitions.ts b/libs/state/src/core/state-definitions.ts index 1c09b071e99..c2dbccd6fdf 100644 --- a/libs/state/src/core/state-definitions.ts +++ b/libs/state/src/core/state-definitions.ts @@ -52,8 +52,6 @@ export const DEVICE_TRUST_DISK_LOCAL = new StateDefinition("deviceTrust", "disk" web: "disk-local", browser: "disk-backup-local-storage", }); -export const KDF_CONFIG_DISK = new StateDefinition("kdfConfig", "disk"); -export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); export const LOGIN_EMAIL_DISK = new StateDefinition("loginEmail", "disk", { web: "disk-local", }); @@ -62,8 +60,6 @@ export const LOGIN_STRATEGY_MEMORY = new StateDefinition("loginStrategy", "memor export const MASTER_PASSWORD_DISK = new StateDefinition("masterPassword", "disk"); export const MASTER_PASSWORD_MEMORY = new StateDefinition("masterPassword", "memory"); export const MASTER_PASSWORD_UNLOCK_DISK = new StateDefinition("masterPasswordUnlock", "disk"); -export const PIN_DISK = new StateDefinition("pinUnlock", "disk"); -export const PIN_MEMORY = new StateDefinition("pinUnlock", "memory"); export const ROUTER_DISK = new StateDefinition("router", "disk"); export const SSO_DISK = new StateDefinition("ssoLogin", "disk"); export const SSO_DISK_LOCAL = new StateDefinition("ssoLoginLocal", "disk", { web: "disk-local" }); @@ -111,13 +107,10 @@ export const NEW_WEB_LAYOUT_BANNER_DISK = new StateDefinition("newWebLayoutBanne export const APPLICATION_ID_DISK = new StateDefinition("applicationId", "disk", { web: "disk-local", }); -export const BIOMETRIC_SETTINGS_DISK = new StateDefinition("biometricSettings", "disk"); export const CLEAR_EVENT_DISK = new StateDefinition("clearEvent", "disk"); export const CONFIG_DISK = new StateDefinition("config", "disk", { web: "disk-local", }); -export const CRYPTO_DISK = new StateDefinition("crypto", "disk"); -export const CRYPTO_MEMORY = new StateDefinition("crypto", "memory"); export const DESKTOP_SETTINGS_DISK = new StateDefinition("desktopSettings", "disk"); export const ENVIRONMENT_DISK = new StateDefinition("environment", "disk"); export const ENVIRONMENT_MEMORY = new StateDefinition("environment", "memory"); @@ -217,3 +210,14 @@ export const VAULT_BROWSER_INTRO_CAROUSEL = new StateDefinition( "vaultBrowserIntroCarousel", "disk", ); + +// KM + +export const BIOMETRIC_SETTINGS_DISK = new StateDefinition("biometricSettings", "disk"); +export const ENCRYPTED_MIGRATION_DISK = new StateDefinition("encryptedMigration", "disk"); +export const PIN_DISK = new StateDefinition("pinUnlock", "disk"); +export const PIN_MEMORY = new StateDefinition("pinUnlock", "memory"); +export const CRYPTO_DISK = new StateDefinition("crypto", "disk"); +export const CRYPTO_MEMORY = new StateDefinition("crypto", "memory"); +export const KDF_CONFIG_DISK = new StateDefinition("kdfConfig", "disk"); +export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk");