mirror of
https://github.com/bitwarden/browser
synced 2026-02-28 10:33:31 +00:00
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`
This commit is contained in:
@@ -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)) {
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
|
||||
@@ -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<void>;
|
||||
|
||||
/**
|
||||
* @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<void>;
|
||||
|
||||
@@ -148,4 +160,18 @@ export abstract class SetInitialPasswordService {
|
||||
credentials: InitializeJitPasswordCredentials,
|
||||
userId: UserId,
|
||||
): Promise<void>;
|
||||
|
||||
/**
|
||||
* 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<void>;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user