bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 15:53:27 +00:00
Code Issues Releases Wiki Activity

Encourage The Use of UserId in CryptoService (#9033)

Browse Source
This commit is contained in:
Justin Baur
2024-05-04 02:04:56 -04:00
committed by GitHub
parent e4ef7d362e
commit 869fa29da6
16 changed files with 92 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts
View File

@@ -140,7 +140,7 @@ describe("AuthRequestLoginStrategy", () => {
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key);
expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey);
expect(deviceTrustService.trustDeviceIfRequired).toHaveBeenCalled();
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, mockUserId);
});
it("sets keys after a successful authentication when only userKey provided in login credentials", async () => {
@@ -164,7 +164,7 @@ describe("AuthRequestLoginStrategy", () => {
// setMasterKeyEncryptedUserKey, setUserKey, and setPrivateKey should still be called
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key);
expect(cryptoService.setUserKey).toHaveBeenCalledWith(decUserKey);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, mockUserId);
// trustDeviceIfRequired should be called
expect(deviceTrustService.trustDeviceIfRequired).not.toHaveBeenCalled();

8
libs/auth/src/common/login-strategies/auth-request-login.strategy.ts
View File

@@ -161,9 +161,13 @@ export class AuthRequestLoginStrategy extends LoginStrategy {
}
}
protected override async setPrivateKey(response: IdentityTokenResponse): Promise<void> {
protected override async setPrivateKey(
response: IdentityTokenResponse,
userId: UserId,
): Promise<void> {
await this.cryptoService.setPrivateKey(
response.privateKey ?? (await this.createKeyPairForOldAccount()),
response.privateKey ?? (await this.createKeyPairForOldAccount(userId)),
userId,
);
}

9
libs/auth/src/common/login-strategies/login.strategy.ts
View File

@@ -252,7 +252,7 @@ export abstract class LoginStrategy {
await this.setMasterKey(response, userId);
await this.setUserKey(response, userId);
await this.setPrivateKey(response);
await this.setPrivateKey(response, userId);
this.messagingService.send("loggedIn");
@@ -262,7 +262,7 @@ export abstract class LoginStrategy {
// The keys comes from different sources depending on the login strategy
protected abstract setMasterKey(response: IdentityTokenResponse, userId: UserId): Promise<void>;
protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise<void>;
protected abstract setPrivateKey(response: IdentityTokenResponse): Promise<void>;
protected abstract setPrivateKey(response: IdentityTokenResponse, userId: UserId): Promise<void>;
// Old accounts used master key for encryption. We are forcing migrations but only need to
// check on password logins
@@ -270,9 +270,10 @@ export abstract class LoginStrategy {
return false;
}
protected async createKeyPairForOldAccount() {
protected async createKeyPairForOldAccount(userId: UserId) {
try {
const [publicKey, privateKey] = await this.cryptoService.makeKeyPair();
const userKey = await this.cryptoService.getUserKeyWithLegacySupport(userId);
const [publicKey, privateKey] = await this.cryptoService.makeKeyPair(userKey);
await this.apiService.postAccountKeys(new KeysRequest(publicKey, privateKey.encryptedString));
return privateKey.encryptedString;
} catch (e) {

2
libs/auth/src/common/login-strategies/password-login.strategy.spec.ts
View File

@@ -178,7 +178,7 @@ describe("PasswordLoginStrategy", () => {
userId,
);
expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, userId);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, userId);
});
it("does not force the user to update their master password when there are no requirements", async () => {

8
libs/auth/src/common/login-strategies/password-login.strategy.ts
View File

@@ -233,9 +233,13 @@ export class PasswordLoginStrategy extends LoginStrategy {
}
}
protected override async setPrivateKey(response: IdentityTokenResponse): Promise<void> {
protected override async setPrivateKey(
response: IdentityTokenResponse,
userId: UserId,
): Promise<void> {
await this.cryptoService.setPrivateKey(
response.privateKey ?? (await this.createKeyPairForOldAccount()),
response.privateKey ?? (await this.createKeyPairForOldAccount(userId)),
userId,
);
}

8
libs/auth/src/common/login-strategies/sso-login.strategy.ts
View File

@@ -354,12 +354,16 @@ export class SsoLoginStrategy extends LoginStrategy {
await this.cryptoService.setUserKey(userKey);
}
protected override async setPrivateKey(tokenResponse: IdentityTokenResponse): Promise<void> {
protected override async setPrivateKey(
tokenResponse: IdentityTokenResponse,
userId: UserId,
): Promise<void> {
const newSsoUser = tokenResponse.key == null;
if (!newSsoUser) {
await this.cryptoService.setPrivateKey(
tokenResponse.privateKey ?? (await this.createKeyPairForOldAccount()),
tokenResponse.privateKey ?? (await this.createKeyPairForOldAccount(userId)),
userId,
);
}
}

2
libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts
View File

@@ -159,7 +159,7 @@ describe("UserApiLoginStrategy", () => {
await apiLogInStrategy.logIn(credentials);
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, userId);
});
it("gets and sets the master key if Key Connector is enabled", async () => {

8
libs/auth/src/common/login-strategies/user-api-login.strategy.ts
View File

@@ -116,9 +116,13 @@ export class UserApiLoginStrategy extends LoginStrategy {
}
}
protected override async setPrivateKey(response: IdentityTokenResponse): Promise<void> {
protected override async setPrivateKey(
response: IdentityTokenResponse,
userId: UserId,
): Promise<void> {
await this.cryptoService.setPrivateKey(
response.privateKey ?? (await this.createKeyPairForOldAccount()),
response.privateKey ?? (await this.createKeyPairForOldAccount(userId)),
userId,
);
}

2
libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts
View File

@@ -224,7 +224,7 @@ describe("WebAuthnLoginStrategy", () => {
mockPrfPrivateKey,
);
expect(cryptoService.setUserKey).toHaveBeenCalledWith(mockUserKey, userId);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(idTokenResponse.privateKey);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(idTokenResponse.privateKey, userId);
// Master key and private key should not be set
expect(masterPasswordService.mock.setMasterKey).not.toHaveBeenCalled();

8
libs/auth/src/common/login-strategies/webauthn-login.strategy.ts
View File

@@ -139,9 +139,13 @@ export class WebAuthnLoginStrategy extends LoginStrategy {
}
}
protected override async setPrivateKey(response: IdentityTokenResponse): Promise<void> {
protected override async setPrivateKey(
response: IdentityTokenResponse,
userId: UserId,
): Promise<void> {
await this.cryptoService.setPrivateKey(
response.privateKey ?? (await this.createKeyPairForOldAccount()),
response.privateKey ?? (await this.createKeyPairForOldAccount(userId)),
userId,
);
}