mirror of
https://github.com/bitwarden/browser
synced 2026-02-07 20:24:01 +00:00
fix(policy-enforcement): [PM-21085] Fix Bug with Policy Enforcement - Made code more dry.
This commit is contained in:
@@ -115,11 +115,14 @@ export class ChangePasswordComponent implements OnInit {
|
||||
throw new Error("userId not found");
|
||||
}
|
||||
|
||||
await this.changePasswordService.changePassword(
|
||||
passwordInputResult,
|
||||
this.activeUserId,
|
||||
this.forceSetPasswordReason === ForceSetPasswordReason.AdminForcePasswordReset,
|
||||
);
|
||||
if (this.forceSetPasswordReason === ForceSetPasswordReason.AdminForcePasswordReset) {
|
||||
await this.changePasswordService.changePasswordForAccountRecovery(
|
||||
passwordInputResult,
|
||||
this.activeUserId,
|
||||
);
|
||||
} else {
|
||||
await this.changePasswordService.changePassword(passwordInputResult, this.activeUserId);
|
||||
}
|
||||
|
||||
this.toastService.showToast({
|
||||
variant: "success",
|
||||
|
||||
@@ -30,12 +30,12 @@ export abstract class ChangePasswordService {
|
||||
*
|
||||
* @param passwordInputResult credentials object received from the `InputPasswordComponent`
|
||||
* @param userId the `userId`
|
||||
* @param recoverAccount this is a boolean to know if we need to hit the update-temp-password endpoint instead of the password endpoint
|
||||
* @throws if the `userId`, `currentMasterKey`, or `currentServerMasterKeyHash` is not found
|
||||
*/
|
||||
abstract changePassword(
|
||||
abstract changePassword(passwordInputResult: PasswordInputResult, userId: UserId): Promise<void>;
|
||||
|
||||
abstract changePasswordForAccountRecovery(
|
||||
passwordInputResult: PasswordInputResult,
|
||||
userId: UserId,
|
||||
recoverAccount?: boolean,
|
||||
): Promise<void>;
|
||||
}
|
||||
|
||||
@@ -97,7 +97,7 @@ describe("DefaultChangePasswordService", () => {
|
||||
|
||||
it("should throw if a userId was not found", async () => {
|
||||
// Arrange
|
||||
const userId: null = null;
|
||||
const userId: undefined = undefined;
|
||||
|
||||
// Act
|
||||
const testFn = sut.changePassword(passwordInputResult, userId);
|
||||
@@ -109,7 +109,7 @@ describe("DefaultChangePasswordService", () => {
|
||||
it("should throw if a currentMasterKey was not found", async () => {
|
||||
// Arrange
|
||||
const incorrectPasswordInputResult = { ...passwordInputResult };
|
||||
incorrectPasswordInputResult.currentMasterKey = null;
|
||||
incorrectPasswordInputResult.currentMasterKey = undefined;
|
||||
|
||||
// Act
|
||||
const testFn = sut.changePassword(incorrectPasswordInputResult, userId);
|
||||
@@ -123,7 +123,7 @@ describe("DefaultChangePasswordService", () => {
|
||||
it("should throw if a currentServerMasterKeyHash was not found", async () => {
|
||||
// Arrange
|
||||
const incorrectPasswordInputResult = { ...passwordInputResult };
|
||||
incorrectPasswordInputResult.currentServerMasterKeyHash = null;
|
||||
incorrectPasswordInputResult.currentServerMasterKeyHash = undefined;
|
||||
|
||||
// Act
|
||||
const testFn = sut.changePassword(incorrectPasswordInputResult, userId);
|
||||
@@ -174,4 +174,43 @@ describe("DefaultChangePasswordService", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("changePasswordForAccountRecovery()", () => {
|
||||
it("should call the putUpdateTempPassword() API method with the correct UpdateTempPasswordRequest credentials", async () => {
|
||||
// Act
|
||||
await sut.changePasswordForAccountRecovery(passwordInputResult, userId);
|
||||
|
||||
// Assert
|
||||
expect(masterPasswordApiService.putUpdateTempPassword).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
newMasterPasswordHash: passwordInputResult.newServerMasterKeyHash,
|
||||
masterPasswordHint: passwordInputResult.newPasswordHint,
|
||||
key: newMasterKeyEncryptedUserKey[1].encryptedString,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("should throw an error if user key decryption fails", async () => {
|
||||
// Arrange
|
||||
masterPasswordService.decryptUserKeyWithMasterKey.mockResolvedValue(null);
|
||||
|
||||
// Act
|
||||
const testFn = sut.changePasswordForAccountRecovery(passwordInputResult, userId);
|
||||
|
||||
// Assert
|
||||
await expect(testFn).rejects.toThrow("Could not decrypt user key");
|
||||
});
|
||||
|
||||
it("should throw an error if putUpdateTempPassword() fails", async () => {
|
||||
// Arrange
|
||||
masterPasswordApiService.putUpdateTempPassword.mockRejectedValueOnce(new Error("error"));
|
||||
|
||||
// Act
|
||||
const testFn = sut.changePasswordForAccountRecovery(passwordInputResult, userId);
|
||||
|
||||
// Assert
|
||||
await expect(testFn).rejects.toThrow("Could not change password");
|
||||
expect(masterPasswordApiService.putUpdateTempPassword).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
import { PasswordInputResult, ChangePasswordService } from "@bitwarden/auth/angular";
|
||||
import { ChangePasswordService, PasswordInputResult } from "@bitwarden/auth/angular";
|
||||
import { Account } from "@bitwarden/common/auth/abstractions/account.service";
|
||||
import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction";
|
||||
import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request";
|
||||
import { UpdateTempPasswordRequest } from "@bitwarden/common/auth/models/request/update-temp-password.request";
|
||||
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
|
||||
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { UserKey } from "@bitwarden/common/types/key";
|
||||
import { KeyService } from "@bitwarden/key-management";
|
||||
|
||||
export class DefaultChangePasswordService implements ChangePasswordService {
|
||||
@@ -23,11 +25,10 @@ export class DefaultChangePasswordService implements ChangePasswordService {
|
||||
throw new Error("rotateUserKeyMasterPasswordAndEncryptedData() is only implemented in Web");
|
||||
}
|
||||
|
||||
async changePassword(
|
||||
private async preparePasswordChange(
|
||||
passwordInputResult: PasswordInputResult,
|
||||
userId: UserId,
|
||||
recoverAccount: boolean = false,
|
||||
) {
|
||||
): Promise<[UserKey, EncString]> {
|
||||
if (!userId) {
|
||||
throw new Error("userId not found");
|
||||
}
|
||||
@@ -44,34 +45,46 @@ export class DefaultChangePasswordService implements ChangePasswordService {
|
||||
throw new Error("Could not decrypt user key");
|
||||
}
|
||||
|
||||
const newMasterKeyEncryptedUserKey = await this.keyService.encryptUserKeyWithMasterKey(
|
||||
return await this.keyService.encryptUserKeyWithMasterKey(
|
||||
passwordInputResult.newMasterKey,
|
||||
decryptedUserKey,
|
||||
);
|
||||
}
|
||||
|
||||
if (recoverAccount) {
|
||||
const request = new UpdateTempPasswordRequest();
|
||||
request.newMasterPasswordHash = passwordInputResult.newServerMasterKeyHash;
|
||||
request.masterPasswordHint = passwordInputResult.newPasswordHint;
|
||||
request.key = newMasterKeyEncryptedUserKey[1].encryptedString as string;
|
||||
async changePassword(passwordInputResult: PasswordInputResult, userId: UserId) {
|
||||
const newMasterKeyEncryptedUserKey = await this.preparePasswordChange(
|
||||
passwordInputResult,
|
||||
userId,
|
||||
);
|
||||
|
||||
try {
|
||||
await this.masterPasswordApiService.putUpdateTempPassword(request);
|
||||
} catch {
|
||||
throw new Error("Could not change password");
|
||||
}
|
||||
} else {
|
||||
const request = new PasswordRequest();
|
||||
request.masterPasswordHash = passwordInputResult.currentServerMasterKeyHash;
|
||||
request.newMasterPasswordHash = passwordInputResult.newServerMasterKeyHash;
|
||||
request.masterPasswordHint = passwordInputResult.newPasswordHint;
|
||||
request.key = newMasterKeyEncryptedUserKey[1].encryptedString as string;
|
||||
const request = new PasswordRequest();
|
||||
request.masterPasswordHash = passwordInputResult.currentServerMasterKeyHash ?? "";
|
||||
request.newMasterPasswordHash = passwordInputResult.newServerMasterKeyHash;
|
||||
request.masterPasswordHint = passwordInputResult.newPasswordHint;
|
||||
request.key = newMasterKeyEncryptedUserKey[1].encryptedString as string;
|
||||
|
||||
try {
|
||||
await this.masterPasswordApiService.postPassword(request);
|
||||
} catch {
|
||||
throw new Error("Could not change password");
|
||||
}
|
||||
try {
|
||||
await this.masterPasswordApiService.postPassword(request);
|
||||
} catch {
|
||||
throw new Error("Could not change password");
|
||||
}
|
||||
}
|
||||
|
||||
async changePasswordForAccountRecovery(passwordInputResult: PasswordInputResult, userId: UserId) {
|
||||
const newMasterKeyEncryptedUserKey = await this.preparePasswordChange(
|
||||
passwordInputResult,
|
||||
userId,
|
||||
);
|
||||
|
||||
const request = new UpdateTempPasswordRequest();
|
||||
request.newMasterPasswordHash = passwordInputResult.newServerMasterKeyHash;
|
||||
request.masterPasswordHint = passwordInputResult.newPasswordHint;
|
||||
request.key = newMasterKeyEncryptedUserKey[1].encryptedString as string;
|
||||
|
||||
try {
|
||||
await this.masterPasswordApiService.putUpdateTempPassword(request);
|
||||
} catch {
|
||||
throw new Error("Could not change password");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user