mirror of
https://github.com/bitwarden/browser
synced 2025-12-12 22:33:35 +00:00
[PM-23621] Require userId for initAccount on the key-service (#15684)
* require userID for initAccount on key service * add unit test coverage * update consumer
This commit is contained in:
@@ -249,7 +249,7 @@ export class LoginDecryptionOptionsComponent implements OnInit {
|
||||
}
|
||||
|
||||
try {
|
||||
const { publicKey, privateKey } = await this.keyService.initAccount();
|
||||
const { publicKey, privateKey } = await this.keyService.initAccount(this.activeAccountId);
|
||||
const keysRequest = new KeysRequest(publicKey, privateKey.encryptedString);
|
||||
await this.apiService.postAccountKeys(keysRequest);
|
||||
|
||||
|
||||
@@ -386,11 +386,12 @@ export abstract class KeyService {
|
||||
/**
|
||||
* Initialize all necessary crypto keys needed for a new account.
|
||||
* Warning! This completely replaces any existing keys!
|
||||
* @param userId The user id of the target user.
|
||||
* @returns The user's newly created public key, private key, and encrypted private key
|
||||
*
|
||||
* @throws An error if there is no user currently active.
|
||||
* @throws An error if the userId is null or undefined.
|
||||
* @throws An error if the user already has a user key.
|
||||
*/
|
||||
abstract initAccount(): Promise<{
|
||||
abstract initAccount(userId: UserId): Promise<{
|
||||
userKey: UserKey;
|
||||
publicKey: string;
|
||||
privateKey: EncString;
|
||||
|
||||
@@ -1047,4 +1047,105 @@ describe("keyService", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("initAccount", () => {
|
||||
let userKey: UserKey;
|
||||
let mockPublicKey: string;
|
||||
let mockPrivateKey: EncString;
|
||||
|
||||
beforeEach(() => {
|
||||
userKey = makeSymmetricCryptoKey<UserKey>(64);
|
||||
mockPublicKey = "mockPublicKey";
|
||||
mockPrivateKey = makeEncString("mockPrivateKey");
|
||||
|
||||
keyGenerationService.createKey.mockResolvedValue(userKey);
|
||||
jest.spyOn(keyService, "makeKeyPair").mockResolvedValue([mockPublicKey, mockPrivateKey]);
|
||||
jest.spyOn(keyService, "setUserKey").mockResolvedValue();
|
||||
});
|
||||
|
||||
test.each([null as unknown as UserId, undefined as unknown as UserId])(
|
||||
"throws when the provided userId is %s",
|
||||
async (userId) => {
|
||||
await expect(keyService.initAccount(userId)).rejects.toThrow("UserId is required.");
|
||||
expect(keyService.setUserKey).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("throws when user already has a user key", async () => {
|
||||
const existingUserKey = makeSymmetricCryptoKey<UserKey>(64);
|
||||
stateProvider.singleUser.getFake(mockUserId, USER_KEY).nextState(existingUserKey);
|
||||
|
||||
await expect(keyService.initAccount(mockUserId)).rejects.toThrow(
|
||||
"Cannot initialize account, keys already exist.",
|
||||
);
|
||||
expect(logService.error).toHaveBeenCalledWith(
|
||||
"Tried to initialize account with existing user key.",
|
||||
);
|
||||
expect(keyService.setUserKey).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("throws when private key creation fails", async () => {
|
||||
// Simulate failure
|
||||
const invalidPrivateKey = new EncString(
|
||||
"2.AAAw2vTUePO+CCyokcIfVw==|DTBNlJ5yVsV2Bsk3UU3H6Q==|YvFBff5gxWqM+UsFB6BKimKxhC32AtjF3IStpU1Ijwg=",
|
||||
);
|
||||
invalidPrivateKey.encryptedString = null as unknown as EncryptedString;
|
||||
jest.spyOn(keyService, "makeKeyPair").mockResolvedValue([mockPublicKey, invalidPrivateKey]);
|
||||
|
||||
await expect(keyService.initAccount(mockUserId)).rejects.toThrow(
|
||||
"Failed to create valid private key.",
|
||||
);
|
||||
expect(keyService.setUserKey).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("successfully initializes account with new keys", async () => {
|
||||
const keyCreationSize = 512;
|
||||
const privateKeyState = stateProvider.singleUser.getFake(
|
||||
mockUserId,
|
||||
USER_ENCRYPTED_PRIVATE_KEY,
|
||||
);
|
||||
|
||||
const result = await keyService.initAccount(mockUserId);
|
||||
|
||||
expect(keyGenerationService.createKey).toHaveBeenCalledWith(keyCreationSize);
|
||||
expect(keyService.makeKeyPair).toHaveBeenCalledWith(userKey);
|
||||
expect(keyService.setUserKey).toHaveBeenCalledWith(userKey, mockUserId);
|
||||
expect(privateKeyState.nextMock).toHaveBeenCalledWith(mockPrivateKey.encryptedString);
|
||||
expect(result).toEqual({
|
||||
userKey: userKey,
|
||||
publicKey: mockPublicKey,
|
||||
privateKey: mockPrivateKey,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("makeKeyPair", () => {
|
||||
test.each([null as unknown as SymmetricCryptoKey, undefined as unknown as SymmetricCryptoKey])(
|
||||
"throws when the provided key is %s",
|
||||
async (key) => {
|
||||
await expect(keyService.makeKeyPair(key)).rejects.toThrow(
|
||||
"'key' is a required parameter and must be non-null.",
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("generates a key pair and returns public key and encrypted private key", async () => {
|
||||
const mockKey = new SymmetricCryptoKey(new Uint8Array(64));
|
||||
const mockKeyPair: [Uint8Array, Uint8Array] = [new Uint8Array(256), new Uint8Array(256)];
|
||||
const mockPublicKeyB64 = "mockPublicKeyB64";
|
||||
const mockPrivateKeyEncString = makeEncString("encryptedPrivateKey");
|
||||
|
||||
cryptoFunctionService.rsaGenerateKeyPair.mockResolvedValue(mockKeyPair);
|
||||
jest.spyOn(Utils, "fromBufferToB64").mockReturnValue(mockPublicKeyB64);
|
||||
encryptService.wrapDecapsulationKey.mockResolvedValue(mockPrivateKeyEncString);
|
||||
|
||||
const [publicKey, privateKey] = await keyService.makeKeyPair(mockKey);
|
||||
|
||||
expect(cryptoFunctionService.rsaGenerateKeyPair).toHaveBeenCalledWith(2048);
|
||||
expect(Utils.fromBufferToB64).toHaveBeenCalledWith(mockKeyPair[0]);
|
||||
expect(encryptService.wrapDecapsulationKey).toHaveBeenCalledWith(mockKeyPair[1], mockKey);
|
||||
expect(publicKey).toBe(mockPublicKeyB64);
|
||||
expect(privateKey).toBe(mockPrivateKeyEncString);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -661,19 +661,17 @@ export class DefaultKeyService implements KeyServiceAbstraction {
|
||||
* Initialize all necessary crypto keys needed for a new account.
|
||||
* Warning! This completely replaces any existing keys!
|
||||
*/
|
||||
async initAccount(): Promise<{
|
||||
async initAccount(userId: UserId): Promise<{
|
||||
userKey: UserKey;
|
||||
publicKey: string;
|
||||
privateKey: EncString;
|
||||
}> {
|
||||
const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$);
|
||||
|
||||
if (activeUserId == null) {
|
||||
throw new Error("Cannot initilize an account if one is not active.");
|
||||
if (userId == null) {
|
||||
throw new Error("UserId is required.");
|
||||
}
|
||||
|
||||
// Verify user key doesn't exist
|
||||
const existingUserKey = await this.getUserKey(activeUserId);
|
||||
const existingUserKey = await this.getUserKey(userId);
|
||||
|
||||
if (existingUserKey != null) {
|
||||
this.logService.error("Tried to initialize account with existing user key.");
|
||||
@@ -686,9 +684,9 @@ export class DefaultKeyService implements KeyServiceAbstraction {
|
||||
throw new Error("Failed to create valid private key.");
|
||||
}
|
||||
|
||||
await this.setUserKey(userKey, activeUserId);
|
||||
await this.setUserKey(userKey, userId);
|
||||
await this.stateProvider
|
||||
.getUser(activeUserId, USER_ENCRYPTED_PRIVATE_KEY)
|
||||
.getUser(userId, USER_ENCRYPTED_PRIVATE_KEY)
|
||||
.update(() => privateKey.encryptedString!);
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user