mirror of
https://github.com/bitwarden/clients.git
synced 2025-12-22 13:17:30 +00:00
fix(set-initial-password) [PM-28494]: Newly created master password not accepted on unlock until after re-login on browser extension (#17930)
* fix(set-initial-password-service) [PM-28494]: Update MP data and decryption property sets to accommodate legacy and new paths for service. * fix(set-initial-password-component) [PM-28494]: Add salt and mp data to credentials object. * refactor(set-initial-password-service) [PM-28494]: Additional comments. * test(set-initial-password-service) [PM-28494]: Update tests for added credential members.
This commit is contained in:
@@ -19,6 +19,7 @@ import { AccountCryptographicStateService } from "@bitwarden/common/key-manageme
|
||||
import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service";
|
||||
import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string";
|
||||
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
|
||||
import { MasterPasswordSalt } from "@bitwarden/common/key-management/master-password/types/master-password.types";
|
||||
import { KeysRequest } from "@bitwarden/common/models/request/keys.request";
|
||||
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
@@ -62,6 +63,8 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
|
||||
orgSsoIdentifier,
|
||||
orgId,
|
||||
resetPasswordAutoEnroll,
|
||||
newPassword,
|
||||
salt,
|
||||
} = credentials;
|
||||
|
||||
for (const [key, value] of Object.entries(credentials)) {
|
||||
@@ -155,6 +158,20 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
|
||||
userId,
|
||||
);
|
||||
|
||||
// Set master password unlock data for unlock path pointed to with
|
||||
// MasterPasswordUnlockData feature development
|
||||
// (requires: password, salt, kdf, userKey).
|
||||
// As migration to this strategy continues, both unlock paths need supported.
|
||||
// Several invocations in this file become redundant and can be removed once
|
||||
// the feature is enshrined/unwound. These are marked with [PM-23246] below.
|
||||
await this.setMasterPasswordUnlockData(
|
||||
newPassword,
|
||||
salt,
|
||||
kdfConfig,
|
||||
masterKeyEncryptedUserKey[0],
|
||||
userId,
|
||||
);
|
||||
|
||||
/**
|
||||
* Set the private key only for new JIT provisioned users in MP encryption orgs.
|
||||
* (Existing TDE users will have their private key set on sync or on login.)
|
||||
@@ -174,6 +191,7 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
|
||||
);
|
||||
}
|
||||
|
||||
// [PM-23246] "Legacy" master key setting path - to be removed once unlock path migration is complete
|
||||
await this.masterPasswordService.setMasterKeyHash(newLocalMasterKeyHash, userId);
|
||||
|
||||
if (resetPasswordAutoEnroll) {
|
||||
@@ -216,10 +234,40 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi
|
||||
userDecryptionOpts,
|
||||
);
|
||||
await this.kdfConfigService.setKdfConfig(userId, kdfConfig);
|
||||
// [PM-23246] "Legacy" master key setting path - to be removed once unlock path migration is complete
|
||||
await this.masterPasswordService.setMasterKey(masterKey, userId);
|
||||
// [PM-23246] "Legacy" master key setting path - to be removed once unlock path migration is complete
|
||||
await this.masterPasswordService.setMasterKeyEncryptedUserKey(
|
||||
masterKeyEncryptedUserKey[1],
|
||||
userId,
|
||||
);
|
||||
await this.keyService.setUserKey(masterKeyEncryptedUserKey[0], userId);
|
||||
}
|
||||
|
||||
/**
|
||||
* As part of [PM-28494], adding this setting path to accommodate the changes that are
|
||||
* emerging with pm-23246-unlock-with-master-password-unlock-data.
|
||||
* Without this, immediately locking/unlocking the vault with the new password _may_ still fail
|
||||
* if sync has not completed. Sync will eventually set this data, but we want to ensure it's
|
||||
* set right away here to prevent a race condition UX issue that prevents immediate unlock.
|
||||
*/
|
||||
private async setMasterPasswordUnlockData(
|
||||
password: string,
|
||||
salt: MasterPasswordSalt,
|
||||
kdfConfig: KdfConfig,
|
||||
userKey: UserKey,
|
||||
userId: UserId,
|
||||
): Promise<void> {
|
||||
const masterPasswordUnlockData = await this.masterPasswordService.makeMasterPasswordUnlockData(
|
||||
password,
|
||||
kdfConfig,
|
||||
salt,
|
||||
userKey,
|
||||
);
|
||||
|
||||
await this.masterPasswordService.setMasterPasswordUnlockData(masterPasswordUnlockData, userId);
|
||||
}
|
||||
|
||||
private async handleResetPasswordAutoEnroll(
|
||||
masterKeyHash: string,
|
||||
orgId: string,
|
||||
|
||||
@@ -134,6 +134,8 @@ describe("DefaultSetInitialPasswordService", () => {
|
||||
orgSsoIdentifier: "orgSsoIdentifier",
|
||||
orgId: "orgId",
|
||||
resetPasswordAutoEnroll: false,
|
||||
newPassword: "Test@Password123!",
|
||||
salt: "user@example.com" as any,
|
||||
};
|
||||
userType = SetInitialPasswordUserType.JIT_PROVISIONED_MP_ORG_USER;
|
||||
|
||||
@@ -226,6 +228,8 @@ describe("DefaultSetInitialPasswordService", () => {
|
||||
"orgSsoIdentifier",
|
||||
"orgId",
|
||||
"resetPasswordAutoEnroll",
|
||||
"newPassword",
|
||||
"salt",
|
||||
].forEach((key) => {
|
||||
it(`should throw if ${key} is not provided on the SetInitialPasswordCredentials object`, async () => {
|
||||
// Arrange
|
||||
@@ -357,6 +361,10 @@ describe("DefaultSetInitialPasswordService", () => {
|
||||
ForceSetPasswordReason.None,
|
||||
userId,
|
||||
);
|
||||
expect(masterPasswordService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
|
||||
masterKeyEncryptedUserKey[1],
|
||||
userId,
|
||||
);
|
||||
});
|
||||
|
||||
it("should update account decryption properties", async () => {
|
||||
@@ -417,6 +425,36 @@ describe("DefaultSetInitialPasswordService", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("should create and set master password unlock data to prevent race condition with sync", async () => {
|
||||
// Arrange
|
||||
setupMocks();
|
||||
|
||||
const mockUnlockData = {
|
||||
salt: credentials.salt,
|
||||
kdf: credentials.kdfConfig,
|
||||
masterKeyWrappedUserKey: "wrapped_key_string",
|
||||
};
|
||||
|
||||
masterPasswordService.makeMasterPasswordUnlockData.mockResolvedValue(
|
||||
mockUnlockData as any,
|
||||
);
|
||||
|
||||
// Act
|
||||
await sut.setInitialPassword(credentials, userType, userId);
|
||||
|
||||
// Assert
|
||||
expect(masterPasswordService.makeMasterPasswordUnlockData).toHaveBeenCalledWith(
|
||||
credentials.newPassword,
|
||||
credentials.kdfConfig,
|
||||
credentials.salt,
|
||||
masterKeyEncryptedUserKey[0],
|
||||
);
|
||||
expect(masterPasswordService.setMasterPasswordUnlockData).toHaveBeenCalledWith(
|
||||
mockUnlockData,
|
||||
userId,
|
||||
);
|
||||
});
|
||||
|
||||
describe("given resetPasswordAutoEnroll is true", () => {
|
||||
it(`should handle reset password (account recovery) auto enroll`, async () => {
|
||||
// Arrange
|
||||
@@ -586,6 +624,10 @@ describe("DefaultSetInitialPasswordService", () => {
|
||||
credentials.newMasterKey,
|
||||
userId,
|
||||
);
|
||||
expect(masterPasswordService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
|
||||
masterKeyEncryptedUserKey[1],
|
||||
userId,
|
||||
);
|
||||
expect(keyService.setUserKey).toHaveBeenCalledWith(masterKeyEncryptedUserKey[0], userId);
|
||||
});
|
||||
|
||||
@@ -616,6 +658,36 @@ describe("DefaultSetInitialPasswordService", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("should create and set master password unlock data to prevent race condition with sync", async () => {
|
||||
// Arrange
|
||||
setupMocks({ ...defaultMockConfig, userType });
|
||||
|
||||
const mockUnlockData = {
|
||||
salt: credentials.salt,
|
||||
kdf: credentials.kdfConfig,
|
||||
masterKeyWrappedUserKey: "wrapped_key_string",
|
||||
};
|
||||
|
||||
masterPasswordService.makeMasterPasswordUnlockData.mockResolvedValue(
|
||||
mockUnlockData as any,
|
||||
);
|
||||
|
||||
// Act
|
||||
await sut.setInitialPassword(credentials, userType, userId);
|
||||
|
||||
// Assert
|
||||
expect(masterPasswordService.makeMasterPasswordUnlockData).toHaveBeenCalledWith(
|
||||
credentials.newPassword,
|
||||
credentials.kdfConfig,
|
||||
credentials.salt,
|
||||
masterKeyEncryptedUserKey[0],
|
||||
);
|
||||
expect(masterPasswordService.setMasterPasswordUnlockData).toHaveBeenCalledWith(
|
||||
mockUnlockData,
|
||||
userId,
|
||||
);
|
||||
});
|
||||
|
||||
describe("given resetPasswordAutoEnroll is true", () => {
|
||||
it(`should handle reset password (account recovery) auto enroll`, async () => {
|
||||
// Arrange
|
||||
|
||||
@@ -214,6 +214,8 @@ export class SetInitialPasswordComponent implements OnInit {
|
||||
assertTruthy(passwordInputResult.newServerMasterKeyHash, "newServerMasterKeyHash", ctx);
|
||||
assertTruthy(passwordInputResult.newLocalMasterKeyHash, "newLocalMasterKeyHash", ctx);
|
||||
assertTruthy(passwordInputResult.kdfConfig, "kdfConfig", ctx);
|
||||
assertTruthy(passwordInputResult.newPassword, "newPassword", ctx);
|
||||
assertTruthy(passwordInputResult.salt, "salt", ctx);
|
||||
assertTruthy(this.orgSsoIdentifier, "orgSsoIdentifier", ctx);
|
||||
assertTruthy(this.orgId, "orgId", ctx);
|
||||
assertTruthy(this.userType, "userType", ctx);
|
||||
@@ -231,6 +233,8 @@ export class SetInitialPasswordComponent implements OnInit {
|
||||
orgSsoIdentifier: this.orgSsoIdentifier,
|
||||
orgId: this.orgId,
|
||||
resetPasswordAutoEnroll: this.resetPasswordAutoEnroll,
|
||||
newPassword: passwordInputResult.newPassword,
|
||||
salt: passwordInputResult.salt,
|
||||
};
|
||||
|
||||
await this.setInitialPasswordService.setInitialPassword(
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { MasterPasswordSalt } from "@bitwarden/common/key-management/master-password/types/master-password.types";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { MasterKey } from "@bitwarden/common/types/key";
|
||||
import { KdfConfig } from "@bitwarden/key-management";
|
||||
@@ -50,6 +51,8 @@ export interface SetInitialPasswordCredentials {
|
||||
orgSsoIdentifier: string;
|
||||
orgId: string;
|
||||
resetPasswordAutoEnroll: boolean;
|
||||
newPassword: string;
|
||||
salt: MasterPasswordSalt;
|
||||
}
|
||||
|
||||
export interface SetInitialPasswordTdeOffboardingCredentials {
|
||||
|
||||
Reference in New Issue
Block a user