From 4c2aec162d0d79b3fb2d53dca7c64cf1ac0a6cad Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Thu, 26 Feb 2026 09:51:19 -0800 Subject: [PATCH] refactor(input-password-flows): [Auth/PM-27086] Use new KM Data Types in InputPasswordComponent flows - TDE Offboarding (#18204) Updates the `setInitialPasswordTdeOffboarding` path to use new KM data types: - `MasterPasswordAuthenticationData` - `MasterPasswordUnlockData` This allows us to move away from the deprecated `makeMasterKey()` method (which takes email as salt) as we seek to eventually separate the email from the salt. Behind feature flag: `pm-27086-update-authentication-apis-for-input-password` --- ...initial-password.service.implementation.ts | 55 +++++- ...fault-set-initial-password.service.spec.ts | 169 +++++++++++++++++- .../set-initial-password.component.ts | 51 +++++- ...et-initial-password.service.abstraction.ts | 32 +++- ...update-tde-offboarding-password.request.ts | 19 ++ 5 files changed, 310 insertions(+), 16 deletions(-) diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts index 317030c25aa..c6eb9ca7367 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts @@ -45,8 +45,9 @@ import { InitializeJitPasswordCredentials, SetInitialPasswordCredentials, SetInitialPasswordService, - SetInitialPasswordTdeOffboardingCredentials, SetInitialPasswordUserType, + SetInitialPasswordTdeOffboardingCredentialsOld, + SetInitialPasswordTdeOffboardingCredentials, SetInitialPasswordTdeUserWithPermissionCredentials, } from "./set-initial-password.service.abstraction"; @@ -222,6 +223,58 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi async setInitialPasswordTdeOffboarding( credentials: SetInitialPasswordTdeOffboardingCredentials, userId: UserId, + ) { + const ctx = "Could not set initial password."; + assertTruthy(credentials.newPassword, "newPassword", ctx); + assertTruthy(credentials.salt, "salt", ctx); + assertNonNullish(credentials.kdfConfig, "kdfConfig", ctx); + assertNonNullish(credentials.newPasswordHint, "newPasswordHint", ctx); + + if (userId == null) { + throw new Error("userId not found. Could not set password."); + } + + const { newPassword, salt, kdfConfig, newPasswordHint } = credentials; + + const userKey = await firstValueFrom(this.keyService.userKey$(userId)); + if (userKey == null) { + throw new Error("userKey not found. Could not set password."); + } + + const authenticationData: MasterPasswordAuthenticationData = + await this.masterPasswordService.makeMasterPasswordAuthenticationData( + newPassword, + kdfConfig, + salt, + ); + + const unlockData: MasterPasswordUnlockData = + await this.masterPasswordService.makeMasterPasswordUnlockData( + newPassword, + kdfConfig, + salt, + userKey, + ); + + const request = UpdateTdeOffboardingPasswordRequest.newConstructorWithHint( + authenticationData, + unlockData, + newPasswordHint, + ); + + await this.masterPasswordApiService.putUpdateTdeOffboardingPassword(request); + + // TODO: investigate removing this call to clear forceSetPasswordReason in https://bitwarden.atlassian.net/browse/PM-32660 + // Clear force set password reason to allow navigation back to vault. + await this.masterPasswordService.setForceSetPasswordReason(ForceSetPasswordReason.None, userId); + } + + /** + * @deprecated To be removed in PM-28143 + */ + async setInitialPasswordTdeOffboardingOld( + credentials: SetInitialPasswordTdeOffboardingCredentialsOld, + userId: UserId, ) { const { newMasterKey, newServerMasterKeyHash, newPasswordHint } = credentials; for (const [key, value] of Object.entries(credentials)) { diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts index d68bf2c7d01..5543a631225 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts @@ -65,6 +65,7 @@ import { SetInitialPasswordCredentials, SetInitialPasswordService, SetInitialPasswordTdeOffboardingCredentials, + SetInitialPasswordTdeOffboardingCredentialsOld, SetInitialPasswordTdeUserWithPermissionCredentials, SetInitialPasswordUserType, } from "./set-initial-password.service.abstraction"; @@ -757,10 +758,160 @@ describe("DefaultSetInitialPasswordService", () => { }); }); - describe("setInitialPasswordTdeOffboarding(...)", () => { - // Mock function parameters + describe("setInitialPasswordTdeOffboarding()", () => { + // Mock method parameters let credentials: SetInitialPasswordTdeOffboardingCredentials; + // Mock method data + let userKey: UserKey; + let authenticationData: MasterPasswordAuthenticationData; + let unlockData: MasterPasswordUnlockData; + let request: UpdateTdeOffboardingPasswordRequest; + + beforeEach(() => { + credentials = { + newPassword: "new-Password", + salt: "salt" as MasterPasswordSalt, + kdfConfig: DEFAULT_KDF_CONFIG, + newPasswordHint: "newPasswordHint", + }; + + userKey = makeSymmetricCryptoKey(64) as UserKey; + + authenticationData = { + salt: credentials.salt, + kdf: credentials.kdfConfig, + masterPasswordAuthenticationHash: + "masterPasswordAuthenticationHash" as MasterPasswordAuthenticationHash, + }; + + unlockData = { + salt: credentials.salt, + kdf: credentials.kdfConfig, + masterKeyWrappedUserKey: "masterKeyWrappedUserKey" as MasterKeyWrappedUserKey, + } as MasterPasswordUnlockData; + + request = UpdateTdeOffboardingPasswordRequest.newConstructorWithHint( + authenticationData, + unlockData, + credentials.newPasswordHint, + ); + + keyService.userKey$.mockReturnValue(of(userKey)); + masterPasswordService.makeMasterPasswordAuthenticationData.mockResolvedValue( + authenticationData, + ); + masterPasswordService.makeMasterPasswordUnlockData.mockResolvedValue(unlockData); + }); + + describe("general error handling", () => { + ["newPassword", "salt"].forEach((key) => { + it(`should throw if ${key} is an empty string (falsy) on the SetInitialPasswordTdeOffboardingCredentials object`, async () => { + // Arrange + const invalidCredentials: SetInitialPasswordTdeOffboardingCredentials = { + ...credentials, + [key]: "", + }; + + // Act + const promise = sut.setInitialPasswordTdeOffboarding(invalidCredentials, userId); + + // Assert + await expect(promise).rejects.toThrow(`${key} is falsy. Could not set initial password.`); + }); + }); + + ["kdfConfig", "newPasswordHint"].forEach((key) => { + it(`should throw if ${key} is null/undefined on the SetInitialPasswordTdeOffboardingCredentials object`, async () => { + // Arrange + const invalidCredentials: SetInitialPasswordTdeOffboardingCredentials = { + ...credentials, + [key]: null, + }; + + // Act + const promise = sut.setInitialPasswordTdeOffboarding(invalidCredentials, userId); + + // Assert + await expect(promise).rejects.toThrow( + `${key} is null or undefined. Could not set initial password.`, + ); + }); + }); + + it(`should throw if the userId was not passed in`, async () => { + // Arrange + userId = null; + + // Act + const promise = sut.setInitialPasswordTdeOffboarding(credentials, userId); + + // Assert + await expect(promise).rejects.toThrow("userId not found. Could not set password."); + }); + + it(`should throw if the userKey was not found`, async () => { + // Arrange + keyService.userKey$.mockReturnValue(of(null)); + + // Act + const promise = sut.setInitialPasswordTdeOffboarding(credentials, userId); + + // Assert + await expect(promise).rejects.toThrow("userKey not found. Could not set password."); + }); + }); + + it("should call makeMasterPasswordAuthenticationData and makeMasterPasswordUnlockData with the correct parameters", async () => { + // Act + await sut.setInitialPasswordTdeOffboarding(credentials, userId); + + // Assert + expect(masterPasswordService.makeMasterPasswordAuthenticationData).toHaveBeenCalledWith( + credentials.newPassword, + credentials.kdfConfig, + credentials.salt, + ); + + expect(masterPasswordService.makeMasterPasswordUnlockData).toHaveBeenCalledWith( + credentials.newPassword, + credentials.kdfConfig, + credentials.salt, + userKey, + ); + }); + + it("should call the API method to set a master password", async () => { + // Act + await sut.setInitialPasswordTdeOffboarding(credentials, userId); + + // Assert + expect(masterPasswordApiService.putUpdateTdeOffboardingPassword).toHaveBeenCalledTimes(1); + expect(masterPasswordApiService.putUpdateTdeOffboardingPassword).toHaveBeenCalledWith( + request, + ); + }); + + it("should set the ForceSetPasswordReason to None", async () => { + // Act + await sut.setInitialPasswordTdeOffboarding(credentials, userId); + + // Assert + expect(masterPasswordService.setForceSetPasswordReason).toHaveBeenCalledWith( + ForceSetPasswordReason.None, + userId, + ); + }); + }); + + /** + * @deprecated To be removed in PM-28143. When you remove this, check also if there are any imports/properties + * in the test setup above that are now un-used and can also be removed. + */ + describe("setInitialPasswordTdeOffboardingOld(...)", () => { + // Mock function parameters + let credentials: SetInitialPasswordTdeOffboardingCredentialsOld; + beforeEach(() => { // Mock function parameters credentials = { @@ -785,7 +936,7 @@ describe("DefaultSetInitialPasswordService", () => { request.masterPasswordHint = credentials.newPasswordHint; // Act - await sut.setInitialPasswordTdeOffboarding(credentials, userId); + await sut.setInitialPasswordTdeOffboardingOld(credentials, userId); // Assert expect(masterPasswordApiService.putUpdateTdeOffboardingPassword).toHaveBeenCalledTimes(1); @@ -800,7 +951,7 @@ describe("DefaultSetInitialPasswordService", () => { setupTdeOffboardingMocks(); // Act - await sut.setInitialPasswordTdeOffboarding(credentials, userId); + await sut.setInitialPasswordTdeOffboardingOld(credentials, userId); // Assert expect(masterPasswordApiService.putUpdateTdeOffboardingPassword).toHaveBeenCalledTimes(1); @@ -815,13 +966,13 @@ describe("DefaultSetInitialPasswordService", () => { ["newMasterKey", "newServerMasterKeyHash", "newPasswordHint"].forEach((key) => { it(`should throw if ${key} is not provided on the SetInitialPasswordTdeOffboardingCredentials object`, async () => { // Arrange - const invalidCredentials: SetInitialPasswordTdeOffboardingCredentials = { + const invalidCredentials: SetInitialPasswordTdeOffboardingCredentialsOld = { ...credentials, [key]: null, }; // Act - const promise = sut.setInitialPasswordTdeOffboarding(invalidCredentials, userId); + const promise = sut.setInitialPasswordTdeOffboardingOld(invalidCredentials, userId); // Assert await expect(promise).rejects.toThrow(`${key} not found. Could not set password.`); @@ -833,7 +984,7 @@ describe("DefaultSetInitialPasswordService", () => { userId = null; // Act - const promise = sut.setInitialPasswordTdeOffboarding(credentials, userId); + const promise = sut.setInitialPasswordTdeOffboardingOld(credentials, userId); // Assert await expect(promise).rejects.toThrow("userId not found. Could not set password."); @@ -844,7 +995,7 @@ describe("DefaultSetInitialPasswordService", () => { keyService.userKey$.mockReturnValue(of(null)); // Act - const promise = sut.setInitialPasswordTdeOffboarding(credentials, userId); + const promise = sut.setInitialPasswordTdeOffboardingOld(credentials, userId); // Assert await expect(promise).rejects.toThrow("userKey not found. Could not set password."); @@ -857,7 +1008,7 @@ describe("DefaultSetInitialPasswordService", () => { setupTdeOffboardingMocks(); // Act - const promise = sut.setInitialPasswordTdeOffboarding(credentials, userId); + const promise = sut.setInitialPasswordTdeOffboardingOld(credentials, userId); // Assert await expect(promise).rejects.toThrow( diff --git a/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.component.ts b/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.component.ts index 1680bf57720..7a2279f305a 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.component.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.component.ts @@ -48,6 +48,7 @@ import { SetInitialPasswordCredentials, SetInitialPasswordService, SetInitialPasswordTdeOffboardingCredentials, + SetInitialPasswordTdeOffboardingCredentialsOld, SetInitialPasswordTdeUserWithPermissionCredentials, SetInitialPasswordUserType, } from "./set-initial-password.service.abstraction"; @@ -201,7 +202,13 @@ export class SetInitialPasswordComponent implements OnInit { break; case SetInitialPasswordUserType.OFFBOARDED_TDE_ORG_USER: - await this.setInitialPasswordTdeOffboarding(passwordInputResult); + if (passwordInputResult.newApisWithInputPasswordFlagEnabled) { + await this.setInitialPasswordTdeOffboarding(passwordInputResult); + return; + } + + await this.setInitialPasswordTdeOffboardingOld(passwordInputResult); + break; default: this.logService.error( @@ -438,6 +445,44 @@ export class SetInitialPasswordComponent implements OnInit { } private async setInitialPasswordTdeOffboarding(passwordInputResult: PasswordInputResult) { + const ctx = "Could not set initial password."; + assertTruthy(passwordInputResult.newPassword, "newPassword", ctx); + assertTruthy(passwordInputResult.salt, "salt", ctx); + assertNonNullish(passwordInputResult.kdfConfig, "kdfConfig", ctx); + assertNonNullish(passwordInputResult.newPasswordHint, "newPasswordHint", ctx); // can have an empty string as a valid value, so check non-nullish + assertTruthy(this.userId, "userId", ctx); + + try { + const credentials: SetInitialPasswordTdeOffboardingCredentials = { + newPassword: passwordInputResult.newPassword, + salt: passwordInputResult.salt, + kdfConfig: passwordInputResult.kdfConfig, + newPasswordHint: passwordInputResult.newPasswordHint, + }; + + await this.setInitialPasswordService.setInitialPasswordTdeOffboarding( + credentials, + this.userId, + ); + + this.showSuccessToastByUserType(); + + // TODO: investigate refactoring logout and follow-up routing in https://bitwarden.atlassian.net/browse/PM-32660 + await this.logoutService.logout(this.userId); + // navigate to root so redirect guard can properly route next active user or null user to correct page + await this.router.navigate(["/"]); + } catch (e) { + this.logService.error("Error setting initial password during TDE offboarding", e); + this.validationService.showError(e); + } finally { + this.submitting = false; + } + } + + /** + * @deprecated To be removed in PM-28143 + */ + private async setInitialPasswordTdeOffboardingOld(passwordInputResult: PasswordInputResult) { const ctx = "Could not set initial password."; assertTruthy(passwordInputResult.newMasterKey, "newMasterKey", ctx); assertTruthy(passwordInputResult.newServerMasterKeyHash, "newServerMasterKeyHash", ctx); @@ -445,13 +490,13 @@ export class SetInitialPasswordComponent implements OnInit { assertNonNullish(passwordInputResult.newPasswordHint, "newPasswordHint", ctx); // can have an empty string as a valid value, so check non-nullish try { - const credentials: SetInitialPasswordTdeOffboardingCredentials = { + const credentials: SetInitialPasswordTdeOffboardingCredentialsOld = { newMasterKey: passwordInputResult.newMasterKey, newServerMasterKeyHash: passwordInputResult.newServerMasterKeyHash, newPasswordHint: passwordInputResult.newPasswordHint, }; - await this.setInitialPasswordService.setInitialPasswordTdeOffboarding( + await this.setInitialPasswordService.setInitialPasswordTdeOffboardingOld( credentials, this.userId, ); diff --git a/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.service.abstraction.ts b/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.service.abstraction.ts index 5a68b787e28..8b1de87043d 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.service.abstraction.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/set-initial-password.service.abstraction.ts @@ -65,12 +65,22 @@ export interface SetInitialPasswordTdeUserWithPermissionCredentials { resetPasswordAutoEnroll: boolean; } -export interface SetInitialPasswordTdeOffboardingCredentials { +/** + * @deprecated To be removed in PM-28143 + */ +export interface SetInitialPasswordTdeOffboardingCredentialsOld { newMasterKey: MasterKey; newServerMasterKeyHash: string; newPasswordHint: string; } +export interface SetInitialPasswordTdeOffboardingCredentials { + newPassword: string; + salt: MasterPasswordSalt; + kdfConfig: KdfConfig; + newPasswordHint: string; +} + /** * Credentials required to initialize a just-in-time (JIT) provisioned user with a master password. */ @@ -127,6 +137,8 @@ export abstract class SetInitialPasswordService { ) => Promise; /** + * @deprecated To be removed in PM-28143 + * * Sets an initial password for a user who logs in after their org offboarded from * trusted device encryption and is now a master-password-encryption org: * - {@link SetInitialPasswordUserType.OFFBOARDED_TDE_ORG_USER} @@ -134,8 +146,8 @@ export abstract class SetInitialPasswordService { * @param passwordInputResult credentials object received from the `InputPasswordComponent` * @param userId the account `userId` */ - abstract setInitialPasswordTdeOffboarding: ( - credentials: SetInitialPasswordTdeOffboardingCredentials, + abstract setInitialPasswordTdeOffboardingOld: ( + credentials: SetInitialPasswordTdeOffboardingCredentialsOld, userId: UserId, ) => Promise; @@ -148,4 +160,18 @@ export abstract class SetInitialPasswordService { credentials: InitializeJitPasswordCredentials, userId: UserId, ): Promise; + + /** + * Sets an initial password for a user who logs in after their org offboarded from + * trusted device encryption and is now a master-password-encryption org: + * - {@link SetInitialPasswordUserType.OFFBOARDED_TDE_ORG_USER} + * + * @param credentials An object of the credentials needed to set the initial password + * @param userId the account `userId` + * @throws if `userId`, `userKey`, or necessary credentials are not found + */ + abstract setInitialPasswordTdeOffboarding: ( + credentials: SetInitialPasswordTdeOffboardingCredentials, + userId: UserId, + ) => Promise; } diff --git a/libs/common/src/auth/models/request/update-tde-offboarding-password.request.ts b/libs/common/src/auth/models/request/update-tde-offboarding-password.request.ts index 8a107aa6c32..77ff8253c88 100644 --- a/libs/common/src/auth/models/request/update-tde-offboarding-password.request.ts +++ b/libs/common/src/auth/models/request/update-tde-offboarding-password.request.ts @@ -3,7 +3,26 @@ // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { OrganizationUserResetPasswordRequest } from "@bitwarden/admin-console/common"; +import { + MasterPasswordAuthenticationData, + MasterPasswordUnlockData, +} from "@bitwarden/common/key-management/master-password/types/master-password.types"; export class UpdateTdeOffboardingPasswordRequest extends OrganizationUserResetPasswordRequest { masterPasswordHint: string; + + // This will eventually be changed to be an actual constructor, once all callers are updated. + // The body of this request will be changed to carry the authentication data and unlock data. + // https://bitwarden.atlassian.net/browse/PM-23234 + static newConstructorWithHint( + authenticationData: MasterPasswordAuthenticationData, + unlockData: MasterPasswordUnlockData, + masterPasswordHint: string, + ): UpdateTdeOffboardingPasswordRequest { + const request = new UpdateTdeOffboardingPasswordRequest(); + request.newMasterPasswordHash = authenticationData.masterPasswordAuthenticationHash; + request.key = unlockData.masterKeyWrappedUserKey; + request.masterPasswordHint = masterPasswordHint; + return request; + } }