1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 08:13:42 +00:00

use user verification as a part of key rotation (#9722)

This commit is contained in:
Jake Fink
2024-07-01 11:15:18 -04:00
committed by GitHub
parent 5965f779b9
commit 9531d1c655
2 changed files with 31 additions and 37 deletions

View File

@@ -3,9 +3,8 @@ import { BehaviorSubject } from "rxjs";
import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/common/admin-console/abstractions/organization-user/requests"; import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/common/admin-console/abstractions/organization-user/requests";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request"; import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request";
import { FakeMasterPasswordService } from "@bitwarden/common/auth/services/master-password/fake-master-password.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
@@ -32,6 +31,7 @@ import { UserKeyRotationService } from "./user-key-rotation.service";
describe("KeyRotationService", () => { describe("KeyRotationService", () => {
let keyRotationService: UserKeyRotationService; let keyRotationService: UserKeyRotationService;
let mockUserVerificationService: MockProxy<UserVerificationService>;
let mockApiService: MockProxy<UserKeyRotationApiService>; let mockApiService: MockProxy<UserKeyRotationApiService>;
let mockCipherService: MockProxy<CipherService>; let mockCipherService: MockProxy<CipherService>;
let mockFolderService: MockProxy<FolderService>; let mockFolderService: MockProxy<FolderService>;
@@ -42,10 +42,8 @@ describe("KeyRotationService", () => {
let mockCryptoService: MockProxy<CryptoService>; let mockCryptoService: MockProxy<CryptoService>;
let mockEncryptService: MockProxy<EncryptService>; let mockEncryptService: MockProxy<EncryptService>;
let mockConfigService: MockProxy<ConfigService>; let mockConfigService: MockProxy<ConfigService>;
let mockKdfConfigService: MockProxy<KdfConfigService>;
let mockSyncService: MockProxy<SyncService>; let mockSyncService: MockProxy<SyncService>;
let mockWebauthnLoginAdminService: MockProxy<WebauthnLoginAdminService>; let mockWebauthnLoginAdminService: MockProxy<WebauthnLoginAdminService>;
let mockMasterPasswordService: FakeMasterPasswordService = new FakeMasterPasswordService();
const mockUser = { const mockUser = {
id: "mockUserId" as UserId, id: "mockUserId" as UserId,
@@ -55,7 +53,7 @@ describe("KeyRotationService", () => {
}; };
beforeAll(() => { beforeAll(() => {
mockMasterPasswordService = new FakeMasterPasswordService(); mockUserVerificationService = mock<UserVerificationService>();
mockApiService = mock<UserKeyRotationApiService>(); mockApiService = mock<UserKeyRotationApiService>();
mockCipherService = mock<CipherService>(); mockCipherService = mock<CipherService>();
mockFolderService = mock<FolderService>(); mockFolderService = mock<FolderService>();
@@ -66,12 +64,11 @@ describe("KeyRotationService", () => {
mockCryptoService = mock<CryptoService>(); mockCryptoService = mock<CryptoService>();
mockEncryptService = mock<EncryptService>(); mockEncryptService = mock<EncryptService>();
mockConfigService = mock<ConfigService>(); mockConfigService = mock<ConfigService>();
mockKdfConfigService = mock<KdfConfigService>();
mockSyncService = mock<SyncService>(); mockSyncService = mock<SyncService>();
mockWebauthnLoginAdminService = mock<WebauthnLoginAdminService>(); mockWebauthnLoginAdminService = mock<WebauthnLoginAdminService>();
keyRotationService = new UserKeyRotationService( keyRotationService = new UserKeyRotationService(
mockMasterPasswordService, mockUserVerificationService,
mockApiService, mockApiService,
mockCipherService, mockCipherService,
mockFolderService, mockFolderService,
@@ -81,7 +78,6 @@ describe("KeyRotationService", () => {
mockDeviceTrustService, mockDeviceTrustService,
mockCryptoService, mockCryptoService,
mockEncryptService, mockEncryptService,
mockKdfConfigService,
mockSyncService, mockSyncService,
mockWebauthnLoginAdminService, mockWebauthnLoginAdminService,
); );
@@ -95,7 +91,6 @@ describe("KeyRotationService", () => {
let privateKey: BehaviorSubject<UserPrivateKey>; let privateKey: BehaviorSubject<UserPrivateKey>;
beforeEach(() => { beforeEach(() => {
mockCryptoService.makeMasterKey.mockResolvedValue("mockMasterKey" as any);
mockCryptoService.makeUserKey.mockResolvedValue([ mockCryptoService.makeUserKey.mockResolvedValue([
new SymmetricCryptoKey(new Uint8Array(64)) as UserKey, new SymmetricCryptoKey(new Uint8Array(64)) as UserKey,
{ {
@@ -109,6 +104,12 @@ describe("KeyRotationService", () => {
encryptedString: "mockEncryptedData", encryptedString: "mockEncryptedData",
} as any); } as any);
// Mock user verification
mockUserVerificationService.verifyUserByMasterPassword.mockResolvedValue({
masterKey: "mockMasterKey" as any,
policyOptions: null,
});
// Mock user key // Mock user key
mockCryptoService.userKey$.mockReturnValue(new BehaviorSubject("mockOriginalUserKey" as any)); mockCryptoService.userKey$.mockReturnValue(new BehaviorSubject("mockOriginalUserKey" as any));
@@ -162,14 +163,6 @@ describe("KeyRotationService", () => {
).rejects.toThrow(); ).rejects.toThrow();
}); });
it("throws if master key creation fails", async () => {
mockCryptoService.makeMasterKey.mockResolvedValueOnce(null);
await expect(
keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser),
).rejects.toThrow();
});
it("throws if user key creation fails", async () => { it("throws if user key creation fails", async () => {
mockCryptoService.makeUserKey.mockResolvedValueOnce([null, null]); mockCryptoService.makeUserKey.mockResolvedValueOnce([null, null]);
@@ -186,13 +179,14 @@ describe("KeyRotationService", () => {
).rejects.toThrow(); ).rejects.toThrow();
}); });
it("saves the master key in state after creation", async () => { it("throws if master password is incorrect", async () => {
await keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser); mockUserVerificationService.verifyUserByMasterPassword.mockRejectedValueOnce(
new Error("Invalid master password"),
expect(mockMasterPasswordService.mock.setMasterKey).toHaveBeenCalledWith(
"mockMasterKey" as any,
mockUser.id,
); );
await expect(
keyRotationService.rotateUserKeyAndEncryptedData("mockMasterPassword", mockUser),
).rejects.toThrow();
}); });
it("throws if server rotation fails", async () => { it("throws if server rotation fails", async () => {

View File

@@ -3,8 +3,9 @@ import { firstValueFrom } from "rxjs";
import { AccountInfo } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountInfo } from "@bitwarden/common/auth/abstractions/account.service";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
import { EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string";
@@ -25,7 +26,7 @@ import { UserKeyRotationApiService } from "./user-key-rotation-api.service";
@Injectable() @Injectable()
export class UserKeyRotationService { export class UserKeyRotationService {
constructor( constructor(
private masterPasswordService: InternalMasterPasswordServiceAbstraction, private userVerificationService: UserVerificationService,
private apiService: UserKeyRotationApiService, private apiService: UserKeyRotationApiService,
private cipherService: CipherService, private cipherService: CipherService,
private folderService: FolderService, private folderService: FolderService,
@@ -35,7 +36,6 @@ export class UserKeyRotationService {
private deviceTrustService: DeviceTrustServiceAbstraction, private deviceTrustService: DeviceTrustServiceAbstraction,
private cryptoService: CryptoService, private cryptoService: CryptoService,
private encryptService: EncryptService, private encryptService: EncryptService,
private kdfConfigService: KdfConfigService,
private syncService: SyncService, private syncService: SyncService,
private webauthnLoginAdminService: WebauthnLoginAdminService, private webauthnLoginAdminService: WebauthnLoginAdminService,
) {} ) {}
@@ -58,19 +58,19 @@ export class UserKeyRotationService {
); );
} }
// Create master key to validate the master password // Verify master password
const masterKey = await this.cryptoService.makeMasterKey( // UV service sets master key on success since it is stored in memory and can be lost on refresh
masterPassword, const verification = {
type: VerificationType.MasterPassword,
secret: masterPassword,
} as MasterPasswordVerification;
const { masterKey } = await this.userVerificationService.verifyUserByMasterPassword(
verification,
user.id,
user.email, user.email,
await this.kdfConfigService.getKdfConfig(),
); );
if (!masterKey) {
throw new Error("Master key could not be created");
}
// Set master key again in case it was lost (could be lost on refresh)
await this.masterPasswordService.setMasterKey(masterKey, user.id);
const [newUserKey, newEncUserKey] = await this.cryptoService.makeUserKey(masterKey); const [newUserKey, newEncUserKey] = await this.cryptoService.makeUserKey(masterKey);
if (!newUserKey || !newEncUserKey) { if (!newUserKey || !newEncUserKey) {